This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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:


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]