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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73230