MIPS textrel fix

MIPS textrel fix

[Daniel Jacobowitz]

While stuck offline earlier today, I revisited the textrel-1 MIPS
failure.  Eric originally tried setting DF_TEXTREL during section
relocation, but it's too late: we add the entry in size_dynamic_sections.
But I couldn't see any other way to get it right, since there's no
hook to predict whether elf-eh-frame.c will eliminate a relocation.

The easiest approach I found was to annul the DT_TEXTREL and DT_FLAGS
changes in finish_dynamic_sections if no text relocations were
actually generated.  This is not immensely pretty, but does work.
OK?

For background, the problem arises from the use of absolute addresses
in .eh_frame.  In this case, they're being produced by gas CFI
directives.  Is there a reason we can't mix and match encodings?
i.e. why not have gas use a PC-relative format?  I'm sure there's
a reason, but I can't think of it...

-- 
Daniel Jacobowitz
CodeSourcery

2006-05-18  Daniel Jacobowitz  <dan@codesourcery.com>

	* elflink.c (_bfd_elf_add_dynamic_entry): Remove DT_TEXTREL
	check.
	(bfd_elf_final_link): Add a late DT_TEXTREL check.
	* elfxx-mips.c (MIPS_ELF_READONLY_SECTION): Define.
	(mips_elf_create_dynamic_relocation): Set DF_TEXTREL.
	(_bfd_mips_elf_check_relocs): Delete MIPS_READONLY_SECTION.
	Use MIPS_ELF_READONLY_SECTION.
	(_bfd_mips_elf_size_dynamic_sections): Clear DF_TEXTREL after
	creating DT_TEXTREL.
	(_bfd_mips_elf_finish_dynamic_sections): Clear textrel markers
	if no text relocations were generated.

