This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH, MIPS] When calculating a relocation using an undefined weak symbol don't check for overflow.
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Andrew Bennett <Andrew dot Bennett at imgtec dot com>
- Cc: "binutils\ at sourceware dot org" <binutils at sourceware dot org>
- Date: Mon, 01 Dec 2014 22:24:40 +0000
- Subject: Re: [PATCH, MIPS] When calculating a relocation using an undefined weak symbol don't check for overflow.
- Authentication-results: sourceware.org; auth=none
- References: <0DA23CC379F5F945ACB41CF394B9827720F35D28 at LEMAIL01 dot le dot imgtec dot org>
Andrew Bennett <Andrew.Bennett@imgtec.com> writes:
> On a side note I noticed that for the R_MIPS_26 relocation the
> overflow check is only performed for global non undefined weak
> symbols. I was wondering why it is not done for local symbols?
I think it's just because the psABI definition of the reloc is a bit
weird and doesn't give a sensible overflow condition for local symbols.
Specifically the value for local symbols is:
(((A << 2) | (P & 0xf0000000) + S) >> 2
First note the unbalanced brackets :-) Second note the use of P rather
than P+4. Third, it adds the address of the instruction and the symbol
value together and so doesn't always reflect where the instruction
actually jumps. In the case where S and P are both kernel addresses
0x80…, say, the sum is 0x0[01]…. As far as the natural test goes this
would seem to be an overflow, even though the relocation is actually valid.
The bfd code to some extent follows this:
if (was_local_p)
value = addend | ((p + 4) & (0xfc000000 << shift));
[...]
value = (value + symbol) >> shift;
[...]
value &= howto->dst_mask;
but the whole | (p ...) thing is redundant, because it only introduces
bits that are later discarded. A bold person might change it to:
if (howto->partial_inplace)
value = _bfd_mips_elf_sign_extend (addend, 26 + shift);
else
value = addend;
value = (value + symbol) >> shift;
if (was_local_p || h->root.root.type != bfd_link_hash_undefweak)
overflowed_p = (value >> 26) != ((p + 4) >> (26 + shift));
value &= howto->dst_mask;
but it's clearly not what the ABI says.
> bfd/
> * elfxx-mips.c (mips_elf_calculate_relocation): Only check for overflow
> on non-weak undefined symbols.
>
> ld/testsuite/
> * ld-mips-elf/mips-elf.exp: Add in undefined weak overflow tests for
> o32, n32 and n64.
> * ld-mips-elf/undefweak-overflow.s: New test.
> * ld-mips-elf/undefweak-overflow.d: New test.
> * ld-mips-elf/undefweak-overflow-n64.d: New test.
> * ld-mips-elf/undefweak-overflow-n64.d: New test.
OK, thanks, and sorry for the slow reply.
Richard