[ELF] --wrap: retain __wrap_foo if foo is defined in an object/bitcode file

[MaskRay]

Summary

If foo is referenced in any object file, bitcode file or shared object,
__wrap_foo should be retained as the redirection target of sym
(f96ff3c0f8ebd941b3f6b345164c3d858b781484).

If the object file defining foo has foo references, we cannot easily distinguish
the case from cases where foo is not referenced (we haven't scanned
relocations). Retain __wrap_foo because we choose to wrap sym references
regardless of whether sym is defined to keep non-LTO/LTO/relocatable links' behaviors similar
https://sourceware.org/bugzilla/show_bug.cgi?id=26358 .

If foo is defined in a shared object, __wrap_foo can still be omitted
(wrap-dynamic-undef.s).

[jhenderson]

Apart from a couple of nits, I'll leave reviewing this to @andrewng, since I know he's looked into the issue more (he and I work together).

lld/ELF/Driver.cpp

1930

lld/ELF/Options.td

475–476

This line looks quite long?

[andrewng]

Apart from the nits already pointed out by @jhenderson, LGTM.

[MaskRay]

lld/ELF/Driver.cpp

1930

. is a valid part of a URL and this will make clicking the URL broken in many systems...

[MaskRay]

wrap long line

lld/ELF/Driver.cpp

for (auto *arg : args.filtered(OPT_wrap)) {
 
     // We want to tell LTO not to inline symbols to be overwritten
     // because LTO doesn't know the final symbol contents after renaming.
     real->canInline = false;
     sym->canInline = false;
 
     // Tell LTO not to eliminate these symbols.
     sym->isUsedInRegularObj = true;
-    if (sym->referenced)
+    // If sym is referenced in any object file, bitcode file or shared object,
+    // retain wrap which is the redirection target of sym. If the object file
+    // defining sym has sym references, we cannot easily distinguish the case
+    // from cases where sym is not referenced. Retain wrap because we choose to
+    // wrap sym references regardless of whether sym is defined
+    // (https://sourceware.org/bugzilla/show_bug.cgi?id=26358).
+    if (sym->referenced || sym->isDefined())
       wrap->isUsedInRegularObj = true;
   }
   return v;
 }
 
 // Do renaming for -wrap and foo@v1 by updating pointers to symbols.
 //
 // When this function is executed, only InputFiles and symbol table

lld/ELF/Options.td

defm warn_symbol_ordering: BB<"warn-symbol-ordering",
     "Do not warn about problems with the symbol ordering file">;
 
 def warn_unresolved_symbols: F<"warn-unresolved-symbols">,
   HelpText<"Report unresolved symbols as warnings">;
 
 defm whole_archive: B<"whole-archive",
     "Force load of all members in a static library",
     "Do not force load of all members in a static library (default)">;
 
-defm wrap: Eq<"wrap", "Use wrapper functions for symbol">,
+defm wrap : Eq<"wrap", "Redirect symbol references to __wrap_symbol and "
+                       "__real_symbol references to symbol">,
             MetaVarName<"<symbol>">;
 
 def z: JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,
   HelpText<"Linker option extensions">;
 
 def visual_studio_diagnostics_format : F<"vs-diagnostics">,
 HelpText<"Format diagnostics for Visual Studio compatibility">;
 
 // Aliases

lld/docs/ld.lld.1

 warning.
 This flag has been introduced in late 2018, has no counter part in ld and gold
 linkers, and may be removed in the future.
 .It Fl -warn-unresolved-symbols
 Report unresolved symbols as warnings.
 .It Fl -whole-archive
 Force load of all members in a static library.
 .It Fl -wrap Ns = Ns Ar symbol
-Use wrapper functions for symbol.
+Redirect
+.Ar symbol
+references to
+.Ar __wrap_symbol
+and
+.Ar __real_symbol
+references to
+.Ar symbol.
 .It Fl z Ar option
 Linker option extensions.
 .Bl -tag -width indent -compact
 .Pp
 .It Cm dead-reloc-in-nonalloc Ns = Ns Ar section_glob=value
 Resolve a relocation in a matched non-SHF_ALLOC section referencing a discarded symbol to
 .Ar value
 Accepts globs, in the event of a section matching more than one option, the last

