[LLD][COFF] Preserve original symbol name when resolving weak aliases.

[cjacek]

Instead of replacing it with target's name.

[cjacek]

I noticed it while looking at corner cases of ARM64EC patch that I plan to send, but it's observable in other cases, like the test from this commit. Without the patch, we'd emit "main" symbol twice. GNU ld in this case preserves the name and even the fact that it's a weak symbol (which we can't do with current architecture), but it gets the target of the symbol wrong, so I don't expect anything to depend on that.

[llvmbot]

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Jacek Caban (cjacek)

Changes

Instead of replacing it with target's name.

---

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

4 Files Affected:

• (modified) lld/COFF/SymbolTable.cpp (+1-13)
• (modified) lld/COFF/Symbols.cpp (+23)
• (modified) lld/COFF/Symbols.h (+3)
• (modified) lld/test/COFF/symtab.test (+18)

diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 1dfff0a90f4aee..a5f155bc05bc9e 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -494,20 +494,8 @@ void SymbolTable::resolveRemainingUndefines() {
     StringRef name = undef->getName();
 
     // A weak alias may have been resolved, so check for that.
-    if (Defined *d = undef->getWeakAlias()) {
-      // We want to replace Sym with D. However, we can't just blindly
-      // copy sizeof(SymbolUnion) bytes from D to Sym because D may be an
-      // internal symbol, and internal symbols are stored as "unparented"
-      // Symbols. For that reason we need to check which type of symbol we
-      // are dealing with and copy the correct number of bytes.
-      if (isa<DefinedRegular>(d))
-        memcpy(sym, d, sizeof(DefinedRegular));
-      else if (isa<DefinedAbsolute>(d))
-        memcpy(sym, d, sizeof(DefinedAbsolute));
-      else
-        memcpy(sym, d, sizeof(SymbolUnion));
+    if (undef->resolveWeakAlias())
       continue;
-    }
 
     // If we can resolve a symbol by removing __imp_ prefix, do that.
     // This odd rule is for compatibility with MSVC linker.
diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp
index ff8ad1e619116f..b098abb80d6f1e 100644
--- a/lld/COFF/Symbols.cpp
+++ b/lld/COFF/Symbols.cpp
@@ -136,6 +136,29 @@ Defined *Undefined::getWeakAlias() {
   return nullptr;
 }
 
+bool Undefined::resolveWeakAlias() {
+  Defined *d = getWeakAlias();
+  if (!d)
+    return false;
+
+  // We want to replace Sym with D. However, we can't just blindly
+  // copy sizeof(SymbolUnion) bytes from D to Sym because D may be an
+  // internal symbol, and internal symbols are stored as "unparented"
+  // Symbols. For that reason we need to check which type of symbol we
+  // are dealing with and copy the correct number of bytes.
+  StringRef name = getName();
+  if (isa<DefinedRegular>(d))
+    memcpy(this, d, sizeof(DefinedRegular));
+  else if (isa<DefinedAbsolute>(d))
+    memcpy(this, d, sizeof(DefinedAbsolute));
+  else
+    memcpy(this, d, sizeof(SymbolUnion));
+
+  nameData = name.data();
+  nameSize = name.size();
+  return true;
+}
+
 MemoryBufferRef LazyArchive::getMemberBuffer() {
   Archive::Child c =
       CHECK(sym.getMember(), "could not get the member for symbol " +
diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h
index 56b137d56873aa..c427a062dc82b2 100644
--- a/lld/COFF/Symbols.h
+++ b/lld/COFF/Symbols.h
@@ -341,6 +341,9 @@ class Undefined : public Symbol {
   // symbol by searching the chain of fallback symbols. Returns the symbol if
   // successful, otherwise returns null.
   Defined *getWeakAlias();
+
+  // If this symbol is external weak, replace this object with aliased symbol.
+  bool resolveWeakAlias();
 };
 
 // Windows-specific classes.
diff --git a/lld/test/COFF/symtab.test b/lld/test/COFF/symtab.test
index 45e8ed39737a46..6ef2b4d47503c7 100644
--- a/lld/test/COFF/symtab.test
+++ b/lld/test/COFF/symtab.test
@@ -86,6 +86,15 @@
 # CHECK-NEXT:     StorageClass: External (0x2)
 # CHECK-NEXT:     AuxSymbolCount: 0
 # CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: weak_main
+# CHECK-NEXT:     Value: 0
+# CHECK-NEXT:     Section: .text (1)
+# CHECK-NEXT:     BaseType: Null (0x0)
+# CHECK-NEXT:     ComplexType: Null (0x0)
+# CHECK-NEXT:     StorageClass: External (0x2)
+# CHECK-NEXT:     AuxSymbolCount: 0
+# CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
 # NO:       Symbols [
@@ -237,4 +246,13 @@ symbols:
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_NULL
     StorageClass:    IMAGE_SYM_CLASS_LABEL
+  - Name:            weak_main
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_WEAK_EXTERNAL
+    WeakExternal:
+      TagIndex:        10
+      Characteristics: IMAGE_WEAK_EXTERN_SEARCH_ALIAS
 ...

[mstorsjo]

LGTM