Safe ICF for i386.

Sriraman Tallam tmsriram@google.com
Fri Feb 26 05:21:00 GMT 2010


Hi,

     I have attached a patch to use relocation types during safe ICF
and detect function pointer taken in i386. For i386, relocation types
function pointers are always different from function calls. Function
pointer reloc types can be one of R_386_32, R_386_16, R_386_8,
R_386_GOTOFF, R_386_GOT32 depending on whether it is compiled with
pic, pie or not. Please let me know what you think.

	* i386.cc (Target_i386::can_check_for_function_pointers): New function.
	(Scan::possible_function_pointer_reloc): New function.
	(Scan::local_reloc_may_be_function_pointer): Change to call
	possible_function_pointer_reloc.
	(Scan::global_reloc_may_be_function_pointer): Ditto.
	* icf.h (Icf::check_section_for_function_pointers): Change to reject
	relocations in ".data.rel.ro._ZTV" section.
	* testsuite/icf_safe_so_test.sh: Change to pass i386.
	* testsuite/icf_safe_so_test.cc: Ditto.
	* testsuite/icf_safe_test.cc: Ditto.
	* testsuite/icf_safe_test.sh: Ditto.

Thanks,
-Sri.
-------------- next part --------------
Index: i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.114
diff -u -u -p -r1.114 i386.cc
--- i386.cc	13 Feb 2010 02:04:20 -0000	1.114
+++ i386.cc	26 Feb 2010 03:05:01 -0000
@@ -64,6 +64,10 @@ class Target_i386 : public Target_freebs
       got_mod_index_offset_(-1U), tls_base_symbol_defined_(false)
   { }
 
+  inline bool
+  can_check_for_function_pointers() const
+  { return true; }
+
   // Process the relocations to determine unreferenced sections for 
   // garbage collection.
   void
@@ -203,24 +207,27 @@ class Target_i386 : public Target_freebs
 	   Symbol* gsym);
 
     inline bool
-    local_reloc_may_be_function_pointer(Symbol_table* , Layout* , Target_i386* ,
-		  			Sized_relobj<32, false>* ,
-		  			unsigned int ,
-		  			Output_section* ,
-		  			const elfcpp::Rel<32, false>& ,
-	 				unsigned int ,
-		  			const elfcpp::Sym<32, false>&)
-    { return false; }
+    local_reloc_may_be_function_pointer(Symbol_table* symtab, Layout* layout,
+ 					Target_i386* target,
+	  				Sized_relobj<32, false>* object,
+	  				unsigned int data_shndx,
+	  				Output_section* output_section,
+	  				const elfcpp::Rel<32, false>& reloc,
+					unsigned int r_type,
+	  				const elfcpp::Sym<32, false>& lsym);
+
+    inline bool
+    global_reloc_may_be_function_pointer(Symbol_table* symtab, Layout* layout,
+					 Target_i386* target,
+	   				 Sized_relobj<32, false>* object,
+				         unsigned int data_shndx,
+	   				 Output_section* output_section,
+					 const elfcpp::Rel<32, false>& reloc,
+					 unsigned int r_type,
+			   		 Symbol* gsym);
 
     inline bool
-    global_reloc_may_be_function_pointer(Symbol_table* , Layout* ,
-					 Target_i386* ,
-		   			 Sized_relobj<32, false>* ,
-		   			 unsigned int ,
-		   			 Output_section* ,
-		   			 const elfcpp::Rel<32, false>& ,
-					 unsigned int , Symbol*)
-    { return false; }
+    possible_function_pointer_reloc(unsigned int r_type);
 
     static void
     unsupported_reloc_local(Sized_relobj<32, false>*, unsigned int r_type);
@@ -1234,6 +1241,53 @@ Target_i386::Scan::unsupported_reloc_glo
 	     object->name().c_str(), r_type, gsym->demangled_name().c_str());
 }
 
+inline bool
+Target_i386::Scan::possible_function_pointer_reloc(unsigned int r_type)
+{
+  switch (r_type)
+    {
+    case elfcpp::R_386_32:
+    case elfcpp::R_386_16:
+    case elfcpp::R_386_8:
+    case elfcpp::R_386_GOTOFF:
+    case elfcpp::R_386_GOT32:
+      {
+        return true;
+      }
+    }
+  return false;
+}
+
+inline bool
+Target_i386::Scan::local_reloc_may_be_function_pointer(
+  Symbol_table* ,
+  Layout* ,
+  Target_i386* ,
+  Sized_relobj<32, false>* ,
+  unsigned int ,
+  Output_section* ,
+  const elfcpp::Rel<32, false>& ,
+  unsigned int r_type,
+  const elfcpp::Sym<32, false>&)
+{
+  return (possible_function_pointer_reloc(r_type));
+}
+
+inline bool
+Target_i386::Scan::global_reloc_may_be_function_pointer(
+  Symbol_table* ,
+  Layout* ,
+  Target_i386* ,
+  Sized_relobj<32, false>* ,
+  unsigned int ,
+  Output_section* ,
+  const elfcpp::Rel<32, false>& ,
+  unsigned int r_type,
+  Symbol*)
+{
+  return (possible_function_pointer_reloc(r_type));
+}
+
 // Scan a relocation for a global symbol.
 
 inline void
