[PATCH] D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static

[Martin Storsjö via Phabricator via llvm-commits]

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

If we have references to the same extern_weak in multiple objects, all of them would generate external symbols with the same name. Make them static to avoid duplicate definitions; nothing should need to refer to this symbol outside of the current object.

GCC/binutils seems to handle the same by not using a fixed string for the ".default" suffix, but instead using the name of some other defined external symbol from the same object (which is supposed to be unique among objects unless there are other duplicate definitions).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71711

Files:
  llvm/lib/MC/WinCOFFObjectWriter.cpp
  llvm/test/MC/COFF/weak-alias-local.s
  llvm/test/MC/COFF/weak-val.s
  llvm/test/MC/COFF/weak.s


Index: llvm/test/MC/COFF/weak.s
===================================================================
--- llvm/test/MC/COFF/weak.s
+++ llvm/test/MC/COFF/weak.s
@@ -64,7 +64,7 @@
 // CHECK-NEXT:   Section:             IMAGE_SYM_ABSOLUTE (-1)
 // CHECK-NEXT:   BaseType:            Null
 // CHECK-NEXT:   ComplexType:         Null
-// CHECK-NEXT:   StorageClass:        External
+// CHECK-NEXT:   StorageClass:        Static
 // CHECK-NEXT:   AuxSymbolCount:      0
 // CHECK-NEXT: }
 
@@ -88,6 +88,6 @@
 // CHECK-NEXT:   Section: .text
 // CHECK-NEXT:   BaseType: Null
 // CHECK-NEXT:   ComplexType: Null
-// CHECK-NEXT:   StorageClass: External
+// CHECK-NEXT:   StorageClass: Static
 // CHECK-NEXT:   AuxSymbolCount: 0
 // CHECK-NEXT: }
Index: llvm/test/MC/COFF/weak-val.s
===================================================================
--- llvm/test/MC/COFF/weak-val.s
+++ llvm/test/MC/COFF/weak-val.s
@@ -28,6 +28,6 @@
 // CHECK-NEXT:   Section: .data (2)
 // CHECK-NEXT:   BaseType: Null (0x0)
 // CHECK-NEXT:   ComplexType: Null (0x0)
-// CHECK-NEXT:   StorageClass: External (0x2)
+// CHECK-NEXT:   StorageClass: Static (0x3)
 // CHECK-NEXT:   AuxSymbolCount: 0
 // CHECK-NEXT: }
Index: llvm/test/MC/COFF/weak-alias-local.s
===================================================================
--- llvm/test/MC/COFF/weak-alias-local.s
+++ llvm/test/MC/COFF/weak-alias-local.s
@@ -38,6 +38,6 @@
 // CHECK-NEXT:   Section: .data (2)
 // CHECK-NEXT:   BaseType: Null (0x0)
 // CHECK-NEXT:   ComplexType: Null (0x0)
-// CHECK-NEXT:   StorageClass: External (0x2)
+// CHECK-NEXT:   StorageClass: Static (0x3)
 // CHECK-NEXT:   AuxSymbolCount: 0
 // CHECK-NEXT: }
Index: llvm/lib/MC/WinCOFFObjectWriter.cpp
===================================================================
--- llvm/lib/MC/WinCOFFObjectWriter.cpp
+++ llvm/lib/MC/WinCOFFObjectWriter.cpp
@@ -411,6 +411,11 @@
       Local->Data.StorageClass = IsExternal ? COFF::IMAGE_SYM_CLASS_EXTERNAL
                                             : COFF::IMAGE_SYM_CLASS_STATIC;
     }
+    // If the original symbol was weak, and we created a new default
+    // symbol here, make it static, in order to not conflict with similar
+    // default symbols for the same weak in other translation units.
+    if (SymbolCOFF.isWeakExternal())
+      Local->Data.StorageClass = COFF::IMAGE_SYM_CLASS_STATIC;
   }
 
   Sym->MC = &MCSym;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71711.234733.patch
Type: text/x-patch
Size: 2383 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191219/0af11790/attachment.bin>