[LLD][ELF] --wrap: __real_foo references should trigger archive extraction for foo

[bd1976llvm]

Summary

A reference to __real_foo should trigger archive extraction of the input file that defines foo, otherwise a link using --wrap=foo might fail to link with an undefined reference to foo.
This matches bfd linker behaviour.

Replaces https://reviews.llvm.org/D135737.

[smeenai]

This makes sense to me, and I tested it internally and confirmed it doesn't change anything for us, but I'd prefer @MaskRay to take a look as well.

lld/test/ELF/wrap-extract-real.ll

19

Why does this become weak? (It's global for the assembly test case, which is what I'd expect.)

[MaskRay]

The new semantics look good.

lld/ELF/Driver.cpp

2249

If __real_ is referenced, pull in the symbol if it is lazy

lld/test/ELF/wrap-extract-real.s

17

1 => [[#]]

[bd1976llvm]

lld/test/ELF/wrap-extract-real.ll

19

Good question! It seems to be a bug. Weak binding is assigned by this piece of code:

llvm\lib\LTO\LTO.cpp:

// For symbols re-defined with linker -wrap and -defsym options,
// set the linkage to weak to inhibit IPO. The linkage will be
// restored by the linker.
if (Res.LinkerRedefined)
  GV->setLinkage(GlobalValue::WeakAnyLinkage);

However, LLD doesn't seem to have fulfilled its side of the bargain alluded to in this comment.

I'll look at this more tomorrow.

[smeenai]

lld/test/ELF/wrap-extract-real.ll

19

Ah, I remember this now; that's https://github.com/llvm/llvm-project/issues/51346. It's independent of your change though, so you can certainly land this without needing to fix that.

[MaskRay]

lld/test/ELF/wrap-extract-real.ll

19

So this is related to a WeakAnyLinkage workaround D33621. It'd be difficult to fix properly but it's good to re-think how to handle it.

[bd1976llvm]

lld/test/ELF/wrap-extract-real.ll

19

Thanks.

19

Interesting. Thanks.

[bd1976llvm]

Addressed review comments.

lld/ELF/Driver.cpp

for (auto *arg : args.filtered(OPT_wrap)) {
     StringRef name = arg->getValue();
     if (!seen.insert(name).second)
       continue;
 
     Symbol *sym = symtab.find(name);
     if (!sym)
       continue;
 
-    Symbol *real = addUnusedUndefined(saver().save("__real_" + name));
     Symbol *wrap =
         addUnusedUndefined(saver().save("__wrap_" + name), sym->binding);
+
+    // If there is a reference to __real_ pull in the symbol being wrapped.
+    // Do this after processing the __wrap_ symbol as that may have referenced
+    // the __real_ symbol.
+    StringRef realName = saver().save("__real_" + name);
+    if (symtab.find(realName))
+      addUnusedUndefined(name, sym->binding);
+
+    Symbol *real = addUnusedUndefined(realName);
     v.push_back({sym, real, 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->scriptDefined = true;
     sym->scriptDefined = true;
 
     // If a symbol is referenced in any object file, bitcode file or shared

lld/test/ELF/wrap-extract-real.ll

+# REQUIRES: x86
+## --wrap=xxx should trigger archive extraction for symbol xxx for references to __real_xxx.
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-as _start.ll -o _start.o
+# RUN: llvm-as _start_ref__real_foo.ll -o _start_ref__real_foo.o
+# RUN: llvm-as wrap.ll -o wrap.o
+# RUN: llvm-as foo.ll -o foo.o
+
+## Test when the reference to __real_foo is not in __wrap_foo.
+# RUN: ld.lld _start_ref__real_foo.o --start-lib foo.o --end-lib --wrap foo -o %t_real.elf
+# RUN: llvm-readelf --symbols %t_real.elf | FileCheck %s --check-prefix=REAL
+
+# REAL:      Symbol table '.symtab' contains 5 entries:
+# REAL-NEXT:        Value              Size Type    Bind   Vis       Ndx Name
+# REAL-NEXT:        {{.*}}           {{.*}} NOTYPE  LOCAL  DEFAULT   UND
+# REAL-NEXT:        {{.*}}           {{.*}} FILE    LOCAL  DEFAULT   ABS ld-temp.o
+# REAL-NEXT:        {{.*}}           {{.*}} FUNC    GLOBAL DEFAULT     2 _start
+# REAL-NEXT:        {{.*}}           {{.*}} FUNC    WEAK   DEFAULT     2 foo
+# REAL-NEXT:        {{.*}}           {{.*}} NOTYPE  GLOBAL DEFAULT   UND __wrap_foo
+
+## Test when the reference to __real_foo is in __wrap_foo.
+# RUN: ld.lld _start.o --start-lib wrap.o --end-lib --start-lib foo.o --end-lib --wrap foo -o %t_wrap_real.elf
+# RUN: llvm-readelf --symbols %t_wrap_real.elf | FileCheck %s --check-prefix=WRAP_REAL
+
+# WRAP_REAL:      Symbol table '.symtab' contains 5 entries:
+# WRAP_REAL-NEXT:    Value              Size Type    Bind   Vis       Ndx Name
+# WRAP_REAL-NEXT:    {{.*}}           {{.*}} NOTYPE  LOCAL  DEFAULT   UND
+# WRAP_REAL-NEXT:    {{.*}}           {{.*}} FILE    LOCAL  DEFAULT   ABS ld-temp.o
+# WRAP_REAL-NEXT:    {{.*}}           {{.*}} FUNC    GLOBAL DEFAULT     2 _start
+# WRAP_REAL-NEXT:    {{.*}}           {{.*}} FUNC    GLOBAL DEFAULT     2 __wrap_foo
+# WRAP_REAL-NEXT:    {{.*}}           {{.*}} FUNC    WEAK   DEFAULT     2 foo
+
+#--- _start.ll
+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-elf"
+define void @_start() {
+  ret void
+}
+
+#--- _start_ref__real_foo.ll
+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-elf"
+define void @_start() {
+  call void @__real_foo()
+  ret void
+}
+
+declare void @__real_foo()
+
+#--- wrap.ll
+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-elf"
+define void @__wrap_foo() {
+  call void @__real_foo()
+  ret void
+}
+
+declare void @__real_foo()
+
+#--- foo.ll
+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-elf"
+define void @foo() {
+  ret void
+}

lld/test/ELF/wrap-extract-real.s

+# REQUIRES: x86
+## --wrap=xxx should trigger archive extraction for symbol xxx for references to __real_xxx.
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 _start.s -o _start.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 ref__real_foo.s -o ref__real_foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 wrap.s -o wrap.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 foo.s -o foo.o
+
+## Test when the reference to __real_foo is not in __wrap_foo.
+# RUN: ld.lld _start.o ref__real_foo.o --start-lib foo.o --end-lib --wrap foo -o %t_real.elf
+# RUN: llvm-readelf --symbols %t_real.elf | FileCheck %s --check-prefix=REAL
+
+# REAL:      Symbol table '.symtab' contains 4 entries:
+# REAL-NEXT:        Value             Size Type    Bind   Vis       Ndx Name
+# REAL-NEXT:        {{.*}}               0 NOTYPE  LOCAL  DEFAULT   UND
+# REAL-NEXT:        {{.*}}               0 NOTYPE  GLOBAL DEFAULT     1 _start
+# REAL-NEXT:        {{.*}}               0 NOTYPE  GLOBAL DEFAULT     1 foo
+# REAL-NEXT:        {{.*}}               0 NOTYPE  GLOBAL DEFAULT   UND __wrap_foo
+
+## Test when the reference to __real_foo is in __wrap_foo.
+# RUN: ld.lld _start.o --start-lib wrap.o --end-lib --start-lib foo.o --end-lib --wrap foo -o %t_wrap_real.elf
+# RUN: llvm-readelf --symbols %t_wrap_real.elf | FileCheck %s --check-prefix=WRAP_REAL
+
+# WRAP_REAL:      Symbol table '.symtab' contains 4 entries:
+# WRAP_REAL-NEXT:        Value             Size Type    Bind   Vis       Ndx Name
+# WRAP_REAL-NEXT:        {{.*}}               0 NOTYPE  LOCAL  DEFAULT   UND
+# WRAP_REAL-NEXT:        {{.*}}               0 NOTYPE  GLOBAL DEFAULT     1 _start
+# WRAP_REAL-NEXT:        {{.*}}               0 NOTYPE  GLOBAL DEFAULT     1 __wrap_foo
+# WRAP_REAL-NEXT:        {{.*}}               0 NOTYPE  GLOBAL DEFAULT     1 foo
+
+#--- _start.s
+.global _start; _start:; ret
+
+#--- ref__real_foo.s
+call __real_foo
+
+#--- wrap.s
+.global __wrap_foo; __wrap_foo:; call __real_foo
+
+#--- foo.s
+.global foo; foo:; ret