[PATCH] Change MIPS linker stubs to allow for more than 2^15 symbols.

[PATCH] Change MIPS linker stubs to allow for more than 2^15 symbols.

[David Daney]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

As noted in this thread:

http://sourceware.org/ml/binutils/2006-06/msg00086.html

The mips linker would not reliably allow for more than 2^15 dynamic 
symbols.  This prevents GCC's libgcj java runtime library from being 
correctly linked.

The consensus was to increase the size of the linker stubs by one 
instruction so that objects with many dynamic symbols could be supported.

I increased the size of the stub from 16 to 20 bytes.  The new stubs 
support up to 2^31 dynamic symbols.  That should be enough for at least 
the short term.  With the patch applied I can now successfully build and 
run libgcj from a recent GCC-4.2 snapshot on mipsel-linux.

Tested with a mipsel-linux cross build running on i686-pc-linux-gnu with 
no regressions in the testsuite.

Perhaps someone with a mips64 system could test there, as this patch 
effects that architecture as well.

Comments?

OK to commit?

How about to the 2.17 branch?

David Daney

bfd:
2006-06-07  David Daney  <ddaney@avtrex.com>

	* elfxx-mips.c (STUB_LI16): Made non sign extending.
	(STUB_LUI): New macro.
	(MIPS_FUNCTION_STUB_SIZE): Increased to 20.
	(_bfd_mips_elf_finish_dynamic_symbol): Changed stub generation
	to allow larger symbol table indexes.

[-- Attachment #2: mips-stubs.diff.txt --]
[-- Type: text/plain, Size: 2560 bytes --]

? doc/bfd.info
Index: elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.169
diff -c -p -r1.169 elfxx-mips.c
*** elfxx-mips.c	22 May 2006 15:06:22 -0000	1.169
--- elfxx-mips.c	7 Jun 2006 22:58:04 -0000
*************** static bfd *reldyn_sorting_bfd;
*** 631,642 ****
     ((ABI_64_P (abfd)						\
       ? 0x03e0782d		/* daddu t7,ra */		\
       : 0x03e07821))		/* addu t7,ra */
  #define STUB_JALR 0x0320f809	/* jalr t9,ra */
! #define STUB_LI16(abfd)                                         \
!   ((ABI_64_P (abfd)						\
!    ? 0x64180000			/* daddiu t8,zero,0 */		\
!    : 0x24180000))		/* addiu t8,zero,0 */
! #define MIPS_FUNCTION_STUB_SIZE (16)
  
  /* The name of the dynamic interpreter.  This is put in the .interp
     section.  */
--- 631,640 ----
     ((ABI_64_P (abfd)						\
       ? 0x03e0782d		/* daddu t7,ra */		\
       : 0x03e07821))		/* addu t7,ra */
+ #define STUB_LUI 0x3c180000     /* lui t8,0 */
  #define STUB_JALR 0x0320f809	/* jalr t9,ra */
! #define STUB_LI16 0x34180000	/* ori t8,zero,0 */
! #define MIPS_FUNCTION_STUB_SIZE (20)
  
  /* The name of the dynamic interpreter.  This is put in the .interp
     section.  */
*************** _bfd_mips_elf_finish_dynamic_symbol (bfd
*** 8013,8027 ****
  				   MIPS_ELF_STUB_SECTION_NAME (dynobj));
        BFD_ASSERT (s != NULL);
  
-       /* FIXME: Can h->dynindx be more than 64K?  */
-       if (h->dynindx & 0xffff0000)
- 	return FALSE;
- 
        /* Fill the stub.  */
        bfd_put_32 (output_bfd, STUB_LW (output_bfd), stub);
        bfd_put_32 (output_bfd, STUB_MOVE (output_bfd), stub + 4);
!       bfd_put_32 (output_bfd, STUB_JALR, stub + 8);
!       bfd_put_32 (output_bfd, STUB_LI16 (output_bfd) + h->dynindx, stub + 12);
  
        BFD_ASSERT (h->plt.offset <= s->size);
        memcpy (s->contents + h->plt.offset, stub, MIPS_FUNCTION_STUB_SIZE);
--- 8011,8023 ----
  				   MIPS_ELF_STUB_SECTION_NAME (dynobj));
        BFD_ASSERT (s != NULL);
  
        /* Fill the stub.  */
        bfd_put_32 (output_bfd, STUB_LW (output_bfd), stub);
        bfd_put_32 (output_bfd, STUB_MOVE (output_bfd), stub + 4);
