Re: [PATCH] MIPS16 addiupc relocation fix

[Richard Sandiford <rdsandiford at googlemail dot com>]

Chung-Lin Tang <cltang@codesourcery.com> writes:
> Index: elfxx-mips.c
> ===================================================================
> RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
> retrieving revision 1.297
> diff -u -p -r1.297 elfxx-mips.c
> --- elfxx-mips.c	8 Dec 2011 20:47:24 -0000	1.297
> +++ elfxx-mips.c	10 Dec 2011 09:21:06 -0000
> @@ -5553,7 +5553,9 @@ mips_elf_calculate_relocation (bfd *abfd
>  	  /* See the comment for R_MIPS16_HI16 above for the reason
>  	     for this conditional.  */
>  	  if (r_type == R_MIPS16_LO16)
> -	    value = addend + gp - p;
> +	    /* MIPS16 addiupc clears the lower two PC address bits
> +	       before adding.  */
> +	    value = addend + gp - (p & ~(bfd_vma) 0x3);
>  	  else if (r_type == R_MICROMIPS_LO16
>  		   || r_type == R_MICROMIPS_HI0_LO16)
>  	    value = addend + gp - p + 3;

Let's adjust the comment:

	  /* For MIPS16 ABI code we generate this sequence
	        0: li      $v0,%hi(_gp_disp)
	        4: addiupc $v1,%lo(_gp_disp)
	        8: sll     $v0,16
	       12: addu    $v0,$v1
	       14: move    $gp,$v0
	     So the offsets of hi and lo relocs are the same, but the
	     $pc is four higher than $t9 would be, so reduce
	     both reloc addends by 4. */

to say:

	  /* For MIPS16 ABI code we generate this sequence
		0: li      $v0,%hi(_gp_disp)
		4: addiupc $v1,%lo(_gp_disp)
		8: sll     $v0,16
	       12: addu    $v0,$v1
	       14: move    $gp,$v0
	     So the offsets of %hi and %lo relocs are the same, but the
	     base $pc is that used by the ADDIUPC instruction at $t9 + 4.
	     ADDIUPC clears the low two bits of the instruction address,
	     so the base is ($t9 + 4) & ~3.  */

Then we can leave the commentary above the LO16 unchanged; the
existing reference to HI16 is enough.  I'd also like to see the
~3 applied there for consistency, even though it doesn't affect
the final result:

	  if (r_type == R_MIPS16_HI16)
	    value = mips_elf_high (addend + gp - ((p + 4) & ~(bfd_vma) 3));

(I just think this whole HI16/LO16 stuff is subtle enough as it is
without giving the reader extra work by using different addresses.)

OK with those changes, thanks.

Richard