bug: GOLD handling of Sparc PLTREL

David Miller davem@davemloft.net
Tue Feb 9 06:01:00 GMT 2010


From: Ian Lance Taylor <iant@google.com>
Date: Mon, 08 Feb 2010 21:53:50 -0800

> David Miller <davem@davemloft.net> writes:
> 
>> Several test cases in GOLD fail on sparc because of how it lays out
>> the .rela.plt and .rela.dyn sections.
>>
>> On Sparc, the .rela.dyn must include .rela.plt in it's range.  If this
>> is not followed, the dynamic linker will reference past the end of the
>> relocations.  This is true on 32-bit PowerPC and 32-bit S390 as well.
>>
>> But GOLD isn't doing this, it makes the size of .rela.dyn only include
>> the .rela.dyn relocs, it doesn't include the  .rela.plt reloc size
>> too.
> 
> I don't see how this could be a problem, since the only thing the
> dynamic linker sees are the segments and the dynamic tags.  The
> dynamic linker doesn't see anything about the .rela.dyn or .rela.plt
> sections.

It is a problem, GLIBC has code which does the following when the
macro ELF_MACHINE_PLTREL_OVERLAP is defined:

/* On some machines, notably SPARC, DT_REL* includes DT_JMPREL in its
   range.  Note that according to the ELF spec, this is completely legal!
   But conditionally define things so that on machines we know this will
   not happen we do something more optimal.  */

