2010-02-12 Sriraman Tallam <tmsriram@google.com>

* arm.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * sparc.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * powerpc.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * i386.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. * x86_64.cc (Scan::local_reloc_may_be_function_pointer): New function. (Scan::global_reloc_may_be_function_pointer): New function. (Scan::possible_function_pointer_reloc): New function. (Target_x86_64::can_check_for_function_pointers): 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. (section_has_function_pointers): New function. (set_section_has_function_pointers): New function. (check_section_for_function_pointers): New function. * options.h: Fix comment for safe ICF option. * target.h (can_check_for_function_pointers): 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.
2010-02-13 02:04:21 +00:00 · 2010-02-13 02:04:21 +00:00 · 21bb3914d6
commit 21bb3914d6
parent 6076632b70
17 changed files with 556 additions and 24 deletions
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@ -1,3 +1,38 @@
+2010-02-12  Sriraman Tallam  <tmsriram@google.com>
+
+	* arm.cc (Scan::local_reloc_may_be_function_pointer): New function.
+	(Scan::global_reloc_may_be_function_pointer): New function.
+	* sparc.cc (Scan::local_reloc_may_be_function_pointer): New function.
+	(Scan::global_reloc_may_be_function_pointer): New function.
+	* powerpc.cc (Scan::local_reloc_may_be_function_pointer): New function.
+	(Scan::global_reloc_may_be_function_pointer): New function.
+	* i386.cc (Scan::local_reloc_may_be_function_pointer): New function.
+	(Scan::global_reloc_may_be_function_pointer): New function.
+	* x86_64.cc (Scan::local_reloc_may_be_function_pointer): New function.
+	(Scan::global_reloc_may_be_function_pointer): New function.
+	(Scan::possible_function_pointer_reloc): New function.
+	(Target_x86_64::can_check_for_function_pointers): 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.
+	(section_has_function_pointers): New function.
+	(set_section_has_function_pointers): New function.
+	(check_section_for_function_pointers): New function.
+	* options.h: Fix comment for safe ICF option.
+	* target.h (can_check_for_function_pointers): 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.
+
 2010-02-12  David S. Miller  <davem@davemloft.net>

 	* output.h (Output_reloc<SHT_REL>::Output_reloc): Add
--- a/gold/arm.cc
+++ b/gold/arm.cc
@ -2171,6 +2171,25 @@ class Target_arm : public Sized_target<32, big_endian>
 	   const elfcpp::Rel<32, big_endian>& reloc, unsigned int r_type,
 	   Symbol* gsym);