Index: elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.210.2.3
diff -u -p -r1.210.2.3 elflink.c
--- elflink.c	17 May 2006 05:14:21 -0000	1.210.2.3
+++ elflink.c	18 May 2006 21:24:41 -0000
@@ -2810,10 +2810,6 @@ _bfd_elf_add_dynamic_entry (struct bfd_l
   if (! is_elf_hash_table (hash_table))
     return FALSE;
 
-  if (info->warn_shared_textrel && info->shared && tag == DT_TEXTREL)
-    _bfd_error_handler
-      (_("warning: creating a DT_TEXTREL in a shared object."));
-
   bed = get_elf_backend_data (hash_table->dynobj);
   s = bfd_get_section_by_name (hash_table->dynobj, ".dynamic");
   BFD_ASSERT (s != NULL);
@@ -8655,6 +8651,32 @@ bfd_elf_final_link (bfd *abfd, struct bf
       if (! (*bed->elf_backend_finish_dynamic_sections) (abfd, info))
 	goto error_return;
 
+      /* Check for DT_TEXTREL (late, in case the backend removes it).  */
+      if (info->warn_shared_textrel && info->shared)
+	{
+	  bfd_byte *dyncon, *dynconend;
+
+	  /* Fix up .dynamic entries.  */
+	  o = bfd_get_section_by_name (dynobj, ".dynamic");
+	  BFD_ASSERT (o != NULL);
+
+	  dyncon = o->contents;
+	  dynconend = o->contents + o->size;
+	  for (; dyncon < dynconend; dyncon += bed->s->sizeof_dyn)
+	    {
+	      Elf_Internal_Dyn dyn;
+
+	      bed->s->swap_dyn_in (dynobj, dyncon, &dyn);
+
+	      if (dyn.d_tag == DT_TEXTREL)
+		{
+		  _bfd_error_handler
+		    (_("warning: creating a DT_TEXTREL in a shared object."));
+		  break;
+		}
+	    }
+	}
+
       for (o = dynobj->sections; o != NULL; o = o->next)
 	{
 	  if ((o->flags & SEC_HAS_CONTENTS) == 0
Index: elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.165
diff -u -p -r1.165 elfxx-mips.c
--- elfxx-mips.c	27 Mar 2006 11:30:53 -0000	1.165
+++ elfxx-mips.c	18 May 2006 21:24:42 -0000
@@ -535,6 +535,11 @@ static bfd *reldyn_sorting_bfd;
 #define MIPS_ELF_OPTIONS_SECTION_NAME_P(NAME) \
   (strcmp (NAME, ".MIPS.options") == 0 || strcmp (NAME, ".options") == 0)
 
+/* Whether the section is readonly.  */
+#define MIPS_ELF_READONLY_SECTION(sec) \
+  ((sec->flags & (SEC_ALLOC | SEC_LOAD | SEC_READONLY))		\
+   == (SEC_ALLOC | SEC_LOAD | SEC_READONLY))
+
 /* The name of the stub section.  */
 #define MIPS_ELF_STUB_SECTION_NAME(abfd) ".MIPS.stubs"
 
@@ -4909,6 +4914,12 @@ mips_elf_create_dynamic_relocation (bfd 
 	}
     }
 
+  /* If we've written this relocation for a readonly section,
+     we need to set DF_TEXTREL again, so that we do not delete the
+     DT_TEXTREL tag.  */
+  if (MIPS_ELF_READONLY_SECTION (input_section))
+    info->flags |= DF_TEXTREL;
+
   return TRUE;
 }
 \f
@@ -6513,15 +6524,13 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
 		  if (sreloc == NULL)
 		    return FALSE;
 		}
-#define MIPS_READONLY_SECTION (SEC_ALLOC | SEC_LOAD | SEC_READONLY)
 	      if (info->shared)
 		{
 		  /* When creating a shared object, we must copy these
 		     reloc types into the output file as R_MIPS_REL32
 		     relocs.  Make room for this reloc in .rel(a).dyn.  */
 		  mips_elf_allocate_dynamic_relocations (dynobj, info, 1);
-		  if ((sec->flags & MIPS_READONLY_SECTION)
-		      == MIPS_READONLY_SECTION)
+		  if (MIPS_ELF_READONLY_SECTION (sec))
 		    /* We tell the dynamic linker that there are
 		       relocations against the text segment.  */
 		    info->flags |= DF_TEXTREL;
@@ -6534,8 +6543,7 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
                      defined in a dynamic object.  */
 		  hmips = (struct mips_elf_link_hash_entry *) h;
 		  ++hmips->possibly_dynamic_relocs;
-		  if ((sec->flags & MIPS_READONLY_SECTION)
-		      == MIPS_READONLY_SECTION)
+		  if (MIPS_ELF_READONLY_SECTION (sec))
 		    /* We need it to tell the dynamic linker if there
 		       are relocations against the text segment.  */
 		    hmips->readonly_reloc = TRUE;
@@ -7452,6 +7460,12 @@ _bfd_mips_elf_size_dynamic_sections (bfd
 	{
 	  if (! MIPS_ELF_ADD_DYNAMIC_ENTRY (info, DT_TEXTREL, 0))
 	    return FALSE;
+
+	  /* Clear the DF_TEXTREL flag.  It will be set again if we
+	     write out an actual text relocation; we may not, because
+	     at this point we do not know whether e.g. any .eh_frame
+	     absolute relocations have been converted to PC-relative.  */
+	  info->flags &= ~DF_TEXTREL;
 	}
 
       if (! MIPS_ELF_ADD_DYNAMIC_ENTRY (info, DT_PLTGOT, 0))
@@ -8478,6 +8492,7 @@ _bfd_mips_elf_finish_dynamic_sections (b
   if (elf_hash_table (info)->dynamic_sections_created)
     {
       bfd_byte *b;
+      int dyn_to_skip = 0, dyn_skipped = 0;
 
       BFD_ASSERT (sdyn != NULL);
       BFD_ASSERT (g != NULL);
@@ -8632,15 +8647,44 @@ _bfd_mips_elf_finish_dynamic_sections (b
 				+ htab->srelplt->output_offset);
 	      break;
 
+	    case DT_TEXTREL:
+	      /* If we didn't need any text relocations after all, delete
+		 the dynamic tag.  */
+	      if (!(info->flags & DF_TEXTREL))
+		{
+		  dyn_to_skip = MIPS_ELF_DYN_SIZE (dynobj);
+		  swap_out_p = FALSE;
+		}
+	      break;
+
+	    case DT_FLAGS:
+	      /* If we didn't need any text relocations after all, clear
+		 DF_TEXTREL from DT_FLAGS.  */
+	      if (!(info->flags & DF_TEXTREL))
+		dyn.d_un.d_val &= ~DF_TEXTREL;
+	      else
+		swap_out_p = FALSE;
+	      break;
+
 	    default:
 	      swap_out_p = FALSE;
 	      break;
 	    }
 
-	  if (swap_out_p)
+	  if (swap_out_p || dyn_skipped)
 	    (*get_elf_backend_data (dynobj)->s->swap_dyn_out)
-	      (dynobj, &dyn, b);
+	      (dynobj, &dyn, b - dyn_skipped);
+
+	  if (dyn_to_skip)
+	    {
+	      dyn_skipped += dyn_to_skip;
+	      dyn_to_skip = 0;
+	    }
 	}
+
+      /* Wipe out any trailing entries if we shifted down a dynamic tag.  */
+      if (dyn_skipped > 0)
+	memset (b - dyn_skipped, 0, dyn_skipped);
     }
 
   if (sgot != NULL && sgot->size > 0)

Re: MIPS textrel fix

[Daniel Jacobowitz]

On Thu, May 18, 2006 at 05:31:49PM -0400, Daniel Jacobowitz wrote:
> 2006-05-18  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* elflink.c (_bfd_elf_add_dynamic_entry): Remove DT_TEXTREL
> 	check.
> 	(bfd_elf_final_link): Add a late DT_TEXTREL check.
> 	* elfxx-mips.c (MIPS_ELF_READONLY_SECTION): Define.
> 	(mips_elf_create_dynamic_relocation): Set DF_TEXTREL.
> 	(_bfd_mips_elf_check_relocs): Delete MIPS_READONLY_SECTION.
> 	Use MIPS_ELF_READONLY_SECTION.
> 	(_bfd_mips_elf_size_dynamic_sections): Clear DF_TEXTREL after
> 	creating DT_TEXTREL.
> 	(_bfd_mips_elf_finish_dynamic_sections): Clear textrel markers
> 	if no text relocations were generated.

Oh, there's a textrel-1.d part of this patch too; the test bitrotted
while it was failing.  That part is obvious.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MIPS textrel fix

[Eric Christopher]

> actually generated.  This is not immensely pretty, but does work.
> OK?

OK everywhere.

> For background, the problem arises from the use of absolute addresses
> in .eh_frame.  In this case, they're being produced by gas CFI
> directives.  Is there a reason we can't mix and match encodings?
> i.e. why not have gas use a PC-relative format?  I'm sure there's
> a reason, but I can't think of it...

Well, mips is missing relocations which is why I had to do it this  
way...

gas in general mixing and matching schemes? I dunno, I haven't looked
into it recently. I think I'm going to need to anyhow soon though. :)

-eric

Re: MIPS textrel fix

[David Daney]

Daniel Jacobowitz wrote:
> While stuck offline earlier today, I revisited the textrel-1 MIPS
> failure.  Eric originally tried setting DF_TEXTREL during section
> relocation, but it's too late: we add the entry in size_dynamic_sections.
> But I couldn't see any other way to get it right, since there's no
> hook to predict whether elf-eh-frame.c will eliminate a relocation.
> 
> The easiest approach I found was to annul the DT_TEXTREL and DT_FLAGS
> changes in finish_dynamic_sections if no text relocations were
> actually generated.  This is not immensely pretty, but does work.
> OK?
> 
> For background, the problem arises from the use of absolute addresses
> in .eh_frame.  In this case, they're being produced by gas CFI
> directives.  Is there a reason we can't mix and match encodings?
> i.e. why not have gas use a PC-relative format?  I'm sure there's
> a reason, but I can't think of it...
> 

Could it be related to the reason that GCC no longer generates a 
PC-relative .eh_frame?

I never fully understood the reason, but I think it has to do with 
MIPS-ELF specifications not allowing the needed relocation types (even 
though they work well in binutils/glibc/linux).

David Daney

Re: MIPS textrel fix

[Thiemo Seufer]

David Daney wrote:
> Daniel Jacobowitz wrote:
> >While stuck offline earlier today, I revisited the textrel-1 MIPS
> >failure.  Eric originally tried setting DF_TEXTREL during section
> >relocation, but it's too late: we add the entry in size_dynamic_sections.
> >But I couldn't see any other way to get it right, since there's no
> >hook to predict whether elf-eh-frame.c will eliminate a relocation.
> >
> >The easiest approach I found was to annul the DT_TEXTREL and DT_FLAGS
> >changes in finish_dynamic_sections if no text relocations were
> >actually generated.  This is not immensely pretty, but does work.
> >OK?
> >
> >For background, the problem arises from the use of absolute addresses
> >in .eh_frame.  In this case, they're being produced by gas CFI
> >directives.  Is there a reason we can't mix and match encodings?
> >i.e. why not have gas use a PC-relative format?  I'm sure there's
> >a reason, but I can't think of it...
> >
> 
> Could it be related to the reason that GCC no longer generates a 
> PC-relative .eh_frame?
> 
> I never fully understood the reason, but I think it has to do with 
> MIPS-ELF specifications not allowing the needed relocation types (even 
> though they work well in binutils/glibc/linux).

The needed relocation types were originally defined for embedded PIC
MIPS and are not available in the ABI spec, their use for eh_frame
was an accident. When the removal of embedded PIC support broke the
toolchain, the fix chosen was to become ABI compatible again.


Thiemo

Re: MIPS textrel fix

[David Daney]

Thiemo Seufer wrote:
> David Daney wrote:
> 
>>Daniel Jacobowitz wrote:
>>
>>>While stuck offline earlier today, I revisited the textrel-1 MIPS
>>>failure.  Eric originally tried setting DF_TEXTREL during section
>>>relocation, but it's too late: we add the entry in size_dynamic_sections.
>>>But I couldn't see any other way to get it right, since there's no
>>>hook to predict whether elf-eh-frame.c will eliminate a relocation.
>>>
>>>The easiest approach I found was to annul the DT_TEXTREL and DT_FLAGS
>>>changes in finish_dynamic_sections if no text relocations were
>>>actually generated.  This is not immensely pretty, but does work.
>>>OK?
>>>
>>>For background, the problem arises from the use of absolute addresses
>>>in .eh_frame.  In this case, they're being produced by gas CFI
>>>directives.  Is there a reason we can't mix and match encodings?
>>>i.e. why not have gas use a PC-relative format?  I'm sure there's
>>>a reason, but I can't think of it...
>>>
>>
>>Could it be related to the reason that GCC no longer generates a 
>>PC-relative .eh_frame?
>>
>>I never fully understood the reason, but I think it has to do with 
>>MIPS-ELF specifications not allowing the needed relocation types (even 
>>though they work well in binutils/glibc/linux).
> 
> 
> The needed relocation types were originally defined for embedded PIC
> MIPS and are not available in the ABI spec, their use for eh_frame
> was an accident. When the removal of embedded PIC support broke the
> toolchain, the fix chosen was to become ABI compatible again.
> 

Not to beat a dead horse, but it would be nice to have a gnu extension 
that allows us to have read-only eh_frame sections.  Large C++ shared 
libraries have way to many relocations.  It is a lot of dirty pages in 
the name of ABI compliance.

David Daney.

Re: MIPS textrel fix

[Eric Christopher]

>
> Not to beat a dead horse, but it would be nice to have a gnu  
> extension that allows us to have read-only eh_frame sections.   
> Large C++ shared libraries have way to many relocations.  It is a  
> lot of dirty pages in the name of ABI compliance.

As long as it doesn't impact handling of other things I'm willing to  
look into it. Need to make sure that we don't base everything on it -  
otherwise we break irix.

-eric

Re: MIPS textrel fix

[Daniel Jacobowitz]

On Fri, May 19, 2006 at 01:11:01AM +0100, Thiemo Seufer wrote:
> The needed relocation types were originally defined for embedded PIC
> MIPS and are not available in the ABI spec, their use for eh_frame
> was an accident. When the removal of embedded PIC support broke the
> toolchain, the fix chosen was to become ABI compatible again.

Right.  I'd forgotten this.  Really, the ABI ought to have the
necessary relocations; but I'm not going to poke my nose in further
right now.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MIPS textrel fix

[Thiemo Seufer]

Daniel Jacobowitz wrote:
> On Fri, May 19, 2006 at 01:11:01AM +0100, Thiemo Seufer wrote:
> > The needed relocation types were originally defined for embedded PIC
> > MIPS and are not available in the ABI spec, their use for eh_frame
> > was an accident. When the removal of embedded PIC support broke the
> > toolchain, the fix chosen was to become ABI compatible again.
> 
> Right.  I'd forgotten this.  Really, the ABI ought to have the
> necessary relocations; but I'm not going to poke my nose in further
> right now.

I'm not opposed to extending the ABI, but it shouldn't happen as an
accident. Back when the embedded PIC was removed, nobody seemed to care
much about keeping the PC-relative relocations as general extension.


Thiemo