This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[GOLD] Support --icf=safe with -pie for x86_64


--icf=safe folds identical functions only if the functions do not have their
address taken. It uses relocation types to distinguish between a function call
and taking the address of a function. This works fine for all architectures
except x86_64, where the same relocation type (R_X86_64_PC32) may be used for
both function call and taking the address of a function (when compiled with
-fPIE). Currently Target_x86_64::do_can_check_for_function_pointers returns
false if the linker is invoked with -pie, and --icf=safe folds only ctors and
dtors.

This patch adds improved support for --icf=safe when used with -pie. For the
ambiguous case of R_X86_64_PC32 relocation, it checks the instruction opcode to
distinguish between function calls and taking the address of a function.

OK?

        * 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.
        * testsuite/Makefile.am (icf_safe_pie_test): New test case.
        * testsuite/Makefile.in: Regenerate.
        * testsuite/icf_safe_pie_test.sh: New shell script.

-- 
Rahul
diff --git gold/ChangeLog gold/ChangeLog
index 6cc97528a8..05e081186a 100644
--- gold/ChangeLog
+++ gold/ChangeLog
@@ -1,3 +1,16 @@
+2017-01-12  Rahul Chaudhry  <rahulchaudhry@google.com>
+
+	* 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.
+	* testsuite/Makefile.am (icf_safe_pie_test): New test case.
+	* testsuite/Makefile.in: Regenerate.
+	* testsuite/icf_safe_pie_test.sh: New shell script.
+
 2017-01-11  Cary Coutant  <ccoutant@gmail.com>
 
 	PR gold/21040
diff --git gold/testsuite/Makefile.am gold/testsuite/Makefile.am
index d9480abb42..d0803d251a 100644
--- gold/testsuite/Makefile.am
+++ 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 gold/testsuite/Makefile.in gold/testsuite/Makefile.in
index 2c67bf5fe2..133e733cf1 100644
--- gold/testsuite/Makefile.in
+++ 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 gold/testsuite/icf_safe_pie_test.sh gold/testsuite/icf_safe_pie_test.sh
new file mode 100755
index 0000000000..f91a80c0b9
--- /dev/null
+++ 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 <tmsriram@google.com>.
+# Modified by Rahul Chaudhry <rahulchaudhry@google.com>.
+
+# 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 gold/x86_64.cc gold/x86_64.cc
index ffa876116a..8360033d8c 100644
--- gold/x86_64.cc
+++ gold/x86_64.cc
@@ -729,10 +729,13 @@ class Target_x86_64 : public Sized_target<size, false>
   // 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<size, false>
     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<size, false>* src_obj,
+                                    unsigned int src_indx,
+                                    unsigned int r_offset,
+                                    unsigned int r_type);
 
     bool
     reloc_needs_plt_for_ifunc(Sized_relobj_file<size, false>*,
@@ -3274,7 +3280,11 @@ Target_x86_64<size>::Scan::unsupported_reloc_global(
 // Returns true if this relocation type could be that of a function pointer.
 template<int size>
 inline bool
-Target_x86_64<size>::Scan::possible_function_pointer_reloc(unsigned int r_type)
+Target_x86_64<size>::Scan::possible_function_pointer_reloc(
+    Sized_relobj_file<size, false>* src_obj,
+    unsigned int src_indx,
+    unsigned int r_offset,
+    unsigned int r_type)
 {
   switch (r_type)
     {
@@ -3293,6 +3303,36 @@ Target_x86_64<size>::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.
+        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;
 }
@@ -3307,18 +3347,21 @@ Target_x86_64<size>::Scan::local_reloc_may_be_function_pointer(
   Symbol_table* ,
   Layout* ,
   Target_x86_64<size>* ,
-  Sized_relobj_file<size, false>* ,
-  unsigned int ,
+  Sized_relobj_file<size, false>* src_obj,
+  unsigned int src_indx,
   Output_section* ,
-  const elfcpp::Rela<size, false>& ,
+  const elfcpp::Rela<size, false>& reloc,
   unsigned int r_type,
   const elfcpp::Sym<size, false>&)
 {
   // 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
@@ -3331,20 +3374,23 @@ Target_x86_64<size>::Scan::global_reloc_may_be_function_pointer(
   Symbol_table*,
   Layout* ,
   Target_x86_64<size>* ,
-  Sized_relobj_file<size, false>* ,
-  unsigned int ,
+  Sized_relobj_file<size, false>* src_obj,
+  unsigned int src_indx,
   Output_section* ,
-  const elfcpp::Rela<size, false>& ,
+  const elfcpp::Rela<size, false>& 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.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]