+    inline bool
+    local_reloc_may_be_function_pointer(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>&)
+    { return false; }
+
+    inline bool
+    global_reloc_may_be_function_pointer(Symbol_table* , Layout* , Target_arm* ,
+  	           			 Sized_relobj<32, big_endian>* ,
+	           			 unsigned int ,
+	           			 Output_section* ,
+	           			 const elfcpp::Rel<32, big_endian>& ,
+					 unsigned int , Symbol*)
+    { return false; }
+
   private:
    static void
    unsupported_reloc_local(Sized_relobj<32, big_endian>*,
--- a/gold/gc.h
+++ b/gold/gc.h
@ -163,18 +163,19 @@ inline void
 gc_process_relocs(
    Symbol_table* symtab,
    Layout*,
-    Target_type* ,
+    Target_type* target,
    Sized_relobj<size, big_endian>* src_obj,
    unsigned int src_indx,
    const unsigned char* prelocs,
    size_t reloc_count,
    Output_section*,
-    bool ,
+    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;
@ -186,8 +187,14 @@ gc_process_relocs(
  bool is_icf_tracked = false;
  const char* cident_section_name = NULL;

+  std::string src_section_name = (parameters->options().icf_enabled()
+                                  ? src_obj->section_name(src_indx)
+                                  : "");
+
+  bool check_section_for_function_pointers = false;
+
  if (parameters->options().icf_enabled()
-      && is_section_foldable_candidate(src_obj->section_name(src_indx).c_str()))
+      && is_section_foldable_candidate(src_section_name.c_str()))
    {
      is_icf_tracked = true;
      Section_id src_id(src_obj, src_indx);
@ -196,11 +203,16 @@ gc_process_relocs(
      addendvec = &symtab->icf()->addend_reloc_list()[src_id];
    }

+  check_section_for_function_pointers =
+    symtab->icf()->check_section_for_function_pointers(src_section_name,
+                                                       target);
+
  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 +237,18 @@ 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 (check_section_for_function_pointers
+              && lsym.get_st_type() != elfcpp::STT_OBJECT
+ 	      && scan.local_reloc_may_be_function_pointer(symtab, NULL, NULL,
+							  src_obj, src_indx,
+			                       		  NULL, reloc, r_type,
+							  lsym))
+            symtab->icf()->set_section_has_function_pointers(
+              src_obj, lsym.get_st_shndx());
+
          if (shndx == src_indx)
            continue;
        }
@ -239,9 +263,21 @@ gc_process_relocs(
          bool is_ordinary;
          dst_obj = gsym->object();
          dst_indx = gsym->shndx(&is_ordinary);
+          Section_id dst_id(dst_obj, dst_indx);
+
+	  // When doing safe folding, check to see if this relocation is that
+	  // of a function pointer being taken.
+	  if (check_section_for_function_pointers
+              && gsym->type() != elfcpp::STT_OBJECT
+              && (!is_ordinary
+                  || scan.global_reloc_may_be_function_pointer(
+                       symtab, NULL, NULL, src_obj, src_indx, NULL, reloc,
+                       r_type, gsym)))
+            symtab->icf()->set_section_has_function_pointers(dst_obj, dst_indx);
+
          if (!is_ordinary)
            continue;
-          Section_id dst_id(dst_obj, dst_indx);
+
          // If the symbol name matches '__start_XXX' then the section with
          // the C identifier like name 'XXX' should not be garbage collected.
          // A similar treatment to symbols with the name '__stop_XXX'.
--- a/gold/i386.cc
+++ b/gold/i386.cc
@ -202,6 +202,26 @@ class Target_i386 : public Target_freebsd<32, false>
 	   const elfcpp::Rel<32, false>& reloc, unsigned int r_type,
 	   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; }
+
+    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; }
+
    static void
    unsupported_reloc_local(Sized_relobj<32, false>*, unsigned int r_type);

--- a/gold/icf.cc
+++ b/gold/icf.cc
@ -101,6 +101,36 @@
 // when folding takes place.  This could lead to unexpected run-time
 // behaviour.
 //
+// Safe Folding :
+// ------------
+//
+// ICF in safe mode folds only ctors and dtors if their function pointers can
+// never be taken.  Also, for X86-64, safe folding uses the relocation
+// type to determine if a function's pointer is taken or not and only folds
+// functions whose pointers are definitely not taken.
+//
+// Caveat with safe folding :
+// ------------------------
+//
+// This applies only to x86_64.
+//
+// Position independent executables are created from PIC objects (compiled
+// with -fPIC) and/or PIE objects (compiled with -fPIE).  For PIE objects, the
+// relocation types for function pointer taken and a call are the same.
+// Now, it is not always possible to tell if an object used in the link of
+// a pie executable is a PIC object or a PIE object.  Hence, for pie
+// executables, using relocation types to disambiguate function pointers is
+// currently disabled.
+//
+// Further, it is not correct to use safe folding to build non-pie
+// executables using PIC/PIE objects.  PIC/PIE objects have different
+// relocation types for function pointers than non-PIC objects, and the
+// current implementation of safe folding does not handle those relocation
+// types.  Hence, if used, functions whose pointers are taken could still be
+// folded causing unpredictable run-time behaviour if the pointers were used
+// in comparisons.
+//
+//
 //
 // How to run  : --icf=[safe|all|none]
 // Optional parameters : --icf-iterations <num> --print-icf-sections
@ -560,6 +590,7 @@ Icf::find_identical_sections(const Input_objects* input_objects,
  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 +608,18 @@ Icf::find_identical_sections(const Input_objects* input_objects,
          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))
+              && !is_function_ctor_or_dtor(mangled_func_name + 1)
+	      && (!target.can_check_for_function_pointers()
+                  || section_has_function_pointers(*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);
--- a/gold/icf.h
+++ b/gold/icf.h
@ -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_()
@ -89,6 +91,36 @@ class Icf
  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
+  section_has_function_pointers(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_has_function_pointers(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.
+  inline bool
+  check_section_for_function_pointers(std::string section_name,
+                                      Target* target)
+  {
+    return (parameters->options().icf_safe_folding()
+	    && target->can_check_for_function_pointers()
+            && !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 +154,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_;
--- a/gold/options.h
+++ b/gold/options.h
@ -914,7 +914,8 @@ 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 ctors, dtors and functions whose"
+                 " pointers are definitely not taken."),
 	      ("[none,all,safe]"),	
              {"none", "all", "safe"});

--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@ -183,6 +183,28 @@ class Target_powerpc : public Sized_target<size, big_endian>
 	   const elfcpp::Rela<size, big_endian>& reloc, unsigned int r_type,
 	   Symbol* gsym);

+    inline bool
+    local_reloc_may_be_function_pointer(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>&)
+    { return false; }
+
+    inline bool
+    global_reloc_may_be_function_pointer(Symbol_table* , Layout* ,
+					 Target_powerpc* ,
+		   			 Sized_relobj<size, big_endian>* ,
+		   			 unsigned int ,
+		   			 Output_section* ,
+		   			 const elfcpp::Rela<size,
+							    big_endian>& ,
+					 unsigned int , Symbol*)
+    { return false; }
+
  private:
    static void
    unsupported_reloc_local(Sized_relobj<size, big_endian>*,
--- a/gold/sparc.cc
+++ b/gold/sparc.cc
@ -193,6 +193,29 @@ class Target_sparc : public Sized_target<size, big_endian>
 	   const elfcpp::Rela<size, big_endian>& reloc, unsigned int r_type,
 	   Symbol* gsym);

+    inline bool
+    local_reloc_may_be_function_pointer(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>&)
+    { return false; }
+
+    inline bool
+    global_reloc_may_be_function_pointer(Symbol_table* , Layout* ,
+			 		 Target_sparc* ,
+		   			 Sized_relobj<size, big_endian>* ,
+		   			 unsigned int ,
+		   			 Output_section* ,
+		   			 const elfcpp::Rela<size,
+	 						    big_endian>& ,
+ 					 unsigned int , Symbol*)
+    { return false; }
+
+
  private:
    static void
    unsupported_reloc_local(Sized_relobj<size, big_endian>*,
--- a/gold/target.h
+++ b/gold/target.h
@ -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
+  can_check_for_function_pointers() const
+  { return false; }
+
  // Return the bit size that this target implements.  This should
  // return 32 or 64.
  int
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@ -170,14 +170,28 @@ icf_keep_unique_test.stdout: icf_keep_unique_test
 	$(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
 	$(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
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@ -61,6 +61,7 @@ check_PROGRAMS = object_unittest$(EXEEXT) binary_unittest$(EXEEXT) \
@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_so_test.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared.sh weak_plt.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	debug_msg.sh undef_symbol.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_1.sh ver_test_2.sh \
@ -86,7 +87,10 @@ check_PROGRAMS = object_unittest$(EXEEXT) binary_unittest$(EXEEXT) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	gc_orphan_section_test.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_test.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_keep_unique_test.stdout \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test.stdout \
+@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_so_test_1.stdout \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_so_test_2.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared.dbg \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	weak_plt_shared.so debug_msg.err \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	debug_msg_so.err \
@ -107,7 +111,7 @@ check_PROGRAMS = object_unittest$(EXEEXT) binary_unittest$(EXEEXT) \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	gc_comdat_test gc_tls_test \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	gc_orphan_section_test icf_test \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_keep_unique_test \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	icf_safe_test icf_safe_so_test \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared.dbg \
@GCC_TRUE@@NATIVE_LINKER_TRUE@	alt/weak_undef_lib.so
@GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_4 = basic_test \
@ -2648,8 +2652,18 @@ uninstall-am:
@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXCOMPILE) -O0 -c -ffunction-sections -g -o $@ $<
@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_test: icf_safe_test.o gcctestdir/ld
@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_test.o
-@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_test.stdout: icf_safe_test
-@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_NM) icf_safe_test > icf_safe_test.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_test_1.stdout: icf_safe_test
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_NM) icf_safe_test > icf_safe_test_1.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_test_2.stdout: icf_safe_test
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) -h icf_safe_test > icf_safe_test_2.stdout
+@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
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_so_test.o -fPIC -shared
+@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_so_test_1.stdout: icf_safe_so_test
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_NM) icf_safe_so_test > icf_safe_so_test_1.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_safe_so_test_2.stdout: icf_safe_so_test
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
@GCC_TRUE@@NATIVE_LINKER_TRUE@basic_test.o: basic_test.cc
@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXCOMPILE) -O0 -c -o $@ $<
@GCC_TRUE@@NATIVE_LINKER_TRUE@basic_test: basic_test.o gcctestdir/ld
--- a/gold/testsuite/icf_safe_so_test.cc
+++ b/gold/testsuite/icf_safe_so_test.cc
@ -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 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;
+}
+
--- a/gold/testsuite/icf_safe_so_test.sh
+++ b/gold/testsuite/icf_safe_so_test.sh
@ -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"
+
--- a/gold/testsuite/icf_safe_test.cc
+++ b/gold/testsuite/icf_safe_test.cc
@ -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 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;
 }
