[PATCH] D73230: [X86][ELF] Prefer to lower MC_GlobalAddress operands to .Lfoo$local

Mon Aug 10 10:44:25 PDT 2020

bd1976llvm added a comment.

In D73230#2207258 <https://reviews.llvm.org/D73230#2207258>, @MaskRay wrote:

> In D73230#2207159 <https://reviews.llvm.org/D73230#2207159>, @bd1976llvm wrote:
>
>> In D73230#2207023 <https://reviews.llvm.org/D73230#2207023>, @MaskRay wrote:
>>
>>> In D73230#2206232 <https://reviews.llvm.org/D73230#2206232>, @bd1976llvm wrote:
>>>
>>>> @MaskRay - this change causes a behaviour difference for --wrap.
>>>>
>>>> Here is the --wrap behaviour before this change:
>>>>
>>>>   ben at ben-VirtualBox:~/tests/wrap$ more main.c
>>>>   void __wrap_foo () {
>>>>   	puts ("__wrap_foo");
>>>>   	__real_foo();
>>>>   }
>>>>   
>>>>   void foo () { puts("foo()"); }
>>>>   
>>>>   int main() {
>>>>   	__real_foo();
>>>>   	puts("---");
>>>>   	__wrap_foo();
>>>>   	puts("---");
>>>>   	foo();
>>>>   	return 0;
>>>>   }
>>>>   ben at ben-VirtualBox:~/tests/wrap$ gcc main.c -Wl,--wrap=foo -ffunction-sections -fuse-ld=lld -o lld.elf -Wno-implicit-function-declaration
>>>>   ben at ben-VirtualBox:~/tests/wrap$ ./lld.elf 
>>>>   foo()
>>>>   ---
>>>>   __wrap_foo
>>>>   foo()
>>>>   ---
>>>>   __wrap_foo
>>>>   foo()
>>>>
>>>> … and here is the behaviour after this change:
>>>>
>>>>   ben at ben-VirtualBox:~/tests/wrap$ ./lld.elf 
>>>>   foo()
>>>>   ---
>>>>   __wrap_foo
>>>>   foo()
>>>>   ---
>>>>   foo()
>>>>
>>>> There is no behaviour change for -flto builds so the behaviour for --wrap is now effectively different for LTO vs normal builds.
>>>
>>> I think you missed a point in the description of --wrap:
>>>
>>>   You may wish to provide a "__real_malloc" function as well, so that links without the --wrap option will succeed.  If you do this, you
>>>   should not put the definition of "__real_malloc" in the same file as "__wrap_malloc"; if you do, the assembler may resolve the call
>>>   before the linker has a chance to wrap it to "malloc".
>>>
>>> Providing `foo` definition in the translation unit where they are referenced is not reliable when you are using `--wrap`.
>>> Actually, this is where GNU ld and LLD differ. See https://sourceware.org/bugzilla/show_bug.cgi?id=26358 and the history of `lld/test/ELF/wrap-shlib-undefined.s`
>>>
>>> If you want to get guaranteed semantics, don't define `foo` when it is referenced. You may also try gcc and gcc -fPIC -fno-semantic-interposition, the behavior is similar to latest clang.
>>
>> Thanks for the summary. I am not particularly concerned about which behaviour we have w.r.t. wrapping intra-translation-unit references (although I have seen some evidence that lld's behaviour is useful e.g. https://stackoverflow.com/questions/13961774/gnu-gcc-ld-wrapping-a-call-to-symbol-with-caller-and-callee-defined-in-the-sam). However, you stated in https://sourceware.org/bugzilla/show_bug.cgi?id=26358 that for lld -r, lto, and normal links have the same behaviour - that is not true after this change. Furthermore, with the current clang it is not possible to go back to the old behaviour using -fsemantic-interposition for hidden symbols. IIUC I think that hidden symbols are probably the majority of opensource symbols now as the GNU toolchain encourages the use of -fvisiblity=hidden.
>
> Let me summarize the cases:
>
> - `{clang,gcc} -fuse-ld=lld main.c  -Wl,--wrap=foo` => wrapped (in LLD, --wrap is done after (global) symbol resolution. Definitions are wrapped as well)
> - `{clang,gcc} -fuse-ld=bfd main.c  -Wl,--wrap=foo` => not wrapped (in GNU ld, --wrap is per object file. --wrap is not effective when the symbol is defined)
> - `{clang,gcc} -fuse-ld=lld main.c -fPIC -fno-semantic-interposition -Wl,--wrap=foo` => not wrapped (references go through `.Lfoo$local` which cannot be wrapped)
>
> I think your make an integration between this commit and 872c5fb1432493c0a09b6f210765c0d94ce9b5d0 <https://reviews.llvm.org/rG872c5fb1432493c0a09b6f210765c0d94ce9b5d0>, so for -fno-PIC or -fPIE, you observe the `-fPIC -fno-semantic-interposition` behavior as well. If you cherry pick 872c5fb1432493c0a09b6f210765c0d94ce9b5d0 <https://reviews.llvm.org/rG872c5fb1432493c0a09b6f210765c0d94ce9b5d0> and don't use -fno-semantic-interposition, and use LLD, you should get a wrapping behavior. (Clang traditionally has some -fno-semantic-interposition behaviors, so in the future we might be able to make -fno-semantic-interposition default for -fPIC.)
>
> I indeed prefer the LLD behavior, so I filed https://sourceware.org/bugzilla/show_bug.cgi?id=26358 yesterday, but I cannot say the wrapping behavior is promised. If you want better portability, make `foo` weak (and be aware of side effects with the change).

@MaskRay - Thanks for taking the time to look into this :)