Index: icf.h
===================================================================
RCS file: /cvs/src/src/gold/icf.h,v
retrieving revision 1.6
diff -u -u -p -r1.6 icf.h
--- icf.h	21 Feb 2010 00:57:59 -0000	1.6
+++ icf.h	26 Feb 2010 03:05:01 -0000
@@ -127,6 +127,7 @@ class Icf
     return (parameters->options().icf_safe_folding()
 	    && target->can_check_for_function_pointers()
             && !is_prefix_of(".rodata._ZTV", section_name.c_str())
+	    && !is_prefix_of(".data.rel.ro._ZTV", section_name.c_str())
             && !is_prefix_of(".eh_frame", section_name.c_str()));
   }
 
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/icf_safe_so_test.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_so_test.cc,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_so_test.cc
--- testsuite/icf_safe_so_test.cc	13 Feb 2010 02:04:21 -0000	1.1
+++ testsuite/icf_safe_so_test.cc	26 Feb 2010 03:05:01 -0000
@@ -24,7 +24,8 @@
 // in safe mode correctly folds functions in a shared object. The
 // foo_* functions below should not be folded.  For x86-64,
 // foo_glob and bar_glob should be folded as their function pointers
-// are addresses of PLT entries in shared objects.
+// are addresses of PLT entries in shared objects.  For 32-bit X86,
+// the hidden protected and internal symbols can be folded.
 
 int  __attribute__ ((visibility ("protected")))
 foo_prot()
Index: testsuite/icf_safe_so_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_so_test.sh,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_so_test.sh
--- testsuite/icf_safe_so_test.sh	13 Feb 2010 02:04:21 -0000	1.1
+++ testsuite/icf_safe_so_test.sh	26 Feb 2010 03:05:01 -0000
@@ -25,8 +25,26 @@
 # to verify if identical code folding in safe mode correctly folds
 # functions in a shared object.
 
+error_if_symbol_absent()
+{
+    is_symbol_present $1 $2
+    if [ $? != 0 ];
+    then
+      echo "Symbol" $2 "not present, possibly folded."
+      exit 1
+    fi
+}
+
+is_symbol_present()
+{
+    result=`grep $2 $1`
+    return $?
+}
+
 check_nofold()
 {
+    error_if_symbol_absent $1 $2
+    error_if_symbol_absent $1 $3
     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 ];
@@ -38,6 +56,18 @@ check_nofold()
 
 check_fold()
 {
+    is_symbol_present $1 $2
+    if [ $? != 0 ];
+    then
+      return 0
+    fi
+
+    is_symbol_present $1 $3
+    if [ $? != 0 ];
+    then
+      return 0
+    fi
+
     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 ];
@@ -49,21 +79,31 @@ check_fold()
 
 arch_specific_safe_fold()
 {
-    grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
-    if [ $? == 0 ];
+    if [ $1 == 0 ];
     then
-      check_fold $1 $3 $4
+      check_fold $2 $3 $4
     else
-      check_nofold $1 $3 $4
+      check_nofold $2 $3 $4
     fi
 }
 
-check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_hidden"
-check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_internal"
-check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_static"
-check_nofold icf_safe_so_test_1.stdout "foo_hidden" "foo_internal"
-check_nofold icf_safe_so_test_1.stdout "foo_hidden" "foo_static"
-check_nofold icf_safe_so_test_1.stdout "foo_internal" "foo_static"
-arch_specific_safe_fold icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout \
-  "foo_glob" "bar_glob"
+X86_32_specific_safe_fold()
+{
+    grep_x86_32=`grep -q -e "Intel 80386" $1`
+    arch_specific_safe_fold $? $2 $3 $4
+}
 
+X86_64_specific_safe_fold()
+{
+    grep_x86_64=`grep -q -e "Advanced Micro Devices X86-64" $1`
+    arch_specific_safe_fold $? $2 $3 $4
+}
+
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_hidden"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_internal"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_static"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_hidden" "foo_internal"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_hidden" "foo_static"
+X86_32_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_internal" "foo_static"
+X86_64_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout \
+  "foo_glob" "bar_glob"
Index: testsuite/icf_safe_test.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.cc,v
retrieving revision 1.2
diff -u -u -p -r1.2 icf_safe_test.cc
--- testsuite/icf_safe_test.cc	13 Feb 2010 02:04:21 -0000	1.2
+++ testsuite/icf_safe_test.cc	26 Feb 2010 03:05:01 -0000
@@ -22,10 +22,10 @@
 
 // The goal of this program is to verify if identical code folding
 // in safe mode correctly folds only ctors and dtors. kept_func_1 must
-// not be folded into kept_func_2 other than for X86-64 which can
-// use relocation types to determine if function pointers are taken.
-// kept_func_3 should never be folded as its pointer is taken. The ctor
-// and dtor of class A must be folded.
+// not be folded into kept_func_2 other than for X86 (32 and 64 bit)
+// which can use relocation types to determine if function pointers are
+// taken.  kept_func_3 should never be folded as its pointer is taken.
+// The ctor and dtor of class A must be folded.
 
 class A
 {
Index: testsuite/icf_safe_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.sh,v
retrieving revision 1.2
diff -u -u -p -r1.2 icf_safe_test.sh
--- testsuite/icf_safe_test.sh	13 Feb 2010 02:04:21 -0000	1.2
+++ testsuite/icf_safe_test.sh	26 Feb 2010 03:05:01 -0000
@@ -22,8 +22,8 @@
 
 # The goal of this program is to verify if --icf=safe  works as expected.
 # File icf_safe_test.cc is in this test. This program checks if only
-# ctors and dtors are folded, except for x86-64, which uses relocation
-# types to detect if function pointers are taken.
+# ctors and dtors are folded, except for x86 (32 and 64 bit), which
+# uses relocation types to detect if function pointers are taken.
 
 check_nofold()
 {
@@ -49,7 +49,7 @@ check_fold()
 
 arch_specific_safe_fold()
 {
-    grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
+    grep_x86=`grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" $2`
     if [ $? == 0 ];
     then
       check_fold $1 $3 $4


More information about the Binutils mailing list