[COFF] Respect weak externals for mangled symbol searching

[smeenai]

Summary

We were previously ignoring weak externals during these searches (which
are used for the entry point, exports, and subsystem inference), which
differed from link.exe behavior. It also meant that we could get
different behavior when linking an object file directly vs. packaging it
into a static library, because static library symbol name directories
include weak externals.

[mstorsjo]

lld/COFF/SymbolTable.cpp

817

What this symbol is non-null but is Undefined (with itself possibly might be a weak symbol)? Do we need to make this a loop which inspects that aspect? Or is it not relevant for the usecases of findMangle where it is enough to see that we apparently did hit a real symbol and don’t need to check for prefixes?

[smeenai]

lld/COFF/SymbolTable.cpp

817

getWeakAlias does that loop: https://github.com/llvm/llvm-project/blob/bbcffb08f0fdc0be8c8cba48410f9cb556ea661d/lld/COFF/Symbols.cpp#L123-L129

[mstorsjo]

This looks reasonable to me!

lld/COFF/SymbolTable.cpp

817

Ah, thanks!

lld/COFF/SymbolTable.cpp

if (name.startswith(prefix) || name.startswith(prefix.drop_front()) ||
         name.drop_front().startswith(prefix.drop_front())) {
       syms.push_back(pair.second);
     }
   }
   return syms;
 }
 
 Symbol *SymbolTable::findMangle(StringRef name) {
-  if (Symbol *sym = find(name))
-    if (!isa<Undefined>(sym))
+  if (Symbol *sym = find(name)) {
+    if (auto *u = dyn_cast<Undefined>(sym)) {
+      // We're specifically looking for weak aliases that ultimately resolve to
+      // defined symbols, hence the call to getWeakAlias() instead of just using
+      // the weakAlias member variable. This matches link.exe's behavior.
+      if (Symbol *weakAlias = u->getWeakAlias())
+        return weakAlias;
+    } else {
       return sym;
+    }
+  }
 
   // Efficient fuzzy string lookup is impossible with a hash table, so iterate
   // the symbol table once and collect all possibly matching symbols into this
   // vector. Then compare each possibly matching symbol with each possible
   // mangling.
   std::vector<Symbol *> syms = getSymsWithPrefix(name);
   auto findByPrefix = [&syms](const Twine &t) -> Symbol * {
     std::string prefix = t.str();

lld/test/COFF/entry-weak-external.s

+# REQUIRES: x86
+# RUN: rm -rf %t && split-file %s %t
+
+## Ensure that we resolve the entry point to the unmangled weak alias instead of
+## the mangled library definition (which would fail with an undefined symbol).
+
+# RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t/entry-weak.obj %t/entry-weak.s
+# RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t/entry-mangled.obj %t/entry-mangled.s
+# RUN: llvm-lib -out:%t/entry-mangled.lib %t/entry-mangled.obj
+# RUN: lld-link -subsystem:console -entry:entry -out:%t/entry-weak-external.exe %t/entry-weak.obj %t/entry-mangled.lib
+# RUN: llvm-readobj --file-headers %t/entry-weak-external.exe | FileCheck %s
+
+## Ensure that we don't resolve the entry point to a weak alias pointing to an
+## undefined symbol (which would have caused the entry point to be 0 instead of
+## an actual address). I can't think of a way of triggering this edge case
+## without using /force:unresolved, which means it likely doesn't matter in
+## practice, but we still match link.exe's behavior for it.
+
+# RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t/entry-weak-undefined.obj %t/entry-weak-undefined.s
+# RUN: lld-link -subsystem:console -entry:entry -force:unresolved -out:%t/entry-undefined-weak-external.exe \
+# RUN:   %t/entry-weak-undefined.obj %t/entry-mangled.lib
+# RUN: llvm-readobj --file-headers %t/entry-undefined-weak-external.exe | FileCheck %s
+
+# CHECK: AddressOfEntryPoint: 0x1000
+
+#--- entry-weak.s
+.globl default_entry
+default_entry:
+  ret
+
+.weak entry
+entry = default_entry
+
+#--- entry-mangled.s
+.globl "?entry@@YAHXZ"
+"?entry@@YAHXZ":
+  jmp does_not_exist
+
+#--- entry-weak-undefined.s
+.weak entry
+entry = does_not_exist

lld/test/COFF/subsystem-inference-weak-external.s

+# REQUIRES: x86
+# RUN: rm -rf %t && split-file %s %t
+
+## Ensure that weak externals are considered during subsystem inference.
+
+# RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t/cui.obj %t/cui.s
+# RUN: lld-link -out:%t/cui.exe %t/cui.obj
+# RUN: llvm-readobj --file-headers %t/cui.exe | FileCheck --check-prefix=CUI %s
+# RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t/gui.obj %t/gui.s
+# RUN: lld-link -out:%t/gui.exe %t/gui.obj
+# RUN: llvm-readobj --file-headers %t/gui.exe | FileCheck --check-prefix=GUI %s
+
+# CUI:     Subsystem: IMAGE_SUBSYSTEM_WINDOWS_CUI
+# GUI:     Subsystem: IMAGE_SUBSYSTEM_WINDOWS_GUI
+
+#--- cui.s
+.global default_main
+default_main:
+  ret
+
+.weak main
+main = default_main
+
+.global mainCRTStartup
+mainCRTStartup:
+  ret
+
+#--- gui.s
+.global default_WinMain
+default_WinMain:
+  ret
+
+.weak WinMain
+WinMain = default_WinMain
+
+.global WinMainCRTStartup
+WinMainCRTStartup:
+  ret