Re: MIPS embedded PIC bug.

[Geoff Keating <geoffk at geoffk dot org>]

> Cc: binutils@sourceware.cygnus.com, nickc@sourceware.cygnus.com
> From: cgd@sibyte.com (Chris G. Demetriou)
> Date: 08 Feb 2001 12:15:30 -0800
> 
> First off, I'd like to thank you for responding to my message in a
> timely manner.  8-)
> 
> 
> Geoff Keating <geoffk@geoffk.org> writes:
> > > Code for MIPS embedded pic switches looks like:
> > > 
> > > $LS20:
> > > 	...
> > >         lw      $3,$L20-$LS20($2)	# can be 'ld' for 64-bit GPRs.
> > > 	...
> > > $L20:
> > > 	... table ...
> > > 
> > > Where $LS20 is in the current section, but $L20 is undefined.  The
> > > change I made to the embedded PIC hack broke that.
> > 
> > How can $L20 be undefined?  It is defined by the line
> 
> It's undefined at the time the 'lw' is assembled.  Yes, it'll be fixed
> up later, typically.  (My patch was broken in this regard.)
> 
> 
> > In fact, the code would still be valid even if $L20 was never defined,
> > because (IIRC) the right kind of 16-bit PC-relative reloc is available.
> 
> Really?  My understanding of this says that if it can't be resolved
> into something PC-relative, you need to emit more complex code, and a
> sample like the above, but with L20 not defined in the same segment,
> can't be resolved into something PC-relative with a single
> instruction.

Sure it can.

The basic PC-relative reloc looks like:

	  .word Lbar-.

This is a PC-relative reloc to Lbar, because the address we need is
the address of Lbar less the address of the PC, which is `.' in assembler.

A similar problem looks like:

Lfoo:
	.word 0
	.word Lbar-Lfoo

The expression 'Lbar-Lfoo' can be rewritten as 'Lbar-Lfoo+.-.',
which (because Lfoo and the .word are in the same segment) can be
rewritten in this case as 'Lbar-4-.', which is a PC-relative reloc to
Lbar-4.

> The embedded PIC code (before I got to it 8-) assumed that as long as
> LS20 is in the current segment, you can do that (regardless of where
> L20 is defined).

Yes, this is true.  You can't do arbitrary subtractions in ELF, or at
least not without some special stuff.

> > > The problem is, with the embedded PIC hack as general as it was,
> > > is that several cases in the MIPS 'empic' testcase (before I got to
> > > it, honest!!! 8-) would _fail_ because of the excessive generality of
> > > the test.  It was letting references to undefined symbols through,
> > > and gas couldn't properly emit relocations for them.  (To demonstrate
> > > this, unapply the first patch mentioned above, and make a mips-elf
> > > toolchain, and look at the errors generated by the MIPS empic test.)
> > 
> > The test didn't fail when I wrote it, did something break since and
> > no-one fix it?
> 
> Hmm.  Looks like it went in on 2000-03-10 (according to the
> ChangeLogs.
> 
> I just did a checkout of binutils using date 2000-03-15 (mmm, the Ides
> of March 8-), using the command:
> 
> cvs -z 9 -d :pserver:anoncvs@anoncvs.cygnus.com:/cvs/src co -D"2000-03-15" binutils
> 
> I then compiled it up for mips-elf.  It fails in exactly the same way
> as I'd seen all along:
> 
> ../as-new  -membedded-pic -mips3 -o dump.o
> /home/cgd/reloc-bug/src/gas/testsuite
> /gas/mips/empic.s
> /home/cgd/reloc-bug/src/gas/testsuite/gas/mips/empic.s: Assembler
> messages:
> /home/cgd/reloc-bug/src/gas/testsuite/gas/mips/empic.s:21: Error: Cannot make BFD_RELOC_LO16 relocation PC relative
> /home/cgd/reloc-bug/src/gas/testsuite/gas/mips/empic.s:23: Error: Cannot make BFD_RELOC_LO16 relocation PC relative
> /home/cgd/reloc-bug/src/gas/testsuite/gas/mips/empic.s:68: Error: Cannot make BFD_RELOC_LO16 relocation PC relative
> /home/cgd/reloc-bug/src/gas/testsuite/gas/mips/empic.s:70: Error: Cannot make BFD_RELOC_LO16 relocation PC relative
> [ ... ]
> 
> I've not gone to the trouble of inspecting diffs betwen the exact
> moments your change went in and 2000-03-15, but the ChangeLogs don't
> show anything interesting that i'm seeing.

This is certainly a bug, but I wrote the patch quite some time before
that and it worked then (it took about 6 months to be approved).

The PC-relative version of BFD_RELOC_LO16 is BFD_RELOC_PCREL_LO16
(which gets output as a R_MIPS_GNU_REL_LO16, just as the comment in
the testcase says), so the error message indicates that the conversion
of a PC-relative BFD_RELOC_LO16 is going wrong.

> A bit more discussion:
> 
> Those testsuite errors are for instructions (really in this case
> pseudo-ops) like:
> 
>         la      $3,g1-l3        # R_MIPS_GNU_REL_HI16   g1   0
>                                 # R_MIPS_GNU_REL_LO16   g1   C
>         la      $3,l1-l3        # R_MIPS_GNU_REL_HI16   .foo 0
>                                 # R_MIPS_GNU_REL_LO16   .foo 114
> 
> (lines 21 and 23).  g1 is not defined in the file, l1 is defined in a
> different section later in the file, and l3 is defined earlier in the
> currently-assembling section.
> 
> Your 'o' operand code change checks for:
> 
>               if (c == 0
>                   && (offset_expr.X_op != O_constant
>                       || offset_expr.X_add_number >= 0x8000
>                       || offset_expr.X_add_number < -0x8000)  
>                   && (mips_pic != EMBEDDED_PIC
>                       || offset_expr.X_op != O_subtract
>                       || (S_GET_SEGMENT (offset_expr.X_op_symbol)
>                           != now_seg)))
>                 break;
> 
> (circa line 7817 in gas/config/tc-mips.c of that date).
> 
> 'break' indicates that arguments don't match.  If you do 'break' here
> in this case for the instructions that fail the testcase, they're
> handled properly: you get the REL_LO16 and REL_HI16 relocs, etc.
> 
> No break indicates this can be handled as a BFD_RELOC_LO16.  That
> can't be converted into something PC-relative unless it happens to
> resolve into a constant by the symbol being defined in the same
> section, later in the file.

I believe that at the end of tc_gen_reloc, there's code for converting
relocs into PC-relative relocs, possibly the problem is that
BFD_RELOC_LO16 is not being handled here?

-- 
- Geoffrey Keating <geoffk@geoffk.org>