[ELF] Drop unused original symbol after wrapping if not defined

[smeenai]

Summary

We were previously only omitting the original of a wrapped symbol if it
was not used by an object file and undefined. We can tighten the second
condition to drop any symbol that isn't defined instead, which lets us
drop a previous check (added in https://reviews.llvm.org/D118756) that
was only covering some such symbols.

[MaskRay]

lld/ELF/Driver.cpp

2182

The comment and code is from D118756. Mention it in the summary?

[smeenai]

Add comment about conditional

[MaskRay]

Comments looks great. Thanks for investigating this area!

lld/ELF/Driver.cpp

static std::vector<WrappedSymbol> addWrappedSymbols(opt::InputArgList &args) {
   DenseSet<StringRef> seen;
 
   for (auto *arg : args.filtered(OPT_wrap)) {
     StringRef name = arg->getValue();
     if (!seen.insert(name).second)
       continue;
 
     Symbol *sym = symtab->find(name);
-    // Avoid wrapping symbols that are lazy and unreferenced at this point, to
-    // not create undefined references. The isUsedInRegularObj check handles the
-    // case of a weak reference, which we still want to wrap even though it
-    // doesn't cause lazy symbols to be extracted.
-    if (!sym || (sym->isLazy() && !sym->isUsedInRegularObj))
+    if (!sym)
       continue;
 
     Symbol *real = addUnusedUndefined(saver().save("__real_" + name));
     Symbol *wrap =
         addUnusedUndefined(saver().save("__wrap_" + name), sym->binding);
     v.push_back({sym, real, wrap});
 
     // We want to tell LTO not to inline symbols to be overwritten

lld/ELF/SymbolTable.cpp

void SymbolTable::wrap(Symbol *sym, Symbol *real, Symbol *wrap) {
   idx2 = idx1;
   idx1 = idx3;
 
   // Propagate symbol usage information to the redirected symbols.
   if (sym->isUsedInRegularObj)
     wrap->isUsedInRegularObj = true;
   if (real->isUsedInRegularObj)
     sym->isUsedInRegularObj = true;
-  else if (sym->isUndefined())
+  else if (!sym->isDefined())
+    // Now that all references to sym have been redirected to wrap, if there are
+    // no references to real (which has been redirected to sym), we only need to
+    // keep sym if it was defined, otherwise it's unused and can be dropped.
     sym->isUsedInRegularObj = false;
 
   // Now renaming is complete, and no one refers to real. We drop real from
   // .symtab and .dynsym. If real is undefined, it is important that we don't
   // leave it in .dynsym, because otherwise it might lead to an undefined symbol
   // error in a subsequent link. If real is defined, we could emit real as an
   // alias for sym, but that could degrade the user experience of some tools
   // that can print out only one symbol for each location: sym is a preferred

lld/test/ELF/lto/wrap-unreferenced-before-codegen.test

 # RUN:   FileCheck --check-prefix=UNWIND-DYNSYM %s
 
 # UNWIND-DISASM:       [[#%x,RELOC:]] R_X86_64_JUMP_SLOT __wrap__Unwind_Resume
 # UNWIND-DISASM-LABEL: <_Z1fv>:
 # UNWIND-DISASM:       callq {{.*}}<__wrap__Unwind_Resume@plt>
 # UNWIND-DISASM-LABEL: <__wrap__Unwind_Resume@plt>:
 # UNWIND-DISASM-NEXT:  jmpq *[[#]](%rip) # [[#%#x,RELOC]]
 
-# UNWIND-DYNSYM:      Symbol table '.dynsym' contains 5 entries:
+# UNWIND-DYNSYM:      Symbol table '.dynsym' contains 4 entries:
 # UNWIND-DYNSYM:      NOTYPE  LOCAL  DEFAULT   UND
 # UNWIND-DYNSYM-NEXT: NOTYPE  GLOBAL DEFAULT   UND throw
-# UNWIND-DYNSYM-NEXT: NOTYPE  GLOBAL DEFAULT   UND _Unwind_Resume
 # UNWIND-DYNSYM-NEXT: NOTYPE  GLOBAL DEFAULT   UND __wrap__Unwind_Resume
 # UNWIND-DYNSYM-NEXT: FUNC    GLOBAL DEFAULT     9 _Z1fv
 
 # RUN: llvm-mc -triple x86_64-elf -filetype=obj -o %t/malloc.o %t/malloc.s
 # RUN: ld.lld -shared -o %t/libmalloc.so -soname libmalloc.so %t/malloc.o
 # RUN: llvm-mc -triple x86_64-elf -filetype=obj -o %t/emutls.o %t/emutls.s
 # RUN: llvm-as -o %t/usetls.bc %t/usetls.ll
 # RUN: ld.lld -shared -o %t/libusetls.so %t/usetls.bc %t/libmalloc.so \
 # RUN:   --start-lib %t/emutls.o -mllvm -emulated-tls --wrap malloc
 # RUN: llvm-objdump --dynamic-reloc --disassemble %t/libusetls.so | \
 # RUN:   FileCheck --check-prefix=USETLS-DISASM %s
 # RUN: llvm-readelf --dyn-syms %t/libusetls.so | \
 # RUN:   FileCheck --check-prefix=USETLS-DYNSYM %s
 
 # USETLS-DISASM:       [[#%x,RELOC:]] R_X86_64_JUMP_SLOT __wrap_malloc
 # USETLS-DISASM-LABEL: <__emutls_get_address>:
 # USETLS-DISASM-NEXT:  jmp{{.*}}<__wrap_malloc@plt>
 # USETLS-DISASM-LABEL: <__wrap_malloc@plt>:
 # USETLS-DISASM-NEXT:  jmpq *[[#]](%rip) # [[#%#x,RELOC]]
 
-# USETLS-DYNSYM:      Symbol table '.dynsym' contains 6 entries:
+# USETLS-DYNSYM:      Symbol table '.dynsym' contains 5 entries:
 # USETLS-DYNSYM:      NOTYPE  LOCAL  DEFAULT   UND
-# USETLS-DYNSYM-NEXT: NOTYPE  GLOBAL DEFAULT   UND malloc
 # USETLS-DYNSYM-NEXT: NOTYPE  GLOBAL DEFAULT   UND __wrap_malloc
 # USETLS-DYNSYM-NEXT: FUNC    GLOBAL DEFAULT     6 f
 # USETLS-DYNSYM-NEXT: NOTYPE  GLOBAL DEFAULT     6 __emutls_get_address
 
 #--- unwind.s
 .globl _Unwind_Resume
 .globl __wrap__Unwind_Resume
 _Unwind_Resume:

lld/test/ELF/wrap-drop-shared-original.s

+## If the original of a wrapped symbol becomes unreferenced after wrapping, it
+## should be dropped from the dynamic symbol table even if defined in a shared
+## library.
+
+# REQUIRES: x86
+
+# RUN: rm -rf %t && split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-elf %t/original.s -o %t/original.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-elf %t/wrapped.s -o %t/wrapped.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-elf %t/ref.s -o %t/ref.o
+# RUN: ld.lld -shared -o %t/liboriginal.so -soname liboriginal.so %t/original.o
+# RUN: ld.lld -shared -o %t/liboriginal-and-wrapped.so \
+# RUN:   -soname liboriginal-and-wrapped.so %t/original.o %t/wrapped.o
+# RUN: ld.lld -shared -o %t/libref-with-original.so %t/ref.o \
+# RUN:   --as-needed %t/liboriginal.so --wrap foo
+# RUN: llvm-readelf --dynamic --dyn-syms %t/libref-with-original.so | \
+# RUN:   FileCheck --check-prefix=ORIGINAL %s
+# RUN: ld.lld -shared -o %t/libref-with-original-and-wrapped.so %t/ref.o \
+# RUN:   --as-needed %t/liboriginal-and-wrapped.so --wrap foo
+# RUN: llvm-readelf --dynamic --dyn-syms %t/libref-with-original-and-wrapped.so | \
+# RUN:   FileCheck --check-prefix=ORIGINAL-AND-WRAPPED %s
+
+# ORIGINAL-NOT: (NEEDED) Shared library: [liboriginal.so]
+# ORIGINAL:      Symbol table '.dynsym' contains 3 entries:
+# ORIGINAL:      NOTYPE  LOCAL  DEFAULT   UND
+# ORIGINAL-NEXT: NOTYPE  GLOBAL DEFAULT   UND __wrap_foo
+# ORIGINAL-NEXT: NOTYPE  GLOBAL DEFAULT     6 ref
+
+# ORIGINAL-AND-WRAPPED: (NEEDED) Shared library: [liboriginal-and-wrapped.so]
+# ORIGINAL-AND-WRAPPED:      Symbol table '.dynsym' contains 3 entries:
+# ORIGINAL-AND-WRAPPED:      NOTYPE  LOCAL  DEFAULT   UND
+# ORIGINAL-AND-WRAPPED-NEXT: NOTYPE  GLOBAL DEFAULT   UND __wrap_foo
+# ORIGINAL-AND-WRAPPED-NEXT: NOTYPE  GLOBAL DEFAULT     6 ref
+
+#--- original.s
+.globl foo
+foo:
+  retq
+
+#--- wrapped.s
+.globl __wrap_foo
+__wrap_foo:
+  retq
+
+#--- ref.s
+.globl ref
+ref:
+  jmp foo@plt

lld/test/ELF/wrap-lazy.test

 # RUN: llvm-readelf -s %t8.so | FileCheck --check-prefix=NO-LAZY %s
 
 # NO-LAZY-NOT: lazy
 # NO-LAZY-NOT: __wrap_lazy
 
 # LAZY-DEF-DAG: [[#]] lazy
 # LAZY-DEF-DAG: UND __wrap_lazy
 
-# LAZY-REF-DAG: UND lazy
+# LAZY-REF-NOT: UND lazy
 # LAZY-REF-DAG: UND __wrap_lazy
+# LAZY-REF-NOT: UND lazy
 
 #--- dummy.s
 .globl dummy
 dummy:
   retq
 
 #--- lazy.ll
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"