Your change to ldlang.c

Your change to ldlang.c

[Mark Mitchell]

  1999-06-23  Ian Lance Taylor  <ian@zembu.com>

	* ldlang.c (section_already_linked): Only discard link once
	sections if we are building constructors.

Ian --

  This change breaks the MIPS ABI with ld -r.  The reason is
that the .reginfo section uses SEC_LINK_ONCE to avoid duplication, and
that is true even when doing ld -r.  In fact, I'm not sure what the
purpose of your change was; I don't see why we should not discard link
once sections for ld -r things.

  Thoughts?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: Your change to ldlang.c

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Thu, 01 Jul 1999 17:58:06 -0700

     1999-06-23  Ian Lance Taylor  <ian@zembu.com>

	   * ldlang.c (section_already_linked): Only discard link once
	   sections if we are building constructors.

     This change breaks the MIPS ABI with ld -r.  The reason is
   that the .reginfo section uses SEC_LINK_ONCE to avoid duplication, and
   that is true even when doing ld -r.  In fact, I'm not sure what the
   purpose of your change was; I don't see why we should not discard link
   once sections for ld -r things.

I thought it was discussed on the list a bit, but I can't find the
messages, so it must have been somewhere else.  Richard, do you
remember?

My main concern was that it would mess up relocations.  If an input
section disappears, relocations against symbols in that section need
to be adjusted to use different symbols, and relocations for that
section need to disappear.  I doubt the backends do either of these
correctly.

We could go back to letting the emultempl file handle the .reginfo
sections for MIPS files when doing a relocateable link, I suppose.
Does anybody have a better suggestion?

Ian

Re: Your change to ldlang.c

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> I thought it was discussed on the list a bit, but I can't
    Ian> find the messages, so it must have been somewhere else.
    Ian> Richard, do you remember?

I vaguely remember you mentioning the patch, but I don't recall the
discussion. 

    Ian> My main concern was that it would mess up relocations.  If an
    Ian> input section disappears, relocations against symbols in that
    Ian> section need to be adjusted to use different symbols, and
    Ian> relocations for that section need to disappear.  I doubt the
    Ian> backends do either of these correctly.

I see.  Of course, in this case, there are no relocations against the
section.  

I guess I don't see why your solution is really any better.  On the
one hand, we may have bogus relocations.  On the other, we've violated
the SEC_LINK_ONCE semantics, especially for fixed-sized sections like
.reginfo. 

I guess I'm thinking we should undo your change, and then attack the
problem of making the back-ends handle the relocations correctly.
That should not be *that* hard.

    Ian> We could go back to letting the emultempl file handle the
    Ian> .reginfo sections for MIPS files when doing a relocateable
    Ian> link, I suppose.  Does anybody have a better suggestion?

I don't know what you mean by this suggestion.  I guess I'm too new to
binutils-land.  Could you elaborate on the emultempl file solution?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: Your change to ldlang.c

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Thu, 01 Jul 1999 20:30:04 -0700

       Ian> My main concern was that it would mess up relocations.  If an
       Ian> input section disappears, relocations against symbols in that
       Ian> section need to be adjusted to use different symbols, and
       Ian> relocations for that section need to disappear.  I doubt the
       Ian> backends do either of these correctly.

   I see.  Of course, in this case, there are no relocations against the
   section.  

   I guess I don't see why your solution is really any better.  On the
   one hand, we may have bogus relocations.  On the other, we've violated
   the SEC_LINK_ONCE semantics, especially for fixed-sized sections like
   .reginfo. 

It's not clear to me what SEC_LINK_ONCE semantics are in the case of a
relocateable link.  My first reaction when I noticed this was actually
that a relocateable link should keep all the sections, and that the
current behaviour was simply a bug.

SEC_LINK_ONCE was originally meant to be something along the lines of
PE COMDAT sections.  Can anybody check what the Microsoft linker does
when doing a relocateable link with multiple COMDAT sections?

   I guess I'm thinking we should undo your change, and then attack the
   problem of making the back-ends handle the relocations correctly.
   That should not be *that* hard.

