[lld/COFF] Fix -start-lib / -end-lib after reviews.llvm.org/D116434

[nico]

That change forgot to set lazy to false before calling addFile() in forceLazy() which caused addFile() to parse the file we want to force a load for to be added as a lazy object again instead of adding the file to ctx.objFileInstances.

This is caught by a pretty simple test (included).

---

Seems like nobody uses these flags in practice at the moment.

Fixes https://crbug.com/383531486

[llvmbot]

@llvm/pr-subscribers-lld-coff
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld

Author: Nico Weber (nico)

Changes

That change forgot to set lazy to false before calling addFile() in forceLazy() which caused addFile() to parse the file we want to force a load for to be added as a lazy object again instead of adding the file to ctx.objFileInstances.

This is caught by a pretty simple test (included).

---

Seems like nobody uses these flags in practice at the moment.

Fixes https://crbug.com/383531486

---

Full diff: https://github.com/llvm/llvm-project/pull/120452.diff

2 Files Affected:

• (modified) lld/COFF/SymbolTable.cpp (+1)
• (modified) lld/test/COFF/start-lib.ll (+75)

diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 6b3375e13e8393..b1d375b2265835 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -116,6 +116,7 @@ static void forceLazy(Symbol *s) {
   }
   case Symbol::Kind::LazyObjectKind: {
     InputFile *file = cast<LazyObject>(s)->file;
+    file->lazy = false;
     file->symtab.addFile(file);
     break;
   }
diff --git a/lld/test/COFF/start-lib.ll b/lld/test/COFF/start-lib.ll
index f4f49ed4e4f6e6..02bf4c254ee185 100644
--- a/lld/test/COFF/start-lib.ll
+++ b/lld/test/COFF/start-lib.ll
@@ -79,3 +79,78 @@ define i32 @bar() {
 
 !llvm.linker.options = !{!0}
 !0 = !{!"/INCLUDE:bar"}
+
+
+; Check that lazy object files trigger loads correctly.
+; If the links succeed, that's enough, no additional tests needed.
+
+; RUN: llc -filetype=obj %t.dir/main2.ll -o %t-main2.obj
+; RUN: llc -filetype=obj %t.dir/foo.ll -o %t-foo.obj
+; RUN: llc -filetype=obj %t.dir/bar.ll -o %t-bar.obj
+; RUN: llc -filetype=obj %t.dir/baz.ll -o %t-baz.obj
+; RUN: opt -thinlto-bc %t.dir/main2.ll -o %t-main2.bc
+; RUN: opt -thinlto-bc %t.dir/foo.ll -o %t-foo.bc
+; RUN: opt -thinlto-bc %t.dir/bar.ll -o %t-bar.bc
+; RUN: opt -thinlto-bc %t.dir/baz.ll -o %t-baz.bc
+
+; RUN: lld-link -out:%t2.exe -entry:main \
+; RUN:     %t-main2.obj %t-foo.obj %t-bar.obj %t-baz.obj
+; RUN: lld-link -out:%t2.exe -entry:main \
+; RUN:     %t-main2.obj /start-lib %t-foo.obj %t-bar.obj %t-baz.obj /end-lib
+; RUN: lld-link -out:%t2.exe -entry:main \
+; RUN:     /start-lib %t-foo.obj %t-bar.obj %t-baz.obj /end-lib %t-main2.obj
+
+; RUN: lld-link -out:%t2.exe -entry:main \
+; RUN:     %t-main2.bc %t-foo.bc %t-bar.bc %t-baz.bc
+; RUN: lld-link -out:%t2.exe -entry:main \
+; RUN:     %t-main2.bc /start-lib %t-foo.bc %t-bar.bc %t-baz.bc /end-lib
+; RUN: lld-link -out:%t2.exe -entry:main \
+; RUN:     /start-lib %t-foo.bc %t-bar.bc %t-baz.bc /end-lib %t-main2.bc
+
+#--- main2.ll
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+declare void @bar()
+
+define void @main() {
+  call void () @bar()
+  ret void
+}
+
+
+#--- foo.ll
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+define void @foo() {
+  ret void
+}
+
+
+#--- bar.ll
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+; One undef from the lazy obj file before it, one from the one after it.
+declare void @foo()
+declare void @baz()
+
+define void @bar() {
+  call void () @foo()
+  call void () @baz()
+  ret void
+}
+
+
+#--- baz.ll
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+define void @baz() {
+  ret void
+}

[github-actions]

✅ With the latest revision this PR passed the undef deprecator.

[nico]

It complains about undef in this comment:

; One undef from the lazy obj file before it, one from the one after it.

I suppose I'll make it say "undefined symbol", but that check seems a bit overeager