--- a/gold/testsuite/icf_safe_test.sh
+++ b/gold/testsuite/icf_safe_test.sh
@ -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"
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@ -39,6 +39,7 @@
 #include "tls.h"
 #include "freebsd.h"
 #include "gc.h"
+#include "icf.h"

 namespace
 {
@ -69,6 +70,16 @@ class Target_x86_64 : public Target_freebsd<64, false>
      tls_base_symbol_defined_(false)
  { }

+  // This function should be defined in targets that can use relocation
+  // types to determine (implemented in local_reloc_may_be_function_pointer
+  // 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.
+  inline bool
+  can_check_for_function_pointers() const
+  { return !parameters->options().pie(); }
+
  // Hook for a new output section.
  void
  do_new_output_section(Output_section*) const;
@ -224,6 +235,26 @@ class Target_x86_64 : public Target_freebsd<64, false>
 	   const elfcpp::Rela<64, false>& reloc, unsigned int r_type,
 	   Symbol* gsym);

+    inline bool
+    local_reloc_may_be_function_pointer(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 bool
+    global_reloc_may_be_function_pointer(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 +266,9 @@ class Target_x86_64 : public Target_freebsd<64, false>
    void
    check_non_pic(Relobj*, unsigned int r_type);

+    inline bool
+    possible_function_pointer_reloc(unsigned int r_type);
+
    // Whether we have issued an error about a non-PIC compilation.
    bool issued_non_pic_error_;
  };
@ -1364,6 +1398,76 @@ Target_x86_64::Scan::unsupported_reloc_global(Sized_relobj<64, false>* object,
 	     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::possible_function_pointer_reloc(unsigned int r_type)
+{
+  if (parameters->options().shared())
+    return false;
+
+  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 bool
+Target_x86_64::Scan::local_reloc_may_be_function_pointer(
+  Symbol_table* ,
+  Layout* ,
+  Target_x86_64* ,
+  Sized_relobj<64, false>* ,
+  unsigned int ,
+  Output_section* ,
+  const elfcpp::Rela<64, false>& ,
+  unsigned int r_type,
+  const elfcpp::Sym<64, 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));
+}
+
+// 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 bool
+Target_x86_64::Scan::global_reloc_may_be_function_pointer(
+  Symbol_table*,
+  Layout* ,
+  Target_x86_64* ,
+  Sized_relobj<64, false>* ,
+  unsigned int ,
+  Output_section* ,
+  const elfcpp::Rela<64, false>& ,
+  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));
+}
+
 // Scan a relocation for a global symbol.

 inline void