Patch for ICF string inline bug for SHT_REL sections.

Sriraman Tallam tmsriram@google.com
Thu Jul 29 07:12:00 GMT 2010
Hi Ian,

     I have attached the patch with that change.

	* arm.cc (Target_arm<big_endian>::gc_process_relocs): Add template
	paramter to the call to gold::gc_process_relocs.
	* i386.cc (Target_i386<big_endian>::gc_process_relocs): Add template
	paramter to the call to gold::gc_process_relocs.
	* x86_64.cc (Target_x86_64<big_endian>::gc_process_relocs): Add template
	parameter to the call to gold::gc_process_relocs.
	* powerpc.cc (Target_powerpc<big_endian>::gc_process_relocs): Add
	template parameter to the call to gold::gc_process_relocs.
	* sparc.cc (Target_sparc<big_endian>::gc_process_relocs): Add template
	paramter to the call to gold::gc_process_relocs.
	* gc.h (get_embedded_addend_size): New function.
	(gc_process_relocs): Save the size of the reloc for use by ICF.
	* icf.cc (get_section_contents): Get the addend from the text section
	for SHT_REL relocation sections.
	* icf.h (Icf::Reloc_addend_size_info): New typedef.
	(Icf::Reloc_info): Add new member reloc_addend_size_info.
	* int_encoding.h (read_from_pointer): New overloaded function.
	* testsuite/Makefile.am (icf_sht_rel_addend_test): New test.
	* testsuite/icf_sht_rel_addend_test.sh: New file.
	* testsuite/icf_sht_rel_addend_test_1.cc: New file.
	* testsuite/icf_sht_rel_addend_test_2.cc: New file.


Thanks,
-Sri.

