[PATCH] MIPS16 addiupc relocation fix

[PATCH] MIPS16 addiupc relocation fix

[Chung-Lin Tang]

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

Hi Richard,
this is a small fix for MIPS16 pc-relative add relocations. The ADDIUPC
instruction clears the lower two address bits before adding.

Thanks,
Chung-Lin

2011-12-10  Chung-Lin Tang  <cltang@codesourcery.com>

        bfd/
        * elfxx-mips.c (mips_elf_calculate_relocation): Correct
        R_MIPS16_LO16 handling of two cleared lower bits.

[-- Attachment #2: addiupc.diff --]
[-- Type: text/plain, Size: 758 bytes --]

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;

Re: [PATCH] MIPS16 addiupc relocation fix

[Richard Sandiford]

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

Re: [PATCH] MIPS16 addiupc relocation fix

[Richard Sandiford]

Forgot to say:

Richard Sandiford <rdsandiford@googlemail.com> writes:
> OK with those changes, thanks.

...2.22 branch too, please!

Richard