Skip to content

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jul 25, 2025

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
MCContext::get*Section calls.
initMachOMCObjectFileInfo defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment.
In addition, move emitLabel/setFragment from switchSection* and
overridden changeSection to MCObjectStreamer::changeSection for
consistency.

  • test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
    MCObjectStreamer. I don't think the MCAsmStreamer output behavior
    change matters.

Created using spr 1.3.5-bogner
@MaskRay MaskRay added the skip-precommit-approval PR for CI feedback, not intended for review label Jul 25, 2025
@llvmbot llvmbot added llvm:mc Machine (object) code and removed skip-precommit-approval PR for CI feedback, not intended for review labels Jul 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2025

@llvm/pr-subscribers-mc

Author: Fangrui Song (MaskRay)

Changes

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
MCContext::get*Section calls.
initMachOMCObjectFileInfo defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment.

  • test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
    MCObjectStreamer. I don't think the MCAsmStreamer output behavior
    change matters.

Full diff: https://github.com/llvm/llvm-project/pull/150574.diff

14 Files Affected:

  • (modified) llvm/include/llvm/MC/MCSection.h (+3-1)
  • (modified) llvm/include/llvm/MC/MCXCOFFStreamer.h (+1)
  • (modified) llvm/lib/MC/ELFObjectWriter.cpp (+1-14)
  • (modified) llvm/lib/MC/MCAssembler.cpp (-1)
  • (modified) llvm/lib/MC/MCContext.cpp (+9-31)
  • (modified) llvm/lib/MC/MCELFStreamer.cpp (+3-1)
  • (modified) llvm/lib/MC/MCMachOStreamer.cpp (+2)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (+20-5)
  • (modified) llvm/lib/MC/MCSection.cpp (+1-2)
  • (modified) llvm/lib/MC/MCStreamer.cpp (+4-8)
  • (modified) llvm/lib/MC/MCXCOFFStreamer.cpp (+14)
  • (modified) llvm/test/CodeGen/XCore/section-name.ll (+2-2)
  • (modified) llvm/test/MC/ELF/section-sym2.s (+20-7)
  • (removed) llvm/test/MC/ELF/undefined-debug.s (-5)
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index c1f3f02b7a7b6..125f849cb4854 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -589,6 +589,8 @@ 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.
@@ -650,7 +652,7 @@ class LLVM_ABI MCSection {
   bool isLinkerRelaxable() const { return LinkerRelaxable; }
   void setLinkerRelaxable() { LinkerRelaxable = true; }
 
-  MCFragment &getDummyFragment() { return *Subsections[0].second.Head; }
+  MCFragment &getDummyFragment() { return DummyFragment; }
 
   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 870d48fe4200c..c3bc2ca94f711 100644
--- a/llvm/include/llvm/MC/MCXCOFFStreamer.h
+++ b/llvm/include/llvm/MC/MCXCOFFStreamer.h
@@ -22,6 +22,7 @@ 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 9f52b3e3e85c0..4510d4db52e82 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -559,20 +559,7 @@ void ELFWriter::computeSymbolTable(const RevGroupMapTy &RevGroupMap) {
     } else {
       const MCSectionELF &Section =
           static_cast<const MCSectionELF &>(Symbol.getSection());
-
-      // 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;
-      }
-
+      assert(Section.isRegistered());
       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 e142ac1860035..8500fd1fef520 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -106,7 +106,6 @@ 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 12b3fbab8fb8f..0ab5e74530083 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -443,17 +443,19 @@ 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");
-  if (Sym && Sym->isUndefined()) {
+  // 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) {
     R = cast<Symbol>(Sym);
   } else {
     SymEntry.second.Used = true;
@@ -461,6 +463,8 @@ Symbol *MCContext::getOrCreateSectionSymbol(StringRef Section) {
     if (!Sym)
       SymEntry.second.Symbol = R;
   }
+  // Mark as section symbol.
+  R->setIndex(-1u);
   return R;
 }
 
@@ -568,7 +572,6 @@ 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;
 }
 