On Wed, Jul 28, 2010 at 1:36 PM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>> +                      case 1:
>> +                        {
>> +                          reloc_addend_value =
>> +                            *(reinterpret_cast<const uint8_t*>(reloc_addend_ptr));
>> +                       break;
>> +                        }
>> +                      case 2:
>> +                        {
>> +                          reloc_addend_value =
>> +                            *(reinterpret_cast<const uint16_t*>(reloc_addend_ptr));
>> +                       break;
>> +                        }
>> +                      case 4:
>> +                        {
>> +                          reloc_addend_value =
>> +                            *(reinterpret_cast<const uint32_t*>(reloc_addend_ptr));
>> +                       break;
>
> You can't use these reinterpret_casts.  In general the addend need not
> be aligned, and this will crash when running on a strict alignment host
> like SPARC.  Without some knowledge that the addend is aligned, you need
> to use Swap_unaligned.  It may be easiest to use read_from_pointer from
> int_encoding.h.
>
> Sorry for the long delay.
>
> Ian
>
-------------- next part --------------
Index: arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.113
diff -u -u -p -r1.113 arm.cc
--- arm.cc	13 Jul 2010 20:07:08 -0000	1.113
+++ arm.cc	29 Jul 2010 01:30:18 -0000
@@ -8213,7 +8213,8 @@ Target_arm<big_endian>::gc_process_reloc
   typedef Target_arm<big_endian> Arm;
   typedef typename Target_arm<big_endian>::Scan Scan;
 
-  gold::gc_process_relocs<32, big_endian, Arm, elfcpp::SHT_REL, Scan>(
+  gold::gc_process_relocs<32, big_endian, Arm, elfcpp::SHT_REL, Scan,
+			  Target_arm::Relocatable_size_for_reloc>(
     symtab,
     layout,
     this,
Index: gc.h
===================================================================
RCS file: /cvs/src/src/gold/gc.h,v
retrieving revision 1.12
diff -u -u -p -r1.12 gc.h
--- gc.h	23 Apr 2010 18:49:22 -0000	1.12
+++ gc.h	29 Jul 2010 01:30:18 -0000
@@ -151,6 +151,20 @@ struct Symbols_data
   section_size_type symbol_names_size;
 };
 
+// Relocations of type SHT_REL store the addend value in their bytes.
+// This function returns the size of the embedded addend which is
+// nothing but the size of the relocation.
+
+template<typename Classify_reloc>
+inline unsigned int
+get_embedded_addend_size(int sh_type, int r_type, Relobj* obj)
+{
+  if (sh_type != elfcpp::SHT_REL)
+    return 0;
+  Classify_reloc classify_reloc;
+  return classify_reloc.get_size_for_reloc(r_type, obj);
+}
+
 // This function implements the generic part of reloc
 // processing to map a section to all the sections it
 // references through relocs.  It is called only during
@@ -158,7 +172,7 @@ struct Symbols_data
 // folding (--icf).
 
 template<int size, bool big_endian, typename Target_type, int sh_type,
-	 typename Scan>
+	 typename Scan, typename Classify_reloc>
 inline void
 gc_process_relocs(
     Symbol_table* symtab,
@@ -185,6 +199,7 @@ gc_process_relocs(
   Icf::Symbol_info* symvec = NULL;
   Icf::Addend_info* addendvec = NULL;
   Icf::Offset_info* offsetvec = NULL;
+  Icf::Reloc_addend_size_info* reloc_addend_size_vec = NULL;
   bool is_icf_tracked = false;
   const char* cident_section_name = NULL;
 
@@ -205,6 +220,7 @@ gc_process_relocs(
       symvec = &reloc_info->symbol_info;
       addendvec = &reloc_info->addend_info;
       offsetvec = &reloc_info->offset_info;
+      reloc_addend_size_vec = &reloc_info->reloc_addend_size_info;
     }
 
   check_section_for_function_pointers =
@@ -243,6 +259,9 @@ gc_process_relocs(
               uint64_t reloc_offset =
                 convert_to_section_size_type(reloc.get_r_offset());
 	      (*offsetvec).push_back(reloc_offset);
+              (*reloc_addend_size_vec).push_back(
+                get_embedded_addend_size<Classify_reloc>(sh_type, r_type,
+                                                         src_obj));
             }
 
 	  // When doing safe folding, check to see if this relocation is that
@@ -316,6 +335,9 @@ gc_process_relocs(
               uint64_t reloc_offset =
                 convert_to_section_size_type(reloc.get_r_offset());
 	      (*offsetvec).push_back(reloc_offset);
+              (*reloc_addend_size_vec).push_back(
+                get_embedded_addend_size<Classify_reloc>(sh_type, r_type,
+                                                         src_obj));
 	    }
 
           if (gsym->source() != Symbol::FROM_OBJECT)
Index: i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.118
diff -u -u -p -r1.118 i386.cc
--- i386.cc	13 Jul 2010 12:04:03 -0000	1.118
+++ i386.cc	29 Jul 2010 01:30:18 -0000
@@ -1624,7 +1624,8 @@ Target_i386::gc_process_relocs(Symbol_ta
                                const unsigned char* plocal_symbols)
 {
   gold::gc_process_relocs<32, false, Target_i386, elfcpp::SHT_REL,
-		          Target_i386::Scan>(
+		          Target_i386::Scan,
+                          Target_i386::Relocatable_size_for_reloc>(
     symtab,
     layout,
     this,
Index: icf.cc
===================================================================
RCS file: /cvs/src/src/gold/icf.cc,v
retrieving revision 1.13
diff -u -u -p -r1.13 icf.cc
--- icf.cc	23 Apr 2010 18:49:22 -0000	1.13
+++ icf.cc	29 Jul 2010 01:30:18 -0000
@@ -145,6 +145,8 @@
 #include "symtab.h"
 #include "libiberty.h"
 #include "demangle.h"
+#include "elfcpp.h"
+#include "int_encoding.h"
 
 namespace gold
 {
@@ -269,12 +271,16 @@ get_section_contents(bool first_iteratio
       Icf::Addend_info a = (it_reloc_info_list->second).addend_info;
       // Stores the offset of the reloc.
       Icf::Offset_info o = (it_reloc_info_list->second).offset_info;
+      Icf::Reloc_addend_size_info reloc_addend_size_info =
+        (it_reloc_info_list->second).reloc_addend_size_info;
       Icf::Sections_reachable_info::iterator it_v = v.begin();
       Icf::Symbol_info::iterator it_s = s.begin();
       Icf::Addend_info::iterator it_a = a.begin();
       Icf::Offset_info::iterator it_o = o.begin();
+      Icf::Reloc_addend_size_info::iterator it_addend_size =
+        reloc_addend_size_info.begin();
 
-      for (; it_v != v.end(); ++it_v, ++it_s, ++it_a, ++it_o)
+      for (; it_v != v.end(); ++it_v, ++it_s, ++it_a, ++it_o, ++it_addend_size)
         {
           // ADDEND_STR stores the symbol value and addend and offset,
           // each atmost 16 hex digits long.  it_a points to a pair
@@ -372,6 +378,41 @@ get_section_contents(bool first_iteratio
                   if (addend < 0xffffff00)
                     offset = offset + addend;
 
+		  // For SHT_REL relocation sections, the addend is stored in the
+		  // text section at the relocation offset.
+		  uint32_t reloc_addend_value = 0;
+                  const unsigned char* reloc_addend_ptr =
+		    contents + static_cast<unsigned long long>(*it_o);
+		  switch(*it_addend_size)
+		    {
+		      case 0:
+		        {
+                          break;
+                        }
+                      case 1:
+                        {
+                          reloc_addend_value =
+                            read_from_pointer<8>(reloc_addend_ptr);
+			  break;
+                        }
+                      case 2:
+                        {
+                          reloc_addend_value =
+                            read_from_pointer<16>(reloc_addend_ptr);
+			  break;
+                        }
+                      case 4:
+                        {
+                          reloc_addend_value =
+                            read_from_pointer<32>(reloc_addend_ptr);
+			  break;
+                        }
+		      default:
+		        gold_unreachable();
+
+		    }
+		  offset = offset + reloc_addend_value;
+
                   section_size_type secn_len;
                   const unsigned char* str_contents =
                   (it_v->first)->section_contents(it_v->second,
Index: icf.h
===================================================================
RCS file: /cvs/src/src/gold/icf.h,v
retrieving revision 1.8
diff -u -u -p -r1.8 icf.h
--- icf.h	25 Jun 2010 00:37:40 -0000	1.8
+++ icf.h	29 Jul 2010 01:30:18 -0000
@@ -43,6 +43,7 @@ class Icf
   typedef std::vector<Symbol*> Symbol_info;
   typedef std::vector<std::pair<long long, long long> > Addend_info;
   typedef std::vector<uint64_t> Offset_info;
+  typedef std::vector<unsigned int> Reloc_addend_size_info;
   typedef Unordered_map<Section_id,
                         unsigned int,
                         Section_id_hash> Uniq_secn_id_map;
@@ -57,6 +58,7 @@ class Icf
     // This stores the symbol value and the addend for a reloc.
     Addend_info addend_info;
     Offset_info offset_info;
+    Reloc_addend_size_info reloc_addend_size_info;
   } Reloc_info;
 
   typedef Unordered_map<Section_id, Reloc_info,
Index: int_encoding.h
===================================================================
RCS file: /cvs/src/src/gold/int_encoding.h,v
retrieving revision 1.1
diff -u -u -p -r1.1 int_encoding.h
--- int_encoding.h	9 Dec 2009 03:02:28 -0000	1.1
+++ int_encoding.h	29 Jul 2010 01:30:18 -0000
@@ -77,6 +77,20 @@ void insert_into_vector(std::vector<unsi
   destination->insert(destination->end(), buffer, buffer + valsize / 8);
 }
 
+// Read a possibly unaligned integer of SIZE from SOURCE.
+
+template <int valsize>
+typename elfcpp::Valtype_base<valsize>::Valtype
+read_from_pointer(const unsigned char* source)
+{
+  typename elfcpp::Valtype_base<valsize>::Valtype return_value;
+  if (parameters->target().is_big_endian())
+    return_value = elfcpp::Swap_unaligned<valsize, true>::readval(source);
+  else
+    return_value = elfcpp::Swap_unaligned<valsize, false>::readval(source);
+  return return_value;
+}
+
 // Read a possibly unaligned integer of SIZE.  Update SOURCE after read.
 
 template <int valsize>
Index: powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.30
diff -u -u -p -r1.30 powerpc.cc
--- powerpc.cc	13 Jul 2010 12:04:03 -0000	1.30
+++ powerpc.cc	29 Jul 2010 01:30:18 -0000
@@ -1493,7 +1493,8 @@ Target_powerpc<size, big_endian>::gc_pro
   typedef Target_powerpc<size, big_endian> Powerpc;
   typedef typename Target_powerpc<size, big_endian>::Scan Scan;
 
-  gold::gc_process_relocs<size, big_endian, Powerpc, elfcpp::SHT_RELA, Scan>(
+  gold::gc_process_relocs<size, big_endian, Powerpc, elfcpp::SHT_RELA, Scan,
+			  Target_powerpc::Relocatable_size_for_reloc>(
     symtab,
     layout,
     this,
Index: sparc.cc
===================================================================
RCS file: /cvs/src/src/gold/sparc.cc,v
retrieving revision 1.39
diff -u -u -p -r1.39 sparc.cc
--- sparc.cc	13 Jul 2010 12:04:03 -0000	1.39
+++ sparc.cc	29 Jul 2010 01:30:18 -0000
@@ -2330,7 +2330,8 @@ Target_sparc<size, big_endian>::gc_proce
   typedef Target_sparc<size, big_endian> Sparc;
   typedef typename Target_sparc<size, big_endian>::Scan Scan;
 
-  gold::gc_process_relocs<size, big_endian, Sparc, elfcpp::SHT_RELA, Scan>(
+  gold::gc_process_relocs<size, big_endian, Sparc, elfcpp::SHT_RELA, Scan,
+			  Target_sparc::Relocatable_size_for_reloc>(
     symtab,
     layout,
     this,
Index: x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.109
diff -u -u -p -r1.109 x86_64.cc
--- x86_64.cc	13 Jul 2010 12:04:03 -0000	1.109
+++ x86_64.cc	29 Jul 2010 01:30:18 -0000
@@ -1781,7 +1781,8 @@ Target_x86_64::gc_process_relocs(Symbol_
     }
 
    gold::gc_process_relocs<64, false, Target_x86_64, elfcpp::SHT_RELA,
-                           Target_x86_64::Scan>(
+                           Target_x86_64::Scan,
+			   Target_x86_64::Relocatable_size_for_reloc>(
     symtab,
     layout,
     this,
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.137
diff -u -u -p -r1.137 Makefile.am
--- testsuite/Makefile.am	10 Jun 2010 17:20:27 -0000	1.137
+++ testsuite/Makefile.am	29 Jul 2010 01:30:18 -0000
@@ -232,6 +232,18 @@ icf_string_merge_test: icf_string_merge_
 icf_string_merge_test.stdout: icf_string_merge_test
 	$(TEST_NM) icf_string_merge_test > icf_string_merge_test.stdout
 
+check_SCRIPTS += icf_sht_rel_addend_test.sh
+check_DATA += icf_sht_rel_addend_test.stdout
+MOSTLYCLEANFILES += icf_sht_rel_addend_test
+icf_sht_rel_addend_test_1.o: icf_sht_rel_addend_test_1.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections -fPIC -g -o $@ $<
+icf_sht_rel_addend_test_2.o: icf_sht_rel_addend_test_2.cc
+	$(CXXCOMPILE) -O0 -c -ffunction-sections -fPIC -g -o $@ $<
+icf_sht_rel_addend_test: icf_sht_rel_addend_test_1.o icf_sht_rel_addend_test_2.o gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=all icf_sht_rel_addend_test_1.o icf_sht_rel_addend_test_2.o
+icf_sht_rel_addend_test.stdout: icf_sht_rel_addend_test
+	$(TEST_NM) icf_sht_rel_addend_test > icf_sht_rel_addend_test.stdout
+
 check_PROGRAMS += basic_test
 check_PROGRAMS += basic_static_test
 check_PROGRAMS += basic_pic_test
Index: testsuite/icf_sht_rel_addend_test.sh
===================================================================
RCS file: testsuite/icf_sht_rel_addend_test.sh
diff -N testsuite/icf_sht_rel_addend_test.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/icf_sht_rel_addend_test.sh	29 Jul 2010 01:30:18 -0000
@@ -0,0 +1,35 @@
+# icf_sht_rel_addend_test.sh -- test --icf=all
+
+# 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.
+
+
+check()
+{
+    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 "Identical Code Folding should not fold" $2 "and" $3
+	exit 1
+    fi
+}
+
+check icf_sht_rel_addend_test.stdout "name1" "name2"
Index: testsuite/icf_sht_rel_addend_test_1.cc
===================================================================
RCS file: testsuite/icf_sht_rel_addend_test_1.cc
diff -N testsuite/icf_sht_rel_addend_test_1.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/icf_sht_rel_addend_test_1.cc	29 Jul 2010 01:30:18 -0000
@@ -0,0 +1,44 @@
+// icf_sht_rel_addend_test_1.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 is strings are handled correctly
+// by ICF when the relocation types are SHT_REL.  ICF inlines strings that
+// can be merged.  To do this, it must get the addend of the relocation
+// pointing to the string.  For SHT_REL relocations, the addend is encoded
+// in the text section at the offset of the relocation.  If ICF fails to
+// get the addend correctly, function name1 will be incorrectly folded with
+// function name2 in icf_sht_rel_addend_test_2.cc.
+
+
+const char* bar()
+{
+  return "AAAAAA";
+}
+const char* name1()
+{
+  return "Name1";
+}
+
+int main()
+{
+  return 0;
+}
Index: testsuite/icf_sht_rel_addend_test_2.cc
===================================================================
RCS file: testsuite/icf_sht_rel_addend_test_2.cc
diff -N testsuite/icf_sht_rel_addend_test_2.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/icf_sht_rel_addend_test_2.cc	29 Jul 2010 01:30:18 -0000
@@ -0,0 +1,39 @@
+// icf_sht_rel_addend_test_2.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 is strings are handled correctly
+// by ICF when the relocation types are SHT_REL.  ICF inlines strings that
+// can be merged.  To do this, it must get the addend of the relocation
+// pointing to the string.  For SHT_REL relocations, the addend is encoded
+// in the text section at the offset of the relocation.  If ICF fails to
+// get the addend correctly, function name1 in icf_sht_rel_addend_test_1.cc
+// will be incorrectly folded with name2.
+
+
+const char* foo()
+{
+  return "AAAAAA";
+}
+const char* name2()
+{
+  return "Name2";
+}


More information about the Binutils mailing list