From 4aebb6312eb5dcd12f2f8420028547584b708907 Mon Sep 17 00:00:00 2001 From: Rahul Chaudhry Date: Wed, 15 Feb 2017 00:37:10 -0800 Subject: [PATCH] Improved support for --icf=safe when used with -pie. gold/ * x86_64.cc (Target_x86_64::do_can_check_for_function_pointers): Return true even when building pie binaries. (Target_x86_64::possible_function_pointer_reloc): Check opcode for R_X86_64_PC32 relocations. (Target_x86_64::local_reloc_may_be_function_pointer): Pass extra arguments to local_reloc_may_be_function_pointer. (Target_x86_64::global_reloc_may_be_function_pointer): Likewise. * gc.h (gc_process_relocs): Add check for STT_FUNC. * testsuite/Makefile.am (icf_safe_pie_test): New test case. * testsuite/Makefile.in: Regenerate. * testsuite/icf_safe_pie_test.sh: New shell script. --- gold/ChangeLog | 14 ++++++ gold/gc.h | 1 + gold/testsuite/Makefile.am | 14 ++++++ gold/testsuite/Makefile.in | 18 ++++++++ gold/testsuite/icf_safe_pie_test.sh | 76 +++++++++++++++++++++++++++++++++ gold/x86_64.cc | 85 +++++++++++++++++++++++++++++-------- 6 files changed, 191 insertions(+), 17 deletions(-) create mode 100755 gold/testsuite/icf_safe_pie_test.sh diff --git a/gold/ChangeLog b/gold/ChangeLog index 56fe966..e811b8e 100644 --- a/gold/ChangeLog +++ b/gold/ChangeLog @@ -1,3 +1,17 @@ +2017-02-03 Rahul Chaudhry + + * x86_64.cc (Target_x86_64::do_can_check_for_function_pointers): + Return true even when building pie binaries. + (Target_x86_64::possible_function_pointer_reloc): Check opcode + for R_X86_64_PC32 relocations. + (Target_x86_64::local_reloc_may_be_function_pointer): Pass + extra arguments to local_reloc_may_be_function_pointer. + (Target_x86_64::global_reloc_may_be_function_pointer): Likewise. + * gc.h (gc_process_relocs): Add check for STT_FUNC. + * testsuite/Makefile.am (icf_safe_pie_test): New test case. + * testsuite/Makefile.in: Regenerate. + * testsuite/icf_safe_pie_test.sh: New shell script. + 2017-02-03 Alan Modra * powerpc.cc (Powerpc_relobj::make_toc_relative): Don't crash diff --git a/gold/gc.h b/gold/gc.h index 45f8d2a..327efc2 100644 --- a/gold/gc.h +++ b/gold/gc.h @@ -295,6 +295,7 @@ gc_process_relocs( // When doing safe folding, check to see if this relocation is that // of a function pointer being taken. if (gsym->source() == Symbol::FROM_OBJECT + && gsym->type() == elfcpp::STT_FUNC && check_section_for_function_pointers && dst_obj != NULL && (!is_ordinary diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am index d9480ab..d0803d2 100644 --- a/gold/testsuite/Makefile.am +++ b/gold/testsuite/Makefile.am @@ -274,6 +274,20 @@ icf_safe_test_1.stdout: icf_safe_test icf_safe_test_2.stdout: icf_safe_test $(TEST_READELF) -h $< > $@ +check_SCRIPTS += icf_safe_pie_test.sh +check_DATA += icf_safe_pie_test_1.stdout icf_safe_pie_test_2.stdout icf_safe_pie_test.map +MOSTLYCLEANFILES += icf_safe_pie_test icf_safe_pie_test.map +icf_safe_pie_test.o: icf_safe_test.cc + $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIE -g -o $@ $< +icf_safe_pie_test: icf_safe_pie_test.o gcctestdir/ld + $(CXXLINK) -o icf_safe_pie_test -Bgcctestdir/ -Wl,--icf=safe,-Map,icf_safe_pie_test.map icf_safe_pie_test.o -pie +icf_safe_pie_test.map: icf_safe_pie_test + @touch icf_safe_pie_test.map +icf_safe_pie_test_1.stdout: icf_safe_pie_test + $(TEST_NM) $< > $@ +icf_safe_pie_test_2.stdout: icf_safe_pie_test + $(TEST_READELF) -h $< > $@ + check_SCRIPTS += icf_safe_so_test.sh check_DATA += icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout icf_safe_so_test.map MOSTLYCLEANFILES += icf_safe_so_test icf_safe_so_test.map diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in index 2c67bf5..133e733 100644 --- a/gold/testsuite/Makefile.in +++ b/gold/testsuite/Makefile.in @@ -82,6 +82,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_keep_unique_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test.sh \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ final_layout.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ text_section_grouping.sh \ @@ -103,6 +104,9 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test_1.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test_2.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test.map \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test_1.stdout \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test_2.stdout \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test_1.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test_2.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test.map \ @@ -125,6 +129,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_test icf_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_keep_unique_test \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_test icf_safe_test.map \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_pie_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test.map \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ final_layout \ @@ -5093,6 +5099,8 @@ icf_keep_unique_test.sh.log: icf_keep_unique_test.sh @p='icf_keep_unique_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) icf_safe_test.sh.log: icf_safe_test.sh @p='icf_safe_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) +icf_safe_pie_test.sh.log: icf_safe_pie_test.sh + @p='icf_safe_pie_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) icf_safe_so_test.sh.log: icf_safe_so_test.sh @p='icf_safe_so_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post) final_layout.sh.log: final_layout.sh @@ -5910,6 +5918,16 @@ uninstall-am: @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_NM) $< > $@ @GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_test_2.stdout: icf_safe_test @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -h $< > $@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test.o: icf_safe_test.cc +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIE -g -o $@ $< +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test: icf_safe_pie_test.o gcctestdir/ld +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -o icf_safe_pie_test -Bgcctestdir/ -Wl,--icf=safe,-Map,icf_safe_pie_test.map icf_safe_pie_test.o -pie +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test.map: icf_safe_pie_test +@GCC_TRUE@@NATIVE_LINKER_TRUE@ @touch icf_safe_pie_test.map +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test_1.stdout: icf_safe_pie_test +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_NM) $< > $@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_pie_test_2.stdout: icf_safe_pie_test +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -h $< > $@ @GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_so_test.o: icf_safe_so_test.cc @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIC -g -o $@ $< @GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_so_test: icf_safe_so_test.o gcctestdir/ld diff --git a/gold/testsuite/icf_safe_pie_test.sh b/gold/testsuite/icf_safe_pie_test.sh new file mode 100755 index 0000000..f91a80c --- /dev/null +++ b/gold/testsuite/icf_safe_pie_test.sh @@ -0,0 +1,76 @@ +#!/bin/sh + +# icf_safe_pie_test.sh -- test --icf=safe -pie + +# Copyright (C) 2009-2017 Free Software Foundation, Inc. +# Written by Sriraman Tallam . +# Modified by Rahul Chaudhry . + +# This file is part of gold. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, +# MA 02110-1301, USA. + +# The goal of this program is to verify if --icf=safe works with +# -pie as expected. File icf_safe_test.cc is in this test. This +# program checks if only ctors and dtors are folded, except for +# the architectures which use relocation types and instruction +# opcodes to detect if function pointers are taken. + +set -e + +check_nofold() +{ + func_addr_1=`grep $2 $1 | awk '{print $1}'` + func_addr_2=`grep $3 $1 | awk '{print $1}'` + if [ $func_addr_1 = $func_addr_2 ] + then + echo "Safe Identical Code Folding folded" $2 "and" $3 + exit 1 + fi +} + +check_fold() +{ + awk " +BEGIN { discard = 0; } +/^Discarded input/ { discard = 1; } +/^Memory map/ { discard = 0; } +/.*\\.text\\..*($2|$3).*/ { act[discard] = act[discard] \" \" \$0; } +END { + # printf \"kept\" act[0] \"\\nfolded\" act[1] \"\\n\"; + if (length(act[0]) == 0 || length(act[1]) == 0) + { + printf \"Safe Identical Code Folding did not fold $2 and $3\\n\" + exit 1; + } + }" $1 +} + +arch_specific_safe_fold() +{ + if grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" -e "ARM" -e "TILE" -e "PowerPC" -e "AArch64" -e "IBM S/390" $2; + then + check_fold $3 $4 $5 + else + check_nofold $1 $4 $5 + fi +} + +arch_specific_safe_fold icf_safe_pie_test_1.stdout icf_safe_pie_test_2.stdout \ + icf_safe_pie_test.map "kept_func_1" "kept_func_2" +check_fold icf_safe_pie_test.map "_ZN1AD2Ev" "_ZN1AC2Ev" +check_nofold icf_safe_pie_test_1.stdout "kept_func_3" "kept_func_1" +check_nofold icf_safe_pie_test_1.stdout "kept_func_3" "kept_func_2" diff --git a/gold/x86_64.cc b/gold/x86_64.cc index d21c268..7f1742d 100644 --- a/gold/x86_64.cc +++ b/gold/x86_64.cc @@ -729,10 +729,13 @@ class Target_x86_64 : public Sized_target // and global_reloc_may_be_function_pointer) // if a function's pointer is taken. ICF uses this in safe mode to only // fold those functions whose pointer is defintely not taken. For x86_64 - // pie binaries, safe ICF cannot be done by looking at relocation types. + // pie binaries, safe ICF cannot be done by looking at only relocation + // types, and for certain cases (e.g. R_X86_64_PC32), the instruction + // opcode is checked as well to distinguish a function call from taking + // a function's pointer. bool do_can_check_for_function_pointers() const - { return !parameters->options().pie(); } + { return true; } // Return the base for a DW_EH_PE_datarel encoding. uint64_t @@ -924,7 +927,10 @@ class Target_x86_64 : public Sized_target check_non_pic(Relobj*, unsigned int r_type, Symbol*); inline bool - possible_function_pointer_reloc(unsigned int r_type); + possible_function_pointer_reloc(Sized_relobj_file* src_obj, + unsigned int src_indx, + unsigned int r_offset, + unsigned int r_type); bool reloc_needs_plt_for_ifunc(Sized_relobj_file*, @@ -3277,7 +3283,11 @@ Target_x86_64::Scan::unsupported_reloc_global( // Returns true if this relocation type could be that of a function pointer. template inline bool -Target_x86_64::Scan::possible_function_pointer_reloc(unsigned int r_type) +Target_x86_64::Scan::possible_function_pointer_reloc( + Sized_relobj_file* src_obj, + unsigned int src_indx, + unsigned int r_offset, + unsigned int r_type) { switch (r_type) { @@ -3296,6 +3306,41 @@ Target_x86_64::Scan::possible_function_pointer_reloc(unsigned int r_type) { return true; } + case elfcpp::R_X86_64_PC32: + { + // This relocation may be used both for function calls and + // for taking address of a function. We distinguish between + // them by checking the opcodes. + uint64_t sh_flags = src_obj->section_flags(src_indx); + bool is_executable = (sh_flags & elfcpp::SHF_EXECINSTR) != 0; + if (is_executable) + { + section_size_type stype; + const unsigned char* view = src_obj->section_contents(src_indx, + &stype, + true); + + // call + if (r_offset >= 1 + && view[r_offset - 1] == 0xe8) + return false; + + // jmp + if (r_offset >= 1 + && view[r_offset - 1] == 0xe9) + return false; + + // jo/jno/jb/jnb/je/jne/jna/ja/js/jns/jp/jnp/jl/jge/jle/jg + if (r_offset >= 2 + && view[r_offset - 2] == 0x0f + && view[r_offset - 1] >= 0x80 + && view[r_offset - 1] <= 0x8f) + return false; + } + + // Be conservative and treat all others as function pointers. + return true; + } } return false; } @@ -3310,18 +3355,21 @@ Target_x86_64::Scan::local_reloc_may_be_function_pointer( Symbol_table* , Layout* , Target_x86_64* , - Sized_relobj_file* , - unsigned int , + Sized_relobj_file* src_obj, + unsigned int src_indx, Output_section* , - const elfcpp::Rela& , + const elfcpp::Rela& reloc, unsigned int r_type, const elfcpp::Sym&) { // When building a shared library, do not fold any local symbols as it is // not possible to distinguish pointer taken versus a call by looking at // the relocation types. - return (parameters->options().shared() - || possible_function_pointer_reloc(r_type)); + if (parameters->options().shared()) + return true; + + return possible_function_pointer_reloc(src_obj, src_indx, + reloc.get_r_offset(), r_type); } // For safe ICF, scan a relocation for a global symbol to check if it @@ -3334,20 +3382,23 @@ Target_x86_64::Scan::global_reloc_may_be_function_pointer( Symbol_table*, Layout* , Target_x86_64* , - Sized_relobj_file* , - unsigned int , + Sized_relobj_file* src_obj, + unsigned int src_indx, Output_section* , - const elfcpp::Rela& , + const elfcpp::Rela& reloc, unsigned int r_type, Symbol* gsym) { // When building a shared library, do not fold symbols whose visibility // is hidden, internal or protected. - return ((parameters->options().shared() - && (gsym->visibility() == elfcpp::STV_INTERNAL - || gsym->visibility() == elfcpp::STV_PROTECTED - || gsym->visibility() == elfcpp::STV_HIDDEN)) - || possible_function_pointer_reloc(r_type)); + if (parameters->options().shared() + && (gsym->visibility() == elfcpp::STV_INTERNAL + || gsym->visibility() == elfcpp::STV_PROTECTED + || gsym->visibility() == elfcpp::STV_HIDDEN)) + return true; + + return possible_function_pointer_reloc(src_obj, src_indx, + reloc.get_r_offset(), r_type); } // Scan a relocation for a global symbol. -- 2.7.4