[lld] r259474 - ELF: Simplify and add comments.

[Rui Ueyama via llvm-commits]

On Tue, Feb 2, 2016 at 2:13 PM, Rafael Espíndola <rafael.espindola at gmail.com
> wrote:

> On 2 February 2016 at 16:43, Rui Ueyama <ruiu at google.com> wrote:
> > I guess that I still don't fully understand this piece of code. When we
> > reach here for what? Can you describe it a little bit more in detail for
> me?
>
> Sure. The code is trying to decide if a given relocation can be fully
> resolved by the static linker or if needs to ask the dynamic linker
> for help (i.e., create a dynamic relocation).
>
> The static linker cannot know the final answer when:
>
> * The symbol can be preempted. That is, the symbol the static linker
> is seeing might not be the one used at runtime.
> * The symbol is the final one, but the value the relocation wants
> depends on where the code is placed in memory.
>
>  I actually find it a bit harder to follow now that part of the code
> is duplicated in the "need got" if. We have:
>
> -------------------------------
>     bool CBP = canBePreempted(Body, /*NeedsGot=*/true);
>       bool Dynrel = Config->Shared && !Target->isRelRelative(Type) &&
>                     !Target->isSizeRel(Type);
>       if (CBP)
>         Body->setUsedInDynamicReloc();
>       if (CBP || Dynrel)
>         Out<ELFT>::RelaDyn->addReloc({&C, &RI});
> ......
>
>
>   if (canBePreempted(Body, /*NeedsGot=*/false)) {
>       Body->setUsedInDynamicReloc();
>       Out<ELFT>::RelaDyn->addReloc({&C, &RI});
>       continue;
>     }
>
>     // If we get here, the code we are handling is not PIC. We need to copy
>     // relocations from object files to the output file, so that the
>     // dynamic linker can fix up addresses. But there are a few exceptions.
>     // If the relocation will not change at runtime, we don't need to copy
>     // them. For example, we don't copy PC-relative relocations because
>     // the distance between two symbols won't change whereever they are
>     // loaded. Likewise, if we are linking an executable, it will be loaded
>     // at a fixed address, so we don't copy relocations.
>     if (Config->Shared && !Target->isRelRelative(Type) &&
>         !Target->isSizeRel(Type))
>        Out<ELFT>::RelaDyn->addReloc({&C, &RI});
> ----------------------------
>
> The two calls to setUsedInDynamicReloc and RelaDyn->addReloc should be
> merged.
>

The point of my patches is to make code a series of the decisions like

 - here we are handling *something* (e.g. relocation against IFUNC), and in
this case we need to create something and something (e.g. GOT and PLT) and
nothing more

Previously, there were bunch of cascading if's that affect each other, so
it was not clear how many cases we need to handle and what we want for each
case.

The overall structure of how we collect information for dynamic
> relocations should be fixed too, I will try to do that next.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160202/f77a496a/attachment.html>