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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D72197#1805964 <https://reviews.llvm.org/D72197#1805964>, @sfertile 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?
>
>
> I had a talk about this once I believe with @hfinkel (or maybe @dblaikie), and this behavior was intentional. Essentially LLVM may perform IPO between the 2 functions, in which case its no longer appropriate  to allow the callee to be preempted at runtime. The back-ends aren't consistent on this behaviour either. For example:
>
>   __attribute__((noinline))
>   int bar(int i) {
>       return i;
>   }
>  
>  
>   int caller(int i) {
>       if (bar(i) == i)
>         return 0;
>  
>       return 1;
>   }
>
>
> Compiling this for PowerPC and AArch64 Linux and -fPIC I get no relocation for the call. Compiling for X86_64 Linux and -fPIC I do get a relocation for the call. So it seems to be pick your poison: Emit a relocation and expose a potential runtime problem, or don't emit a relocation and break ELF interposition semantics.  I'm also not sure if the optimizer actually skips all IPO when the functions are in different sections, so we might end up with both broken shared object interposition and potentially unsafe codegen 😦


Behavior without this patch.

- `clang -target {aarch64,ppc64le} -fPIC -c` => no relocation referencing `bar`
- `clang -target {aarch64,ppc64le} -fPIC -c -ffunction-sections` => a branch relocation referencing `bar`

Look, we are already inconsistent. With this patch, we will consistently emit a relocation. I thinks this patch makes the situation better.

> With 'dso_local' and 'dso_premptable' generated by clang we should have enough information in the IR now to be able to properly implement '-fsemantic-interposition`, if someone was so inclined.

(You added `dso_local/dso_preemptable` in D20217 <https://reviews.llvm.org/D20217>.) I suspect we can simplify some linkage kinds with dso/local/dso_preemptable.  `-fsemantic-interposition` will be nice to have and Clang should still default to `-fno-semantic-interposition`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72197