Here are my results from an e912fffd3a8c6c9f6e09d2eac4c1ee3a32800a22 <https://reviews.llvm.org/rGe912fffd3a8c6c9f6e09d2eac4c1ee3a32800a22> clang using my previous example. I have included a bit more context in the first relocation dump so you can see which relocation.. in the other dumps I strip out more.

With -fpic:

$ /c/u/br2/bin/clang main.c -fno-semantic-interposition -c -fpic -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=default -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o                                           
...
Relocation section '.rela.text.main' at offset 0x330 contains 7 entries:

  Offset             Info             Type               Symbol's Value  Symbol's Name + Addend

...
0000000000000041  0000000600000004 R_X86_64_PLT32         0000000000000000 .text.foo - 4
...
Relocation section '.rela.eh_frame' at offset 0x3d8 contains 3 entries:
...

$ /c/u/br2/bin/clang main.c -fsemantic-interposition -c -fpic -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=default -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o                                           
...
0000000000000041  0000000a00000004 R_X86_64_PLT32         0000000000000000 foo - 4
...

$ /c/u/br2/bin/clang main.c -fno-semantic-interposition -c -fpic -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=hidden -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o                                           
...
0000000000000041  0000000600000004 R_X86_64_PLT32         0000000000000000 .text.foo - 4
...

$ /c/u/br2/bin/clang main.c -fsemantic-interposition -c -fpic -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=hidden -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o                                           
...
0000000000000041  0000000600000004 R_X86_64_PLT32         0000000000000000 .text.foo - 4
...

With -fpie:

$ /c/u/br2/bin/clang main.c -fno-semantic-interposition -c -fpie -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=default -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o
...
0000000000000041  0000000a00000004 R_X86_64_PLT32         0000000000000000 foo - 4
...

$ /c/u/br2/bin/clang main.c -fsemantic-interposition -c -fpie -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=default -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o                                           
...
0000000000000041  0000000a00000004 R_X86_64_PLT32         0000000000000000 foo - 4
...

$ /c/u/br2/bin/clang main.c -fno-semantic-interposition -c -fpie -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=hidden -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o                                           
...
0000000000000041  0000000a00000004 R_X86_64_PLT32         0000000000000000 foo - 4
...

$ /c/u/br2/bin/clang main.c -fsemantic-interposition -c -fpie -target x86_64-linux-gnu -o test.o -ffunction-sections -fdata-sections -fvisibility=hidden -Wno-implicit-function-declaration
$ /c/u/br2/bin/llvm-readelf -r test.o                                           
...
0000000000000041  0000000a00000004 R_X86_64_PLT32         0000000000000000 foo - 4
...

So, clang's behavior has changed so that --wrap no longer wraps symbol definitions for default symbols + -fpic + -fno-semantic-interposition (-fno-semantic-interposition is the default); however, you can restore the old behavior via -fsemantic-interposition. For hidden symbols + -fpic --wrap no longer wraps symbol definitions and there is no way to restore the old definition wrapping behavior.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73230