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

[David Daney <ddaney at avtrex dot com>]

Thiemo Seufer wrote:
> David Daney wrote:
>
> > Thiemo Seufer wrote:
> >
> > > Richard Sandiford wrote:
> > >
> > >
> > > > David Daney <ddaney@avtrex.com> writes:
> > > >
> > > >
> > > > > ! #define MIPS_FUNCTION_STUB_SIZE(INFO) \
> > > > > !   (elf_hash_table (INFO)->dynsymcount > 65536 ? 20 : 16)
> > > >
> > > > Sorry to be a pain, but as I said earlier, I really do think we should
> > > > cache the chosen stub size in mips_elf_link_hash_table (and get rid of
> > > > this macro entirely).  That will emphasise that 
> > > > always_size_dynamic_sections
> > > > is the place that makes the decision, and that it's only safe to use this
> > > > value once that function has been called.  I think that will be more 
> > > > robust
> > > > and easier to understand in future.
> > > >
> > > > Apart from that, and from Thiemo's and Daniel's comments, this looks
> > > > really good to me.  Thanks a lot for doing this!
> > >
> > >
> > > Does this followup patch look ok?
> >
> > FWIW, it looks good to me.  Definitly a little cleaner than mine.
> >
> > One weird thing is that the generated code is different.
> >
> > My libgcj.so now has 16 byte stubs.  With my version of the patch it 
> > generated 20 byte stubs.  This means that at the point you are sampling 
> > info->dynsymcount, it had a different value than when it was sampled in 
> > my patch.  I was wondering about this because even though my patch 
> > generated 20 byte stubs, none of the symbol index values in the stubs 
> > were large enough to require the larger stub.
>
>
> Some of the overflow checks were off by one bit.

Your checks are identical to mine.  The test for stub size is likewise 
unchanged.

Look at this:

$ mipsel-linux-readelf -S libgcj.so.7.0.0
There are 40 section headers, starting at offset 0x35f82b8:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg 
Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00 
 0   0  0
  [ 1] .reginfo          MIPS_REGINFO    000000d4 0000d4 000018 18   A 
 0   0  4
  [ 2] .dynamic          DYNAMIC         000000ec 0000ec 000120 08   A 
 5   0  4
  [ 3] .hash             HASH            0000020c 00020c 069c98 04   A 
 4   0  4
  [ 4] .dynsym           DYNSYM          00069ea4 069ea4 127210 10   A 
 5  17  4
.
.
.

Note the size of the .dynsym section.  According to my calculations it 
contains about 75553 symbols.

$ mipsel-linux-readelf -s -D libgcj.so.7.0.0 | wc
  75539  679837 6082342

$ mipsel-linux-objdump -d -z -j .MIPS.stubs libgcj.so.7.0.0

libgcj.so.7.0.0:     file format elf32-tradlittlemips

Disassembly of section .MIPS.stubs:

01597f30 <.MIPS.stubs>:
 1597f30:       8f998010        lw      t9,-32752(gp)
 1597f34:       03e07821        move    t7,ra
 1597f38:       0320f809        jalr    t9
 1597f3c:       341897cc        li      t8,0x97cc
 1597f40:       8f998010        lw      t9,-32752(gp)
 1597f44:       03e07821        move    t7,ra
 1597f48:       0320f809        jalr    t9
 1597f4c:       341897b1        li      t8,0x97b1
 1597f50:       8f998010        lw      t9,-32752(gp)
 1597f54:       03e07821        move    t7,ra
 1597f58:       0320f809        jalr    t9
 1597f5c:       34189761        li      t8,0x9761
.
.
.


These are 16 bytes stubs, even though there are more than 2^16 dynamic 
symbols.  With my original patch the stubs were 20 bytes, which made 
sense to me.  I don't understand the discrepancy.

David Daney.




> > With your patch my simple tests with libgcj run successfully.  I did 
> > however see FAIL: MIPS multi-got-no-shared, which I think passed before 
> > applying your patch.  These are very brittle tests, so perhaps it is not 
> > surprising.
>
>
> No, that's since H.J.'s COMMONPAGESIZE patch, I have a fix for that.
>
>
> Thiemo