This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: mips: branches to external labels are broken


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, and the fact that it appears to work at the moment
is just because of the hideous hacks in the assembler code, some of
which are only compensating for errors in the handling of R_MIPS_PC16
in the linker.

>   - to allow carefully optimized assembly code

Well, you can always use embedded pic if you really want to do that.

> If it gets in the way of a clean relocation handling rewrite, that's
> probably reason enough to remove it.

I've lost hope of rewriting relocation handling cleanly.  I just don't
see a clean way to do what we're doing with in-place addends (REL)
with out-of-place addends (RELA), and I got tired of trying to find
it.  It's just too broken.  In part because the handling of
R_MIPS_PC16 itself is broken (in that it shifts the addend right when
installing it, but doesn't shift it back left when reading it back
in).

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, such that it does the
right thing and we can then remove at least one of the `This must be
broken, but it appears to work' comments.  For the `addend == 0'
hacks, there's little hope other than using an alternate function
instead of bfd_elf_generic_reloc.  However, I've tried some of that,
without much success.  It's really a horrible mess.

>> /*
>> * 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.

-- 
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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]