[COFF] Assign unique names to autogenerated .weak.<name>.default symbols

These symbols need to be external (MSVC tools error out if a weak
external points at a symbol that isn't external; this was tried before
but had to be reverted in bc5b7217dc,
and this was originally explicitly fixed in
732eeaf2a9).

If multiple object files have weak symbols with defaults, their
defaults could cause linker errors due to duplicate definitions,
unless the names of the defaults are unique.

GNU binutils handles this by appending the name of another symbol
from the same object file to the name of the default symbol. Try
to implement something similar; before writing the object file,
locate a symbol that should have a unique name and use the name of
that one for making the weak defaults unique.

Differential Revision: https://reviews.llvm.org/D75989

llvm/lib/MC/WinCOFFObjectWriter.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -131,6 +132,8 @@ public:
   using symbol_map = DenseMap<MCSymbol const *, COFFSymbol *>;
   using section_map = DenseMap<MCSection const *, COFFSection *>;
 
+  using symbol_list = DenseSet<COFFSymbol *>;
+
   std::unique_ptr<MCWinCOFFObjectTargetWriter> TargetObjectWriter;
 
   // Root level file contents.
@@ -143,6 +146,8 @@ public:
   section_map SectionMap;
   symbol_map SymbolMap;
 
+  symbol_list WeakDefaults;
+
   bool UseBigObj;
 
   bool EmitAddrsigSection = false;
@@ -205,6 +210,7 @@ public:
                         MCValue Target, uint64_t &FixedValue) override;
 
   void createFileSymbols(MCAssembler &Asm);
+  void setWeakDefaultNames();
   void assignSectionNumbers();
   void assignFileOffsets(MCAssembler &Asm, const MCAsmLayout &Layout);
 
@@ -376,6 +382,7 @@ void WinCOFFObjectWriter::DefineSymbol(const MCSymbol &MCSym,
         WeakDefault->Data.SectionNumber = COFF::IMAGE_SYM_ABSOLUTE;
       else
         WeakDefault->Section = Sec;
+      WeakDefaults.insert(WeakDefault);
       Local = WeakDefault;
     }
 
@@ -863,6 +870,47 @@ void WinCOFFObjectWriter::createFileSymbols(MCAssembler &Asm) {
   }
 }
 
