[PATCH] D72197: [MC][ELF] Emit a relocation if target is defined in the same section and is non-local

Mon Jan 13 14:11:47 PST 2020

MaskRay added a comment.

In D72197#1817789 <https://reviews.llvm.org/D72197#1817789>, @bd1976llvm wrote:

> In D72197#1806225 <https://reviews.llvm.org/D72197#1806225>, @MaskRay wrote:
>
> > In D72197#1805438 <https://reviews.llvm.org/D72197#1805438>, @bd1976llvm wrote:
> >
> > > I wonder if is this really a bug or intentional. Clang and LLVM seem to ignore the possibility of symbol interposition (see: -fno-semantic-interposition). What is the LLVM policy for this?
> > >
> > > Also, this code is missing a case for STB_GLOBAL and visibility != STV_DEFAULT.  Actually, I have noticed before that this case is missing in various places in MC. As it stands this patch may cause a performance regression on platforms that use -fvisibility=hidden.
> >
> >
> > Visibility is irrelevant here. For a symbol defined in the same section as the instruction, the relocation record will reference a non-SECTION symbol instead of a STT_SECTION after this change. If the symbol is STV_HIDDEN, the linker will think the symbol is non-preemptible. So, no performance regression.
>
>
> For STB_GLOBAL and visibility != STV_DEFAULT symbols I don't think we need to emit a relocation record for pc-relative references within a section, no?

If this symbols is not STT_GNU_IFUNC, MC can choose whether a relocation referencing a STB_GLOBAL and visibility != STV_DEFAULT symbol should be resolved at assembly time. The symbol will be considered non-preemptible and the linked output will be the same. For non-`-fvisibility=hidden`-non-`-fvisibility-inlines-hidden` users, there aren't going to have many non-`STV_DEFAULT` symbols. I think this optimization will not be valuable.

> Unless I am mistaken, it seems that for projects that are not using -ffunction/data-sections we may be emitting many unnecessary relocation records with this patch? That is what I meant by a performance regression. However, I think that you trying to explain that this optimization isn't done and, rather than no relocation, we currently emit a relocation to the STT_SECTION symbol in the STB_GLOBAL case? If so, LGTM as long as there is a code comment added explaining why STB_LOCAL is the only case in which we consider such relocations resolvable.

The old code did not emit relocations referencing STT_SECTION symbols.
Those projects not using -ffunction-sections will see an increase of relocations referencing in-section non-STB_LOCAL functions.

We should make -fno-semantic-interposition behave more like -fno-semantic-interposition and do it at the correct layer (clang codegen). I'll investigate how to make clang emit STB_LOCAL `.L` (PrivateGlobalPrefix) aliases and jump through the aliases for the non-STB_LOCAL case. Relocations referencing in-section STT_SECTION symbols will be resolved at assembly time.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72197/new/

https://reviews.llvm.org/D72197