!       bfd_put_32 (output_bfd, STUB_LUI + ((h->dynindx >> 16 ) & 0xffff),
!                   stub + 8);
!       bfd_put_32 (output_bfd, STUB_JALR, stub + 12);
!       bfd_put_32 (output_bfd, STUB_LI16 + (h->dynindx & 0xffff), stub + 16);
  
        BFD_ASSERT (h->plt.offset <= s->size);
        memcpy (s->contents + h->plt.offset, stub, MIPS_FUNCTION_STUB_SIZE);

Re: [PATCH] Change MIPS linker stubs to allow for more than 2^15 symbols.

[Thiemo Seufer]

David Daney wrote:
> As noted in this thread:
> 
> http://sourceware.org/ml/binutils/2006-06/msg00086.html
> 
> The mips linker would not reliably allow for more than 2^15 dynamic 
> symbols.  This prevents GCC's libgcj java runtime library from being 
> correctly linked.
> 
> The consensus was to increase the size of the linker stubs by one 
> instruction so that objects with many dynamic symbols could be supported.
> 
> I increased the size of the stub from 16 to 20 bytes.

It may be useful to use 24 bytes, this adheres to the ABI alignment
rule of 2^3.

> The new stubs 
> support up to 2^31 dynamic symbols. That should be enough for at least 
> the short term.  With the patch applied I can now successfully build and 
> run libgcj from a recent GCC-4.2 snapshot on mipsel-linux.

Hm, 2^32 symbols would nicely fit the maximum for NewABI.

[snip]
> *** 631,642 ****
>      ((ABI_64_P (abfd)						\
>        ? 0x03e0782d		/* daddu t7,ra */		\
>        : 0x03e07821))		/* addu t7,ra */
>   #define STUB_JALR 0x0320f809	/* jalr t9,ra */
> ! #define STUB_LI16(abfd)                                         \
> !   ((ABI_64_P (abfd)						\
> !    ? 0x64180000			/* daddiu t8,zero,0 */		\
> !    : 0x24180000))		/* addiu t8,zero,0 */
> ! #define MIPS_FUNCTION_STUB_SIZE (16)
>   
>   /* The name of the dynamic interpreter.  This is put in the .interp
>      section.  */
> --- 631,640 ----
>      ((ABI_64_P (abfd)						\
>        ? 0x03e0782d		/* daddu t7,ra */		\
>        : 0x03e07821))		/* addu t7,ra */
> + #define STUB_LUI 0x3c180000     /* lui t8,0 */
>   #define STUB_JALR 0x0320f809	/* jalr t9,ra */
> ! #define STUB_LI16 0x34180000	/* ori t8,zero,0 */
> ! #define MIPS_FUNCTION_STUB_SIZE (20)

Why not the conventional expansion of lui/(d)addiu?

[snip]
> *** 8013,8027 ****
>   				   MIPS_ELF_STUB_SECTION_NAME (dynobj));
>         BFD_ASSERT (s != NULL);
>   
> -       /* FIXME: Can h->dynindx be more than 64K?  */
> -       if (h->dynindx & 0xffff0000)
> - 	return FALSE;
> - 

When it is 2^31, shoudn't we continue to test for overflow?


Thiemo

Re: [PATCH] Change MIPS linker stubs to allow for more than 2^15 symbols.

[David Daney]

Thiemo Seufer wrote:
> David Daney wrote:
> 
>>As noted in this thread:
>>
>>http://sourceware.org/ml/binutils/2006-06/msg00086.html
>>
>>The mips linker would not reliably allow for more than 2^15 dynamic 
>>symbols.  This prevents GCC's libgcj java runtime library from being 
>>correctly linked.
>>
>>The consensus was to increase the size of the linker stubs by one 
>>instruction so that objects with many dynamic symbols could be supported.
>>
>>I increased the size of the stub from 16 to 20 bytes.
> 
> 
> It may be useful to use 24 bytes, this adheres to the ABI alignment
> rule of 2^3.

Which rule would that be?  Is there such a requirement for function 
entry points?

