[lld] r204606 - [Mips] Sort R_MIPS_LO16 / R_MIPS_HI16 / R_MIPS_GOT16 before finding

[Richard Sandiford]

Simon Atanasyan <simon at atanasyan.com> writes:
> On Tue, Mar 25, 2014 at 2:03 PM, Richard Sandiford
> <rsandifo at linux.vnet.ibm.com> wrote:
>> Simon Atanasyan <simon at atanasyan.com> writes:
>>> Author: atanasyan
>>> Date: Mon Mar 24 09:09:17 2014
>>> New Revision: 204606
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=204606&view=rev
>>> Log:
>>> [Mips] Sort R_MIPS_LO16 / R_MIPS_HI16 / R_MIPS_GOT16 before finding
>>> pairs and calculate AHL addend.
>>
>> Out of interest, what are you sorting on?  It sounds like it might be
>> the relocation offset, whereas the relocations need to be tied based
>> on their original order in .rel.foo.  E.g. for something like:
>>
>> foo:
>>         lw      $4,%lo(X)($5)
>>         ...
>>         beq     $6,$7,foo
>>         lui     $5,%hi(X)
>>
>> the LO16 is against an earlier address, but the assembler makes sure that
>> the HI16 comes immediately before the LO16 in .rel.text.
>>
>> It's done that way because the order can't always be reconstructed
>> otherwise.  E.g. for:
>>
>>         lui     $4,%hi(X)               A
>>         ...
>>         lw      $6,%lo(X+1024)($5)      B
>>         ...
>>         lb      $7,%lo(X)($6)           C
>>         lb      $8,%lo(X+1)($6)         D
>>         ...
>>         lui     $5,%hi(X+1024)          E
>>
>> the order of the incoming .rel.text must be one of:
>>
>>     ..ACD..EB..
>>     ..ADC..EB..
>>     ..EB..ACD..
>>     ..EB..ADC..
>>
>> and the linker should honour whichever order it's given.
>>
>> Sorry if that's what the patch does.  It just sounded odd that any
>> sorting was necessary.
>
> It looks like I do not understand HI16/LO16 relocations handling
> properly. Thanks for pointing out this problem.
>
> I have an object file (buffer.o compiled from VIM source code) with
> the following piece of code:
>
> 7c0:  3c120000        lui     s2,0x0
>               7c0: R_MIPS_HI16        curbuf
> 7c4:  00402821        move    a1,v0
> 7c8:  2404000c        li      a0,12
> 7cc:  3c100000        lui     s0,0x0
>               7cc: R_MIPS_HI16        firstbuf
> 7d0:  afb40034        sw      s4,52(sp)
> 7d4:  afbf003c        sw      ra,60(sp)
> 7d8:  afb50038        sw      s5,56(sp)
> 7dc:  0c000000        jal     0 <buflist_setfpos>
> 7e0:  8e540000        lw      s4,0(s2)
>               7e0: R_MIPS_LO16        curbuf
> 7e4:  8e020000        lw      v0,0(s0)
>               7e4: R_MIPS_LO16        firstbuf
>
> Its .rel.text section contains the following entries exactly in this order:
>
> .rel.text:
> ...
> 0x007e0   R_MIPS_LO16     curbuf
> 0x007e4   R_MIPS_LO16     firstbuf
> 0x0085c   R_MIPS_LO16     firstbuf
> 0x00894   R_MIPS_LO16     curbuf
> 0x008a8   R_MIPS_HI16     first_tabpage
> 0x008ac   R_MIPS_LO16     first_tabpage
> 0x008b4   R_MIPS_HI16     curtab
> 0x008bc   R_MIPS_LO16     curtab
> 0x008c4   R_MIPS_LO16     firstwin
> 0x008f4   R_MIPS_LO16     curtab
> 0x00988   R_MIPS_LO16     firstbuf
> 0x009c0   R_MIPS_HI16     curbuf
> 0x009c4   R_MIPS_LO16     curbuf
> 0x007cc   R_MIPS_HI16     firstbuf
> 0x008b8   R_MIPS_HI16     firstwin
> 0x007c0   R_MIPS_HI16     curbuf
> ...

Yeah, this .rel.text isn't valid.  The assembler should have ordered the
relocations so that HI16s come immediately before the associated LO16.
(It's OK to have several HI16s attached to the same LO16 though.)

Thanks,
Richard