Safe Identical Code Folding for X86-64.

Sriraman Tallam tmsriram@google.com
Fri Jan 22 00:52:00 GMT 2010


Hi,

     I am implementing a safe ICF option for gold that looks at
relocation types and tries to figure out if it is a function pointer
taken versus a function call. Please recall that ICF is not safe when
function pointer checks are present in the code. So, with safe ICF, I
try to fold only functions that are guaranteed to not have their
function pointer taken other than for vtable purposes. Currently, safe
ICF only folds ctors and dtors as their pointers can never be taken.

I have tried to do this for AMD X86-64. Here are the observations I
have used to write the patch. Most of this stuff should be familiar to
you all. I have also attached the patch.

Case (i) : For position dependent code (non-PIC),  there is no
problem. A function call is always a PC relative relocation and a
function pointer is a direct relocation.

Here is an example :

test.cc :

int foo()
{
 return 1;
}

int bar()
{
 int (*p)() = foo;
 p();
}

$ g++ -c test.cc
$ readelf --relocs test.o

Relocation section '.rela.text._Z3barv' at offset 0x660 contains 2 entries:
 Offset          Info           Type           Sym. Value    Sym. Name + Addend
00000000000c  000a0000000b R_X86_64_32S      0000000000000000 _Z3foov + 0
000000000011  000a00000002 R_X86_64_PC32     0000000000000000 _Z3foov
+ fffffffffffffffc


R_X86_64_32S is the relocation type of the pointer and R_X86_64_PC32
is the relocation type of the call. Hence, I can look at the
relocation type and exclude any function from being folded if there is
a direct relocation against it.

Case (ii) : Shared Libraries.

For shared libraries, in x86_64, they *have to be built in -fPIC mode.
Further, for shared libraries, any function whose symbol is exported
will get a PLT entry. Hence, the function pointers for these functions
will be the PLT entry's address and not the real function address.
Hence, it is always safe to fold functions whose symbols are exported
as their pointers *can never be equal*. I do not need any relocation
based disambiguation here.

Here is an example :

t.cc

int foo()
{
 return 1;
}

int bar()
{
 return 1;
}

$ g++ -fPIC -shared t.cc -o libt.so -Wl,--icf
$ nm t.so

000000000000025c T _Z3barv
000000000000025c T _Z3foov

Notice that foo and bar have been folded into one function.

Now,

main.cc

extern int foo();
extern int bar();

int main()
{
 if (foo == bar)
   printf("equal\n");
 else
   printf("not equal\n");
}

$ g++ main.cc -L./ -lt
$ a.out
not equal

However, for shared libraries; protected, hidden, internal symbols and
static functions cause a problem. They dont have a PLT entry and their
relocations for call and pointers are the same. So, safe ICF will
assume that these can never be folded. Fine so far.

Case (iii) Position-independent executables.

This is a monster. AFAIU, PIE is nothing but PIC code that is
executable. However, the function pointer for a function whose symbol
is exported is the address of the function, not the PLT entry. There
is no PLT entry for exported functions because they cannot be
over-ridden.

Further, It can be created in two ways as far as I know.

Sub-case 1) The individual objects can be compiled with -fPIC and then
linked into an executable.

In this case, the relocations for pointer taken versus a function call
are *still* different. The function pointer has a GOTPCREL relocation
whereas a function call has a PC relative relocation. Hence, we can
differentiate and do a safe folding, again accounting for those
special symbols (hidden, protected, internal and static functions.)


Sub-case 2) The individual objects can be compiled with -fPIE and then
linked into an executable.

In this case, the relocations for function pointer taken versus a call
are the same. From what I understood, the difference between -fPIC
objects and -fPIE objects is that objects compiled with -fPIE can
never go into a shared object hence the function pointer relocations
never have to be GOT relative.

Hence, for PIE executables, safe ICF defaults to folding only ctors and dtors.

Case (iv) Building non-PIE binaries using PIE objects.

Safe ICF should not be used in this case. I have a plan to record the
build type of each object in a separate section that can be used by
the linker to decide what kind of safety is possible. Till then,
building non-PIE binaries with PIC/PIE objects is wrong.


