Skip to content

Commit 04c2785

Browse files
MaskRayMaskRay
authored
[MC,COFF] Change how we handle section symbols
13a79bb (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: #96459
1 parent 88f80ae commit 04c2785

File tree

12 files changed

+89
-64
lines changed

12 files changed

+89
-64
lines changed

‎llvm/include/llvm/MC/MCContext.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,9 @@ class MCContext {
351351
MCSymbol *getOrCreateDirectionalLocalSymbol(unsigned LocalLabelVal,
352352
unsigned Instance);
353353

354+
template <typename Symbol>
355+
Symbol *getOrCreateSectionSymbol(StringRef Section);
356+
354357
MCSectionELF *createELFSectionImpl(StringRef Section, unsigned Type,
355358
unsigned Flags, unsigned EntrySize,
356359
const MCSymbolELF *Group, bool IsComdat,
@@ -604,11 +607,9 @@ class MCContext {
604607

605608
MCSectionCOFF *getCOFFSection(StringRef Section, unsigned Characteristics,
606609
StringRef COMDATSymName, int Selection,
607-
unsigned UniqueID = GenericSectionID,
608-
const char *BeginSymName = nullptr);
610+
unsigned UniqueID = GenericSectionID);
609611

610-
MCSectionCOFF *getCOFFSection(StringRef Section, unsigned Characteristics,
611-
const char *BeginSymName = nullptr);
612+
MCSectionCOFF *getCOFFSection(StringRef Section, unsigned Characteristics);
612613

613614
/// Gets or creates a section equivalent to Sec that is associated with the
614615
/// section containing KeySym. For example, to create a debug info section

‎llvm/include/llvm/MC/MCWinCOFFStreamer.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class MCWinCOFFStreamer : public MCObjectStreamer {
4040
/// \{
4141

4242
void initSections(bool NoExecStack, const MCSubtargetInfo &STI) override;
43+
void changeSection(MCSection *Section, uint32_t Subsection = 0) override;
4344
void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
4445
void emitAssemblerFlag(MCAssemblerFlag Flag) override;
4546
void emitThumbFunc(MCSymbol *Func) override;

‎llvm/lib/MC/MCContext.cpp

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,27 @@ MCSymbol *MCContext::getDirectionalLocalSymbol(unsigned LocalLabelVal,
383383
return getOrCreateDirectionalLocalSymbol(LocalLabelVal, Instance);
384384
}
385385

386+
template <typename Symbol>
387+
Symbol *MCContext::getOrCreateSectionSymbol(StringRef Section) {
388+
Symbol *R;
389+
auto &SymEntry = getSymbolTableEntry(Section);
390+
MCSymbol *Sym = SymEntry.second.Symbol;
391+
// A section symbol can not redefine regular symbols. There may be multiple
392+
// sections with the same name, in which case the first such section wins.
393+
if (Sym && Sym->isDefined() &&
394+
(!Sym->isInSection() || Sym->getSection().getBeginSymbol() != Sym))
395+
reportError(SMLoc(), "invalid symbol redefinition");
396+
if (Sym && Sym->isUndefined()) {
397+
R = cast<Symbol>(Sym);
398+
} else {
399+
SymEntry.second.Used = true;
400+
R = new (&SymEntry, *this) Symbol(&SymEntry, /*isTemporary=*/false);
401+
if (!Sym)
402+
SymEntry.second.Symbol = R;
403+
}
404+
return R;
405+
}
406+
386407
MCSymbol *MCContext::lookupSymbol(const Twine &Name) const {
387408
SmallString<128> NameSV;
388409
StringRef NameRef = Name.toStringRef(NameSV);
@@ -497,22 +518,7 @@ MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type,
497518
const MCSymbolELF *Group,
498519
bool Comdat, unsigned UniqueID,
499520
const MCSymbolELF *LinkedToSym) {
500-
MCSymbolELF *R;
501-
MCSymbolTableEntry &SymEntry = getSymbolTableEntry(Section);
502-
MCSymbol *Sym = SymEntry.second.Symbol;
503-
// A section symbol can not redefine regular symbols. There may be multiple
504-
// sections with the same name, in which case the first such section wins.
505-
if (Sym && Sym->isDefined() &&
506-
(!Sym->isInSection() || Sym->getSection().getBeginSymbol() != Sym))
507-
reportError(SMLoc(), "invalid symbol redefinition");
508-
if (Sym && Sym->isUndefined()) {
509-
R = cast<MCSymbolELF>(Sym);
510-
} else {
511-
SymEntry.second.Used = true;
512-
R = new (&SymEntry, *this) MCSymbolELF(&SymEntry, /*isTemporary*/ false);
513-
if (!Sym)
514-
SymEntry.second.Symbol = R;
515-
}
521+
auto *R = getOrCreateSectionSymbol<MCSymbolELF>(Section);
516522
R->setBinding(ELF::STB_LOCAL);
517523
R->setType(ELF::STT_SECTION);
518524

@@ -681,12 +687,19 @@ MCSectionGOFF *MCContext::getGOFFSection(StringRef Section, SectionKind Kind,
681687
MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
682688
unsigned Characteristics,
683689
StringRef COMDATSymName, int Selection,
684-
unsigned UniqueID,
685-
const char *BeginSymName) {
690+
unsigned UniqueID) {
686691
MCSymbol *COMDATSymbol = nullptr;
687692
if (!COMDATSymName.empty()) {
688693
COMDATSymbol = getOrCreateSymbol(COMDATSymName);
689694
COMDATSymName = COMDATSymbol->getName();
695+
// A non-associative COMDAT is considered to define the COMDAT symbol. Check
696+
// the redefinition error.
697+
if (Selection != COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE && COMDATSymbol &&
698+
COMDATSymbol->isDefined() &&
699+
(!COMDATSymbol->isInSection() ||
700+
cast<MCSectionCOFF>(COMDATSymbol->getSection()).getCOMDATSymbol() !=
701+
COMDATSymbol))
702+
reportError(SMLoc(), "invalid symbol redefinition");
690703
}
691704

692705
// Do the lookup, if we have a hit, return it.
@@ -696,23 +709,19 @@ MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
696709
if (!IterBool.second)
697710
return Iter->second;
698711

699-
MCSymbol *Begin = nullptr;
700-
if (BeginSymName)
701-
Begin = createTempSymbol(BeginSymName, false);
702-
703712
StringRef CachedName = Iter->first.SectionName;
713+
MCSymbol *Begin = getOrCreateSectionSymbol<MCSymbolCOFF>(Section);
704714
MCSectionCOFF *Result = new (COFFAllocator.Allocate()) MCSectionCOFF(
705715
CachedName, Characteristics, COMDATSymbol, Selection, Begin);
706716
Iter->second = Result;
707-
allocInitialFragment(*Result);
717+
auto *F = allocInitialFragment(*Result);
718+
Begin->setFragment(F);
708719
return Result;
709720
}
710721

711722
MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
712-
unsigned Characteristics,
713-
const char *BeginSymName) {
714-
return getCOFFSection(Section, Characteristics, "", 0, GenericSectionID,
715-
BeginSymName);
723+
unsigned Characteristics) {
724+
return getCOFFSection(Section, Characteristics, "", 0, GenericSectionID);
716725
}
717726

718727
MCSectionCOFF *MCContext::getAssociativeCOFFSection(MCSectionCOFF *Sec,

‎llvm/lib/MC/MCObjectFileInfo.cpp

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -611,24 +611,21 @@ void MCObjectFileInfo::initCOFFMCObjectFileInfo(const Triple &T) {
611611
COFF::IMAGE_SCN_MEM_READ));
612612

613613
DwarfAbbrevSection = Ctx->getCOFFSection(
614-
".debug_abbrev",
615-
COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
616-
COFF::IMAGE_SCN_MEM_READ,
617-
"section_abbrev");
614+
".debug_abbrev", COFF::IMAGE_SCN_MEM_DISCARDABLE |
615+
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
616+
COFF::IMAGE_SCN_MEM_READ);
618617
DwarfInfoSection = Ctx->getCOFFSection(
619-
".debug_info",
620-
COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
621-
COFF::IMAGE_SCN_MEM_READ,
622-
"section_info");
618+
".debug_info", COFF::IMAGE_SCN_MEM_DISCARDABLE |
619+
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
620+
COFF::IMAGE_SCN_MEM_READ);
623621
DwarfLineSection = Ctx->getCOFFSection(
624622
".debug_line", COFF::IMAGE_SCN_MEM_DISCARDABLE |
625623
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
626624
COFF::IMAGE_SCN_MEM_READ);
627625
DwarfLineStrSection = Ctx->getCOFFSection(
628-
".debug_line_str",
629-
COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
630-
COFF::IMAGE_SCN_MEM_READ,
631-
"section_line_str");
626+
".debug_line_str", COFF::IMAGE_SCN_MEM_DISCARDABLE |
627+
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
628+
COFF::IMAGE_SCN_MEM_READ);
632629
DwarfFrameSection = Ctx->getCOFFSection(
633630
".debug_frame", COFF::IMAGE_SCN_MEM_DISCARDABLE |
634631
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
@@ -738,19 +735,17 @@ void MCObjectFileInfo::initCOFFMCObjectFileInfo(const Triple &T) {
738735
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
739736
COFF::IMAGE_SCN_MEM_READ);
740737
DwarfAccelNamesSection = Ctx->getCOFFSection(
741-
".apple_names",
742-
COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
743-
COFF::IMAGE_SCN_MEM_READ,
744-
"names_begin");
738+
".apple_names", COFF::IMAGE_SCN_MEM_DISCARDABLE |
739+
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
740+
COFF::IMAGE_SCN_MEM_READ);
745741
DwarfAccelNamespaceSection = Ctx->getCOFFSection(
746742
".apple_namespaces", COFF::IMAGE_SCN_MEM_DISCARDABLE |
747743
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
748744
COFF::IMAGE_SCN_MEM_READ);
749745
DwarfAccelTypesSection = Ctx->getCOFFSection(
750-
".apple_types",
751-
COFF::IMAGE_SCN_MEM_DISCARDABLE | COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
752-
COFF::IMAGE_SCN_MEM_READ,
753-
"types_begin");
746+
".apple_types", COFF::IMAGE_SCN_MEM_DISCARDABLE |
747+
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
748+
COFF::IMAGE_SCN_MEM_READ);
754749
DwarfAccelObjCSection = Ctx->getCOFFSection(
755750
".apple_objc", COFF::IMAGE_SCN_MEM_DISCARDABLE |
756751
COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |

‎llvm/lib/MC/MCWinCOFFStreamer.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,15 @@ void MCWinCOFFStreamer::initSections(bool NoExecStack,
8181
switchSection(getContext().getObjectFileInfo()->getTextSection());
8282
}
8383

84+
void MCWinCOFFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
85+
changeSectionImpl(Section, Subsection);
86+
// Ensure that the first and the second symbols relative to the section are
87+
// the section symbol and the COMDAT symbol.
88+
getAssembler().registerSymbol(*Section->getBeginSymbol());
89+
if (auto *Sym = cast<MCSectionCOFF>(Section)->getCOMDATSymbol())
90+
getAssembler().registerSymbol(*Sym);
91+
}
92+
8493
void MCWinCOFFStreamer::emitLabel(MCSymbol *S, SMLoc Loc) {
8594
auto *Symbol = cast<MCSymbolCOFF>(S);
8695
MCObjectStreamer::emitLabel(Symbol, Loc);

‎llvm/lib/MC/WinCOFFObjectWriter.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ void WinCOFFWriter::defineSection(const MCSectionCOFF &MCSec,
324324
COFFSection *Section = createSection(MCSec.getName());
325325
COFFSymbol *Symbol = createSymbol(MCSec.getName());
326326
Section->Symbol = Symbol;
327+
SymbolMap[MCSec.getBeginSymbol()] = Symbol;
327328
Symbol->Section = Section;
328329
Symbol->Data.StorageClass = COFF::IMAGE_SYM_CLASS_STATIC;
329330

@@ -398,15 +399,18 @@ COFFSymbol *WinCOFFWriter::getLinkedSymbol(const MCSymbol &Symbol) {
398399
/// and creates the associated COFF symbol staging object.
399400
void WinCOFFWriter::DefineSymbol(const MCSymbol &MCSym, MCAssembler &Assembler,
400401
const MCAsmLayout &Layout) {
401-
COFFSymbol *Sym = GetOrCreateCOFFSymbol(&MCSym);
402402
const MCSymbol *Base = Layout.getBaseSymbol(MCSym);
403403
COFFSection *Sec = nullptr;
404+
MCSectionCOFF *MCSec = nullptr;
404405
if (Base && Base->getFragment()) {
405-
Sec = SectionMap[Base->getFragment()->getParent()];
406-
if (Sym->Section && Sym->Section != Sec)
407-
report_fatal_error("conflicting sections for symbol");
406+
MCSec = cast<MCSectionCOFF>(Base->getFragment()->getParent());
407+
Sec = SectionMap[MCSec];
408408
}
409409

410+
if (Mode == NonDwoOnly && MCSec && isDwoSection(*MCSec))
411+
return;
412+
413+
COFFSymbol *Sym = GetOrCreateCOFFSymbol(&MCSym);
410414
COFFSymbol *Local = nullptr;
411415
if (cast<MCSymbolCOFF>(MCSym).getWeakExternalCharacteristics()) {
412416
Sym->Data.StorageClass = COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL;

‎llvm/test/DebugInfo/X86/InlinedFnLocalVar.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
; RUN: llc --try-experimental-debuginfo-iterators -mtriple i686-pc-cygwin -O2 %s -o - | FileCheck %s
33
; Check struct X for dead variable xyz from inlined function foo.
44

5-
; CHECK: Lsection_info
5+
; CHECK: .section .debug_info,"dr"
66
; CHECK: DW_TAG_structure_type
77
; CHECK-NEXT: info_string
88

‎llvm/test/DebugInfo/X86/ref_addr_relocation.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
; Make sure this is relocatable.
4848
; and test that we don't create the labels to emit a correct COFF relocation
4949
; ELF-ASM: .quad .debug_info+[[TYPE]] # DW_AT_type
50-
; COFF-ASM: .secrel32 .Lsection_info+[[TYPE]] # DW_AT_type
50+
; COFF-ASM: .secrel32 .debug_info+[[TYPE]] # DW_AT_type
5151
; DARWIN-ASM2: .quad [[TYPE]] ## DW_AT_type
5252
; DARWIN-ASM4: .long [[TYPE]] ## DW_AT_type
5353
; CHECK-NOT: DW_TAG_structure_type

‎llvm/test/ExecutionEngine/RuntimeDyld/X86/COFF_x86_64.s

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66

77
.section section,"rx"
8-
section:
98
.long 0
109
Lreloc:
1110
.long 0

‎llvm/test/MC/COFF/section-comdat-conflict.s

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
// RUN: not --crash llvm-mc -triple i386-pc-win32 -filetype=obj < %s 2>&1 | FileCheck %s
1+
// RUN: not llvm-mc -triple i386-pc-win32 -filetype=obj < %s 2>&1 | FileCheck %s
22

3-
// CHECK: conflicting sections for symbol
3+
// CHECK: <unknown>:0: error: invalid symbol redefinition
44

55
.section .xyz
66
.global bar

‎llvm/test/MC/COFF/section-comdat.s

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
.section assocSec, "dr", discard, "assocSym"
55
.global assocSym
66
assocSym:
7-
.long 1
7+
.long assocSec
88

99
.section secName, "dr", discard, "Symbol1"
1010
.globl Symbol1
1111
Symbol1:
12-
.long 1
12+
.long assocSym
1313

1414
.section secName, "dr", one_only, "Symbol2"
1515
.globl Symbol2
@@ -68,10 +68,10 @@ Symbol8:
6868
# CHECK-NEXT: [ 4](sec 3)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 .bss
6969
# CHECK-NEXT: AUX scnlen 0x0 nreloc 0 nlnno 0 checksum 0x0 assoc 3 comdat 0
7070
# CHECK-NEXT: [ 6](sec 4)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 assocSec
71-
# CHECK-NEXT: AUX scnlen 0x4 nreloc 0 nlnno 0 checksum 0xb8bc6765 assoc 4 comdat 2
71+
# CHECK-NEXT: AUX scnlen 0x4 nreloc 1 nlnno 0 checksum 0x0 assoc 4 comdat 2
7272
# CHECK-NEXT: [ 8](sec 4)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 assocSym
7373
# CHECK-NEXT: [ 9](sec 5)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 secName
74-
# CHECK-NEXT: AUX scnlen 0x4 nreloc 0 nlnno 0 checksum 0xb8bc6765 assoc 5 comdat 2
74+
# CHECK-NEXT: AUX scnlen 0x4 nreloc 1 nlnno 0 checksum 0x0 assoc 5 comdat 2
7575
# CHECK-NEXT: [11](sec 5)(fl 0x00)(ty 0)(scl 2) (nx 0) 0x00000000 Symbol1
7676
# CHECK-NEXT: [12](sec 6)(fl 0x00)(ty 0)(scl 3) (nx 1) 0x00000000 secName
7777
# CHECK-NEXT: AUX scnlen 0x4 nreloc 0 nlnno 0 checksum 0xb8bc6765 assoc 6 comdat 1

‎llvm/test/MC/COFF/section-sym-err.s

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# RUN: not llvm-mc -filetype=obj -triple x86_64-windows-gnu %s -o %t.o 2>&1 | FileCheck %s --implicit-check-not=error:
2+
3+
.section foo0,"xr",discard,foo1
4+
foo0:
5+
foo1:
6+
7+
# CHECK: error: symbol 'foo0' is already defined

0 commit comments

Comments
 (0)