It's not trivial, actually, when you consider relocations against
local symbols in a linkonce section.  In some cases those relocations
can simply be removed, as in a debugging section, but in other cases
you need to actually modify the relocation to a different section.  I
think every backend would have to be modified.

       Ian> We could go back to letting the emultempl file handle the
       Ian> .reginfo sections for MIPS files when doing a relocateable
       Ian> link, I suppose.  Does anybody have a better suggestion?

   I don't know what you mean by this suggestion.  I guess I'm too new to
   binutils-land.  Could you elaborate on the emultempl file solution?

Until quite recently (February 1999), .reginfo sections were not
marked as LINK_ONCE, and they were instead handled in the emultempl
file.  The special handling was removed by this patch:

Wed Feb 17 12:10:06 1999  Stan Cox  <scox@cygnus.com>

	* mpw-elfmips.c (gldelf32ebmip_before_allocation): Remove special
	.reginfo section handling. 
	* emultempl/elf32.em (gld${EMULATION_NAME}_before_allocation): Likewise.
	* emulparams/elf32elmip.sh (INITIAL_READONLY_SECTIONS): Removed
	* emulparams/elf32ebmip.sh (INITIAL_READONLY_SECTIONS): Removed

Index: elf32.em
===================================================================
RCS file: /cvs/cvsfiles/devo/ld/emultempl/elf32.em,v
retrieving revision 1.57
retrieving revision 1.58
diff -u -r1.57 -r1.58
--- elf32.em	1998/09/20 04:16:36	1.57
+++ elf32.em	1999/02/17 19:58:05	1.58
@@ -762,37 +762,6 @@
 	s->_raw_size = 0;
       }
   }
-
-#if defined (TARGET_IS_elf32bmip) || defined (TARGET_IS_elf32lmip)
-  /* For MIPS ELF the .reginfo section requires special handling.
-     Each input section is 24 bytes, and the final output section must
-     also be 24 bytes.  We handle this by clobbering all but the first
-     input section size to 0.  The .reginfo section is handled
-     specially by the backend code anyhow.  */
-  {
-    boolean found = false;
-    LANG_FOR_EACH_INPUT_STATEMENT (is)
-      {
-	asection *s;
-
-	if (is->just_syms_flag)
-	  continue;
-
-	s = bfd_get_section_by_name (is->the_bfd, ".reginfo");
-	if (s == NULL)
-	  continue;
-
-	if (! found)
-	  {
-	    found = true;
-	    continue;
-	  }
-
-	s->_raw_size = 0;
-	s->_cooked_size = 0;
-      }
-  }
-#endif
 }
 
 /* This is called by the before_allocation routine via

Re: Your change to ldlang.c

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> It's not clear to me what SEC_LINK_ONCE semantics are in the
    Ian> case of a relocateable link.  My first reaction when I
    Ian> noticed this was actually that a relocateable link should
    Ian> keep all the sections, and that the current behaviour was
    Ian> simply a bug.

Well, here's what it says in the code.  

	  /* When linking, duplicate sections of the same name should be
	    discarded, rather than being combined into a single section as
	    is usually done.  This is similar to how common symbols are
	    handled.  See SEC_LINK_DUPLICATES below.  */
  #define SEC_LINK_ONCE 0x100000

I think that's pretty clear, actually.  In all cases, "duplicate
sections of the same name should be discarded".  The
SEC_LINK_DUPLICATES flags indicate when to complain about what, but
don't have anything to do with the semantics.

