RFA: MIPS's _gp symbol and the new orphan-placement code

RFA: MIPS's _gp symbol and the new orphan-placement code

[Richard Sandiford]

This patch has been on the back burner since Alan's recent(ish) changes
to the way orphan sections are placed.  This message isn't a complaint
about those changes at all.   However, they did tickle a problem with
the default linker scripts for n32 and n64 on IRIX.

According to the ABI docs, the old .reginfo sections were superceded by
.MIPS.options in n32 and n64, so the associated IRIX linker scripts don't
define a .reginfo section.  Nevertheless, the system /usr/lib32/crt*.o
files do have .reginfo sections, so the linker ends up creating an
output .reginfo and placing it as an orphan.

The new orphan placement code puts this .reginfo section right before
.got.  This should be fine in theory, but unfortunately, the linker
scripts set up _gp using:

  _gp = ALIGN(16) + 0x7ff0;
  .got            : { *(.got.plt) *(.got) }

where the assignment to _gp is provided by OTHER_GOT_SYMBOLS.  bfd's
lazy binding stubs require that _gp - .got be exactly 0x7ff0 and putting
.reginfo before .got will of course break this rule.  As a result, most
objects will quickly segfault.

This definition of _gp has caused problems before (when Thiemo wanted to
decrease the alignment of .got from 16 to the IRIX-standard 4).  In that
instance, we ended up keeping the original alignment for compatibility
with older tools (a good thing IMO), but I think this time we really
will have to change the way _gp is set.

There were lots of different this could be fixed, such as:

  (1) Replacing ALIGN(16) with ADDR(.got).

      --> Unfortunately, this means that every output object will
          get a .got section, which is a bit untidy, although I don't
          know if it would be outright harmful.

  (2) Moving the assignment into .got and using ". + 0x7ff0".

      --> Same problem as (1).

  (3) Defining _gp in the linker instead.

      --> I think it would be difficult to do this and keep backward
          compatibility with existing linker scripts (including the
          kind of scripts used for *-elf targets).

  (4) Just adding .reginfo to the linker script.

      --> Gets us past the current failure point, but it's really just
          hiding the problem.

  (5) Adjusting bfd so that it can cope with offsets other than 0x7ff0.

      --> We don't really want smaller offsets since they'd just lead to
          tighter limits on GOT size.  Going this route would also make
          the bfd code even more complex (I think it would affect the
          stub creation code and multigot partitioning code at the
          very least).

So in the end I went for:

  (6) Defining _gp as follows:

        _gp = (DEFINED (_GLOBAL_OFFSET_TABLE_)
               ? _GLOBAL_OFFSET_TABLE_
               : ALIGN (16)) + 0x7ff0;

      This means that, if the object does need a GOT, _G_O_T_ - gp will
      always be 0x7ff0, just like the bfd linker code requires.  _gp will
      retain its current value otherwise.

Tested with a gcc bootstrap & regression test (all three ABIs)
on mips-sgi-irix6.5.  Also tested against the binutils testsuite
with mips64-elf and mips64-linux-gnu.  OK to install?

Richard


	* emulparams/elf32bmip.sh (OTHER_GOT_SYMBOLS): Make the definition
	of _gp more robust.
	* emulparams/elf32bmipn32-defs.sh (OTHER_GOT_SYMBOLS): Likewise.

Index: ld/emulparams/elf32bmip.sh
===================================================================
RCS file: /cvs/src/src/ld/emulparams/elf32bmip.sh,v
retrieving revision 1.6
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.6 elf32bmip.sh
--- ld/emulparams/elf32bmip.sh	21 Apr 2004 20:52:28 -0000	1.6
+++ ld/emulparams/elf32bmip.sh	6 Nov 2004 23:46:36 -0000
@@ -17,7 +17,9 @@ INITIAL_READONLY_SECTIONS="
 "
 OTHER_TEXT_SECTIONS='*(.mips16.fn.*) *(.mips16.call.*)'
 OTHER_GOT_SYMBOLS='
-  _gp = ALIGN(16) + 0x7ff0;
+  _gp = (DEFINED (_GLOBAL_OFFSET_TABLE_)
+	 ? _GLOBAL_OFFSET_TABLE_
+	 : ALIGN (16)) + 0x7ff0;
 '
 OTHER_SDATA_SECTIONS="
   .lit8         ${RELOCATING-0} : { *(.lit8) }