@@ -579,15 +582,8 @@ MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type,
                                               bool Comdat, unsigned UniqueID,
                                               const MCSymbolELF *LinkedToSym) {
   auto *R = getOrCreateSectionSymbol<MCSymbolELF>(Section);
-  R->setBinding(ELF::STB_LOCAL);
-  R->setType(ELF::STT_SECTION);
-
-  auto *Ret = new (ELFAllocator.Allocate()) MCSectionELF(
+  return new (ELFAllocator.Allocate()) MCSectionELF(
       Section, Type, Flags, EntrySize, Group, Comdat, UniqueID, R, LinkedToSym);
-
-  auto *F = allocInitialFragment(*Ret);
-  R->setFragment(F);
-  return Ret;
 }
 
 MCSectionELF *
@@ -743,7 +739,6 @@ MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
       MCSectionGOFF(CachedName, Kind, IsVirtual, Attributes,
                     static_cast<MCSectionGOFF *>(Parent));
   Iter->second = GOFFSection;
-  allocInitialFragment(*GOFFSection);
   return GOFFSection;
 }
 
@@ -798,8 +793,7 @@ MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
   MCSectionCOFF *Result = new (COFFAllocator.Allocate()) MCSectionCOFF(
       CachedName, Characteristics, COMDATSymbol, Selection, UniqueID, Begin);
   Iter->second = Result;
-  auto *F = allocInitialFragment(*Result);
-  Begin->setFragment(F);
+  Begin->setFragment(&Result->getDummyFragment());
   return Result;
 }
 
@@ -870,8 +864,6 @@ 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;
 }
 
@@ -927,24 +919,11 @@ MCSectionXCOFF *MCContext::getXCOFFSection(
                        MultiSymbolsAllowed);
 
   Entry.second = Result;
-
-  auto *F = allocInitialFragment(*Result);
-
-  // We might miss calculating the symbols difference 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;
 }
 
@@ -964,7 +943,6 @@ 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 b8cbaea52560f..38744a07a2adb 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -89,7 +89,9 @@ void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
     getWriter().markGnuAbi();
 
   MCObjectStreamer::changeSection(Section, Subsection);
-  Asm.registerSymbol(*Section->getBeginSymbol());
+  auto *Sym = static_cast<MCSymbolELF *>(Section->getBeginSymbol());
+  Sym->setBinding(ELF::STB_LOCAL);
+  Sym->setType(ELF::STT_SECTION);
 }
 
 void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) {
diff --git a/llvm/lib/MC/MCMachOStreamer.cpp b/llvm/lib/MC/MCMachOStreamer.cpp
index 8c3332cd8935f..493481524be19 100644
--- a/llvm/lib/MC/MCMachOStreamer.cpp
+++ b/llvm/lib/MC/MCMachOStreamer.cpp
@@ -140,6 +140,8 @@ 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 fcd5cbfbd7341..37b662947cbe5 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);
 
-  // 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();
+  // 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;
   Symbol->setFragment(F);
   Symbol->setOffset(F->getContents().size());
 
@@ -247,6 +247,15 @@ 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)
@@ -262,7 +271,13 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   Section->CurFragList = &Subsections[I].second;
   CurFrag = Section->CurFragList->Tail;
 
-  getAssembler().registerSection(*Section);
+  // 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);
+  }
 }
 
 void MCObjectStreamer::switchSectionNoPrint(MCSection *Section) {
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 023f7f27de0aa..e738a22ec11cd 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -22,8 +22,7 @@ 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) {
-  // The initial subsection number is 0. Create a fragment list.
-  CurFragList = &Subsections.emplace_back(0u, FragList{}).second;
+  DummyFragment.setParent(this);
 }
 
 MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index e14a32f5dc0ce..899a7dfc3b9f5 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -1314,8 +1314,10 @@ 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 *Section, uint32_t) {
-  CurFrag = &Section->getDummyFragment();
+void MCStreamer::changeSection(MCSection *Sec, uint32_t) {
+  CurFrag = &Sec->getDummyFragment();
+  if (auto *Sym = Sec->getBeginSymbol())
+    Sym->setFragment(&Sec->getDummyFragment());
 }
 void MCStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {}
 void MCStreamer::emitBytes(StringRef Data) {}
@@ -1358,9 +1360,6 @@ 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);
   }
 }
 
