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.

17 files changed, 556 insertions, 24 deletions

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 936d0b9..eb98592 100644
--- 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
diff --git a/gold/arm.cc b/gold/arm.cc
index d40ab2c..bdbdbd1 100644
--- 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>*,
diff --git a/gold/gc.h b/gold/gc.h
index 1ffb590..d4fd02e 100644
--- 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'.
@@ -265,7 +301,7 @@ gc_process_relocs(
                         static_cast<long long>(sized_gsym->value());
               (*addendvec).push_back(std::make_pair(symvalue,
                                         static_cast<long long>(addend)));
-            }
+	    }
         }
       if (parameters->options().gc_sections())
         {
diff --git a/gold/i386.cc b/gold/i386.cc
index 97d98d7..eb4ef0a 100644
--- 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);
 
diff --git a/gold/icf.cc b/gold/icf.cc
index 48cb9bf..ec3269c 100644
--- 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))
-	    continue;
+              && !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);
diff --git a/gold/icf.h b/gold/icf.h
index e336572..76e491c 100644
--- 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_()
@@ -88,7 +90,37 @@ 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
+  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_;
diff --git a/gold/options.h b/gold/options.h
index 22662db..fee76dc 100644
--- 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"});
 
diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index cc46782..0eda0a9 100644
--- 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>*,
diff --git a/gold/sparc.cc b/gold/sparc.cc
index ab25203..5355c7b 100644
--- 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>*,
diff --git a/gold/target.h b/gold/target.h
index 20de7a4..673153f 100644
--- 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
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index a1889bb..3797a92 100644
--- 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 
+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
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index e5fd65f..e3c356a 100644
--- 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
diff --git a/gold/testsuite/icf_safe_so_test.cc b/gold/testsuite/icf_safe_so_test.cc
new file mode 100644
index 0000000..5e4453b
--- /dev/null
+++ 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;
+}
+
diff --git a/gold/testsuite/icf_safe_so_test.sh b/gold/testsuite/icf_safe_so_test.sh
new file mode 100755
index 0000000..db2e73e
--- /dev/null
+++ 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"
+
diff --git a/gold/testsuite/icf_safe_test.cc b/gold/testsuite/icf_safe_test.cc
index 9e8a369..3edb865 100644
--- 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;
 }
diff --git a/gold/testsuite/icf_safe_test.sh b/gold/testsuite/icf_safe_test.sh
index f77559c..540dcc2 100755
--- 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"
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index 802d498..e9dd5ae 100644
--- 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