Index: ld/emulparams/elf32bmipn32-defs.sh
===================================================================
RCS file: /cvs/src/src/ld/emulparams/elf32bmipn32-defs.sh,v
retrieving revision 1.6
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.6 elf32bmipn32-defs.sh
--- ld/emulparams/elf32bmipn32-defs.sh	14 Feb 2004 11:45:25 -0000	1.6
+++ ld/emulparams/elf32bmipn32-defs.sh	6 Nov 2004 23:46:36 -0000
@@ -37,7 +37,9 @@ ENTRY=__start
 
 # GOT-related settings.  
 OTHER_GOT_SYMBOLS='
-  _gp = ALIGN(16) + 0x7ff0;
+  _gp = (DEFINED (_GLOBAL_OFFSET_TABLE_)
+	 ? _GLOBAL_OFFSET_TABLE_
+	 : ALIGN (16)) + 0x7ff0;
 '
 OTHER_SDATA_SECTIONS="
   .lit8         ${RELOCATING-0} : { *(.lit8) }

Re: RFA: MIPS's _gp symbol and the new orphan-placement code

[Alan Modra]

On Sun, Nov 07, 2004 at 09:06:04AM +0000, Richard Sandiford wrote:
> The new orphan placement code puts this .reginfo section right before
> .got.  This should be fine in theory, but unfortunately, the linker
> scripts set up _gp using:
> 
>   _gp = ALIGN(16) + 0x7ff0;
>   .got            : { *(.got.plt) *(.got) }

Sorry about the breakage.  It's quite difficult to decide where an
orphan section can be inserted.  We really need some sort of syntactic
hint that ties the assignment to the section.  One such hint is to
write:

  . = .;
  _gp = ALIGN(16) + 0x7ff0;
  .got            : { *(.got.plt) *(.got) }

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: RFA: MIPS's _gp symbol and the new orphan-placement code

[Daniel Jacobowitz]

On Mon, Nov 08, 2004 at 09:18:25AM +1030, Alan Modra wrote:
> On Sun, Nov 07, 2004 at 09:06:04AM +0000, Richard Sandiford wrote:
> > The new orphan placement code puts this .reginfo section right before
> > .got.  This should be fine in theory, but unfortunately, the linker
> > scripts set up _gp using:
> > 
> >   _gp = ALIGN(16) + 0x7ff0;
> >   .got            : { *(.got.plt) *(.got) }
> 
> Sorry about the breakage.  It's quite difficult to decide where an
> orphan section can be inserted.  We really need some sort of syntactic
> hint that ties the assignment to the section.  One such hint is to
> write:
> 
>   . = .;
>   _gp = ALIGN(16) + 0x7ff0;
>   .got            : { *(.got.plt) *(.got) }

Or maybe require/support syntactic hints saying where orphan sections
can be inserted?

-- 
Daniel Jacobowitz

Re: RFA: MIPS's _gp symbol and the new orphan-placement code

[Alan Modra]

On Sun, Nov 07, 2004 at 08:16:36PM -0500, Daniel Jacobowitz wrote:
> On Mon, Nov 08, 2004 at 09:18:25AM +1030, Alan Modra wrote:
> > We really need some sort of syntactic
> > hint that ties the assignment to the section.  One such hint is to
> > write:
> > 
> >   . = .;
> >   _gp = ALIGN(16) + 0x7ff0;
> >   .got            : { *(.got.plt) *(.got) }
> 
> Or maybe require/support syntactic hints saying where orphan sections
> can be inserted?

Yes, extra braces would work, or even taking note of blank lines.  The
latter has the advantage that many existing scripts would "just work".

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: RFA: MIPS's _gp symbol and the new orphan-placement code

[Richard Sandiford]

Alan Modra <amodra@bigpond.net.au> writes:
> On Sun, Nov 07, 2004 at 09:06:04AM +0000, Richard Sandiford wrote:
>> The new orphan placement code puts this .reginfo section right before
>> .got.  This should be fine in theory, but unfortunately, the linker
>> scripts set up _gp using:
>> 
>>   _gp = ALIGN(16) + 0x7ff0;
>>   .got            : { *(.got.plt) *(.got) }
>
> Sorry about the breakage.

Not at all...

> It's quite difficult to decide where an orphan section can be
> inserted.

...I totally agree, and like I said in my original post, I've no
complaint against your change at all.  Setting _gp outside .got
was always a flaky construct.

All the follow-ups were about changing the placement heuristic, but I
don't think any of the suggestions would make existing MIPS scripts work
out of the box.  Any tweak which _does_ make them work could well break
other scripts.

I think we're going to have change the MIPS scripts whatever happens,
and if we're going to do that, we might as well try to make them more
robust.  Does anyone have any comments about the new _gp assignment in
my patch?  Is it better, or could it lead to other problems?  Do you
prefer the idea of introducing new orphan placement hints and keeping
the assignment as-is?

FWIW:

