Re: [PATCH] MIPS/GAS: Complete constant JMP relocs straight away

[David Daney <ddaney dot cavm at gmail dot com>]

On 09/21/2012 10:14 AM, Maciej W. Rozycki wrote:
> Hi,
>
>   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.

David Daney


> just as in the case of several other relocs.  However
> unlike with most other relocs the JMP relocation is not marked as complete
> and a fixup is created.
>
>   That fixup then has another chance for resolution at the final fix-up
> phase, in md_apply_fix.  Nothing is done there for JMP relocations though
> and owing to the lack of an associated symbol (as the expression is
> constant and therefore refers to none) the relocation decays to nil (and
> the instruction's argument has already been initialised).  There is no
> other processing between append_insn and md_apply_fix applied to such
> relocations either.
>
>   I suspect there might have been an additional check intended, to see if
> the final destination of any jump instruction concerned matches the
> segment the instruction will appear in.  However I am not really sure if
> that makes sense at all as the ultimate segment will only be known to the
> linker anyway.  And the current arrangement makes it impossible to detect
> a case where the expression used as the argument to a jump instruction is
> only resolved in the fix-up phase, which we do not currently handle at
> all -- and should warn about.
>
>   I have therefore decided to remove that special arrangement and let
> append_insn complete constant JMP relocations.  No regressions with this
> change in MIPS testing.  OK to apply?
>
> 2012-09-21  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.
>
>    Maciej
>
> binutils-gas-mips-jmp-reloc-complete.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:17:26.250930536 +0100
> +++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2012-09-17 22:18:33.511052691 +0100
> @@ -4062,7 +4062,6 @@ 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;
>   	  }
>   	  break;
>
> @@ -4074,7 +4073,6 @@ 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;
>   	  break;
>
>   	case BFD_RELOC_16_PCREL_S2: