This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] microMIPS/GAS: Avoid relocation overflow with forced 16-bit branches
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>, Iain Sandoe <iain at codesourcery dot com>
- Cc: <binutils at sourceware dot org>
- Date: Thu, 25 Oct 2012 23:50:15 +0100
- Subject: Re: [PATCH] microMIPS/GAS: Avoid relocation overflow with forced 16-bit branches
- References: <alpine.DEB.firstname.lastname@example.org> <email@example.com>
On Thu, 25 Oct 2012, Richard Sandiford wrote:
> > Overall it looks to me the whole idea of using fixp->fx_size to check for
> > relocatable field overflows is missing the point, at least for the MIPS
> > target and relocations in text, where the size of the fixup reflects the
> > width of the instruction a relocation is applied to and has little to do
> > with the range supported.
> > Anyway, to fix this immediate problem I have gladly accepted Chao-ying's
> > proposal.
> > No regressions in testing across the usual MIPS targets. OK to apply?
> > 2012-10-24 Chao-ying Fu <firstname.lastname@example.org>
> > gas/
> > * config/tc-mips.c (append_insn): Set fx_no_overflow for 16-bit
> > microMIPS branch relocations.
> > gas/testsuite/
> > * gas/mips/micromips-b16.d: New test.
> > * gas/mips/micromips-b16.s: New test source.
> > * gas/mips/mips.exp: Run the new test.
> I take your point about the write.c check (been hit by that before too),
> but I suppose the obvious question is: do we still report branches that
> genuinely overflow?
Of course we do, for symbols resolved at the assembly time GAS does that
and for all the others the linker does:
$ cat b.s
$ mips-linux-gnu-as -o b.o b.s
b.s: Assembler messages:
b.s:3: Error: Branch out of range
b.s:7: Error: Branch out of range
$ cat b1.s
.type foo, @function
$ cat b2.s
.type bar, @function
$ mips-linux-gnu-as -o b1.o b1.s
$ mips-linux-gnu-as -o b2.o b2.s
$ mips-linux-gnu-ld -e 0 -o b12 b1.o b2.o
b1.o: In function `foo':
(.text+0x40000): relocation truncated to fit: R_MIPS_PC16 against `bar'
b2.o: In function `bar':
(.text+0x40000): relocation truncated to fit: R_MIPS_PC16 against `foo'
Unsure of MIPS16; from experience its reloc overflow handling may be
> Although you've used the same wording, this isn't
> quite the same as the preceding:
> /* These relocations can have an addend that won't fit in
> 4 octets for 64bit assembly. */
> if (HAVE_64BIT_GPRS
> && ! howto->partial_inplace
> because in that case the RELA entry contains the full, untruncated addend.
I saw this check while looking into this issue and frankly it makes
little sense to me. First of all it looks to me it shouldn't be checking
howto->partial_inplace -- while our 64-bit ABIs want RELA relocations we
do support REL for them, at least to some extent, and the overflow does
not depend on whether the addend is stored in the reloc entry or in the
instruction. Offhand at least HIGHEST, HIGHER, HI16 and LO16 relocations
never overflow no matter the addend, the ABI and the reloc style.
Furthermore for many relocations fixp->fx_size is 4 and the size of the
relocatable field -- only 2. Examples: HI16, LO16, GPREL16, GOT16, PC16,
etc. As noted above, of these HI16 and LO16 never overflow, GPREL16 and
GOT16 overflow at 2 (rather than 4) and PC16 overflow at 18 *bits*. Etc.,
> In contrast, 16-bit branches _can_ overflow, just not in the way that
> write.c expects.
As can 32-bit branches, regardless of the ISA mode. In the microMIPS
mode we don't however check for branch range overflows in GAS as we always
produce a relocation for the linker to resolve once any linker relaxation
has been made. The linker then complains about any relocation overflows
as shown above.
I realise this is suboptimal and Iain has been working on making things
better. I am not sure what his schedule exactly is though -- Iain?
> Would be nice to have a test for that too, if we don't already.
We do have some tests, but only for BPOSGE32. ;) And for the linker we
check some other relocations, but we don't check the branch ones. I'll
see what I can do, but I hope that is not a prerequisite for this bug fix,
I'd like to tick it off.
> I realise this fix is the same as that used in md_convert_frag for
> relaxing microMIPS branches, but the same question applies there
> in the "forced to fit" case.
In md_convert_frag we only produce a 16-bit branch once we've determined
its destination is in range, see relaxed_micromips_16bit_branch_length.
Overall thanks for your review. Also I forgot to cc you on the LUI issue
-- would you please have a look too?