Alan Modra <amodra@bigpond.net.au> writes:
> On Sun, Nov 07, 2004 at 08:16:36PM -0500, Daniel Jacobowitz wrote:
>> On Mon, Nov 08, 2004 at 09:18:25AM +1030, Alan Modra wrote:
>> > We really need some sort of syntactic
>> > hint that ties the assignment to the section.  One such hint is to
>> > write:
>> > 
>> >   . = .;
>> >   _gp = ALIGN(16) + 0x7ff0;
>> >   .got            : { *(.got.plt) *(.got) }
>> 
>> Or maybe require/support syntactic hints saying where orphan sections
>> can be inserted?
>
> Yes, extra braces would work, or even taking note of blank lines.  The
> latter has the advantage that many existing scripts would "just work".

Given that scripts aren't white-space sensitive right now, it might be
a bit confusing to give special meaning to blank lines.  Also (from a
selfish MIPS standpoint) it wouldn't be enough to make the IRIX case
work since the default script has no blank lines. ;)

Perhaps one possibility would be: don't insert orphan sections after any
assignment with a positive offset from something based on ".".  But I
don't like that much. ;)  Some unequivocal syntax would be better...

Richard

Re: RFA: MIPS's _gp symbol and the new orphan-placement code

[Alan Modra]

On Sat, Nov 13, 2004 at 09:37:23PM +0000, Richard Sandiford wrote:
> robust.  Does anyone have any comments about the new _gp assignment in
> my patch?  Is it better, or could it lead to other problems?  Do you
> prefer the idea of introducing new orphan placement hints and keeping
> the assignment as-is?

I think your patch is OK, but I prefer the ". = ." hint for simplicity.
As far as I'm concerned, it's up to one of the mips maintainers to
say what they like.  ;)

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: RFA: MIPS's _gp symbol and the new orphan-placement code

[Eric Christopher]

On Sun, 2004-11-14 at 17:41, Alan Modra wrote:
> On Sat, Nov 13, 2004 at 09:37:23PM +0000, Richard Sandiford wrote:
> > robust.  Does anyone have any comments about the new _gp assignment in
> > my patch?  Is it better, or could it lead to other problems?  Do you
> > prefer the idea of introducing new orphan placement hints and keeping
> > the assignment as-is?
> 
> I think your patch is OK, but I prefer the ". = ." hint for simplicity.
> As far as I'm concerned, it's up to one of the mips maintainers to
> say what they like.  ;)

I'm fine with it as he suggests. I don't have a strong opinion as long
as it's documented in some way :)

I can't remember if you commented the hints or not, could you please do
that if you didn't?

thanks.

-eric

-- 
Eric Christopher <echristo@redhat.com>

