Re: mips: branches to external labels are broken

[Alexandre Oliva <aoliva at redhat dot com>]

On Nov 26, 2002, Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> wrote:

> Alexandre Oliva wrote:
>> On Nov 26, 2002, Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de> wrote:
>> 
>> > I added some support for branches to external labels for two reasons:
>> >   - to support what the spec wants
>> 
>> Which specs?  There's no spec I'm aware of that introduces any
>> relocation type that could be used for branch targets.  R_MIPS_PC16 is
>> certainly not it,

> R_MIPS_PC16 is defined in the ELF ABI as relocation against an external
> symbol, and branches are the only PC-relative MIPS insns.

But nowhere in the ABI does it say that the computed value for the
relocation should be divided by 4, like we do before my patch is
installed, so it's useless for branches.



> [snip]
>> >> /*
>> >> * We need to save the bits in the instruction since fixup_segment()
>> >> * might be deleting the relocation entry (i.e., a branch within
>> >> * the current segment).
>> >> */
>> >> -      if (!fixP->fx_done && value != 0)
>> >> +      if (!fixP->fx_done && (value != 0 || HAVE_NEWABI))
>> >> break;
>> 
>> > Is HAVE_NEWABI the right check here? !EMBEDDED_PIC might be better.
>> 
>> Yup.  HAVE_NEWABI is what determines whether we're generating RELA or
>> REL relocations, and, when doing RELA, it makes no sense to install
>> the addend in-place, and even less so to artificially force the addend
>> to a non-zero value by compensating the change with a modification in
>> the in-place addend, since the in-place addend won't be used at all.
>> 
>> So, yes, HAVE_NEWABI is the right check.  It has nothing to do with
>> EMBEDDED_PIC, even though I do concede that, if we get to that point
>> with mips_pic != EMBEDDED_PIC with !fixP->fx_done, we're going to emit
>> an error message because we're going to need a relocation that we
>> can't represent.

> I don't understand this reasoning. We are in the BFD_RELOC_16_PCREL_S2
> case here

It was BFD_RELOC_16_PCREL too.

> which is invalid for anything else than EMBEDDED_PIC.

There's no reason to assume so.  The hack becomes pointless (and
wrong) when we do RELA instead of REL.  EMBEDDED_PIC has nothing to do
with RELAness, it's HAVE_NEWABI that does, so that's the right test.

> HAVE_NEWABI must always be false there.

I don't know that EMBEDDED_PIC and NEWABI are incompatible, and I
don't really care.  EMBEDDED_PIC is not the right test there, since
the real test to be done is whether the addend is going to be
installed in place or not.

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer