Re: mips: branches to external labels are broken

[Thiemo Seufer <ica2_ts at csv dot ica dot uni-stuttgart dot de>]

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.

[snip]
> If we're to use R_MIPS_PC16 for branches, we should stop lying to bfd
> and actually tell it that its rightshift is 2,

We shouldn't, because the ELF ABI specifies that R_MIPS_PC16 has no
rightshift. If we need external branches in future, we should either
R_MIPS_PC16 the ABI way (as braindread as this may be) or use the
GNU extension with shifted encoding from embedded mips for it.

[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, which is invalid for anything else than EMBEDDED_PIC.
HAVE_NEWABI must always be false there.


Thiemo