@@ -1387,9 +1386,6 @@ 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 63381b4f81859..78e4b953ed550 100644
--- a/llvm/lib/MC/MCXCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCXCOFFStreamer.cpp
@@ -36,6 +36,20 @@ 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 difference 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 0fa2cc606a5e0..b2176ecaa8380 100644
--- a/llvm/test/CodeGen/XCore/section-name.ll
+++ b/llvm/test/CodeGen/XCore/section-name.ll
@@ -1,4 +1,4 @@
-; RUN: not llc < %s -mtriple=xcore -o /dev/null 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=xcore | FileCheck %s
 
 @bar = internal global i32 zeroinitializer
 
@@ -6,4 +6,4 @@ define void @".dp.bss"() {
   ret void
 }
 
-; CHECK: <unknown>:0: error: symbol '.dp.bss' is already defined
+; CHECK: .dp.bss:
diff --git a/llvm/test/MC/ELF/section-sym2.s b/llvm/test/MC/ELF/section-sym2.s
index 167fc8c938ea4..fe2b904c9f368 100644
--- a/llvm/test/MC/ELF/section-sym2.s
+++ b/llvm/test/MC/ELF/section-sym2.s
@@ -1,27 +1,40 @@
 # RUN: llvm-mc -filetype=obj -triple x86_64 %s -o %t
-# RUN: llvm-readelf -Srs %t | FileCheck %s
+# RUN: llvm-readelf -SrsX %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 8 entries:
-# CHECK-NEXT:    Num:    Value          Size Type    Bind   Vis       Ndx Name
+# CHECK:      Symbol table '.symtab' contains 10 entries:
+# CHECK-NEXT:    Num:
 # CHECK-NEXT:  0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND
-# 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 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 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
deleted file mode 100644
index 95ead70ef9714..0000000000000
--- a/llvm/test/MC/ELF/undefined-debug.s
+++ /dev/null
@@ -1,5 +0,0 @@
-// 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

MaskRay added 2 commits July 24, 2025 21:57
Created using spr 1.3.5-bogner
.
Created using spr 1.3.5-bogner
@MaskRay MaskRay merged commit 1955a01 into main Jul 25, 2025
7 of 9 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/mc-allocate-initial-fragment-and-define-section-symbol-in-changesection branch July 25, 2025 06:37
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 25, 2025
… changeSection

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment.
In addition, move `emitLabel`/`setFragment` from `switchSection*` and
overridden changeSection to `MCObjectStreamer::changeSection` for
consistency.

* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Pull Request: llvm/llvm-project#150574
@dyung
Copy link
Collaborator

dyung commented Jul 25, 2025

@MaskRay this change seems to be causing a test failure on the AArch64 MacOS bot, can you take a look?

https://lab.llvm.org/buildbot/#/builders/190/builds/24187

@zeroomega
Copy link
Contributor

We are seeing test failure on Clang :: InstallAPI/diagnostics-dsym.test on mac-arm64 builders, error message:

