[PATCH] MIPS/GAS: Correct the handling of %hi/%lo with subtraction
Maciej W. Rozycki
macro@codesourcery.com
Fri Sep 21 17:07:00 GMT 2012
Hi,
When generating code to handle switch statements GCC produces code like:
$BB0:
[...]
lui $4,%hi($BB1-$BB0)
addiu $4,%lo($BB1-$BB0)
[...]
$BB1:
This happens to work for small distances between $BB0 and $BB1, but with
larger ones, beyond 32kB, GAS fails with a "relocation overflow" error
message referring to the line with the %lo operator.
The check GAS trips over for the LO16 reloc is annotated with:
/* FIXME: Now that embedded-PIC is gone, some of this code/comment
may be safe to remove, but if so it's not obvious. */
/* When handling an embedded PIC switch statement, we can wind
up deleting a LO16 reloc. See the 'o' case in mips_ip. */
and is apparently related to code removed with:
2001-10-31 Chris Demetriou <cgd@broadcom.com>
* config/tc-mips.c (HAVE_32BIT_ADDRESSES): If compiling embedded
PIC code, assume pointers the same size as GPRs.
(macro): In M_LA_AB handling for embedded PIC code, support
"la $treg,foo-bar($breg)". In load/store handling
(label ld_st) support "<op> $treg,<sym>-<local_sym>($breg)"
which is used by the compiler for switch statements.
In load/store double multi-instruction macro handling
(label ldd_std) add a comment that no special handling
is currently done for embedded PIC.
(mips_ip): In 'o' (16-bit offset) case, only accept 16
bit offsets.
where it guarantees that the assumption made in mips_ip that the ultimate
difference between two symbols will fit in 16 bits. For embedded PIC
apparently no code for the corresponding high part was ever produced by
GCC. This was further explained as follows:
/* If this value won't fit into a 16 bit offset, then go
find a macro that will generate the 32 bit offset
code pattern. As a special hack, we accept the
difference of two local symbols as a constant. This
is required to suppose embedded PIC switches, which
use an instruction which looks like
lw $4,$L12-$LS12($4)
The problem with handling this in a more general
fashion is that the macro function doesn't expect to
see anything which can be handled in a single
constant instruction. */
Here, however, we have a legitimate calculation involving both 16-bit
halves of the 32-bit difference and while we still have no o32 relocation
to express a subtraction of a symbol if the subtrahend turns out to be an
external reference, in the case of local symbols there's no particular
need to forbid this calculation.
I have therefore decided that this check should be removed, but that
revealed further problems. This is the complete list of problems
addressed as a result of this observation:
1. Obsolete embedded PIC support prevented constant offsets outside the
-0x8000:0x7fff range in LO16 relocs where the final value of the offset
was only determined at the end of assembly -- e.g. as in absolute
expressions involving forward symbol references.
2. In-range offsets in MIPS16 LO16 relocs lacked bit shuffling required to
embed the immediate value in the correct places of the extended MIPS16
instruction, code corruption resulted.
3. A non-zero high part of any offset used was ignored in HI16 relocs
where the final value of the offset was only determined at the end of
assembly as in #1 above, the immediate operand always came out as zero.
4. MIPS16 LI instruction refused negative HI16 values as its immediate
argument even though the normal constant load sequence will left-shift
it by 16 bits with SLL straight away and as a side effect sign-extend
the value properly (LI's 16-bit argument is somewhat unusually
zero-extended; there's no MIPS16 LUI).
5. The issue #3 applied to many other relocations, including but not
limited to JMP relocations, where a non-zero final value of the
constant used was silently ignored.
6. Outdated/leftover code prevented correct diagnostics of the issue #4
for JMP, there was no way to check if a correct value of the offset had
already been applied by the original assembly pass.
7. The ISA bit was lost in references to labels applied to the instruction
being assembled.
Issues #1-3 are handled by the patch below. I have removed any range
checks for the LO16 relocation and added further code to handle the MIPS16
case right. I have added complementing code for HI16 relocations, that is
similar to that for LO16, but I've decided not to obfuscate it too much
and didn't factor it out.
Issue #4 is fixed by allowing negative values in HI16 relocations applied
to the MIPS16 LI instruction as a special exception. Sent separately.
Issue #5 is fixed simply by checking for the condition and reporting an
error -- GAS shouldn't silently produce wrong code, but such usage is
currently unknown. It may well be that it'll trigger somewhere and some
further cases may have to be implemented. I have deliberately disregarded
the cases of HIGHER and HIGHEST relocs that are relevant to the original
scenario reported in the issue where the n64 ABI is used -- for them to
have any significance the distance between the symbols would have to be
greater than 2GB and 128TB respectively. I have therefore concluded that
implementing any handling would be a wasted effort at this time. Sent
separately.
Issue #6 is fixed by another small change removing unneeded code used in
the original assembly pass while handling JMP relocations. That code
prevented the fix for the issue #4 from working for these relocations and
triggered regressions in the GAS test suite. Sent separately.
Issue #7 was discovered in the preparation of a test case to cover this
issue. It is fixed with the separate patch sent previously, that also
fixes another test case that accidentally relied on that phenomenon.
No regression in MIPS testing and a test case will be sent separately as
it relies on fixes #4 and #7 as well.
OK to apply?
2012-09-21 Maciej W. Rozycki <macro@codesourcery.com>
gas/
* gas/config/tc-mips.c (md_apply_fix)
<BFD_RELOC_HI16_S, BFD_RELOC_MIPS16_HI16_S,
BFD_RELOC_MICROMIPS_HI16_S>: Handle completed fixups.
<BFD_RELOC_LO16, BFD_RELOC_MIPS16_LO16, BFD_RELOC_MICROMIPS_LO16>:
Remove embedded PIC checks, correct the handling of completed
MIPS16 fixups.
Maciej
binutils-gas-mips-lo16-subsy.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c 2012-09-17 22:16:44.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c 2012-09-17 22:17:26.250930536 +0100
@@ -15405,7 +15405,7 @@ void
md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
{
bfd_byte *buf;
- long insn;
+ unsigned long insn;
reloc_howto_type *howto;
/* We ignore generic BFD relocations we don't know about. */
@@ -15491,7 +15491,6 @@ md_apply_fix (fixS *fixP, valueT *valP,
case BFD_RELOC_MIPS_RELGOT:
case BFD_RELOC_MIPS_JALR:
case BFD_RELOC_HI16:
- case BFD_RELOC_HI16_S:
case BFD_RELOC_GPREL16:
case BFD_RELOC_MIPS_LITERAL:
case BFD_RELOC_MIPS_CALL16:
@@ -15505,7 +15504,6 @@ md_apply_fix (fixS *fixP, valueT *valP,
case BFD_RELOC_MIPS16_GOT16:
case BFD_RELOC_MIPS16_CALL16:
case BFD_RELOC_MIPS16_HI16:
- case BFD_RELOC_MIPS16_HI16_S:
case BFD_RELOC_MIPS16_JMP:
case BFD_RELOC_MICROMIPS_JMP:
case BFD_RELOC_MICROMIPS_GOT_DISP:
@@ -15517,7 +15515,6 @@ md_apply_fix (fixS *fixP, valueT *valP,
case BFD_RELOC_MICROMIPS_SCN_DISP:
case BFD_RELOC_MICROMIPS_JALR:
case BFD_RELOC_MICROMIPS_HI16:
- case BFD_RELOC_MICROMIPS_HI16_S:
case BFD_RELOC_MICROMIPS_GPREL16:
case BFD_RELOC_MICROMIPS_LITERAL:
case BFD_RELOC_MICROMIPS_CALL16:
@@ -15562,24 +15559,89 @@ md_apply_fix (fixS *fixP, valueT *valP,
md_number_to_chars ((char *) buf, *valP, fixP->fx_size);
break;
+ case BFD_RELOC_HI16_S:
+ case BFD_RELOC_MIPS16_HI16_S:
+ case BFD_RELOC_MICROMIPS_HI16_S:
+ /* When handling an instruction like
+
+ lui $reg,%hi(.L1-.L0)
+
+ we can wind up deleting a HI16 reloc. If this is the case,
+ then we must fill in the value now. */
+ if (fixP->fx_done)
+ {
+ bfd_boolean use_extend;
+ unsigned short extend;
+ valueT val = (*valP + 0x8000) >> 16;
+
+ if (target_big_endian)
+ insn = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
+ else if (fixP->fx_r_type == BFD_RELOC_HI16_S)
+ insn = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+ else
+ insn = (buf[1] << 24) | (buf[0] << 16) | (buf[3] << 8) | buf[2];
+
+ if (fixP->fx_r_type == BFD_RELOC_MIPS16_HI16_S)
+ {
+ mips16_immed (fixP->fx_file, fixP->fx_line,
+ 'k', ((val & 0xffff) ^ 0x8000) - 0x8000,
+ FALSE, FALSE, TRUE, &insn, &use_extend, &extend);
+ gas_assert (use_extend);
+ insn |= (unsigned long) extend << 16;
+ }
+ else
+ insn |= val & 0xffff;
+
+ if (fixP->fx_r_type == BFD_RELOC_HI16_S)
+ md_number_to_chars ((char *) buf, insn, 4);
+ else
+ {
+ md_number_to_chars ((char *) buf, insn >> 16, 2);
+ md_number_to_chars ((char *) buf + 2, insn & 0xffff, 2);
+ }
+ }
+ break;
+
case BFD_RELOC_LO16:
case BFD_RELOC_MIPS16_LO16:
case BFD_RELOC_MICROMIPS_LO16:
- /* FIXME: Now that embedded-PIC is gone, some of this code/comment
- may be safe to remove, but if so it's not obvious. */
- /* When handling an embedded PIC switch statement, we can wind
- up deleting a LO16 reloc. See the 'o' case in mips_ip. */
+ /* When handling an instruction like
+
+ lw $reg,%lo(.L1-.L0)($reg)
+
+ we can wind up deleting a LO16 reloc. If this is the case,
+ then we must fill in the value now. */
if (fixP->fx_done)
{
- if (*valP + 0x8000 > 0xffff)
- as_bad_where (fixP->fx_file, fixP->fx_line,
- _("relocation overflow"));
- /* 32-bit microMIPS instructions are divided into two halfwords.
- Relocations always refer to the second halfword, regardless
- of endianness. */
- if (target_big_endian || fixP->fx_r_type == BFD_RELOC_MICROMIPS_LO16)
- buf += 2;
- md_number_to_chars ((char *) buf, *valP, 2);
+ bfd_boolean use_extend;
+ unsigned short extend;
+ valueT val = *valP;
+
+ if (target_big_endian)
+ insn = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
+ else if (fixP->fx_r_type == BFD_RELOC_LO16)
+ insn = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];
+ else
+ insn = (buf[1] << 24) | (buf[0] << 16) | (buf[3] << 8) | buf[2];
+
+ if (fixP->fx_r_type == BFD_RELOC_MIPS16_LO16)
+ {
+ mips16_immed (fixP->fx_file, fixP->fx_line,
+ 'k', ((val & 0xffff) ^ 0x8000) - 0x8000,
+ FALSE, FALSE, TRUE, &insn, &use_extend, &extend);
+ gas_assert (use_extend);
+ insn |= (unsigned long) extend << 16;
+ }
+ else
+ insn |= val & 0xffff;
+
+ if (fixP->fx_r_type == BFD_RELOC_LO16)
+ md_number_to_chars ((char *) buf, insn, 4);
+ else
+ {
+ md_number_to_chars ((char *) buf, insn >> 16, 2);
+ md_number_to_chars ((char *) buf + 2, insn & 0xffff, 2);
+ }
}
break;
More information about the Binutils
mailing list