Here is the patch to implement really safe ICF for X86-64. Some
experiments I have conducted show that safe ICF for X86-64 is 98 % as
effective as full ICF in reducing code size with the safety guarantee
against function pointer checks.

Please let me know what you think ?


2010-01-21  Sriraman Tallam  <tmsriram@google.com>

	* arm.cc (Scan::local_for_icf): New function.
	(Scan::global_for_icf): New function.
	* sparc.cc (Scan::local_for_icf): New function.
	(Scan::global_for_icf): New function.
	* powerpc.cc (Scan::local_for_icf): New function.
	(Scan::global_for_icf): New function.
	* i386.cc (Scan::local_for_icf): New function.
	(Scan::global_for_icf): New function.
	* x86_64.cc (Scan::local_for_icf): New function.
	(Scan::global_for_icf): New function.
	(Scan::is_non_pic_reloc_type_func_ptr): New function.
	* gc.h (gc_process_relocs): Scan relocation types to determine if
        function pointers were taken for targets that support it.
	* icf.cc (Icf::find_identical_sections): Include functions for
	folding in safe ICF whose pointer is not taken.
	* icf.h (Secn_fptr_taken_set): New typedef.
	(fptr_section_id_): New member.
	(is_section_fptr_taken): New function.
	(set_section_fptr_taken): New function.
	(check_section_for_fptr_taken): New function.
	* options.h: Fix comment for safe ICF option.
	* target.h (is_fptr_taken_checked): New function.
	* testsuite/Makefile.am: Add icf_safe_so_test test case.
	Modify icf_safe_test for X86-64.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/icf_safe_so_test.cc: New file.
	* testsuite/icf_safe_so_test.sh: New file.
	* testsuite/icf_safe_test.cc (kept_func_3): New function.
	(main): Change to take pointer to function kept_func_3.
	* testsuite/icf_safe_test.sh (arch_specific_safe_fold): Check if safe
	folding is done correctly for X86-64.

Thanks,
-Sriraman.
-------------- next part --------------
Index: arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.59
diff -u -u -p -r1.59 arm.cc
--- arm.cc	20 Jan 2010 17:29:52 -0000	1.59
+++ arm.cc	22 Jan 2010 00:18:11 -0000
@@ -1873,6 +1873,24 @@ class Target_arm : public Sized_target<3
 	   const elfcpp::Rel<32, big_endian>& reloc, unsigned int r_type,
 	   Symbol* gsym);
 