FAIL: Clang :: InstallAPI/diagnostics-dsym.test (13614 of 21953)
******************** TEST 'Clang :: InstallAPI/diagnostics-dsym.test' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
rm -rf /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp # RUN: at line 4
+ rm -rf /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp
split-file /Volumes/Work/s/w/ir/x/w/llvm-llvm-project/clang/test/InstallAPI/diagnostics-dsym.test /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp # RUN: at line 5
+ split-file /Volumes/Work/s/w/ir/x/w/llvm-llvm-project/clang/test/InstallAPI/diagnostics-dsym.test /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp
/Volumes/Work/s/w/ir/x/w/llvm_build/bin/clang --target=arm64-apple-macos11 -g -dynamiclib /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/foo.c  -current_version 1 -compatibility_version 1 -L/Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/usr/lib  -save-temps -dynamiclib  -o /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/foo.dylib -install_name /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/foo.dylib # RUN: at line 8
+ /Volumes/Work/s/w/ir/x/w/llvm_build/bin/clang --target=arm64-apple-macos11 -g -dynamiclib /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/foo.c -current_version 1 -compatibility_version 1 -L/Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/usr/lib -save-temps -dynamiclib -o /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/foo.dylib -install_name /Volumes/Work/s/w/ir/x/w/llvm_build/tools/clang/test/InstallAPI/Output/diagnostics-dsym.test.tmp/foo.dylib
foo.s:252:1: error: assembler local symbol 'Lsection_line' not defined

^
foo.s:252:1: error: assembler local symbol 'Lnames_begin' not defined

^
foo.s:252:1: error: assembler local symbol 'Ltypes_begin' not defined

^
foo.s:252:1: error: assembler local symbol 'Lsection_abbrev' not defined

^

--

********************

Build task: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-mac-arm64/b8708387743983202977/overview

Could you take a look please? If it takes a while to fix, could you revert it first?

dyung added a commit that referenced this pull request Jul 26, 2025
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 26, 2025
MaskRay added a commit that referenced this pull request Jul 26, 2025
MCObjectFileInfo::initMachOMCObjectFileInfo creates DWARF sections with
a temporary label as the `Begin` symbol, different from other object
file formats' section symbol.

 #150574 caused a regression that removed the label for MCAsmStreamer,
which was caught by no test but a ystem-darwin/target-aarch64 specific diagnostics-dsym.test
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 26, 2025
Reland llvm#150574 with a MCStreamer::changeSection change:
In Mach-O, DWARF sections use Begin as a temporary label, requiring a label
definition, unlike section symbols in other file formats.

---

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment.
In addition, move `emitLabel`/`setFragment` from `switchSection*` and
overridden changeSection to `MCObjectStreamer::changeSection` for
consistency.

* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Pull Request: llvm#150574
MaskRay added a commit that referenced this pull request Jul 26, 2025
Reland #150574 with a MCStreamer::changeSection change:
In Mach-O, DWARF sections use Begin as a temporary label, requiring a label
definition, unlike section symbols in other file formats.
(Tested by dec9780)

---

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment. In addition,

* Move `emitLabel`/`setFragment` from `switchSection*` and
  overridden changeSection to `MCObjectStreamer::changeSection` for
  consistency.
* De-virtualize `switchSectionNoPrint`.
* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Pull Request: #150574
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 26, 2025
… changeSection

Reland #150574 with a MCStreamer::changeSection change:
In Mach-O, DWARF sections use Begin as a temporary label, requiring a label
definition, unlike section symbols in other file formats.
(Tested by dec9780)

---

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment. In addition,

* Move `emitLabel`/`setFragment` from `switchSection*` and
  overridden changeSection to `MCObjectStreamer::changeSection` for
  consistency.
* De-virtualize `switchSectionNoPrint`.
* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Pull Request: llvm/llvm-project#150574
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 27, 2025
The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

When switching section or subsection, we cannot reuse the
FragList::Tail, because it is no associated with fragment space tracked
by `FragSpace`. We need to allocate a new fragment, which was less
expensive after llvm#150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 27, 2025
The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

When switching section or subsection, we cannot reuse the
FragList::Tail, because it is no associated with fragment space tracked
by `FragSpace`. We need to allocate a new fragment, which was less
expensive after llvm#150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

Pull Request: llvm#150846
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment.
In addition, move `emitLabel`/`setFragment` from `switchSection*` and
overridden changeSection to `MCObjectStreamer::changeSection` for
consistency.

* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Pull Request: llvm#150574
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
MCObjectFileInfo::initMachOMCObjectFileInfo creates DWARF sections with
a temporary label as the `Begin` symbol, different from other object
file formats' section symbol.

 llvm#150574 caused a regression that removed the label for MCAsmStreamer,
