Author: dyung Date: 2025-07-25T19:59:08-07:00 New Revision: 5972f29670b2d45c35814267366d81390eea1dfb
URL: https://github.com/llvm/llvm-project/commit/5972f29670b2d45c35814267366d81390eea1dfb DIFF: https://github.com/llvm/llvm-project/commit/5972f29670b2d45c35814267366d81390eea1dfb.diff LOG: Revert "MC: Allocate initial fragment and define section symbol in changeSection" This reverts commit 1955a01d63eceaf806faa29c2ed7c61e135889eb. Added: llvm/test/MC/ELF/undefined-debug.s Modified: llvm/include/llvm/MC/MCContext.h llvm/include/llvm/MC/MCSection.h llvm/include/llvm/MC/MCXCOFFStreamer.h llvm/lib/MC/ELFObjectWriter.cpp llvm/lib/MC/MCAssembler.cpp llvm/lib/MC/MCContext.cpp llvm/lib/MC/MCELFStreamer.cpp llvm/lib/MC/MCMachOStreamer.cpp llvm/lib/MC/MCObjectStreamer.cpp llvm/lib/MC/MCSection.cpp llvm/lib/MC/MCStreamer.cpp llvm/lib/MC/MCXCOFFStreamer.cpp llvm/test/CodeGen/XCore/section-name.ll llvm/test/MC/ELF/section-sym2.s Removed: ################################################################################ diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h index ddac161fe0ff9..c137f6184a9a7 100644 --- a/llvm/include/llvm/MC/MCContext.h +++ b/llvm/include/llvm/MC/MCContext.h @@ -333,6 +333,8 @@ class MCContext { void reportCommon(SMLoc Loc, std::function<void(SMDiagnostic &, const SourceMgr *)>); + MCFragment *allocInitialFragment(MCSection &Sec); + MCSymbolTableEntry &getSymbolTableEntry(StringRef Name); MCSymbol *createSymbolImpl(const MCSymbolTableEntry *Name, bool IsTemporary); diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h index 125f849cb4854..c1f3f02b7a7b6 100644 --- a/llvm/include/llvm/MC/MCSection.h +++ b/llvm/include/llvm/MC/MCSection.h @@ -589,8 +589,6 @@ class LLVM_ABI MCSection { /// offset between two locations may not be fully resolved. bool LinkerRelaxable : 1; - MCFragment DummyFragment; - // Mapping from subsection number to fragment list. At layout time, the // subsection 0 list is replaced with concatenated fragments from all // subsections. @@ -652,7 +650,7 @@ class LLVM_ABI MCSection { bool isLinkerRelaxable() const { return LinkerRelaxable; } void setLinkerRelaxable() { LinkerRelaxable = true; } - MCFragment &getDummyFragment() { return DummyFragment; } + MCFragment &getDummyFragment() { return *Subsections[0].second.Head; } FragList *curFragList() const { return CurFragList; } iterator begin() const { return iterator(CurFragList->Head); } diff --git a/llvm/include/llvm/MC/MCXCOFFStreamer.h b/llvm/include/llvm/MC/MCXCOFFStreamer.h index c3bc2ca94f711..870d48fe4200c 100644 --- a/llvm/include/llvm/MC/MCXCOFFStreamer.h +++ b/llvm/include/llvm/MC/MCXCOFFStreamer.h @@ -22,7 +22,6 @@ class MCXCOFFStreamer : public MCObjectStreamer { XCOFFObjectWriter &getWriter(); - void changeSection(MCSection *Section, uint32_t Subsection = 0) override; bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size, Align ByteAlignment) override; diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp index ae8dffc297de4..6099bb81d6e8b 100644 --- a/llvm/lib/MC/ELFObjectWriter.cpp +++ b/llvm/lib/MC/ELFObjectWriter.cpp @@ -559,7 +559,20 @@ void ELFWriter::computeSymbolTable(const RevGroupMapTy &RevGroupMap) { } else { const MCSectionELF &Section = static_cast<const MCSectionELF &>(Symbol.getSection()); - assert(Section.isRegistered()); + + // We may end up with a situation when section symbol is technically + // defined, but should not be. That happens because we explicitly + // pre-create few .debug_* sections to have accessors. + // And if these sections were not really defined in the code, but were + // referenced, we simply error out. + if (!Section.isRegistered()) { + assert(static_cast<const MCSymbolELF &>(Symbol).getType() == + ELF::STT_SECTION); + Ctx.reportError(SMLoc(), + "Undefined section reference: " + Symbol.getName()); + continue; + } + if (Mode == NonDwoOnly && isDwoSection(Section)) continue; MSD.SectionIndex = Section.getOrdinal(); diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp index 8500fd1fef520..e142ac1860035 100644 --- a/llvm/lib/MC/MCAssembler.cpp +++ b/llvm/lib/MC/MCAssembler.cpp @@ -106,6 +106,7 @@ void MCAssembler::reset() { bool MCAssembler::registerSection(MCSection &Section) { if (Section.isRegistered()) return false; + assert(Section.curFragList()->Head && "allocInitialFragment not called"); Sections.push_back(&Section); Section.setIsRegistered(true); return true; diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp index c0448aead7101..12b3fbab8fb8f 100644 --- a/llvm/lib/MC/MCContext.cpp +++ b/llvm/lib/MC/MCContext.cpp @@ -200,6 +200,16 @@ MCInst *MCContext::createMCInst() { return new (MCInstAllocator.Allocate()) MCInst; } +// Allocate the initial MCFragment for the begin symbol. +MCFragment *MCContext::allocInitialFragment(MCSection &Sec) { + assert(!Sec.curFragList()->Head); + auto *F = allocFragment<MCFragment>(); + F->setParent(&Sec); + Sec.curFragList()->Head = F; + Sec.curFragList()->Tail = F; + return F; +} + //===----------------------------------------------------------------------===// // Symbol Manipulation //===----------------------------------------------------------------------===// @@ -433,19 +443,17 @@ MCSymbol *MCContext::getDirectionalLocalSymbol(unsigned LocalLabelVal, return getOrCreateDirectionalLocalSymbol(LocalLabelVal, Instance); } -// Create a section symbol, with a distinct one for each section of the same. -// The first symbol is used for assembly code references. 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"); - // Use the symbol's index to track if it has been used as a section symbol. - // Set to -1 to catch potential bugs if misused as a symbol index. - if (Sym && Sym->getIndex() != -1u) { + if (Sym && Sym->isUndefined()) { R = cast<Symbol>(Sym); } else { SymEntry.second.Used = true; @@ -453,8 +461,6 @@ Symbol *MCContext::getOrCreateSectionSymbol(StringRef Section) { if (!Sym) SymEntry.second.Symbol = R; } - // Mark as section symbol. - R->setIndex(-1u); return R; } @@ -562,6 +568,7 @@ MCSectionMachO *MCContext::getMachOSection(StringRef Segment, StringRef Section, MCSectionMachO(Segment, Name.substr(Name.size() - Section.size()), TypeAndAttributes, Reserved2, Kind, Begin); R.first->second = Ret; + allocInitialFragment(*Ret); return Ret; } @@ -572,8 +579,15 @@ MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type, bool Comdat, unsigned UniqueID, const MCSymbolELF *LinkedToSym) { auto *R = getOrCreateSectionSymbol<MCSymbolELF>(Section); - return new (ELFAllocator.Allocate()) MCSectionELF( + R->setBinding(ELF::STB_LOCAL); + R->setType(ELF::STT_SECTION); + + auto *Ret = new (ELFAllocator.Allocate()) MCSectionELF( Section, Type, Flags, EntrySize, Group, Comdat, UniqueID, R, LinkedToSym); + + auto *F = allocInitialFragment(*Ret); + R->setFragment(F); + return Ret; } MCSectionELF * @@ -729,6 +743,7 @@ MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name, MCSectionGOFF(CachedName, Kind, IsVirtual, Attributes, static_cast<MCSectionGOFF *>(Parent)); Iter->second = GOFFSection; + allocInitialFragment(*GOFFSection); return GOFFSection; } @@ -783,7 +798,8 @@ MCSectionCOFF *MCContext::getCOFFSection(StringRef Section, MCSectionCOFF *Result = new (COFFAllocator.Allocate()) MCSectionCOFF( CachedName, Characteristics, COMDATSymbol, Selection, UniqueID, Begin); Iter->second = Result; - Begin->setFragment(&Result->getDummyFragment()); + auto *F = allocInitialFragment(*Result); + Begin->setFragment(F); return Result; } @@ -854,6 +870,8 @@ MCSectionWasm *MCContext::getWasmSection(const Twine &Section, SectionKind Kind, MCSectionWasm(CachedName, Kind, Flags, GroupSym, UniqueID, Begin); Entry.second = Result; + auto *F = allocInitialFragment(*Result); + Begin->setFragment(F); return Result; } @@ -909,11 +927,24 @@ MCSectionXCOFF *MCContext::getXCOFFSection( MultiSymbolsAllowed); Entry.second = Result; + + auto *F = allocInitialFragment(*Result); + + // We might miss calculating the symbols diff erence as absolute value before + // adding fixups when symbol_A without the fragment set is the csect itself + // and symbol_B is in it. + // TODO: Currently we only set the fragment for XMC_PR csects and DWARF + // sections because we don't have other cases that hit this problem yet. + if (IsDwarfSec || CsectProp->MappingClass == XCOFF::XMC_PR) + QualName->setFragment(F); + return Result; } MCSectionSPIRV *MCContext::getSPIRVSection() { MCSectionSPIRV *Result = new (SPIRVAllocator.Allocate()) MCSectionSPIRV(); + + allocInitialFragment(*Result); return Result; } @@ -933,6 +964,7 @@ MCSectionDXContainer *MCContext::getDXContainerSection(StringRef Section, new (DXCAllocator.Allocate()) MCSectionDXContainer(Name, K, nullptr); // The first fragment will store the header + allocInitialFragment(*MapIt->second); return MapIt->second; } diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp index 38744a07a2adb..b8cbaea52560f 100644 --- a/llvm/lib/MC/MCELFStreamer.cpp +++ b/llvm/lib/MC/MCELFStreamer.cpp @@ -89,9 +89,7 @@ void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) { getWriter().markGnuAbi(); MCObjectStreamer::changeSection(Section, Subsection); - auto *Sym = static_cast<MCSymbolELF *>(Section->getBeginSymbol()); - Sym->setBinding(ELF::STB_LOCAL); - Sym->setType(ELF::STT_SECTION); + Asm.registerSymbol(*Section->getBeginSymbol()); } void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) { diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp index 493481524be19..8c3332cd8935f 100644 --- a/llvm/lib/MC/MCMachOStreamer.cpp +++ b/llvm/lib/MC/MCMachOStreamer.cpp @@ -140,8 +140,6 @@ void MCMachOStreamer::changeSection(MCSection *Section, uint32_t Subsection) { MCSymbol *Label = getContext().createLinkerPrivateTempSymbol(); Section->setBeginSymbol(Label); HasSectionLabel[Section] = true; - if (!Label->isInSection()) - emitLabel(Label); } } diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp index 37b662947cbe5..fcd5cbfbd7341 100644 --- a/llvm/lib/MC/MCObjectStreamer.cpp +++ b/llvm/lib/MC/MCObjectStreamer.cpp @@ -185,10 +185,10 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) { getAssembler().registerSymbol(*Symbol); - // Set the fragment and offset. This function might be called by - // changeSection, when the section stack top hasn't been changed to the new - // section. - MCFragment *F = CurFrag; + // If there is a current fragment, mark the symbol as pointing into it. + // Otherwise queue the label and set its fragment pointer when we emit the + // next fragment. + MCFragment *F = getCurrentFragment(); Symbol->setFragment(F); Symbol->setOffset(F->getContents().size()); @@ -247,15 +247,6 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) { assert(Section && "Cannot switch to a null section!"); getContext().clearDwarfLocSeen(); - // Register the section and create an initial fragment for subsection 0 - // if `Subsection` is non-zero. - bool NewSec = getAssembler().registerSection(*Section); - MCFragment *F0 = nullptr; - if (NewSec && Subsection) { - changeSection(Section, 0); - F0 = CurFrag; - } - auto &Subsections = Section->Subsections; size_t I = 0, E = Subsections.size(); while (I != E && Subsections[I].first < Subsection) @@ -271,13 +262,7 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) { Section->CurFragList = &Subsections[I].second; CurFrag = Section->CurFragList->Tail; - // Define the section symbol at subsection 0's initial fragment if required. - if (!NewSec) - return; - if (auto *Sym = Section->getBeginSymbol()) { - Sym->setFragment(Subsection ? F0 : CurFrag); - getAssembler().registerSymbol(*Sym); - } + getAssembler().registerSection(*Section); } void MCObjectStreamer::switchSectionNoPrint(MCSection *Section) { diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp index e738a22ec11cd..023f7f27de0aa 100644 --- a/llvm/lib/MC/MCSection.cpp +++ b/llvm/lib/MC/MCSection.cpp @@ -22,7 +22,8 @@ MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsBss, MCSymbol *Begin) : Begin(Begin), HasInstructions(false), IsRegistered(false), IsText(IsText), IsBss(IsBss), LinkerRelaxable(false), Name(Name), Variant(V) { - DummyFragment.setParent(this); + // The initial subsection number is 0. Create a fragment list. + CurFragList = &Subsections.emplace_back(0u, FragList{}).second; } MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) { diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp index 899a7dfc3b9f5..e14a32f5dc0ce 100644 --- a/llvm/lib/MC/MCStreamer.cpp +++ b/llvm/lib/MC/MCStreamer.cpp @@ -1314,10 +1314,8 @@ void MCStreamer::emitZerofill(MCSection *, MCSymbol *, uint64_t, Align, SMLoc) { } void MCStreamer::emitTBSSSymbol(MCSection *Section, MCSymbol *Symbol, uint64_t Size, Align ByteAlignment) {} -void MCStreamer::changeSection(MCSection *Sec, uint32_t) { - CurFrag = &Sec->getDummyFragment(); - if (auto *Sym = Sec->getBeginSymbol()) - Sym->setFragment(&Sec->getDummyFragment()); +void MCStreamer::changeSection(MCSection *Section, uint32_t) { + CurFrag = &Section->getDummyFragment(); } void MCStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {} void MCStreamer::emitBytes(StringRef Data) {} @@ -1360,6 +1358,9 @@ void MCStreamer::switchSection(MCSection *Section, uint32_t Subsection) { changeSection(Section, Subsection); SectionStack.back().first = MCSectionSubPair(Section, Subsection); assert(!Section->hasEnded() && "Section already ended"); + MCSymbol *Sym = Section->getBeginSymbol(); + if (Sym && !Sym->isInSection()) + emitLabel(Sym); } } @@ -1386,6 +1387,9 @@ void MCStreamer::switchSectionNoPrint(MCSection *Section) { SectionStack.back().second = SectionStack.back().first; SectionStack.back().first = MCSectionSubPair(Section, 0); changeSection(Section, 0); + MCSymbol *Sym = Section->getBeginSymbol(); + if (Sym && !Sym->isInSection()) + emitLabel(Sym); } MCSymbol *MCStreamer::endSection(MCSection *Section) { diff --git a/llvm/lib/MC/MCXCOFFStreamer.cpp b/llvm/lib/MC/MCXCOFFStreamer.cpp index 78e4b953ed550..63381b4f81859 100644 --- a/llvm/lib/MC/MCXCOFFStreamer.cpp +++ b/llvm/lib/MC/MCXCOFFStreamer.cpp @@ -36,20 +36,6 @@ XCOFFObjectWriter &MCXCOFFStreamer::getWriter() { return static_cast<XCOFFObjectWriter &>(getAssembler().getWriter()); } -void MCXCOFFStreamer::changeSection(MCSection *Section, uint32_t Subsection) { - MCObjectStreamer::changeSection(Section, Subsection); - auto *Sec = cast<MCSectionXCOFF>(Section); - // We might miss calculating the symbols diff erence as absolute value before - // adding fixups when symbol_A without the fragment set is the csect itself - // and symbol_B is in it. - // TODO: Currently we only set the fragment for XMC_PR csects and DWARF - // sections because we don't have other cases that hit this problem yet. - // if (IsDwarfSec || CsectProp->MappingClass == XCOFF::XMC_PR) - // QualName->setFragment(F); - if (Sec->isDwarfSect() || Sec->getMappingClass() == XCOFF::XMC_PR) - Sec->getQualNameSymbol()->setFragment(CurFrag); -} - bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym, MCSymbolAttr Attribute) { auto *Symbol = cast<MCSymbolXCOFF>(Sym); diff --git a/llvm/test/CodeGen/XCore/section-name.ll b/llvm/test/CodeGen/XCore/section-name.ll index b2176ecaa8380..0fa2cc606a5e0 100644 --- a/llvm/test/CodeGen/XCore/section-name.ll +++ b/llvm/test/CodeGen/XCore/section-name.ll @@ -1,4 +1,4 @@ -; RUN: llc < %s -mtriple=xcore | FileCheck %s +; RUN: not llc < %s -mtriple=xcore -o /dev/null 2>&1 | FileCheck %s @bar = internal global i32 zeroinitializer @@ -6,4 +6,4 @@ define void @".dp.bss"() { ret void } -; CHECK: .dp.bss: +; CHECK: <unknown>:0: error: symbol '.dp.bss' is already defined diff --git a/llvm/test/MC/ELF/section-sym2.s b/llvm/test/MC/ELF/section-sym2.s index fe2b904c9f368..167fc8c938ea4 100644 --- a/llvm/test/MC/ELF/section-sym2.s +++ b/llvm/test/MC/ELF/section-sym2.s @@ -1,40 +1,27 @@ # RUN: llvm-mc -filetype=obj -triple x86_64 %s -o %t -# RUN: llvm-readelf -SrsX %t | FileCheck %s +# RUN: llvm-readelf -Srs %t | FileCheck %s ## Test that we can forward reference a section. mov .rodata, %rsi -mov data, %rsi mov .debug_info, %rsi -mov .debug_abbrev, %rsi .section .rodata,"a" -.pushsection data, 2; .long 2; .popsection -.section data; .long 1 .section .debug_info,"G",@progbits,11,comdat; .long x1 .section .debug_info,"G",@progbits,22,comdat; .long x2 .section .debug_info,"",@progbits; .long x0 -.text -mov data, %rdi - -# CHECK: Relocation section '.rela.text' -# CHECK: R_X86_64_32S {{.*}} data + 0 -# CHECK: R_X86_64_32S {{.*}} data + 0 - # CHECK: Relocation section '.rela.debug_info' at offset {{.*}} contains 1 # CHECK: Relocation section '.rela.debug_info' at offset {{.*}} contains 1 # CHECK: Relocation section '.rela.debug_info' at offset {{.*}} contains 1 -# CHECK: Symbol table '.symtab' contains 10 entries: -# CHECK-NEXT: Num: +# CHECK: Symbol table '.symtab' contains 8 entries: +# CHECK-NEXT: Num: Value Size Type Bind Vis Ndx Name # CHECK-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND -# CHECK-NEXT: 0000000000000000 0 SECTION LOCAL DEFAULT [[#]] (.rodata) .rodata -# CHECK-NEXT: 0000000000000000 0 SECTION LOCAL DEFAULT [[#]] (data) data -# CHECK-NEXT: 0000000000000000 0 SECTION LOCAL DEFAULT [[#]] (.debug_info) .debug_info -# CHECK-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT [[#]] (.group) 11 -# CHECK-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT [[#]] (.group) 22 -# CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND .debug_abbrev +# CHECK-NEXT: 0000000000000000 0 SECTION LOCAL DEFAULT 4 .rodata +# CHECK-NEXT: 0000000000000000 0 SECTION LOCAL DEFAULT 11 .debug_info +# CHECK-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT 5 11 +# CHECK-NEXT: 0000000000000000 0 NOTYPE LOCAL DEFAULT 8 22 # CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND x1 # CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND x2 # CHECK-NEXT: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND x0 diff --git a/llvm/test/MC/ELF/undefined-debug.s b/llvm/test/MC/ELF/undefined-debug.s new file mode 100644 index 0000000000000..95ead70ef9714 --- /dev/null +++ b/llvm/test/MC/ELF/undefined-debug.s @@ -0,0 +1,5 @@ +// RUN: not llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t 2>&1 | FileCheck %s +// CHECK: error: Undefined section reference: .debug_pubnames + +.section .foo,"",@progbits + .long .debug_pubnames _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
