[llvm] 04c2785 - [MC,COFF] Change how we handle section symbols
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 25 14:00:51 PDT 2024
Author: Fangrui Song
Date: 2024-06-25T14:00:47-07:00
New Revision: 04c27852e47093f7efa18609dbf57b3ce58a3ffa
URL: https://github.com/llvm/llvm-project/commit/04c27852e47093f7efa18609dbf57b3ce58a3ffa
DIFF: https://github.com/llvm/llvm-project/commit/04c27852e47093f7efa18609dbf57b3ce58a3ffa.diff
LOG: [MC,COFF] Change how we handle section symbols
13a79bbfe583e1d8cc85d241b580907260065eb8 (2017) unified `BeginSymbol` and
section symbol for ELF. This patch does the same for COFF.
* In getCOFFSection, all sections now have a `BeginSymbol` (section
symbol). We do not need a dummy symbol name when `getBeginSymbol` is
needed (used by AsmParser::Run and DWARF generation).
* Section symbols are in the global symbol table. `call .text` will
reference the section symbol instead of an undefined symbol. This
matches GNU assembler. Unlike GNU, redefining the section symbol will
cause a "symbol 'foo0' is already defined" error (see
`section-sym-err.s`).
Pull Request: https://github.com/llvm/llvm-project/pull/96459
Added:
llvm/test/MC/COFF/section-sym-err.s
Modified:
llvm/include/llvm/MC/MCContext.h
llvm/include/llvm/MC/MCWinCOFFStreamer.h
llvm/lib/MC/MCContext.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/lib/MC/MCWinCOFFStreamer.cpp
llvm/lib/MC/WinCOFFObjectWriter.cpp
llvm/test/DebugInfo/X86/InlinedFnLocalVar.ll
llvm/test/DebugInfo/X86/ref_addr_relocation.ll
llvm/test/ExecutionEngine/RuntimeDyld/X86/COFF_x86_64.s
llvm/test/MC/COFF/section-comdat-conflict.s
llvm/test/MC/COFF/section-comdat.s
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h
index 69a20587eecfb..822a062f56d37 100644
--- a/llvm/include/llvm/MC/MCContext.h
+++ b/llvm/include/llvm/MC/MCContext.h
@@ -351,6 +351,9 @@ class MCContext {
MCSymbol *getOrCreateDirectionalLocalSymbol(unsigned LocalLabelVal,
unsigned Instance);
+ template <typename Symbol>
+ Symbol *getOrCreateSectionSymbol(StringRef Section);
+
MCSectionELF *createELFSectionImpl(StringRef Section, unsigned Type,
unsigned Flags, unsigned EntrySize,
const MCSymbolELF *Group, bool IsComdat,
@@ -604,11 +607,9 @@ class MCContext {
MCSectionCOFF *getCOFFSection(StringRef Section, unsigned Characteristics,
StringRef COMDATSymName, int Selection,
- unsigned UniqueID = GenericSectionID,
- const char *BeginSymName = nullptr);
+ unsigned UniqueID = GenericSectionID);
- MCSectionCOFF *getCOFFSection(StringRef Section, unsigned Characteristics,
- const char *BeginSymName = nullptr);
+ MCSectionCOFF *getCOFFSection(StringRef Section, unsigned Characteristics);
/// Gets or creates a section equivalent to Sec that is associated with the
/// section containing KeySym. For example, to create a debug info section
diff --git a/llvm/include/llvm/MC/MCWinCOFFStreamer.h b/llvm/include/llvm/MC/MCWinCOFFStreamer.h
index 0d346871df821..893a70f74ee3b 100644
--- a/llvm/include/llvm/MC/MCWinCOFFStreamer.h
+++ b/llvm/include/llvm/MC/MCWinCOFFStreamer.h
@@ -40,6 +40,7 @@ class MCWinCOFFStreamer : public MCObjectStreamer {
/// \{
void initSections(bool NoExecStack, const MCSubtargetInfo &STI) override;
+ void changeSection(MCSection *Section, uint32_t Subsection = 0) override;
void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
void emitAssemblerFlag(MCAssemblerFlag Flag) override;
void emitThumbFunc(MCSymbol *Func) override;
diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index 36603d06c2a29..26a6fac927d49 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -383,6 +383,27 @@ MCSymbol *MCContext::getDirectionalLocalSymbol(unsigned LocalLabelVal,
return getOrCreateDirectionalLocalSymbol(LocalLabelVal, Instance);
}
+template <typename Symbol>
+Symbol *MCContext::getOrCreateSectionSymbol(StringRef Section) {
+ Symbol *R;
+ auto &SymEntry = getSymbolTableEntry(Section);
+ MCSymbol *Sym = SymEntry.second.Symbol;
+ // A section symbol can not redefine regular symbols. There may be multiple
+ // sections with the same name, in which case the first such section wins.
+ if (Sym && Sym->isDefined() &&
+ (!Sym->isInSection() || Sym->getSection().getBeginSymbol() != Sym))
+ reportError(SMLoc(), "invalid symbol redefinition");
+ if (Sym && Sym->isUndefined()) {
+ R = cast<Symbol>(Sym);
+ } else {
+ SymEntry.second.Used = true;
+ R = new (&SymEntry, *this) Symbol(&SymEntry, /*isTemporary=*/false);
+ if (!Sym)
+ SymEntry.second.Symbol = R;
+ }
+ return R;
+}
+
MCSymbol *MCContext::lookupSymbol(const Twine &Name) const {
SmallString<128> NameSV;
StringRef NameRef = Name.toStringRef(NameSV);
@@ -497,22 +518,7 @@ MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type,
const MCSymbolELF *Group,
bool Comdat, unsigned UniqueID,
const MCSymbolELF *LinkedToSym) {
- MCSymbolELF *R;
- MCSymbolTableEntry &SymEntry = getSymbolTableEntry(Section);
- MCSymbol *Sym = SymEntry.second.Symbol;
- // A section symbol can not redefine regular symbols. There may be multiple
- // sections with the same name, in which case the first such section wins.
- if (Sym && Sym->isDefined() &&
- (!Sym->isInSection() || Sym->getSection().getBeginSymbol() != Sym))
- reportError(SMLoc(), "invalid symbol redefinition");
- if (Sym && Sym->isUndefined()) {
- R = cast<MCSymbolELF>(Sym);
- } else {
- SymEntry.second.Used = true;
- R = new (&SymEntry, *this) MCSymbolELF(&SymEntry, /*isTemporary*/ false);
- if (!Sym)
- SymEntry.second.Symbol = R;
- }
+ auto *R = getOrCreateSectionSymbol<MCSymbolELF>(Section);
R->setBinding(ELF::STB_LOCAL);
R->setType(ELF::STT_SECTION);
@@ -681,12 +687,19 @@ MCSectionGOFF *MCContext::getGOFFSection(StringRef Section, SectionKind Kind,
MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
unsigned Characteristics,
StringRef COMDATSymName, int Selection,
- unsigned UniqueID,
- const char *BeginSymName) {
+ unsigned UniqueID) {
MCSymbol *COMDATSymbol = nullptr;
if (!COMDATSymName.empty()) {
COMDATSymbol = getOrCreateSymbol(COMDATSymName);
COMDATSymName = COMDATSymbol->getName();
+ // A non-associative COMDAT is considered to define the COMDAT symbol. Check
+ // the redefinition error.
+ if (Selection != COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE && COMDATSymbol &&
+ COMDATSymbol->isDefined() &&
+ (!COMDATSymbol->isInSection() ||
+ cast<MCSectionCOFF>(COMDATSymbol->getSection()).getCOMDATSymbol() !=
+ COMDATSymbol))
+ reportError(SMLoc(), "invalid symbol redefinition");
}
// Do the lookup, if we have a hit, return it.
@@ -696,23 +709,19 @@ MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
if (!IterBool.second)
return Iter->second;
- MCSymbol *Begin = nullptr;
- if (BeginSymName)
- Begin = createTempSymbol(BeginSymName, false);
-
StringRef CachedName = Iter->first.SectionName;
+ MCSymbol *Begin = getOrCreateSectionSymbol<MCSymbolCOFF>(Section);
MCSectionCOFF *Result = new (COFFAllocator.Allocate()) MCSectionCOFF(
CachedName, Characteristics, COMDATSymbol, Selection, Begin);
Iter->second = Result;
- allocInitialFragment(*Result);
+ auto *F = allocInitialFragment(*Result);
+ Begin->setFragment(F);
return Result;
}
MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
- unsigned Characteristics,
- const char *BeginSymName) {
- return getCOFFSection(Section, Characteristics, "", 0, GenericSectionID,
- BeginSymName);
+ unsigned Characteristics) {
+ return getCOFFSection(Section, Characteristics, "", 0, GenericSectionID);
}
MCSectionCOFF *MCContext::getAssociativeCOFFSection(MCSectionCOFF *Sec,
diff --git a/llvm/lib/MC/MCObjectFileInfo.cpp b/llvm/lib/MC/MCObjectFileInfo.cpp
index daa996311d1cf..7a2b43a954b10 100644
--- a/llvm/lib/MC/MCObjectFileInfo.cpp
+++ b/llvm/lib/MC/MCObjectFileInfo.cpp
@@ -611,24 +611,21 @@ void MCObjectFileInfo::initCOFFMCObjectFileInfo(const Triple &T) {
COFF::IMAGE_SCN_MEM_READ));
DwarfAbbrevSection = Ctx->getCOFFSection(
- ".debug_abbrev",
- COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
- COFF::IMAGE_SCN_MEM_READ,
- "section_abbrev");
+ ".debug_abbrev", COFF::IMAGE_SCN_MEM_DISCARDABLE |
+ COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+ COFF::IMAGE_SCN_MEM_READ);
DwarfInfoSection = Ctx->getCOFFSection(
- ".debug_info",
- COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
- COFF::IMAGE_SCN_MEM_READ,
- "section_info");
+ ".debug_info", COFF::IMAGE_SCN_MEM_DISCARDABLE |
+ COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+ COFF::IMAGE_SCN_MEM_READ);
DwarfLineSection = Ctx->getCOFFSection(
".debug_line", COFF::IMAGE_SCN_MEM_DISCARDABLE |
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
COFF::IMAGE_SCN_MEM_READ);
DwarfLineStrSection = Ctx->getCOFFSection(
- ".debug_line_str",
- COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
- COFF::IMAGE_SCN_MEM_READ,
- "section_line_str");
+ ".debug_line_str", COFF::IMAGE_SCN_MEM_DISCARDABLE |
+ COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+ COFF::IMAGE_SCN_MEM_READ);
DwarfFrameSection = Ctx->getCOFFSection(
".debug_frame", COFF::IMAGE_SCN_MEM_DISCARDABLE |
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
@@ -738,19 +735,17 @@ void MCObjectFileInfo::initCOFFMCObjectFileInfo(const Triple &T) {
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
COFF::IMAGE_SCN_MEM_READ);
DwarfAccelNamesSection = Ctx->getCOFFSection(
- ".apple_names",
- COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
- COFF::IMAGE_SCN_MEM_READ,
- "names_begin");
+ ".apple_names", COFF::IMAGE_SCN_MEM_DISCARDABLE |
+ COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+ COFF::IMAGE_SCN_MEM_READ);
DwarfAccelNamespaceSection = Ctx->getCOFFSection(
".apple_namespaces", COFF::IMAGE_SCN_MEM_DISCARDABLE |
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
COFF::IMAGE_SCN_MEM_READ);
DwarfAccelTypesSection = Ctx->getCOFFSection(
- ".apple_types",
- COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
- COFF::IMAGE_SCN_MEM_READ,
- "types_begin");
+ ".apple_types", COFF::IMAGE_SCN_MEM_DISCARDABLE |
+ COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+ COFF::IMAGE_SCN_MEM_READ);
DwarfAccelObjCSection = Ctx->getCOFFSection(
".apple_objc", COFF::IMAGE_SCN_MEM_DISCARDABLE |
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
diff --git a/llvm/lib/MC/MCWinCOFFStreamer.cpp b/llvm/lib/MC/MCWinCOFFStreamer.cpp
index 634524d4df6fe..c4c3570223e47 100644
--- a/llvm/lib/MC/MCWinCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCWinCOFFStreamer.cpp
@@ -81,6 +81,15 @@ void MCWinCOFFStreamer::initSections(bool NoExecStack,
switchSection(getContext().getObjectFileInfo()->getTextSection());
}
+void MCWinCOFFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
+ changeSectionImpl(Section, Subsection);
+ // Ensure that the first and the second symbols relative to the section are
+ // the section symbol and the COMDAT symbol.
+ getAssembler().registerSymbol(*Section->getBeginSymbol());
+ if (auto *Sym = cast<MCSectionCOFF>(Section)->getCOMDATSymbol())
+ getAssembler().registerSymbol(*Sym);
+}
+
void MCWinCOFFStreamer::emitLabel(MCSymbol *S, SMLoc Loc) {
auto *Symbol = cast<MCSymbolCOFF>(S);
MCObjectStreamer::emitLabel(Symbol, Loc);
diff --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp
index e5925586d99e6..a58ce90902ed3 100644
--- a/llvm/lib/MC/WinCOFFObjectWriter.cpp
+++ b/llvm/lib/MC/WinCOFFObjectWriter.cpp
@@ -324,6 +324,7 @@ void WinCOFFWriter::defineSection(const MCSectionCOFF &MCSec,
COFFSection *Section = createSection(MCSec.getName());
COFFSymbol *Symbol = createSymbol(MCSec.getName());
Section->Symbol = Symbol;
+ SymbolMap[MCSec.getBeginSymbol()] = Symbol;
Symbol->Section = Section;
Symbol->Data.StorageClass = COFF::IMAGE_SYM_CLASS_STATIC;
@@ -398,15 +399,18 @@ COFFSymbol *WinCOFFWriter::getLinkedSymbol(const MCSymbol &Symbol) {
/// and creates the associated COFF symbol staging object.
void WinCOFFWriter::DefineSymbol(const MCSymbol &MCSym, MCAssembler &Assembler,
const MCAsmLayout &Layout) {
- COFFSymbol *Sym = GetOrCreateCOFFSymbol(&MCSym);
const MCSymbol *Base = Layout.getBaseSymbol(MCSym);
COFFSection *Sec = nullptr;
+ MCSectionCOFF *MCSec = nullptr;
if (Base && Base->getFragment()) {
- Sec = SectionMap[Base->getFragment()->getParent()];
- if (Sym->Section && Sym->Section != Sec)
- report_fatal_error("conflicting sections for symbol");
+ MCSec = cast<MCSectionCOFF>(Base->getFragment()->getParent());
+ Sec = SectionMap[MCSec];
}
+ if (Mode == NonDwoOnly && MCSec && isDwoSection(*MCSec))
+ return;
+
+ COFFSymbol *Sym = GetOrCreateCOFFSymbol(&MCSym);
COFFSymbol *Local = nullptr;
if (cast<MCSymbolCOFF>(MCSym).getWeakExternalCharacteristics()) {
Sym->Data.StorageClass = COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL;
diff --git a/llvm/test/DebugInfo/X86/InlinedFnLocalVar.ll b/llvm/test/DebugInfo/X86/InlinedFnLocalVar.ll
index 0b27ec8194a1c..1a21725ecaf68 100644
--- a/llvm/test/DebugInfo/X86/InlinedFnLocalVar.ll
+++ b/llvm/test/DebugInfo/X86/InlinedFnLocalVar.ll
@@ -2,7 +2,7 @@
; RUN: llc --try-experimental-debuginfo-iterators -mtriple i686-pc-cygwin -O2 %s -o - | FileCheck %s
; Check struct X for dead variable xyz from inlined function foo.
-; CHECK: Lsection_info
+; CHECK: .section .debug_info,"dr"
; CHECK: DW_TAG_structure_type
; CHECK-NEXT: info_string
diff --git a/llvm/test/DebugInfo/X86/ref_addr_relocation.ll b/llvm/test/DebugInfo/X86/ref_addr_relocation.ll
index ba31b2498a384..21e54e73edddb 100644
--- a/llvm/test/DebugInfo/X86/ref_addr_relocation.ll
+++ b/llvm/test/DebugInfo/X86/ref_addr_relocation.ll
@@ -47,7 +47,7 @@
; Make sure this is relocatable.
; and test that we don't create the labels to emit a correct COFF relocation
; ELF-ASM: .quad .debug_info+[[TYPE]] # DW_AT_type
-; COFF-ASM: .secrel32 .Lsection_info+[[TYPE]] # DW_AT_type
+; COFF-ASM: .secrel32 .debug_info+[[TYPE]] # DW_AT_type
; DARWIN-ASM2: .quad [[TYPE]] ## DW_AT_type
; DARWIN-ASM4: .long [[TYPE]] ## DW_AT_type
; CHECK-NOT: DW_TAG_structure_type
diff --git a/llvm/test/ExecutionEngine/RuntimeDyld/X86/COFF_x86_64.s b/llvm/test/ExecutionEngine/RuntimeDyld/X86/COFF_x86_64.s
index d63bcfc6df344..e559e51f63496 100644
--- a/llvm/test/ExecutionEngine/RuntimeDyld/X86/COFF_x86_64.s
+++ b/llvm/test/ExecutionEngine/RuntimeDyld/X86/COFF_x86_64.s
@@ -5,7 +5,6 @@
.section section,"rx"
-section:
.long 0
Lreloc:
.long 0
diff --git a/llvm/test/MC/COFF/section-comdat-conflict.s b/llvm/test/MC/COFF/section-comdat-conflict.s
index 2710b76be565f..7b3935338503b 100644
--- a/llvm/test/MC/COFF/section-comdat-conflict.s
+++ b/llvm/test/MC/COFF/section-comdat-conflict.s
@@ -1,6 +1,6 @@
-// RUN: not --crash llvm-mc -triple i386-pc-win32 -filetype=obj < %s 2>&1 | FileCheck %s
+// RUN: not llvm-mc -triple i386-pc-win32 -filetype=obj < %s 2>&1 | FileCheck %s
-// CHECK: conflicting sections for symbol
+// CHECK: <unknown>:0: error: invalid symbol redefinition
.section .xyz
.global bar
diff --git a/llvm/test/MC/COFF/section-comdat.s b/llvm/test/MC/COFF/section-comdat.s
index 7843637b4a7d8..449131145599f 100644
--- a/llvm/test/MC/COFF/section-comdat.s
+++ b/llvm/test/MC/COFF/section-comdat.s
@@ -4,12 +4,12 @@
.section assocSec, "dr", discard, "assocSym"
.global assocSym
assocSym:
-.long 1
+.long assocSec
.section secName, "dr", discard, "Symbol1"
.globl Symbol1
Symbol1:
-.long 1
+.long assocSym
.section secName, "dr", one_only, "Symbol2"
.globl Symbol2
@@ -68,10 +68,10 @@ Symbol8:
# CHECK-NEXT: [ 4](sec 3)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 .bss
# CHECK-NEXT: AUX scnlen 0x0 nreloc 0 nlnno 0 checksum 0x0 assoc 3 comdat 0
# CHECK-NEXT: [ 6](sec 4)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 assocSec
-# CHECK-NEXT: AUX scnlen 0x4 nreloc 0 nlnno 0 checksum 0xb8bc6765 assoc 4 comdat 2
+# CHECK-NEXT: AUX scnlen 0x4 nreloc 1 nlnno 0 checksum 0x0 assoc 4 comdat 2
# CHECK-NEXT: [ 8](sec 4)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 assocSym
# CHECK-NEXT: [ 9](sec 5)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 secName
-# CHECK-NEXT: AUX scnlen 0x4 nreloc 0 nlnno 0 checksum 0xb8bc6765 assoc 5 comdat 2
+# CHECK-NEXT: AUX scnlen 0x4 nreloc 1 nlnno 0 checksum 0x0 assoc 5 comdat 2
# CHECK-NEXT: [11](sec 5)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 Symbol1
# CHECK-NEXT: [12](sec 6)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 secName
# CHECK-NEXT: AUX scnlen 0x4 nreloc 0 nlnno 0 checksum 0xb8bc6765 assoc 6 comdat 1
diff --git a/llvm/test/MC/COFF/section-sym-err.s b/llvm/test/MC/COFF/section-sym-err.s
new file mode 100644
index 0000000000000..db982ac378098
--- /dev/null
+++ b/llvm/test/MC/COFF/section-sym-err.s
@@ -0,0 +1,7 @@
+# RUN: not llvm-mc -filetype=obj -triple x86_64-windows-gnu %s -o %t.o 2>&1 | FileCheck %s --implicit-check-not=error:
+
+.section foo0,"xr",discard,foo1
+foo0:
+foo1:
+
+# CHECK: error: symbol 'foo0' is already defined
More information about the llvm-commits
mailing list