+    inline void
+    local_for_icf(Symbol_table* , Layout* , Target_arm* ,
+  	          Sized_relobj<32, big_endian>* ,
+        	  unsigned int ,
+  	          Output_section* ,
+	          const elfcpp::Rel<32, big_endian>& , unsigned int ,
+ 	          const elfcpp::Sym<32, big_endian>& )
+    { }
+
+    inline void
+    global_for_icf(Symbol_table* , Layout* , Target_arm* ,
+  	           Sized_relobj<32, big_endian>* ,
+	           unsigned int ,
+	           Output_section* ,
+	           const elfcpp::Rel<32, big_endian>& , unsigned int ,
+  	           Symbol* )
+    { }
+
    private:
     static void
     unsupported_reloc_local(Sized_relobj<32, big_endian>*,
Index: gc.h
===================================================================
RCS file: /cvs/src/src/gold/gc.h,v
retrieving revision 1.9
diff -u -u -p -r1.9 gc.h
--- gc.h	20 Jan 2010 17:29:52 -0000	1.9
+++ gc.h	22 Jan 2010 00:18:11 -0000
@@ -162,19 +162,20 @@ template<int size, bool big_endian, type
 inline void
 gc_process_relocs(
     Symbol_table* symtab,
-    Layout*,
-    Target_type* ,
+    Layout* ,
+    Target_type* target,
     Sized_relobj<size, big_endian>* src_obj,
     unsigned int src_indx,
     const unsigned char* prelocs,
     size_t reloc_count,
-    Output_section*,
+    Output_section* ,
     bool ,
     size_t local_count,
     const unsigned char* plocal_syms)
 {
   Object *dst_obj;
   unsigned int dst_indx;
+  Scan scan;
 
   typedef typename Reloc_types<sh_type, size, big_endian>::Reloc Reltype;
   const int reloc_size = Reloc_types<sh_type, size, big_endian>::reloc_size;
@@ -185,6 +186,7 @@ gc_process_relocs(
   std::vector<std::pair<long long, long long> >* addendvec = NULL;
   bool is_icf_tracked = false;
   const char* cident_section_name = NULL;
+  std::string src_section_name;
 
   if (parameters->options().icf_enabled()
       && is_section_foldable_candidate(src_obj->section_name(src_indx).c_str()))
@@ -196,11 +198,19 @@ gc_process_relocs(
       addendvec = &symtab->icf()->addend_reloc_list()[src_id];
     }
 
+  if (parameters->options().icf_enabled()
+      && parameters->options().icf_safe_folding()
+      && target->is_fptr_taken_checked())
+    {
+      src_section_name = src_obj->section_name(src_indx);
+    }
+
   for (size_t i = 0; i < reloc_count; ++i, prelocs += reloc_size)
     {
       Reltype reloc(prelocs);
       typename elfcpp::Elf_types<size>::Elf_WXword r_info = reloc.get_r_info();
       unsigned int r_sym = elfcpp::elf_r_sym<size>(r_info);
+      unsigned int r_type = elfcpp::elf_r_type<size>(r_info);
       typename elfcpp::Elf_types<size>::Elf_Swxword addend =
       Reloc_types<sh_type, size, big_endian>::get_reloc_addend_noerror(&reloc);
 
@@ -225,6 +235,15 @@ gc_process_relocs(
               (*addendvec).push_back(std::make_pair(symvalue,
                                               static_cast<long long>(addend)));
             }
+	  // When doing safe folding, check to see if this relocation is that
+	  // of a function pointer being taken.
+	  if (parameters->options().icf_enabled()
+              && symtab->icf()->check_section_for_fptr_taken(src_section_name,
+                                                             target))
+            {
+ 	      scan.local_for_icf(symtab, NULL, NULL, src_obj, src_indx,
+	 	                 NULL, reloc, r_type, lsym);
+            }
           if (shndx == src_indx)
             continue;
         }
@@ -265,6 +284,15 @@ gc_process_relocs(
                         static_cast<long long>(sized_gsym->value());
               (*addendvec).push_back(std::make_pair(symvalue,
                                         static_cast<long long>(addend)));
+	    }
+	  // When doing safe folding, check to see if this relocation is that
+	  // of a function pointer being taken.
+	  if (parameters->options().icf_enabled()
+              && symtab->icf()->check_section_for_fptr_taken(src_section_name,
+                                                             target))
+            {
+              scan.global_for_icf(symtab, NULL, NULL, src_obj, src_indx,
+		                  NULL, reloc, r_type, gsym);
             }
         }
       if (parameters->options().gc_sections())
Index: i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.111
diff -u -u -p -r1.111 i386.cc
--- i386.cc	8 Jan 2010 19:33:17 -0000	1.111
+++ i386.cc	22 Jan 2010 00:18:11 -0000
@@ -202,6 +202,24 @@ class Target_i386 : public Target_freebs
 	   const elfcpp::Rel<32, false>& reloc, unsigned int r_type,
 	   Symbol* gsym);
 
+    inline void
+    local_for_icf(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>& )
+    { }
+
+    inline void
+    global_for_icf(Symbol_table* , Layout* , Target_i386* ,
+		   Sized_relobj<32, false>* ,
+		   unsigned int ,
+		   Output_section* ,
+		   const elfcpp::Rel<32, false>& , unsigned int ,
+		   Symbol* )
+    { }
+
     static void
     unsupported_reloc_local(Sized_relobj<32, false>*, unsigned int r_type);
 
Index: icf.cc
===================================================================
RCS file: /cvs/src/src/gold/icf.cc,v
retrieving revision 1.7
diff -u -u -p -r1.7 icf.cc
--- icf.cc	4 Jan 2010 19:08:39 -0000	1.7
+++ icf.cc	22 Jan 2010 00:18:11 -0000
@@ -101,6 +101,19 @@
 // when folding takes place.  This could lead to unexpected run-time
 // behaviour.
 //
+// Safe Folding :
+// ------------
+//
+// ICF in safe mode, folds only ctors and dtors as their function pointers can
+// never be taken.  Also, for AMD X86-64, safe folding uses the relocation
+// type to determine if a function's pointer was taken or not and only folds
+// functions whose pointers are definitely not taken.
+//
+// For X86-64, it is not correct to use safe folding to build non-pie
+// executables using PIC/PIE objects.  This can cause the resulting binary
+// to have unexpected run-time behaviour in the presence of pointer
+// comparisons.
+//
 //
 // How to run  : --icf=[safe|all|none]
 // Optional parameters : --icf-iterations <num> --print-icf-sections
@@ -535,7 +548,7 @@ match_sections(unsigned int iteration_nu
 // This function returns true if the mangled function name is a ctor or a
 // dtor.
 
-static bool
+bool
 is_function_ctor_or_dtor(const char* mangled_func_name)
 {
   if ((is_prefix_of("_ZN", mangled_func_name)
@@ -560,6 +573,7 @@ Icf::find_identical_sections(const Input
   std::vector<unsigned int> num_tracked_relocs;
   std::vector<bool> is_secn_or_group_unique;
   std::vector<std::string> section_contents;
+  const Target& target = parameters->target();
 
   // Decide which sections are possible candidates first.
 
@@ -577,14 +591,18 @@ Icf::find_identical_sections(const Input
           if (parameters->options().gc_sections()
               && symtab->gc()->is_section_garbage(*p, i))
               continue;
+	  const char* mangled_func_name = strrchr(section_name, '.');
+	  gold_assert(mangled_func_name != NULL);
 	  // With --icf=safe, check if the mangled function name is a ctor
 	  // or a dtor.  The mangled function name can be obtained from the
 	  // section name by stripping the section prefix.
-	  const char* mangled_func_name = strrchr(section_name, '.');
-	  gold_assert(mangled_func_name != NULL);
 	  if (parameters->options().icf_safe_folding()
-	      && !is_function_ctor_or_dtor(mangled_func_name + 1))
-	    continue;
+              && !is_function_ctor_or_dtor(mangled_func_name + 1)
+	      && (target.is_fptr_taken_checked() == false
+                  || is_section_fptr_taken(*p, i)))
+            {
+	      continue;
+            }
           this->id_section_.push_back(Section_id(*p, i));
           this->section_id_[Section_id(*p, i)] = section_num;
           this->kept_section_id_.push_back(section_num);
Index: icf.h
===================================================================
RCS file: /cvs/src/src/gold/icf.h,v
retrieving revision 1.4
diff -u -u -p -r1.4 icf.h
--- icf.h	20 Jan 2010 17:29:52 -0000	1.4
+++ icf.h	22 Jan 2010 00:18:11 -0000
@@ -50,9 +50,11 @@ class Icf
   typedef Unordered_map<Section_id,
                         unsigned int,
                         Section_id_hash> Uniq_secn_id_map;
+  typedef Unordered_set<Section_id, Section_id_hash> Secn_fptr_taken_set;
 
   Icf()
   : id_section_(), section_id_(), kept_section_id_(),
+    fptr_section_id_(),
     num_tracked_relocs(NULL), icf_ready_(false),
     section_reloc_list_(), symbol_reloc_list_(),
     addend_reloc_list_()
@@ -88,7 +90,36 @@ class Icf
   // given section.
   bool
   is_section_folded(Object* obj, unsigned int shndx);
-    
+
+  // Given an object and a section index, this returns true if the
+  // pointer of the function defined in this section is taken.
+  bool
+  is_section_fptr_taken(Object *obj, unsigned int shndx)
+  {
+    return (this->fptr_section_id_.find(Section_id(obj, shndx))
+            != this->fptr_section_id_.end());
+  }
+
+  // Records that a pointer of the function defined in this section
+  // is taken.
+  void
+  set_section_fptr_taken(Object *obj, unsigned int shndx)
+  {
+    this->fptr_section_id_.insert(Section_id(obj, shndx));
+  }
+
+  // Checks if the section_name should be searched for relocs
+  // corresponding to taken function pointers.  Ignores eh_frame
+  // and vtable sections.
+  bool
+  check_section_for_fptr_taken(std::string section_name, Target* target)
+  {
+    return (parameters->options().icf_safe_folding()
+	    && target->is_fptr_taken_checked()
+            && !is_prefix_of(".rodata._ZTV", section_name.c_str())
+            && !is_prefix_of(".eh_frame", section_name.c_str()));
+  }
+
   // Returns a map of a section to a list of all sections referenced
   // by its relocations.
   Section_list&
@@ -122,6 +153,10 @@ class Icf
   // section.  If the id's are the same then this section is
   // not folded.
   std::vector<unsigned int> kept_section_id_;
+  // Given a section id, this says if the pointer to this
+  // function is taken in which case it is dangerous to fold
+  // this function.
+  Secn_fptr_taken_set fptr_section_id_;
   unsigned int* num_tracked_relocs;
   // Flag to indicate if ICF has been run.
   bool icf_ready_;
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.138
diff -u -u -p -r1.138 options.h
--- options.h	15 Jan 2010 04:58:34 -0000	1.138
+++ options.h	22 Jan 2010 00:18:11 -0000
@@ -905,7 +905,10 @@ class General_options
 
   DEFINE_enum(icf, options::TWO_DASHES, '\0', "none",
               N_("Identical Code Folding. "
-                 "\'--icf=safe\' folds only ctors and dtors."),
+                 "\'--icf=safe\' folds only ctors and dtors.  "
+                 "For X86-64, also folds functions whose pointers are "
+                 "definitely not taken.  For X86-64, do not use safe "
+                 "ICF to build non-pie executables with pic objects."),
 	      ("[none,all,safe]"),	
               {"none", "all", "safe"});
 
Index: powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.27
diff -u -u -p -r1.27 powerpc.cc
--- powerpc.cc	7 Jan 2010 20:43:35 -0000	1.27
+++ powerpc.cc	22 Jan 2010 00:18:11 -0000
@@ -183,6 +183,24 @@ class Target_powerpc : public Sized_targ
 	   const elfcpp::Rela<size, big_endian>& reloc, unsigned int r_type,
 	   Symbol* gsym);
 
+    inline void
+    local_for_icf(Symbol_table* , Layout* , Target_powerpc* ,
+	          Sized_relobj<size, big_endian>* ,
+	          unsigned int ,
+	          Output_section* ,
+	          const elfcpp::Rela<size, big_endian>& , unsigned int ,
+	          const elfcpp::Sym<size, big_endian>& )
+    { }
+
+    inline void
+    global_for_icf(Symbol_table* , Layout* , Target_powerpc* ,
+		   Sized_relobj<size, big_endian>* ,
+		   unsigned int ,
+		   Output_section* ,
+		   const elfcpp::Rela<size, big_endian>& , unsigned int ,
+		   Symbol* )
+    { }
+
   private:
     static void
     unsupported_reloc_local(Sized_relobj<size, big_endian>*,
Index: sparc.cc
===================================================================
RCS file: /cvs/src/src/gold/sparc.cc,v
retrieving revision 1.31
diff -u -u -p -r1.31 sparc.cc
--- sparc.cc	7 Jan 2010 20:43:35 -0000	1.31
+++ sparc.cc	22 Jan 2010 00:18:11 -0000
@@ -193,6 +193,25 @@ class Target_sparc : public Sized_target
 	   const elfcpp::Rela<size, big_endian>& reloc, unsigned int r_type,
 	   Symbol* gsym);
 
+    inline void
+    local_for_icf(Symbol_table* , Layout* , Target_sparc* ,
+	          Sized_relobj<size, big_endian>* ,
+       	          unsigned int ,
+	          Output_section* ,
+	          const elfcpp::Rela<size, big_endian>& , unsigned int ,
+	          const elfcpp::Sym<size, big_endian>& )
+    { }
+
+    inline void
+    global_for_icf(Symbol_table* , Layout* , Target_sparc* ,
+		   Sized_relobj<size, big_endian>* ,
+		   unsigned int ,
+		   Output_section* ,
+		   const elfcpp::Rela<size, big_endian>& , unsigned int ,
+		   Symbol* )
+    { }
+
+
   private:
     static void
     unsupported_reloc_local(Sized_relobj<size, big_endian>*,
Index: target.h
===================================================================
RCS file: /cvs/src/src/gold/target.h,v
retrieving revision 1.43
diff -u -u -p -r1.43 target.h
--- target.h	8 Jan 2010 19:33:18 -0000	1.43
+++ target.h	22 Jan 2010 00:18:11 -0000
@@ -64,6 +64,13 @@ class Target
   virtual ~Target()
   { }
 
+  // Virtual function which is set to return true by a target if
+  // it can use relocation types to determine if a function's
+  // pointer is taken.
+  virtual bool
+  is_fptr_taken_checked() const
+  { return false; }
+
   // Return the bit size that this target implements.  This should
   // return 32 or 64.
   int
Index: x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.104
diff -u -u -p -r1.104 x86_64.cc
--- x86_64.cc	8 Jan 2010 19:33:18 -0000	1.104
+++ x86_64.cc	22 Jan 2010 00:18:12 -0000
@@ -39,6 +39,7 @@
 #include "tls.h"
 #include "freebsd.h"
 #include "gc.h"
+#include "icf.h"
 
 namespace
 {
@@ -69,6 +70,15 @@ class Target_x86_64 : public Target_free
       tls_base_symbol_defined_(false)
   { }
 
+  // This function should be defined in targets that can use relocation
+  // types to determine (implemented in local_for_icf and global_for_icf)
+  // 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.
+  inline bool
+  is_fptr_taken_checked() const
+  { return !parameters->options().pie(); } 
+
   // Hook for a new output section.
   void
   do_new_output_section(Output_section*) const;
@@ -224,6 +234,22 @@ class Target_x86_64 : public Target_free
 	   const elfcpp::Rela<64, false>& reloc, unsigned int r_type,
 	   Symbol* gsym);
 
+    inline void
+    local_for_icf(Symbol_table* symtab, Layout* layout, Target_x86_64* target,
+	  	  Sized_relobj<64, false>* object,
+		  unsigned int data_shndx,
+		  Output_section* output_section,
+		  const elfcpp::Rela<64, false>& reloc, unsigned int r_type,
+		  const elfcpp::Sym<64, false>& lsym);
+
+    inline void
+    global_for_icf(Symbol_table* symtab, Layout* layout, Target_x86_64* target,
+         	   Sized_relobj<64, false>* object,
+        	   unsigned int data_shndx,
+		   Output_section* output_section,
+		   const elfcpp::Rela<64, false>& reloc, unsigned int r_type,
+		   Symbol* gsym);
+
   private:
     static void
     unsupported_reloc_local(Sized_relobj<64, false>*, unsigned int r_type);
@@ -235,6 +261,9 @@ class Target_x86_64 : public Target_free
     void
     check_non_pic(Relobj*, unsigned int r_type);
 
+    inline bool
+    is_non_pic_reloc_type_func_ptr(unsigned int r_type);
+
     // Whether we have issued an error about a non-PIC compilation.
     bool issued_non_pic_error_;
   };
@@ -1364,6 +1393,94 @@ Target_x86_64::Scan::unsupported_reloc_g
 	     object->name().c_str(), r_type, gsym->demangled_name().c_str());
 }
 
+// Returns true if this relocation type could be that of a function pointer
+// only if the target is not position-independent code.
+inline bool
+Target_x86_64::Scan::is_non_pic_reloc_type_func_ptr(unsigned int r_type)
+{
+  gold_assert (!parameters->options().shared()
+       	       && !parameters->options().pie());
+  switch (r_type)
+    {
+    case elfcpp::R_X86_64_64:
+    case elfcpp::R_X86_64_32:
+    case elfcpp::R_X86_64_32S:
+    case elfcpp::R_X86_64_16:
+    case elfcpp::R_X86_64_8:
+      {
+        return true;
+      }
+    }
+  return false;
+}
+
+// For safe ICF, scan a relocation for a local symbol to check if it
+// corresponds to a function pointer being taken.  In that case mark
+// the function whose pointer was taken as not foldable.
+
+inline void
+Target_x86_64::Scan::local_for_icf(Symbol_table* symtab,
+	                           Layout* ,
+        	                   Target_x86_64* ,
+                	           Sized_relobj<64, false>* object,
+                        	   unsigned int ,
+	                           Output_section* ,
+        	                   const elfcpp::Rela<64, false>& ,
+                	           unsigned int r_type,
+                        	   const elfcpp::Sym<64, false>& lsym)
+{
+  // 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.
+  if (parameters->options().shared())
+    {
+      symtab->icf()->set_section_fptr_taken(object,
+  	                      		    lsym.get_st_shndx());
+      return; 
+    }
+ 
+  if (is_non_pic_reloc_type_func_ptr(r_type))
+    symtab->icf()->set_section_fptr_taken(object, lsym.get_st_shndx());
+}
+
+// For safe ICF, scan a relocation for a global symbol to check if it
+// corresponds to a function pointer being taken.  In that case mark
+// the function whose pointer was taken as not foldable.
+
+inline void
+Target_x86_64::Scan::global_for_icf(Symbol_table* symtab,
+                            	    Layout* ,
+                            	    Target_x86_64* ,
+                            	    Sized_relobj<64, false>* ,
+                            	    unsigned int ,
+                            	    Output_section* ,
+                            	    const elfcpp::Rela<64, false>& ,
+                            	    unsigned int r_type,
+                            	    Symbol* gsym)
+{
+  if (parameters->options().shared())
+    {
+      // When building a shared library, do not fold symbols whose visibility
+      // is hidden, internal or protected. 
+      if (gsym->visibility() == elfcpp::STV_INTERNAL
+	  || gsym->visibility() == elfcpp::STV_PROTECTED
+	  || gsym->visibility() == elfcpp::STV_HIDDEN)
+        {
+	  bool is_ordinary;
+          symtab->icf()->set_section_fptr_taken(gsym->object(),
+		                      		gsym->shndx(&is_ordinary));
+        }
+      return;
+    }
+
+  if (is_non_pic_reloc_type_func_ptr(r_type))
+    {
+      bool is_ordinary;
+      symtab->icf()->set_section_fptr_taken(gsym->object(),
+   	                        	    gsym->shndx(&is_ordinary));
+    }
+}
+
 // Scan a relocation for a global symbol.
 
 inline void
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.118
diff -u -u -p -r1.118 Makefile.am
--- testsuite/Makefile.am	12 Jan 2010 19:12:40 -0000	1.118
+++ testsuite/Makefile.am	22 Jan 2010 00:18:12 -0000
@@ -170,14 +170,28 @@ icf_keep_unique_test.stdout: icf_keep_un
 	$(TEST_NM) -C icf_keep_unique_test > icf_keep_unique_test.stdout
 
 check_SCRIPTS += icf_safe_test.sh
-check_DATA += icf_safe_test.stdout
+check_DATA += icf_safe_test_1.stdout icf_safe_test_2.stdout
 MOSTLYCLEANFILES += icf_safe_test
-icf_safe_test.o: icf_safe_test.cc 
+icf_safe_test.o: icf_safe_test.cc
 	$(CXXCOMPILE) -O0 -c -ffunction-sections -g -o $@ $<
 icf_safe_test: icf_safe_test.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_test.o
-icf_safe_test.stdout: icf_safe_test
-	$(TEST_NM) icf_safe_test > icf_safe_test.stdout
+icf_safe_test_1.stdout: icf_safe_test
+	$(TEST_NM) icf_safe_test > icf_safe_test_1.stdout
+icf_safe_test_2.stdout: icf_safe_test
+	$(TEST_READELF) -h icf_safe_test > icf_safe_test_2.stdout
+
+check_SCRIPTS += icf_safe_so_test.sh
+check_DATA += icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout
+MOSTLYCLEANFILES += icf_safe_so_test
+icf_safe_so_test.o: icf_safe_so_test.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections -fPIC -g -o $@ $<
+icf_safe_so_test: icf_safe_so_test.o gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_so_test.o -fPIC -shared
+icf_safe_so_test_1.stdout: icf_safe_so_test
+	$(TEST_NM) icf_safe_so_test > icf_safe_so_test_1.stdout
+icf_safe_so_test_2.stdout: icf_safe_so_test
+	$(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
 
 check_PROGRAMS += basic_test
 check_PROGRAMS += basic_static_test
Index: testsuite/icf_safe_so_test.cc
===================================================================
RCS file: testsuite/icf_safe_so_test.cc
diff -N testsuite/icf_safe_so_test.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/icf_safe_so_test.cc	22 Jan 2010 00:18:12 -0000
@@ -0,0 +1,73 @@
+// icf_safe_so_test.cc -- a test case for gold
+
+// Copyright 2010 Free Software Foundation, Inc.
+// Written by Sriraman Tallam <tmsriram@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 identical code folding
+// in safe mode correctly folds functions in a shared object. The
+// foo_* functions below should not be folded.  For AMD x86-64,
+// foo_glob and bar_glob should be folded as their function pointers
+// are addresses of PLT entries in shared objects.
+
+int  __attribute__ ((visibility ("protected")))
+foo_prot()
+{
+  return 1;
+}
+
+int __attribute__ ((visibility ("hidden")))
+foo_hidden()
+{
+  return 1;
+}
+
+int __attribute__ ((visibility ("internal")))
+foo_internal()
+{
+  return 1;
+}
+
+static int
+foo_static()
+{
+  return 1;
+}
+
+int foo_glob()
+{
+  return 2;
+}
+
+int bar_glob()
+{
+  return 2;
+}
+
+int main()
+{
+  int (*p)() = foo_glob;
+  (void)p;
+  foo_static();
+  foo_prot();
+  foo_hidden();
+  foo_internal();
+  return 0;
+}
+
Index: testsuite/icf_safe_so_test.sh
===================================================================
RCS file: testsuite/icf_safe_so_test.sh
diff -N testsuite/icf_safe_so_test.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/icf_safe_so_test.sh	22 Jan 2010 00:18:12 -0000
@@ -0,0 +1,69 @@
+# icf_safe_so_test.sh -- test --icf=safe
+
+# Copyright 2010 Free Software Foundation, Inc.
+# Written by Sriraman Tallam <tmsriram@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 as expected.
+# File icf_safe_so_test.cc is in this test.  The goal of this script is
+# to verify if identical code folding in safe mode correctly folds
+# functions in a shared object.
+
+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()
+{
+    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 did not fold " $2 "and" $3
+	exit 1
+    fi
+}
+
+arch_specific_safe_fold()
+{
+    grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
+    if [ $? == 0 ];
+    then
+      check_fold $1 $3 $4
+    else
+      check_nofold $1 $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"
+
Index: testsuite/icf_safe_test.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.cc,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_test.cc
--- testsuite/icf_safe_test.cc	13 Oct 2009 21:17:43 -0000	1.1
+++ testsuite/icf_safe_test.cc	22 Jan 2010 00:18:12 -0000
@@ -22,8 +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.  The ctor and dtor of class A must
-// be folded.
+// not be folded into kept_func_2 other than for AMD 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.
 
 class A
 {
@@ -48,7 +50,14 @@ int kept_func_2()
   return 1;
 }
 
+int kept_func_3()
+{
+  return 1;
+}
+
 int main()
 {
+  int (*p)() = kept_func_3;
+  p();
   return 0;
 }
Index: testsuite/icf_safe_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.sh,v
retrieving revision 1.1
diff -u -u -p -r1.1 icf_safe_test.sh
--- testsuite/icf_safe_test.sh	13 Oct 2009 21:17:43 -0000	1.1
+++ testsuite/icf_safe_test.sh	22 Jan 2010 00:18:12 -0000
@@ -22,7 +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.
+# ctors and dtors are folded, except for x86-64, which uses relocation
+# types to detect if function pointers are taken.
 
 check_nofold()
 {
@@ -46,5 +47,19 @@ check_fold()
     fi
 }
 
-check_nofold icf_safe_test.stdout "kept_func_1" "kept_func_2"
-check_fold   icf_safe_test.stdout "_ZN1AD1Ev" "_ZN1AC1Ev"
+arch_specific_safe_fold()
+{
+    grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
+    if [ $? == 0 ];
+    then
+      check_fold $1 $3 $4
+    else
+      check_nofold $1 $3 $4
+    fi
+}
+
+arch_specific_safe_fold icf_safe_test_1.stdout icf_safe_test_2.stdout \
+ "kept_func_1" "kept_func_2"
+check_fold   icf_safe_test_1.stdout "_ZN1AD1Ev" "_ZN1AC1Ev"
+check_nofold icf_safe_test_1.stdout "kept_func_3" "kept_func_1"
+check_nofold icf_safe_test_1.stdout "kept_func_3" "kept_func_2"


More information about the Binutils mailing list