lld/test/ELF/lto/wrap-defined.ll

+; REQUIRES: x86
+;; Similar to ../wrap-defined.s but for LTO.
+
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld -shared %t.o -wrap=bar -o %t.so
+; RUN: llvm-objdump -d %t.so | FileCheck %s
+
+; CHECK:      <_start>:
+; CHECK-NEXT:   jmp {{.*}} <__wrap_bar@plt>
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @bar() {
+  ret void
+}
+
+define void @_start() {
+  call void @bar()
+  ret void
+}

lld/test/ELF/wrap-defined.s

+# REQUIRES: x86
+
+# RUN: rm -rf %t && split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/main.s -o %t/main.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/wrap.s -o %t/wrap.o
+# RUN: ld.lld -shared --soname=fixed %t/wrap.o -o %t/wrap.so
+
+## GNU ld does not wrap a defined symbol in an object file
+## https://sourceware.org/bugzilla/show_bug.cgi?id=26358
+## We choose to wrap defined symbols so that LTO, non-LTO and relocatable links
+## behave the same. The 'call bar' in main.o will reference __wrap_bar. We cannot
+## easily distinguish the case from cases where bar is not referenced, so we
+## export __wrap_bar whenever bar is defined, regardless of whether it is indeed
+## referenced.
+
+# RUN: ld.lld -shared %t/main.o --wrap bar -o %t1.so
+# RUN: llvm-objdump -d %t1.so | FileCheck %s
+# RUN: ld.lld %t/main.o %t/wrap.so --wrap bar -o %t1
+# RUN: llvm-objdump -d %t1 | FileCheck %s
+
+# CHECK:      <_start>:
+# CHECK-NEXT:   callq {{.*}} <__wrap_bar@plt>
+
+#--- main.s
+.globl _start, bar
+_start:
+  call bar
+bar:
+
+#--- wrap.s
+.globl __wrap_bar
+__wrap_bar:
+  retq

lld/test/ELF/wrap-shlib-undefined.s

 ## --no-allow-shlib-undefined errors because __real_foo is not defined.
 # RUN: not ld.lld %t/main.o %t/bar.so -o /dev/null 2>&1 | FileCheck --check-prefix=ERR %s
 # ERR: {{.*}}.so: undefined reference to __real_foo [--no-allow-shlib-undefined]
 
 ## --wrap=foo defines __real_foo.
 # RUN: ld.lld %t/main.o %t/bar.so --wrap=foo -o %t2
 # RUN: llvm-readelf --dyn-syms %t2 | FileCheck %s --check-prefix=CHECK2
 
-## The reference __real_foo from %t/bar.so causes foo to be exported.
-## __wrap_foo is not used, thus not exported.
-# CHECK2:      Symbol table '.dynsym' contains 3 entries:
+## See wrap-plt2.s why __wrap_foo is retained.
+# CHECK2:      Symbol table '.dynsym' contains 4 entries:
 # CHECK2:      NOTYPE  LOCAL  DEFAULT  UND
 # CHECK2-NEXT: NOTYPE  GLOBAL DEFAULT  UND bar
+# CHECK2-NEXT: NOTYPE  GLOBAL DEFAULT  UND __wrap_foo
 # CHECK2-NEXT: NOTYPE  GLOBAL DEFAULT    6 foo
 
 ## __wrap_bar is undefined.
 # RUN: ld.lld -shared %t/main.o --wrap=bar -o %t3.so
 # RUN: llvm-readelf -r --dyn-syms %t3.so | FileCheck %s --check-prefix=CHECK3
 # CHECK3:      R_X86_64_JUMP_SLOT 0000000000000000 __wrap_bar + 0
 # CHECK3:      Symbol table '.dynsym' contains 4 entries:
 # CHECK3:      NOTYPE  LOCAL  DEFAULT  UND