This is the mail archive of the binutils@sourceware.org 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: [RFC] [gold] Simplify relocation strategy logic


> Well, I don't know how much it counts for, but I know I was deliberately
> following that model when doing the get_reference_flags() stuff. :-)

Sorry for not noticing the discrepancy then. When I first added the
ABSOLUTE_REF flag, it was meant to indicate the type of reference at
the location being relocated.

By the way, have you noticed that the RELATIVE_REF flag is returned
for many relocs, but gold never tests for it? I think I'm going to get
rid of it.

> I agree it's confusing[*], but as things stand, I don't think
> Symbol::needs_dynamic_reloc() inherently assumes what you say.
> It depends on the calling context. ?If you wanted to call
> Symbol::needs_dynamic_reloc() directly from relocate() to decide
> whether a GOT reloc itself needs to become dynamic, then yes,
> the function would give the wrong answer. ?But nothing calls
> the function in that context, because GOT relocs themselves
> are always resolved statically. ?The only question is whether
> the GOT _entry_ needs a reloc.

I started this little exercise with the premise that there were only
three strategies: normal, reference to PLT, and dynamic relocation.
(The "normal" case included references to the GOT.) As I worked
through the logic, it became clear that it was (at least
theoretically) possible for a relocation that references a PLT entry
to also require a dynamic relocation (e.g., an absolute reference to a
PLT entry in a position-independent output). I'm not sure if that's
ever a sensible thing, but it seems possible, and that's one of the
cases I support. I suppose I could add "reference to a GOT entry" as
another strategy, but it doesn't seem to simplify any logic (although
it might simplify it conceptually). [On second thought, it might
actually simplify the code at the top of Relocate::relocate(). I'll
play with that idea...]

If the GOT entry itself needs a dynamic reloc, that's already taken
care of in Scan::global(). The GOT entries never have static
relocations, either -- their values are computed during
Output_data_got::Got_entry::write(). The only thing we care about when
using get_reference_flags() is whether the relocation we're applying
should reference the address of a PLT entry, a GOT entry, or the
actual symbol value, and whether a dynamic relocation is necessary at
that location.

> The case where Symbol::needs_dynamic_reloc() _does_ need to describe
> the GOT entry itself is for the current definition of use_plt_offset().
> That function is called for all relocations (including GOT relocations)
> at the beginning of the target's relocate() function. ?In the case of
> GOT relocs, use_plt_offset() says whether the _GOT entry_ should be
> redirected to the PLT. ?(This is of course sometimes true for ifuncs,
> but should never be true otherwise.) ?So I claim that, in the context
> of use_plt_offset(), needs_dynamic_reloc() really is telling you about
> the GOT entry rather than the GOT reloc itself.

We never call Symbol::use_plt_offset() when relocating a GOT entry. I
think you're confusing that with the use_plt_offset_ flag in the
Got_entry. That bit is set explicitly for GOT entries that reference a
PLT entry directly, and as far as I can tell, it is set only for IFUNC
symbols.

> [*] and I'd like to think that the confusion predates my changes. :-)
> ? ?Don't get me wrong, your plans to clean this up are very welcome to me.
> ? ?I just dispute that the current code is wrong.

Yes, it was plenty confusing before (some was my fault for sure, but I
think some is just intrinsic complexity), and will probably always
remain so. Our best hope is just to minimize how confusing it is. :-)

Oh, and thanks to the newly-simplified logic and our subsequent
discussion, I've discovered what I think is another lurking bug --
gold says "no dynamic reloc necessary" for any reference to an
absolute symbol, but it doesn't consider the case of a pc-relative
reference to an absolute symbol in position-independent code!

-cary


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