I see no reason why `ld -r' should behave differently here.  In fact,
in my Makefiles, I generally assume that `ld -r' followed by plain
`ld' on the resulting object is roughly the same as plain `ld' on all
the objects I used `ld -r' on in the first place; `ld -r' is just a
way of dividing the whole link into stages.  For example, I'd be
wicked surprised to find that `ld -r' followed by `ld' yielded a
substantially bigger executable than plain `ld' in the first place,
but unless I misunderstand, that's what your patch does.

I don't get the relocation issue, but I guess I'm just being dumb.  It
seems like these semantics say that you can take the first input file
which defines the section, together with the relocations it defines
for that section, and dump everything else.  I *thought* that the
idea was that you used SEC_LINK_ONCE for sections that should be the
same everywhere, but that had no obvious canonical location.  For
example, you might generate a common definition in lots of places, and
then smash them to one definition at link-time, but it shouldn't
matter which one you choose.

Seems easy enough, but guess I'm missing something?

    Ian> Until quite recently (February 1999), .reginfo sections were
    Ian> not marked as LINK_ONCE, and they were instead handled in the
    Ian> emultempl file.  The special handling was removed by this
    Ian> patch:

I see.  If I can't persuade you that my interpretation of
SEC_LINK_ONCE is the One True Way :-), then may we revert this change?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: Your change to ldlang.c

[H.J. Lu]

> Seems easy enough, but guess I'm missing something?
> 
>     Ian> Until quite recently (February 1999), .reginfo sections were
>     Ian> not marked as LINK_ONCE, and they were instead handled in the
>     Ian> emultempl file.  The special handling was removed by this
>     Ian> patch:
> 
> I see.  If I can't persuade you that my interpretation of
> SEC_LINK_ONCE is the One True Way :-), then may we revert this change?
> 

I believe this change is needed for Linux/MIPS. Check with the
Linux/MIPS people before reverting it.

-- 
H.J. Lu (hjl@gnu.org)

Re: Your change to ldlang.c

[Mikey]

On Thu, 01 Jul 1999 23:15:49 -0700, you wrote:

>>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:
>
>    Ian> It's not clear to me what SEC_LINK_ONCE semantics are in the
>    Ian> case of a relocateable link.  My first reaction when I
>    Ian> noticed this was actually that a relocateable link should
>    Ian> keep all the sections, and that the current behaviour was
>    Ian> simply a bug.
>
>Well, here's what it says in the code.  
>
>	  /* When linking, duplicate sections of the same name should be
>	    discarded, rather than being combined into a single section as
>	    is usually done.  This is similar to how common symbols are
>	    handled.  See SEC_LINK_DUPLICATES below.  */

This is incorrect, it should read 

/* When linking, duplicate sections of the same name AND COMDAT SYMBOL should be discarded.

Pei ld/bfd does not implement this 
correctly, which is part of the reason
linking with vc5 implibs doesn't work. 

The rest is the sorting of the symbol table
debug or local symbols can't be comdats, ;-)
and the section sorting of .idata$X's still isn't right-(

>  #define SEC_LINK_ONCE 0x100000
>
>I think that's pretty clear, actually.  In all cases, "duplicate
>sections of the same name should be discarded".  The
>SEC_LINK_DUPLICATES flags indicate when to complain about what, but
>don't have anything to do with the semantics.
>
>I see no reason why `ld -r' should behave differently here.  In fact,
>in my Makefiles, I generally assume that `ld -r' followed by plain
>`ld' on the resulting object is roughly the same as plain `ld' on all
>the objects I used `ld -r' on in the first place; `ld -r' is just a
>way of dividing the whole link into stages.  For example, I'd be
>wicked surprised to find that `ld -r' followed by `ld' yielded a
>substantially bigger executable than plain `ld' in the first place,
>but unless I misunderstand, that's what your patch does.
>
>I don't get the relocation issue, but I guess I'm just being dumb.  It
>seems like these semantics say that you can take the first input file
>which defines the section, together with the relocations it defines
>for that section, and dump everything else.  I *thought* that the
>idea was that you used SEC_LINK_ONCE for sections that should be the
>same everywhere, but that had no obvious canonical location.  For
>example, you might generate a common definition in lots of places, and
>then smash them to one definition at link-time, but it shouldn't
>matter which one you choose.
>
>Seems easy enough, but guess I'm missing something?
>
>    Ian> Until quite recently (February 1999), .reginfo sections were
>    Ian> not marked as LINK_ONCE, and they were instead handled in the
>    Ian> emultempl file.  The special handling was removed by this
>    Ian> patch:
>
>I see.  If I can't persuade you that my interpretation of
>SEC_LINK_ONCE is the One True Way :-), then may we revert this change?

Re: Your change to ldlang.c

[Ian Lance Taylor]

   From: jeffdbREMOVETHIS@goodnet.com (Mikey)
   Date: Fri, 02 Jul 1999 06:13:57 GMT

   >	  /* When linking, duplicate sections of the same name should be
   >	    discarded, rather than being combined into a single section as
   >	    is usually done.  This is similar to how common symbols are
   >	    handled.  See SEC_LINK_DUPLICATES below.  */

   This is incorrect, it should read 

   /* When linking, duplicate sections of the same name AND COMDAT SYMBOL should be discarded.

   Pei ld/bfd does not implement this 
   correctly, which is part of the reason
   linking with vc5 implibs doesn't work. 

Yes, I didn't want to get mired in this aspect of the issue.  It's
true that although SEC_LINK_ONCE sections were inspired by COMDAT
sections, they don't actually implement them correctly.

Fortunately Don Terry has patches to fix these problems with the PE
format, and he is working with me to get them into the mainline
sources.

Ian

Re: Your change to ldlang.c

[Ian Lance Taylor]

   Date: Thu, 1 Jul 1999 23:51:39 -0700 (PDT)
   From: hjl@lucon.org (H.J. Lu)

   > Seems easy enough, but guess I'm missing something?
   > 
   >     Ian> Until quite recently (February 1999), .reginfo sections were
   >     Ian> not marked as LINK_ONCE, and they were instead handled in the
   >     Ian> emultempl file.  The special handling was removed by this
   >     Ian> patch:
   > 
   > I see.  If I can't persuade you that my interpretation of
   > SEC_LINK_ONCE is the One True Way :-), then may we revert this change?
   > 

   I believe this change is needed for Linux/MIPS. Check with the
   Linux/MIPS people before reverting it.

Actually, it was the Linux/MIPS people who suggested using LINK_ONCE
for .reginfo.  Once that is done, the change to elf32.em becomes no
longer necessary (modulo concerns about ld -r).  That is why they took
it out--not because they needed the change, but because other changes
they made meant that that code was no longer needed.

Ian

Re: Your change to ldlang.c

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Thu, 01 Jul 1999 23:15:49 -0700

       Ian> It's not clear to me what SEC_LINK_ONCE semantics are in the
       Ian> case of a relocateable link.  My first reaction when I
       Ian> noticed this was actually that a relocateable link should
       Ian> keep all the sections, and that the current behaviour was
       Ian> simply a bug.

   Well, here's what it says in the code.  

	     /* When linking, duplicate sections of the same name should be
	       discarded, rather than being combined into a single section as
	       is usually done.  This is similar to how common symbols are
	       handled.  See SEC_LINK_DUPLICATES below.  */
     #define SEC_LINK_ONCE 0x100000

   I think that's pretty clear, actually.  In all cases, "duplicate
   sections of the same name should be discarded".  The
   SEC_LINK_DUPLICATES flags indicate when to complain about what, but
   don't have anything to do with the semantics.

Well, I wrote that comment in the first place.  The fact that I forgot
to think about ld -r then doesn't necessarily imply that I am wrong
now.

   I see no reason why `ld -r' should behave differently here.  In fact,
   in my Makefiles, I generally assume that `ld -r' followed by plain
   `ld' on the resulting object is roughly the same as plain `ld' on all
   the objects I used `ld -r' on in the first place; `ld -r' is just a
   way of dividing the whole link into stages.  For example, I'd be
   wicked surprised to find that `ld -r' followed by `ld' yielded a
   substantially bigger executable than plain `ld' in the first place,
   but unless I misunderstand, that's what your patch does.

I hope you misunderstand.  I agree that that would be very bad.
However, although they sections are not discarded during ld -r, they
should nevertheless be discarded during the final link.

But now that I think about it a bit more, I see what you mean.  The ld
-r will wind up merging all the sections into one, and thus you will
get a large link once section.  Actually, the original ones should be
copied into the output file unchanged.

You're right, I messed up here.  So maybe we should just revert that
patch, and then try to sort out the relocation issues, although I'm
afraid that will be fairly complicated.

Richard, I recall that you mentioned encountering a bug with ld -r and
link once sections.  Can you recall any details?

   I don't get the relocation issue, but I guess I'm just being dumb.  It
   seems like these semantics say that you can take the first input file
   which defines the section, together with the relocations it defines
   for that section, and dump everything else.  I *thought* that the
   idea was that you used SEC_LINK_ONCE for sections that should be the
   same everywhere, but that had no obvious canonical location.  For
   example, you might generate a common definition in lots of places, and
   then smash them to one definition at link-time, but it shouldn't
   matter which one you choose.

   Seems easy enough, but guess I'm missing something?

I'm not all that worried about the relocations for the input section
being discarded, although in some object file formats that will indeed
be a pain to deal with.

My concern is related to relocations in other input sections which
refer to symbols defined in the input section being discarded,
particularly if those relocations refer to local symbols in the
section being discarded.  The correct semantics would seem to be to
convert those relocations to instead refer to the link once section
which is being kept.  But actually if the relocations in question are
found in the debugging information, then I'm not even sure whether
that is right.

       Ian> Until quite recently (February 1999), .reginfo sections were
       Ian> not marked as LINK_ONCE, and they were instead handled in the
       Ian> emultempl file.  The special handling was removed by this
       Ian> patch:

   I see.  If I can't persuade you that my interpretation of
   SEC_LINK_ONCE is the One True Way :-), then may we revert this change?

Reverting that change is not a problem.  It was only done as a point
of cleanliness when it appeared that it was no longer needed.


Actually, I'm going to revert my ld -r change, at least for now.
You've convinced me that it will never do the right thing as it
stands, whereas removing it will at least sometimes do the right
thing.  We can put it back again if we figure out some different way
to solve the problems that it causes.

Ian

Re: Your change to ldlang.c

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> Well, I wrote that comment in the first place.  The fact that
    Ian> I forgot to think about ld -r then doesn't necessarily imply
    Ian> that I am wrong now.

True 'nuff.

    Ian> My concern is related to relocations in other input sections
    Ian> which refer to symbols defined in the input section being
    Ian> discarded, particularly if those relocations refer to local
    Ian> symbols in the section being discarded.  The correct
    Ian> semantics would seem to be to convert those relocations to
    Ian> instead refer to the link once section which is being kept.

OK, now I see what's worrying you.  Obviously, a local symbol in the
LINK_ONCE section can only be a problem if it is referred to by
something outside the LINK_ONCE section.  The semantics of that are
really weird, anyhow, IMHO, although I am by no means a PE expert.
(I've actually researched PE to some degree, but by no means
exhaustively, in the past.)

    Ian> Actually, I'm going to revert my ld -r change, at least for
    Ian> now.  You've convinced me that it will never do the right
    Ian> thing as it stands, whereas removing it will at least
    Ian> sometimes do the right thing.  We can put it back again if we
    Ian> figure out some different way to solve the problems that it
    Ian> causes.

Thanks!

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: Your change to ldlang.c

[Ralf Baechle]

On Fri, Jul 02, 1999 at 10:26:20AM -0400, Ian Lance Taylor wrote:

>    I believe this change is needed for Linux/MIPS. Check with the
>    Linux/MIPS people before reverting it.
> 
> Actually, it was the Linux/MIPS people who suggested using LINK_ONCE
> for .reginfo.  Once that is done, the change to elf32.em becomes no
> longer necessary (modulo concerns about ld -r).  That is why they took
> it out--not because they needed the change, but because other changes
> they made meant that that code was no longer needed.

It was me who suggested that change but it's not a Linux/MIPS specific
change.  The problem was that ld combined all the .reginfo sections,
each 0x18 bytes long.  Now assume we have a few hundred object files for
a thing like libc.  Then the sum of these files gets longer than let's
say two pages.  The linker assigns the addresses to the sections, then
does the segment layout.  Finally the MIPS backend changes the size of
the combined .reginfo section back to 0x18 bytes.

The result: We now get an unused page in the memory layout, so ld will
split that segment into two segments and we run out of space in the
header table.  Boom, linker crash.  That' how I remember things, not
shure if the details are correct.

  Ralf

Re: Your change to ldlang.c

[Richard Henderson]

On Fri, Jul 02, 1999 at 10:39:28AM -0400, Ian Lance Taylor wrote:
> Richard, I recall that you mentioned encountering a bug with ld -r and
> link once sections.  Can you recall any details?

Nope.  I just remember seeing a Case that complained about it.
I am insufficiently facile with the new problem logging widget
to be able to find it again from home.


r~