This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS/GAS: Fix NewABI reloc handling with the LD/SD macro
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Maciej W. Rozycki" <macro at linux-mips dot org>
- Cc: binutils at sourceware dot org
- Date: Mon, 25 Oct 2010 22:30:40 +0100
- Subject: Re: [PATCH] MIPS/GAS: Fix NewABI reloc handling with the LD/SD macro
- References: <alpine.LFD.2.00.1010241201020.15889@eddie.linux-mips.org>
"Maciej W. Rozycki" <macro@linux-mips.org> writes:
> All the infrastructure is in place though and here is my proposal to fix
> the problem. A side effect is these macros now handle relocations other
> than LO16 even on o32. While easily doable I think it makes no sense to
> insist on the previous behaviour of these macros on o32 -- I think where
> the address offset is hardware-expressable (i.e. matches the "o(b)"
> format) it makes perfect sense to respect percent-ops even in macros.
> What do you think?
Yeah, the previous o32 behaviour was a silent wrong-code bug.
> Tested with the usual set of targets: mips-linux, mips64-linux,
> mipstx39-elf, mipsisa64-elf and mips-ecoff targets and their respective
> little-endian counterparts. At least a smoke test for the regression
> caused here would be a good idea; I'll think of something.
Yeah, I think the patch does need a testcase. Like you say, it doesn't
have to be anything fancy.
> 2010-10-24 Maciej W. Rozycki <macro@linux-mips.org>
>
> gas/
> * config/tc-mips.c (macro)[M_LD_OB, M_SD_OB]: Use any offset
> reloc supplied.
Looks good, but I think it would be easier to make the 'o' case in
mips_ip set offset_expr appropriately, so that we can use it
unconditionally. I.e.:
case 'o': /* 16 bit offset */
offset_reloc[0] = BFD_RELOC_LO16;
offset_reloc[1] = BFD_RELOC_UNUSED;
offset_reloc[2] = BFD_RELOC_UNUSED;
This won't affect non-macro uses, since 'o' only accepts non-reloc
constants that don't overflow a LO16.
(As far as setting [1] and [2] goes: I think it would be better if all
the cases in mips_ip set all three reloc elements, rather than just the
first as some do. That's a separate change though.)
With the change to 'o', let's leave "r" as a scalar.
Also, while we're here, would you mind fixing the unaligned load/store
macros in the same way?
This may be a known bug, and is certainly nothing to do with your patches,
but I notice:
ld $4,0x7ffc($5)
fails to work correctly (or trigger a diagnostic) in o32 mode.
0x8000 works fine of course.
Ah, the fun of macros.
Richard