which was caught by no test but a ystem-darwin/target-aarch64 specific diagnostics-dsym.test
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
Reland llvm#150574 with a MCStreamer::changeSection change:
In Mach-O, DWARF sections use Begin as a temporary label, requiring a label
definition, unlike section symbols in other file formats.
(Tested by dec9780)

---

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment. In addition,

* Move `emitLabel`/`setFragment` from `switchSection*` and
  overridden changeSection to `MCObjectStreamer::changeSection` for
  consistency.
* De-virtualize `switchSectionNoPrint`.
* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Pull Request: llvm#150574
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jul 28, 2025
13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment.
In addition, move `emitLabel`/`setFragment` from `switchSection*` and
overridden changeSection to `MCObjectStreamer::changeSection` for
consistency.

* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Pull Request: llvm#150574
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 29, 2025
The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

FragList::Tail cannot be reused when switching sections or subsections,
as it is not associated with the fragment space tracked by `FragSpace`.
Instead, allocate a new fragment, which becomes less expensive after llvm#150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

Pull Request: llvm#150846
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Jul 29, 2025
The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

FragList::Tail cannot be reused when switching sections or subsections,
as it is not associated with the fragment space tracked by `FragSpace`.
Instead, allocate a new fragment, which becomes less expensive after llvm#150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

Pull Request: llvm#150846
MaskRay added a commit that referenced this pull request Jul 29, 2025
The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

FragList::Tail cannot be reused when switching sections or subsections,
as it is not associated with the fragment space tracked by `FragSpace`.
Instead, allocate a new fragment, which becomes less expensive after #150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

Pull Request: #150846
MaskRay added a commit that referenced this pull request Aug 2, 2025
Reapply after #151724 switched to `char *Data`, fixing a
-fsanitize=pointer-overflow issue in MCAssembler::layout.

---

The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

FragList::Tail cannot be reused when switching sections or subsections,
as it is not associated with the fragment space tracked by `FragSpace`.
Instead, allocate a new fragment, which becomes less expensive after #150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

Pull Request: #150846
MaskRay added a commit that referenced this pull request Aug 4, 2025
The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

FragList::Tail cannot be reused when switching sections or subsections,
as it is not associated with the fragment space tracked by `FragSpace`.
Instead, allocate a new fragment, which becomes less expensive after #150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

---

This reverts commit a2fef66,
which reverted the innocent f1aa605 .
Commit df71243, the MCOrgFragment fix,
has fixed the root cause of ClangBuiltLinux/linux#2116
krishna2803 pushed a commit to krishna2803/llvm-project that referenced this pull request Aug 12, 2025
Reapply after llvm#151724 switched to `char *Data`, fixing a
-fsanitize=pointer-overflow issue in MCAssembler::layout.

---

The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

FragList::Tail cannot be reused when switching sections or subsections,
as it is not associated with the fragment space tracked by `FragSpace`.
Instead, allocate a new fragment, which becomes less expensive after llvm#150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

Pull Request: llvm#150846
krishna2803 pushed a commit to krishna2803/llvm-project that referenced this pull request Aug 12, 2025
)

The fixed-size content of the MCFragment object is now stored as
trailing data, replacing ContentStart/ContentEnd with ContentSize. The
available space for trailing data is tracked using `FragSpace`. If the
available space is insufficient, a new block is allocated within the
bump allocator `MCObjectStreamer::FragStorage`.

FragList::Tail cannot be reused when switching sections or subsections,
as it is not associated with the fragment space tracked by `FragSpace`.
Instead, allocate a new fragment, which becomes less expensive after llvm#150574.

Data can only be appended to the tail fragment of a subsection, not to
fragments in the middle. Post-assembler-layout adjustments (such as
.llvm_addrsig and .llvm.call-graph-profile) have been updated to use the
variable-size part instead.

---

This reverts commit a2fef66,
which reverted the innocent f1aa605 .
Commit df71243, the MCOrgFragment fix,
has fixed the root cause of ClangBuiltLinux/linux#2116
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants