[LLD][COFF] Handle imported weak aliases consistently

[glandium]

symTab being a DenseMap, the order in which a symbol and its corresponding import symbol are processed is not guaranteed, and when the latter comes first, it is left undefined.

[llvmbot]

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

@llvm/pr-subscribers-lld

Author: Mike Hommey (glandium)

Changes

symTab being a DenseMap, the order in which a symbol and its corresponding import symbol are processed is not guaranteed, and when the latter comes first, it is left undefined.

---

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

2 Files Affected:

• (modified) lld/COFF/SymbolTable.cpp (+8)
• (added) lld/test/COFF/import_weak_alias.test (+25)

diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index efea16ccbbfea0..6b96d8bbda789c 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -502,6 +502,14 @@ void SymbolTable::resolveRemainingUndefines() {
     // This odd rule is for compatibility with MSVC linker.
     if (name.starts_with("__imp_")) {
       Symbol *imp = find(name.substr(strlen("__imp_")));
+      if (imp) {
+         // The unprefixed symbol might come later in symMap, so handle it now
+	 // so that the condition below can be appropriately applied.
+         auto *undef = dyn_cast<Undefined>(imp);
+         if (undef) {
+            undef->resolveWeakAlias();
+         }
+      }
       if (imp && isa<Defined>(imp)) {
         auto *d = cast<Defined>(imp);
         replaceSymbol<DefinedLocalImport>(sym, ctx, name, d);
diff --git a/lld/test/COFF/import_weak_alias.test b/lld/test/COFF/import_weak_alias.test
new file mode 100644
index 00000000000000..63df565d5dbc9d
--- /dev/null
+++ b/lld/test/COFF/import_weak_alias.test
@@ -0,0 +1,25 @@
+# REQUIRES: x86
+
+# RUN: split-file %s %t.dir
+# RUN: llc --filetype=obj -o %t.foo.obj %t.dir/foo.ll
+# RUN: llvm-mc --filetype=obj -triple=x86_64-windows-msvc %t.dir/qux.s -o %t.qux.obj
+# RUN: lld-link %t.qux.obj %t.foo.obj -out:%t.dll -dll 2>&1 | FileCheck %s
+#
+# CHECK-NOT: lld-link: error: undefined symbol: __declspec(dllimport) foo
+
+#--- foo.ll
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+@foo = weak alias void(), ptr @bar
+
+define void @bar() {
+  ret void
+}
+
+#--- qux.s
+.text
+.global _DllMainCRTStartup
+_DllMainCRTStartup:
+  call __imp_foo
+  ret

[glandium]

I verified that I can (sometimes) get an error with the included testcase on current master, and that the fix makes that go away.

Cc: @mstorsjo

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[mstorsjo]

The change looks good to me overall, thanks! A couple minor comments/discussion on the testcase.

[glandium]

If you're satisfied with the latest version, could you merge it for me?

[mstorsjo]

LGTM, thanks for the updates!

[mstorsjo]

I'll go ahead and merge this right away, as there's another patch in the queue touching the same areas, to simplify conflicts in that area. (Otherwise I would maybe have left the patch for 24h, for people in other time zones to comment on. But this change feels mostly straightforward to me.)