[PATCH] D84507: [MC] [COFF] Make sure that weak external symbols are undefined symbols

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added reviewers: hans, smeenai, amccarth.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

For comdats (e.g. caused by -ffunction-sections), Section is already set here; make sure it's null, for the weak external symbol to be undefined.

This fixes PR46779.

This setup did reveal another issue wrt mingw style associative comdats in combination with weak symbols, when the actual leader symbol isn't named `func` but `.weak.func.default` in these cases. I'm looking at ways of fixing that as well (as a general headsup re potentially getting the fix for that into the release as well).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84507

Files:
  llvm/lib/MC/WinCOFFObjectWriter.cpp
  llvm/test/MC/COFF/weak-comdat.s


Index: llvm/test/MC/COFF/weak-comdat.s
===================================================================
--- /dev/null
+++ llvm/test/MC/COFF/weak-comdat.s
@@ -0,0 +1,34 @@
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-win32 %s -o %t.o
+// RUN: llvm-readobj --symbols %t.o | FileCheck %s
+
+// Test that the weak symbol is properly undefined, while originally being
+// the leader symbol for a comdat. (This can easily happen if building with
+// -ffunction-sections).
+
+        .section .text$func,"xr",one_only,func
+        .weak   func
+func:
+        ret
+
+// CHECK:       Symbol {
+// CHECK:         Name: func
+// CHECK-NEXT:    Value: 0
+// CHECK-NEXT:    Section: IMAGE_SYM_UNDEFINED (0)
+// CHECK-NEXT:    BaseType: Null (0x0)
+// CHECK-NEXT:    ComplexType: Null (0x0)
+// CHECK-NEXT:    StorageClass: WeakExternal (0x69)
+// CHECK-NEXT:    AuxSymbolCount: 1
+// CHECK-NEXT:    AuxWeakExternal {
+// CHECK-NEXT:      Linked: .weak.func.default (10)
+// CHECK-NEXT:      Search: Alias (0x3)
+// CHECK-NEXT:    }
+// CHECK-NEXT:  }
+// CHECK-NEXT:  Symbol {
+// CHECK-NEXT:    Name: .weak.func.default
+// CHECK-NEXT:    Value: 0
+// CHECK-NEXT:    Section: .text$func (4)
+// CHECK-NEXT:    BaseType: Null (0x0)
+// CHECK-NEXT:    ComplexType: Null (0x0)
+// CHECK-NEXT:    StorageClass: External (0x2)
+// CHECK-NEXT:    AuxSymbolCount: 0
+// CHECK-NEXT:  }
Index: llvm/lib/MC/WinCOFFObjectWriter.cpp
===================================================================
--- llvm/lib/MC/WinCOFFObjectWriter.cpp
+++ llvm/lib/MC/WinCOFFObjectWriter.cpp
@@ -375,6 +375,7 @@
   COFFSymbol *Local = nullptr;
   if (cast<MCSymbolCOFF>(MCSym).isWeakExternal()) {
     Sym->Data.StorageClass = COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL;
+    Sym->Section = nullptr;
 
     COFFSymbol *WeakDefault = getLinkedSymbol(MCSym);
     if (!WeakDefault) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84507.280387.patch
Type: text/x-patch
Size: 1849 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200724/39db695b/attachment.bin>