Re: [PATCH] MIPS gas: Fix broken relocation sorting

[Richard Sandiford <richard at codesourcery dot com>]

David Daney <ddaney@avtrex.com> writes:
> Richard Sandiford wrote:
>> I was thinking that if you move a GOT16 reloc in front of a LO16
>> that didn't need it, you might orphan a HI16 reloc that does need it.
>>   
>
> The GOT16 is only moved if it is for *exactly* the same symbol as the 
> LO16.  I cannot think of a situation where you would have a GOT16 and a 
> HI16 for the same symbol, so it would not be possible to have some sort 
> of conflict between the two.

It could easily happen in -mno-shared code if, for instance, you have
inline asm that uses assembler macros.  It could also happen if you're
compiling -mno-shared code in -fno-unit-at-a-time mode and the
information needed to enable the optimised accesses only becomes
available part-way through.  You could then have a mixture of GOT and
HI/LO accesses in the same file.

>> However, I suppose with the way the linker searches for relocations,
>> that isn't really a problem; it will just skip over the GOT16.
>> I'd still argue that the binary was wrong though. ;)
>>
>> To be honest, I'm slightly nervous about your patch.  It's the first
>> use of section_symbol_p in the gas sources,
> Not quite, I just copied the 'if' statement right out of 
> adjust_reloc_syms() in write.c

Grr... I'm an idiot, sorry.  Obviously a grep failure there.

>>  and given that we already
>> handle most section symbols correctly, it seems like a broad stroke.
>>   
>
> How do we know that we handle them correctly?  Are there testcases?  

Yup.  See e.g. gas/mips/elf-rel9.d.

> Both my original patch and the revised version that I committed cause no 
> regressions for mipsel-linux.  Perhaps I should have tested mips64-linux 
> as well.

Oh, I wasn't suggesting your patch would cause regressions.  I was just
wondering why such a general test fixed your comdat testcase even though
non-comdat section symbols already seem to be handled correctly.

>> The clinching factor in your testcase appears to be that the section
>> is comdat.
>>   
> Clinching factor?  For what?

Sorry, I meant that if you turn the section containing the target
symbol into a non-comdat section, the relocs are correctly sorted
without your patch.  The comdatness of the section appears to be
the thing that makes pic_need_relax fall over.

Anyway, given my failure to grep properly, I agree the patch is correct.
I'm still curious about what's going on though.  Perhaps I'll have a poke
around at some point.  Since it really is just satisfying curiousity,
there's no reason why you should.

Richard