This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] MIPS/GAS: Complete constant JMP relocs straight away
"Maciej W. Rozycki" <macro@codesourcery.com> writes:
> David,
>
>> > In the unlikely case a constant is used as the argument of a jump
>> > instruction, e.g.
>> >
>> > j 0xbfc00000
>> >
>> > the associated JMP relocation is resolved straight away in append_insn and
>> > the instruction's immediate field initialised while the instruction is
>> > being assembled,
>>
>> I haven't fully studied the code, but this seems error prone.
>>
>> Why don't we just emit the relocation without resolving it in the assembler?
>> Wouldn't it be nicer to get a warning/error message out of the linker where
>> the address of the J is known and it is determined that we are attempting to
>> span a forbidden boundry?
>>
>> It is possible I am missing the point here. If so, just ignore this.
>
> That could be done. An artificial absolute symbol would have to be
> created and assigned the constant referred. The relocation would then
> refer to that symbol and remain for the linker to resolve and check the
> range while doing that. But is it worth the effort? If so, then please
> go ahead, sure!
Probably stating the obvious, but that would only be needed for REL targets.
RELA could just emit a relocation against symbol zero without losing bits.
I agree that we should be consistent though, and either find a way of
making it work for REL targets or not do it at all.
I think the same argument applies to:
case BFD_RELOC_16_PCREL_S2:
{
int shift;
shift = mips_opts.micromips ? 1 : 2;
if ((address_expr->X_add_number & ((1 << shift) - 1)) != 0)
as_bad (_("branch to misaligned address (0x%lx)"),
(unsigned long) address_expr->X_add_number);
if (!mips_relax_branch)
{
if ((address_expr->X_add_number + (1 << (shift + 15)))
& ~((1 << (shift + 16)) - 1))
as_bad (_("branch address range overflow (0x%lx)"),
(unsigned long) address_expr->X_add_number);
ip->insn_opcode |= ((address_expr->X_add_number >> shift)
& 0xffff);
}
ip->complete_p = 0;
}
In principle, we should either resolve the operand now and set
complete_p to true, or not install any operand and create a fixup.
I.e. something like:
case BFD_RELOC_16_PCREL_S2:
{
int shift;
shift = mips_opts.micromips ? 1 : 2;
if ((address_expr->X_add_number & ((1 << shift) - 1)) != 0)
as_bad (_("branch to misaligned address (0x%lx)"),
(unsigned long) address_expr->X_add_number);
if (!mips_relax_branch && !mips_opts.micromips)
{
if ((address_expr->X_add_number + (1 << (shift + 15)))
& ~((1 << (shift + 16)) - 1))
as_bad (_("branch address range overflow (0x%lx)"),
(unsigned long) address_expr->X_add_number);
ip->insn_opcode |= ((address_expr->X_add_number >> shift)
& 0xffff);
ip->complete_p = 1;
}
else
ip->complete_p = 0; /* Redundant after next patch. */
}
with the new micromips condition forcing the usual branch reloc
to be created. It looks like that doesn't work for:
.set micromips
beq $4,$5,0x4000
though, for reasons I've not investigated. It seems there are other
problems with these kinds of instruction too, even without the change
above. E.g.:
.set micromips
bc1tl 0x4000
triggers an assertion failure while:
.set micromips
beqz $4,0x4000
causes a segfault. So let's leave that for another day and just stick
to the cases you're fixing.
I changed the patch to explicitly set complete_p to 1, for the benefit
of the patch I'm about to post. Applied with that change, thanks.
Richard
2012-09-23 Maciej W. Rozycki <macro@codesourcery.com>
gas/
* config/tc-mips.c (append_insn) <BFD_RELOC_MIPS_JMP>: Don't
mark as incomplete for constant expressions.
<BFD_RELOC_MIPS16_JMP>: Likewise.
Index: gas/config/tc-mips.c
===================================================================
--- gas/config/tc-mips.c 2012-09-23 08:28:29.000000000 +0100
+++ gas/config/tc-mips.c 2012-09-23 08:35:58.643477523 +0100
@@ -4097,7 +4097,7 @@ append_insn (struct mips_cl_insn *ip, ex
(unsigned long) address_expr->X_add_number);
ip->insn_opcode |= ((address_expr->X_add_number >> shift)
& 0x3ffffff);
- ip->complete_p = 0;
+ ip->complete_p = 1;
}
break;
@@ -4109,7 +4109,7 @@ append_insn (struct mips_cl_insn *ip, ex
(((address_expr->X_add_number & 0x7c0000) << 3)
| ((address_expr->X_add_number & 0xf800000) >> 7)
| ((address_expr->X_add_number & 0x3fffc) >> 2));
- ip->complete_p = 0;
+ ip->complete_p = 1;
break;
case BFD_RELOC_16_PCREL_S2: