Re: R_MIPS_PC16 relocation handling, a proposal

[Geoff Keating <geoffk at ozemail dot com dot au>]

> Mailing-List: contact binutils-help@sourceware.cygnus.com; run by ezmlm
> List-Unsubscribe: <mailto:binutils-unsubscribe-geoffk=cygnus.com@sourceware.cygnus.com>
> List-Subscribe: <mailto:binutils-subscribe@sourceware.cygnus.com>
> List-Archive: <http://sourceware.cygnus.com/ml/binutils/>
> List-Post: <mailto:binutils@sourceware.cygnus.com>
> List-Help: <mailto:binutils-help@sourceware.cygnus.com>, <http://sourceware.cygnus.com/ml/#faqs>
> Date: Thu, 14 Oct 1999 17:58:18 +0200 (MET DST)
> From: "Maciej W. Rozycki" <macro@ds2.pg.gda.pl>
> Organization: Technical University of Gdansk
> 
> On Wed, 13 Oct 1999, Geoff Keating wrote:
> 
> > I have already implemented this as a GNU-specific extension which does
> > include the shift, as part of more general work to make embedded-pic
> > work on MIPS.  I had to add other relocs anyway, so there was no
> > benefit on trying to be tricky for just this one.
> 
>  You refer to the code in ECOFF backend, don't you?  Or some private work?

The work is to make embedded-pic happen on ELF.

> > I'm pretty sure your patch wouldn't work with the current assembler.
> 
>  Have you actually checked?  I've optimized the patch against a CVS
> snapshot dated Oct 8, 1999.  Pretty current, I suppose.

I just checked, and indeed it doesn't work.  I used the
'-membedded-pic' flag, of course, which does jal->bal conversion.

> > My testcase looked like:
> > 
> > # Check GNU-specific embedded relocs, for ELF.
> > 
> > 	.text
> > 	.set noreorder
> > 	nop
> > l2:	jal	g1		# R_MIPS_GNU_REL16_S2	g1   -1
> > 	nop
> > 	b	g2		# R_MIPS_GNU_REL16_S2	g2   -1
> > 	nop
> > 	b	g2		# R_MIPS_GNU_REL16_S2	g2   -1
> > 	nop
> > 	jal	l1		# R_MIPS_GNU_REL16_S2	.foo 4

This is what went wrong.

> > 	nop
> > 	jal	l2		# -9
> > 	nop
> > 	b	l1+8		# R_MIPS_GNU_REL16_S2	.foo C
> > 	nop
> > l3:	
> > 	b	l2		# -D
> > 	nop
> > 
> > 	.section ".foo","ax",@progbits
> > 	nop
> > l1:
> > 	nop

With your patch, I get the following:

Disassembly of section .text:

0000000000000000 <l2-0x4>:
   0:   00000000        nop

0000000000000004 <l2>:
   4:   0411ffff        bal     4 <l2>
                        4: R_MIPS_PC16  g1
   8:   00000000        nop
   c:   1000ffff        b       c <l2+0x8>
                        c: R_MIPS_PC16  g2
  10:   00000000        nop
  14:   1000ffff        b       14 <l2+0x10>
                        14: R_MIPS_PC16 g2
  18:   00000000        nop
  1c:   0411fff9        bal     4 <l2>
                        1c: R_MIPS_PC16 .foo
  20:   00000000        nop
  24:   0411fff7        bal     4 <l2>
                        24: R_MIPS_PC16 .text
  28:   00000000        nop
  2c:   1000fff7        b       c <l2+0x8>
                        2c: R_MIPS_PC16 .foo
  30:   00000000        nop

0000000000000034 <l3>:
  34:   1000fff3        b       4 <l2>
                        34: R_MIPS_PC16 .text
  38:   00000000        nop
Disassembly of section .foo:

0000000000000000 <l1-0x4>:
   0:   00000000        nop

0000000000000004 <l1>:
   4:   00000000        nop

and all the branches which are not to globals are wrong if you want
pcrel_offset to be set (which you most certainly do or you'll get
overflows when using ld -r which you wouldn't have gotten otherwise).

>  Anyway, pointing back to the opcode is the only right way for PC-relative
> relocs that cannot cover the whole address space.  So "-1" is OK and with
> my patch I get exactly the same result.

Yes.  This part is right.

> > The assembler was doing the pcrel handling wrong.  IMHO, the
> > assembler's generic pcrel handling is broken, or at least very
> > suboptimal for ELF, but it's possible to work around it.
> 
>  Yes, there are problems.  I've been dealing with R_386_PC16 and R_386_PC8
> relocs some time ago (see the ChangeLog) and there were some nasties but
> it can be done right with some caution.  Basically I had to repeat the
> same steps for MIPS this time.

Yes.  It's ugly.

-- 
Geoffrey Keating <geoffk@cygnus.com>