+void WinCOFFObjectWriter::setWeakDefaultNames() {
+  if (WeakDefaults.empty())
+    return;
+
+  // If multiple object files use a weak symbol (either with a regular
+  // defined default, or an absolute zero symbol as default), the defaults
+  // cause duplicate definitions unless their names are made unique. Look
+  // for a defined extern symbol, that isn't comdat - that should be unique
+  // unless there are other duplicate definitions. And if none is found,
+  // allow picking a comdat symbol, as that's still better than nothing.
+
+  COFFSymbol *Unique = nullptr;
+  for (bool AllowComdat : {false, true}) {
+    for (auto &Sym : Symbols) {
+      // Don't include the names of the defaults themselves
+      if (WeakDefaults.count(Sym.get()))
+        continue;
+      // Only consider external symbols
+      if (Sym->Data.StorageClass != COFF::IMAGE_SYM_CLASS_EXTERNAL)
+        continue;
+      // Only consider symbols defined in a section or that are absolute
+      if (!Sym->Section && Sym->Data.SectionNumber != COFF::IMAGE_SYM_ABSOLUTE)
+        continue;
+      if (!AllowComdat && Sym->Section &&
+          Sym->Section->Header.Characteristics & COFF::IMAGE_SCN_LNK_COMDAT)
+        continue;
+      Unique = Sym.get();
+      break;
+    }
+    if (Unique)
+      break;
+  }
+  // If we didn't find any unique symbol to use for the names, just skip this.
+  if (!Unique)
+    return;
+  for (auto *Sym : WeakDefaults) {
+    Sym->Name.append(".");
+    Sym->Name.append(Unique->Name);
+  }
+}
+
 static bool isAssociative(const COFFSection &Section) {
   return Section.Symbol->Aux[0].Aux.SectionDefinition.Selection ==
          COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE;
@@ -961,6 +1009,7 @@ uint64_t WinCOFFObjectWriter::writeObject(MCAssembler &Asm,
   Header.NumberOfSections = Sections.size();
   Header.NumberOfSymbols = 0;
 
+  setWeakDefaultNames();
   assignSectionNumbers();
   createFileSymbols(Asm);
 

llvm/test/MC/COFF/weak-alias-local.s

@@ -33,7 +33,7 @@ a=b
 // CHECK-NEXT:   }
 // CHECK-NEXT: }
 // CHECK-NEXT: Symbol {
-// CHECK-NEXT:   Name: .weak.a.default
+// CHECK-NEXT:   Name: .weak.a.default{{$}}
 // CHECK-NEXT:   Value: 4
 // CHECK-NEXT:   Section: .data (2)
 // CHECK-NEXT:   BaseType: Null (0x0)

llvm/test/MC/COFF/weak-name.s

@@ -0,0 +1,35 @@
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-win32 %s -defsym case=0 -o %t.case0.o
+// RUN: llvm-readobj --symbols %t.case0.o | FileCheck %s --check-prefix=CHECK-CASE0
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-win32 %s -defsym case=1 -o %t.case1.o
+// RUN: llvm-readobj --symbols %t.case1.o | FileCheck %s --check-prefix=CHECK-CASE1
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-win32 %s -defsym case=2 -o %t.case2.o
+// RUN: llvm-readobj --symbols %t.case2.o | FileCheck %s --check-prefix=CHECK-CASE2
+
+// Test that we prefer a non-comdat symbol for naming weak default symbols,
+// if such a symbol is available.
+
+        .section .text$comdat1,"xr",discard,comdat1
+        .globl   comdat1
+comdat1:
+        call     undeffunc
+
+        .weak    weaksym
+
+        .section .text$comdat2,"xr",discard,comdat2
+        .globl   comdat2
+comdat2:
+        call     undeffunc2
+
+.if case == 0
+        .text
+        .globl   regular
+regular:
+        call     undeffunc3
+.elseif case == 1
+        .globl   abssym
+abssym = 42
+.endif
+
+// CHECK-CASE0: Name: .weak.weaksym.default.regular
+// CHECK-CASE1: Name: .weak.weaksym.default.abssym
+// CHECK-CASE2: Name: .weak.weaksym.default.comdat1

llvm/test/MC/COFF/weak-val.s

@@ -23,7 +23,7 @@ b:
 // CHECK-NEXT:   }
 // CHECK-NEXT: }
 // CHECK-NEXT: Symbol {
-// CHECK-NEXT:   Name: .weak.b.default
+// CHECK-NEXT:   Name: .weak.b.default{{$}}
 // CHECK-NEXT:   Value: 4
 // CHECK-NEXT:   Section: .data (2)
 // CHECK-NEXT:   BaseType: Null (0x0)

llvm/test/MC/COFF/weak.s

@@ -59,7 +59,7 @@ LBB0_2:                                 # %return
 // CHECK-NEXT: }
 
 // CHECK:      Symbol {
-// CHECK:        Name:                .weak._test_weak.default
+// CHECK:        Name:                .weak._test_weak.default._main
 // CHECK-NEXT:   Value:               0
 // CHECK-NEXT:   Section:             IMAGE_SYM_ABSOLUTE (-1)
 // CHECK-NEXT:   BaseType:            Null
@@ -83,7 +83,7 @@ LBB0_2:                                 # %return
 // CHECK-NEXT: }
 
 // CHECK:      Symbol {
-// CHECK:        Name: .weak._test_weak_alias.default
+// CHECK:        Name: .weak._test_weak_alias.default._main
 // CHECK-NEXT:   Value: 0
 // CHECK-NEXT:   Section: .text
 // CHECK-NEXT:   BaseType: Null