> 
> 
>>The new stubs 
>>support up to 2^31 dynamic symbols. That should be enough for at least 
>>the short term.  With the patch applied I can now successfully build and 
>>run libgcj from a recent GCC-4.2 snapshot on mipsel-linux.
> 
> 
> Hm, 2^32 symbols would nicely fit the maximum for NewABI.

I don't know mips64 well enough to know how to load an unsigned 32 bit 
constant.  So It had to be 2^31.


> 
> [snip]
> 
>>*** 631,642 ****
>>     ((ABI_64_P (abfd)						\
>>       ? 0x03e0782d		/* daddu t7,ra */		\
>>       : 0x03e07821))		/* addu t7,ra */
>>  #define STUB_JALR 0x0320f809	/* jalr t9,ra */
>>! #define STUB_LI16(abfd)                                         \
>>!   ((ABI_64_P (abfd)						\
>>!    ? 0x64180000			/* daddiu t8,zero,0 */		\
>>!    : 0x24180000))		/* addiu t8,zero,0 */
>>! #define MIPS_FUNCTION_STUB_SIZE (16)
>>  
>>  /* The name of the dynamic interpreter.  This is put in the .interp
>>     section.  */
>>--- 631,640 ----
>>     ((ABI_64_P (abfd)						\
>>       ? 0x03e0782d		/* daddu t7,ra */		\
>>       : 0x03e07821))		/* addu t7,ra */
>>+ #define STUB_LUI 0x3c180000     /* lui t8,0 */
>>  #define STUB_JALR 0x0320f809	/* jalr t9,ra */
>>! #define STUB_LI16 0x34180000	/* ori t8,zero,0 */
>>! #define MIPS_FUNCTION_STUB_SIZE (20)
> 
> 
> Why not the conventional expansion of lui/(d)addiu?

We are not using hi/lo relocations, so there is no requirement for this. 
  The calculation in the linker is easier this way.

> 
> [snip]
> 
>>*** 8013,8027 ****
>>  				   MIPS_ELF_STUB_SECTION_NAME (dynobj));
>>        BFD_ASSERT (s != NULL);
>>  
>>-       /* FIXME: Can h->dynindx be more than 64K?  */
>>-       if (h->dynindx & 0xffff0000)
>>- 	return FALSE;
>>- 
> 
> 
> When it is 2^31, shoudn't we continue to test for overflow?
> 
Probably, but I got lazy.

Please comment on my responses, and then I will generate a new patch.

David Daney.

Re: [PATCH] Change MIPS linker stubs to allow for more than 2^15 symbols.

[Thiemo Seufer]

David Daney wrote:
> Thiemo Seufer wrote:
> >David Daney wrote:
> >
> >>As noted in this thread:
> >>
> >>http://sourceware.org/ml/binutils/2006-06/msg00086.html
> >>
> >>The mips linker would not reliably allow for more than 2^15 dynamic 
> >>symbols.  This prevents GCC's libgcj java runtime library from being 
> >>correctly linked.
> >>
> >>The consensus was to increase the size of the linker stubs by one 
> >>instruction so that objects with many dynamic symbols could be supported.
> >>
> >>I increased the size of the stub from 16 to 20 bytes.
> >
> >
> >It may be useful to use 24 bytes, this adheres to the ABI alignment
> >rule of 2^3.
> 
> Which rule would that be?  Is there such a requirement for function 
> entry points?

Apparently I misremembered and there is no such rule.

> >>The new stubs 
> >>support up to 2^31 dynamic symbols. That should be enough for at least 
> >>the short term.  With the patch applied I can now successfully build and 
> >>run libgcj from a recent GCC-4.2 snapshot on mipsel-linux.
> >
> >
> >Hm, 2^32 symbols would nicely fit the maximum for NewABI.
> 
> I don't know mips64 well enough to know how to load an unsigned 32 bit 
> constant.  So It had to be 2^31.

It would need one more instruction, so it is relatively expensive.


> >[snip]
> >
> >>*** 8013,8027 ****
> >> 				   MIPS_ELF_STUB_SECTION_NAME (dynobj));
> >>       BFD_ASSERT (s != NULL);
> >> 
> >>-       /* FIXME: Can h->dynindx be more than 64K?  */
> >>-       if (h->dynindx & 0xffff0000)
> >>- 	return FALSE;
> >>- 
> >
> >
> >When it is 2^31, shoudn't we continue to test for overflow?
> >
> Probably, but I got lazy.

Please add it back. :-)


Thiemo