[llvm] 8f540da - [COFF] Assign unique names to autogenerated .weak.<name>.default symbols
Martin Storsjö via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 13 13:45:32 PDT 2020
Author: Martin Storsjö
Date: 2020-03-13T22:44:55+02:00
New Revision: 8f540dad6120d00e3ad896b98cd32bcf00623ccd
URL: https://github.com/llvm/llvm-project/commit/8f540dad6120d00e3ad896b98cd32bcf00623ccd
DIFF: https://github.com/llvm/llvm-project/commit/8f540dad6120d00e3ad896b98cd32bcf00623ccd.diff
LOG: [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 bc5b7217dceecd3eec69593026a9e38dfbfd6908,
and this was originally explicitly fixed in
732eeaf2a930ad2755cb4eb5d99a3deae0de4a72).
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
Added:
llvm/test/MC/COFF/weak-name.s
Modified:
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
Removed:
################################################################################
diff --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp
index cf755dcb2884..6a0a11e2d637 100644
--- a/llvm/lib/MC/WinCOFFObjectWriter.cpp
+++ b/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 @@ class WinCOFFObjectWriter : public MCObjectWriter {
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 @@ class WinCOFFObjectWriter : public MCObjectWriter {
section_map SectionMap;
symbol_map SymbolMap;
+ symbol_list WeakDefaults;
+
bool UseBigObj;
bool EmitAddrsigSection = false;
@@ -205,6 +210,7 @@ class WinCOFFObjectWriter : public MCObjectWriter {
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);
diff --git a/llvm/test/MC/COFF/weak-alias-local.s b/llvm/test/MC/COFF/weak-alias-local.s
index 93a3652428ee..4d0edb51f907 100644
--- a/llvm/test/MC/COFF/weak-alias-local.s
+++ b/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)
diff --git a/llvm/test/MC/COFF/weak-name.s b/llvm/test/MC/COFF/weak-name.s
new file mode 100644
index 000000000000..653232b2d4c2
--- /dev/null
+++ b/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
diff --git a/llvm/test/MC/COFF/weak-val.s b/llvm/test/MC/COFF/weak-val.s
index cb5872b057eb..e0d5592a847f 100644
--- a/llvm/test/MC/COFF/weak-val.s
+++ b/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)
diff --git a/llvm/test/MC/COFF/weak.s b/llvm/test/MC/COFF/weak.s
index 5e216cef2cbd..6eefdd8dcd19 100644
--- a/llvm/test/MC/COFF/weak.s
+++ b/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
More information about the llvm-commits
mailing list