# ifdef ELF_MACHINE_PLTREL_OVERLAP
#  define _ELF_DYNAMIC_DO_RELOC(RELOC, reloc, map, do_lazy, test_rel) \
  do {									      \
    struct { ElfW(Addr) start, size; int lazy; } ranges[3];		      \
    int ranges_index;							      \
									      \
    ranges[0].lazy = ranges[2].lazy = 0;				      \
    ranges[1].lazy = 1;							      \
    ranges[0].size = ranges[1].size = ranges[2].size = 0;		      \
									      \
    if ((map)->l_info[DT_##RELOC])					      \
      {									      \
	ranges[0].start = D_PTR ((map), l_info[DT_##RELOC]);		      \
	ranges[0].size = (map)->l_info[DT_##RELOC##SZ]->d_un.d_val;	      \
      }									      \
									      \
    if ((do_lazy)							      \
	&& (map)->l_info[DT_PLTREL]					      \
	&& (!test_rel || (map)->l_info[DT_PLTREL]->d_un.d_val == DT_##RELOC)) \
      {									      \
	ranges[1].start = D_PTR ((map), l_info[DT_JMPREL]);		      \
	ranges[1].size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val;	      \
	ranges[2].start = ranges[1].start + ranges[1].size;		      \
	ranges[2].size = ranges[0].start + ranges[0].size - ranges[2].start;  \
	ranges[0].size = ranges[1].start - ranges[0].start;		      \
      }									      \
									      \
    for (ranges_index = 0; ranges_index < 3; ++ranges_index)		      \
      elf_dynamic_do_##reloc ((map),					      \
			      ranges[ranges_index].start,		      \
			      ranges[ranges_index].size,		      \
			      ranges[ranges_index].lazy);		      \
  } while (0)

(##RELOC here is RELA)

Think about what happens to those ranges in the "do_lazy" case when
.rela.dyn does not overlap with .rela.plt

The values in ranges[2] will be garbage (size will be negative for one
thing), and elf_dynamic_do_*() will walk over the edge of the
relcation entries.

Because of how this is coded, GLIBC absolutely expects .rela.dyn
to overlap into .rela.plt when present.

Binutils has been ensuring this forever, so I think that GOLD has to
do it too.

> I see a few calls to add_local_relative and add_global_relative that
> are not passing down elfcpp::R_SPARC_RELATIVE as the reloc type.  That
> is rather suspicious and seems like it could cause this sort of
> problem.

It has nothing to do with these crashes.

I'm playng around with the following patch to fix this:

gold/

2010-02-08  David S. Miller  <davem@davemloft.net>

	* output.h (Output_data_dynamic::add_section_size): New method
	that takes two Output_data objects.
	(Output_data_dynamic::Dynamic_entry): Create storage for secondary
	entry param.  Handle it in initializers.
	* output.cc (Output_data_dynamic::Dynamic_entry::write): For
	DYNAMIC_SECTION_SIZE, add in section object size if non-NULL.
	* layout.h (Layout::add_target_dynamic_tags): Add pltrel_overlap arg.
	* layout.cc (Layout::add_target_dynamic_tags): If pltrel_overlap, and
	.rela.plt exists, set DT_REL{,A}SZ to sum of .rela.dyn and .rela.plt
	* arm.cc (Target_arm::do_finalize_sections): Update to pass false
	for pltrel_overlap.
	* i386.cc (Target_i386::do_finalize_sections): Likewise.
	* x86_64.cc (Target_x86_64::do_finalize_sections): Likewise.
	* sparc.cc (Target_sparc::make_plt_entry): Force .rela.dyn to be output
	before .rela.plt
	(Target_sparc::do_finalize_sections): Update to pass true for
	pltrel_overlap.
	* powerpc.cc (Target_powerpc::make_plt_entry): Force .rela.dyn to be
	output before .rela.plt
	(Target_powerpc::do_finalize_sections): Update to pass true for
	pltrel_overlap when 32-bit.

diff --git a/gold/arm.cc b/gold/arm.cc
index bec70db..51a660f 100644
--- a/gold/arm.cc
+++ b/gold/arm.cc
@@ -7066,7 +7066,7 @@ Target_arm<big_endian>::do_finalize_sections(
 				  ? NULL
 				  : this->plt_->rel_plt());
   layout->add_target_dynamic_tags(true, this->got_plt_, rel_plt,
-				  this->rel_dyn_, true);
+				  this->rel_dyn_, true, false);
 
   // Emit any relocs we saved in an attempt to avoid generating COPY
   // relocs.
diff --git a/gold/i386.cc b/gold/i386.cc
index 2eab3f8..f2a7b53 100644
--- a/gold/i386.cc
+++ b/gold/i386.cc
@@ -1609,7 +1609,7 @@ Target_i386::do_finalize_sections(
 				  ? NULL
 				  : this->plt_->rel_plt());
   layout->add_target_dynamic_tags(true, this->got_plt_, rel_plt,
-				  this->rel_dyn_, true);
+				  this->rel_dyn_, true, false);
 
   // Emit any relocs we saved in an attempt to avoid generating COPY
   // relocs.
diff --git a/gold/layout.cc b/gold/layout.cc
index 52989a5..bb74ac0 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -3240,7 +3240,7 @@ void
 Layout::add_target_dynamic_tags(bool use_rel, const Output_data* plt_got,
 				const Output_data* plt_rel,
 				const Output_data_reloc_generic* dyn_rel,
-				bool add_debug)
+				bool add_debug, bool pltrel_overlap)
 {
   Output_data_dynamic* odyn = this->dynamic_data_;
   if (odyn == NULL)
@@ -3261,8 +3261,12 @@ Layout::add_target_dynamic_tags(bool use_rel, const Output_data* plt_got,
     {
       odyn->add_section_address(use_rel ? elfcpp::DT_REL : elfcpp::DT_RELA,
 				dyn_rel);
-      odyn->add_section_size(use_rel ? elfcpp::DT_RELSZ : elfcpp::DT_RELASZ,
-			     dyn_rel);
+      if (plt_rel != NULL && pltrel_overlap)
+	odyn->add_section_size(use_rel ? elfcpp::DT_RELSZ : elfcpp::DT_RELASZ,
+			       dyn_rel, plt_rel);
+      else
+	odyn->add_section_size(use_rel ? elfcpp::DT_RELSZ : elfcpp::DT_RELASZ,
+			       dyn_rel);
       const int size = parameters->target().get_size();
       elfcpp::DT rel_tag;
       int rel_size;
diff --git a/gold/layout.h b/gold/layout.h
index 15e7548..ff375f8 100644
--- a/gold/layout.h
+++ b/gold/layout.h
@@ -563,7 +563,7 @@ class Layout
   add_target_dynamic_tags(bool use_rel, const Output_data* plt_got,
 			  const Output_data* plt_rel,
 			  const Output_data_reloc_generic* dyn_rel,
-			  bool add_debug);
+			  bool add_debug, bool pltrel_overlap);
 
   // Compute and write out the build ID if needed.
   void
diff --git a/gold/output.cc b/gold/output.cc
index a8f03e7..44922bf 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -1560,6 +1560,8 @@ Output_data_dynamic::Dynamic_entry::write(
 
     case DYNAMIC_SECTION_SIZE:
       val = this->u_.od->data_size();
+      if (this->u2_.od)
+	val += this->u2_.od->data_size();
       break;
 
     case DYNAMIC_SYMBOL:
diff --git a/gold/output.h b/gold/output.h
index 60d57de..b35a6f6 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -1997,6 +1997,12 @@ class Output_data_dynamic : public Output_section_data
   add_section_size(elfcpp::DT tag, const Output_data* od)
   { this->add_entry(Dynamic_entry(tag, od, true)); }
 
+  // Add a new dynamic entry with the total size of two output datas.
+  void
+  add_section_size(elfcpp::DT tag, const Output_data* od,
+		   const Output_data* od2)
+  { this->add_entry(Dynamic_entry(tag, od, od2)); }
+
   // Add a new dynamic entry with the address of a symbol.
   void
   add_symbol(elfcpp::DT tag, const Symbol* sym)
@@ -2045,7 +2051,13 @@ class Output_data_dynamic : public Output_section_data
 	offset_(section_size
 		? DYNAMIC_SECTION_SIZE
 		: DYNAMIC_SECTION_ADDRESS)
-    { this->u_.od = od; }
+    { this->u_.od = od; this->u2_.od = NULL; }
+
+    // Create an entry with the size of two sections.
+    Dynamic_entry(elfcpp::DT tag, const Output_data* od, const Output_data* od2)
+      : tag_(tag),
+	offset_(DYNAMIC_SECTION_SIZE)
+    { this->u_.od = od; this->u2_.od = od2; }
 
     // Create an entry with the address of a section plus a constant offset.
     Dynamic_entry(elfcpp::DT tag, const Output_data* od, unsigned int offset)
@@ -2100,7 +2112,7 @@ class Output_data_dynamic : public Output_section_data
       const Symbol* sym;
       // For DYNAMIC_STRING.
       const char* str;
-    } u_;
+    } u_, u2_;
     // The dynamic tag.
     elfcpp::DT tag_;
     // The type of entry (Classification) or offset within a section.
diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index 83bb992..cc46782 100644
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -948,6 +948,11 @@ Target_powerpc<size, big_endian>::make_plt_entry(Symbol_table* symtab,
       // Create the GOT section first.
       this->got_section(symtab, layout);
 
+      // Ensure that .rela.dyn always appears before .rela.plt  This is
+      // necessary due to how, on PowerPC and some other targets, .rela.dyn
+      // needs to include .rela.plt in it's range.
+      this->rela_dyn_section(layout);
+
       this->plt_ = new Output_data_plt_powerpc<size, big_endian>(layout);
       layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
 				      (elfcpp::SHF_ALLOC
@@ -1556,7 +1561,7 @@ Target_powerpc<size, big_endian>::do_finalize_sections(
 				  ? NULL
 				  : this->plt_->rel_plt());
   layout->add_target_dynamic_tags(false, this->plt_, rel_plt,
-				  this->rela_dyn_, true);
+				  this->rela_dyn_, true, size == 32);
 
   // Emit any relocs we saved in an attempt to avoid generating COPY
   // relocs.
diff --git a/gold/sparc.cc b/gold/sparc.cc
index 6075c44..6a98d91 100644
--- a/gold/sparc.cc
+++ b/gold/sparc.cc
@@ -1369,6 +1369,11 @@ Target_sparc<size, big_endian>::make_plt_entry(Symbol_table* symtab,
       // Create the GOT sections first.
       this->got_section(symtab, layout);
 
+      // Ensure that .rela.dyn always appears before .rela.plt  This is
+      // necessary due to how, on Sparc and some other targets, .rela.dyn
+      // needs to include .rela.plt in it's range.
+      this->rela_dyn_section(layout);
+
       this->plt_ = new Output_data_plt_sparc<size, big_endian>(layout);
       layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
 				      (elfcpp::SHF_ALLOC
@@ -2341,7 +2346,7 @@ Target_sparc<size, big_endian>::do_finalize_sections(
 				  ? NULL
 				  : this->plt_->rel_plt());
   layout->add_target_dynamic_tags(false, this->plt_, rel_plt,
-				  this->rela_dyn_, true);
+				  this->rela_dyn_, true, true);
 
   // Emit any relocs we saved in an attempt to avoid generating COPY
   // relocs.
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index 585a499..fea2ec9 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -1741,7 +1741,7 @@ Target_x86_64::do_finalize_sections(
 				  ? NULL
 				  : this->plt_->rela_plt());
   layout->add_target_dynamic_tags(false, this->got_plt_, rel_plt,
-				  this->rela_dyn_, true);
+				  this->rela_dyn_, true, false);
 				  
   // Fill in some more dynamic tags.
   Output_data_dynamic* const odyn = layout->dynamic_data();



More information about the Binutils mailing list