RFA: elf.sc patch (was Re: RFA: MIPS's _gp symbol and the new orphan-placement code)

[Richard Sandiford]

I'm a dunce.  I started this thread in November, when some changes
to the orphan code had the inadvertent (but totally understandable)
side-effect of breaking IRIX:

    http://sources.redhat.com/ml/binutils/2004-11/msg00116.html

My suggested fix was to change the way _gp is set, but although it
seemed to work, and was approved, I was too afraid to apply it for
fear of breaking some obscure target.

Alan's first suggestion was to use ". = ." as a placement hint instead:

    http://sources.redhat.com/ml/binutils/2004-11/msg00122.html

The thread continued with ideas for a more elaborate hint syntax, and
because of that, I'd somehow got the idea that we were talking about
adding a new feature here.  I didn't really like the ". = ." idiom
and put the whole thing on hold.

What I hadn't realised was that ". = ." was _already_ a supported
placement hint (and that that was why Alan was suggesting it).  Doh...

Given that ". = ." is the official syntax, and given that the whole
point of OTHER_GOT_SYMBOLS is to define symbols related to GOT_SECTIONS,
I agree that adding it to the script makes sense.  The patch below does
that and fixes the IRIX problem without any change to OTHER_GOT_SYMBOLS.

Tested on mips-sgi-irix6.5.  OK to install?

Richard


	* scripttempl/elf.sc: Insert ". = .;" before OTHER_GOT_SYMBOLS.

Index: ld/scripttempl/elf.sc
===================================================================
RCS file: /cvs/src/src/ld/scripttempl/elf.sc,v
retrieving revision 1.50
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.50 elf.sc
*** ld/scripttempl/elf.sc	26 Oct 2004 18:41:52 -0000	1.50
--- ld/scripttempl/elf.sc	20 Jan 2005 22:07:48 -0000
*************** cat <<EOF
*** 380,385 ****
--- 380,386 ----
    ${SMALL_DATA_CTOR+${RELOCATING+${CTOR}}}
    ${SMALL_DATA_DTOR+${RELOCATING+${DTOR}}}
    ${DATA_PLT+${PLT_BEFORE_GOT+${PLT}}}
+   ${RELOCATING+${OTHER_GOT_SYMBOLS+. = .;}}
    ${RELOCATING+${OTHER_GOT_SYMBOLS}}
    ${NO_SMALL_DATA-${GOT}}
    ${OTHER_GOT_SECTIONS}

Re: RFA: elf.sc patch (was Re: RFA: MIPS's _gp symbol and the new orphan-placement code)

[Alan Modra]

On Sat, Jan 22, 2005 at 10:43:17AM +0000, Richard Sandiford wrote:
> 	* scripttempl/elf.sc: Insert ". = .;" before OTHER_GOT_SYMBOLS.

I'd prefer you do this by modifying OTHER_GOT_SYMBOLS in elf32bmip.sh
and elf32bmipn32-defs.sh, perhaps with a comment.  OTHER_GOT_SYMBOLS
may not always be used to only define symbols, and placing ".=.;" in the
source close to where you assign a symbol value might clue future
hackers to guard other random symbol assignments.  Also, four other
scripts use OTHER_GOT_SYMBOLS, so if you change elf.sc you really ought
to change them too.  Patch to emulparams/* preapproved.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: RFA: elf.sc patch

[Richard Sandiford]

Alan Modra <amodra@bigpond.net.au> writes:
> On Sat, Jan 22, 2005 at 10:43:17AM +0000, Richard Sandiford wrote:
>> 	* scripttempl/elf.sc: Insert ". = .;" before OTHER_GOT_SYMBOLS.
>
> I'd prefer you do this by modifying OTHER_GOT_SYMBOLS in elf32bmip.sh
> and elf32bmipn32-defs.sh, perhaps with a comment.  OTHER_GOT_SYMBOLS
> may not always be used to only define symbols, and placing ".=.;" in the
> source close to where you assign a symbol value might clue future
> hackers to guard other random symbol assignments.  Also, four other
> scripts use OTHER_GOT_SYMBOLS, so if you change elf.sc you really ought
> to change them too.  Patch to emulparams/* preapproved.

OK, thanks, here's what I checked in.

Richard


	* emulparams/elf32bmip.sh (OTHER_GOT_SECTIONS): Add ". = .;".
	* emulparams/elf32bmipn32-defs.sh (OTHER_GOT_SECTIONS): Likewise.

Index: ld/emulparams/elf32bmip.sh
===================================================================
RCS file: /cvs/src/src/ld/emulparams/elf32bmip.sh,v
retrieving revision 1.6
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.6 elf32bmip.sh
*** ld/emulparams/elf32bmip.sh	21 Apr 2004 20:52:28 -0000	1.6
--- ld/emulparams/elf32bmip.sh	22 Jan 2005 18:08:18 -0000
*************** INITIAL_READONLY_SECTIONS="
*** 16,22 ****
--- 16,27 ----
    .reginfo      ${RELOCATING-0} : { *(.reginfo) }
  "
  OTHER_TEXT_SECTIONS='*(.mips16.fn.*) *(.mips16.call.*)'
+ # If the output has a GOT section, there must be exactly 0x7ff0 bytes
+ # between .got and _gp.  The ". = ." below stops the orphan code from
+ # inserting other sections between the assignment to _gp and the start
+ # of .got.
  OTHER_GOT_SYMBOLS='
+   . = .;
    _gp = ALIGN(16) + 0x7ff0;
  '
  OTHER_SDATA_SECTIONS="
Index: ld/emulparams/elf32bmipn32-defs.sh
===================================================================
RCS file: /cvs/src/src/ld/emulparams/elf32bmipn32-defs.sh,v
retrieving revision 1.6
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.6 elf32bmipn32-defs.sh
*** ld/emulparams/elf32bmipn32-defs.sh	14 Feb 2004 11:45:25 -0000	1.6
--- ld/emulparams/elf32bmipn32-defs.sh	22 Jan 2005 18:08:18 -0000
*************** MAXPAGESIZE=0x100000
*** 36,42 ****
--- 36,47 ----
  ENTRY=__start
  
  # GOT-related settings.  
+ # If the output has a GOT section, there must be exactly 0x7ff0 bytes
+ # between .got and _gp.  The ". = ." below stops the orphan code from
+ # inserting other sections between the assignment to _gp and the start
+ # of .got.
  OTHER_GOT_SYMBOLS='
+   . = .;
    _gp = ALIGN(16) + 0x7ff0;
  '
  OTHER_SDATA_SECTIONS="