This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Richard Sandiford wrote:
David Daney <ddaney@avtrex.com> writes:

The problem here is that some explicit relocations were not getting sorted properly. The %got() and %lo() were being separated by another %got()/%lo() pair. This resulted in incorrect relocations being applied by the linker.

At first I thought this was a GCC bug, but Ian Lance Taylor set me straight here. For those interested, the GCC bug report is here:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29721

The root of the problem is that the relocations are against a local symbol in another section which get converted to relocations against the global section symbol. The relocation sorting was not done for relocations against global symbols.


The section symbol is local, not global.  The bug appears to be
that pic_need_relax doesn't realise this.


There are as far as I can see two easy ways to fix it:

1) In mips_fix_adjustable, don't allow the conversion if it would affect a relocation that could possibly need sorting.

2) In mips_from_file, do the sorting with relocations against global symbols also.

I chose the second option, as it looked like the exclusion of global relocations from the sorting was probably just an optimization. The first option would result in many %got() relocations that have no corresponding %lo() not being converted to be against the section.


It isn't just an optimisation.

The optimization I was referring to is that for R_MIPS_GOT16 against a global symbol, the relocation was never considered for moving. If there is no corresponding R_MIPS_LO16 it will not be moved anyway. So the optimization was the elimination of the search for the corresponding R_MIPS_LO16. That is all my patch changes. It changes nothing to do with whether it is a global or local symbol.


This is one of the nasty warts of
the 32-bit ABI: the meaning of an R_MIPS_GOT16 relocation depends
on whether the symbol is STB_LOCAL.  If the symbol is STB_LOCAL,
the relocation gives the address of the 64k page that contains
the symbol, and you need to add the associated R_MIPS_LO16 to
get the symbol's value.  If the symbol is not STB_LOCAL,
R_MIPS_GOT16 gives the symbol's value up-front.

Thus if we were indeed converting R_MIPS_GOT16 relocations against
local symbols into R_MIPS_GOT16 relocations against global symbols,
(1) would be the right fix.  But like I say, the bug appears at
first glance to be in pic_need_relax.

The comments say that pic_need_relax must agree with adjust_reloc_syms in write.c. This can be changed just for MIPS by doctoring pic_need_relax and mips_fix_adjustable. Which is similar to my option #1 above but with the pic_need_relax change also.


Does this seem like the correct approach?

David Daney




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]