This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS gas: Fix broken relocation sorting
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