Skip to content

Conversation

@MaskRay
Copy link
Member

@MaskRay MaskRay commented Jul 20, 2025

  • Handle non-zero fill values for .fill and .org directives.
  • Restore the fragment type check
    (5ee34ff removed a reachable
    llvm_unreachable) to detect unintended API usage.

Remove virtual functions getVirtualSectionKind (added in
https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics.
The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, and ELF object file formats.

Created using spr 1.3.5-bogner
@MaskRay MaskRay requested a review from aengelke July 20, 2025 16:44
@llvmbot
Copy link
Member

llvmbot commented Jul 20, 2025

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-arm

Author: Fangrui Song (MaskRay)

Changes

Replace getOrCreateDataFragment with getCurrentFragment, adding an assert to ensure CurFrag is either null or an FT_Data fragment.

Fix fragment-in-BSS check to handle non-zero fill values for .fill and .org directives.
Restore the fragment type check
(5ee34ff removed a reachable llvm_unreachable) to detect unintended API usage.

Remove virtual functions getVirtualSectionKind (added in
https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics.
The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, and ELF object file formats.

The two changes are unrelated. Will land them separately.


Patch is 20.15 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149721.diff

18 Files Affected:

  • (modified) llvm/include/llvm/MC/MCObjectStreamer.h (-3)
  • (modified) llvm/include/llvm/MC/MCSection.h (-2)
  • (modified) llvm/include/llvm/MC/MCSectionCOFF.h (-1)
  • (modified) llvm/include/llvm/MC/MCSectionELF.h (-1)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (+4)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+27-5)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (+9-9)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+2-3)
  • (modified) llvm/lib/MC/MCSection.cpp (-2)
  • (modified) llvm/lib/MC/MCSectionCOFF.cpp (-4)
  • (modified) llvm/lib/MC/MCSectionELF.cpp (-2)
  • (modified) llvm/lib/MC/MCWin64EH.cpp (+1-1)
  • (modified) llvm/lib/MC/MCWinCOFFStreamer.cpp (+5-5)
  • (modified) llvm/lib/MC/MCXCOFFStreamer.cpp (+1-1)
  • (modified) llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp (+3-3)
  • (modified) llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp (+6-6)
  • (modified) llvm/test/MC/COFF/bss-text.s (+2-2)
  • (modified) llvm/test/MC/ELF/nobits-non-zero-value.s (+24-7)
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index 633334ee8a33f..429a1bb3321f6 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -72,9 +72,6 @@ class MCObjectStreamer : public MCStreamer {
   MCSymbol *emitCFILabel() override;
   void emitCFISections(bool EH, bool Debug) override;
 
-  // TODO: Change callers to use getCurrentFragment instead.
-  MCFragment *getOrCreateDataFragment() { return getCurrentFragment(); }
-
 protected:
   bool changeSectionImpl(MCSection *Section, uint32_t Subsection);
 
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index fe8c13d108c2c..72751f08e497b 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -179,8 +179,6 @@ class LLVM_ABI MCSection {
   /// Check whether this section is "virtual", that is has no actual object
   /// file contents.
   bool isVirtualSection() const { return IsVirtual; }
-
-  virtual StringRef getVirtualSectionKind() const;
 };
 
 // Represents a contiguous piece of code or data within a section. Its size is
diff --git a/llvm/include/llvm/MC/MCSectionCOFF.h b/llvm/include/llvm/MC/MCSectionCOFF.h
index 4472a128caa6b..f979413a3791e 100644
--- a/llvm/include/llvm/MC/MCSectionCOFF.h
+++ b/llvm/include/llvm/MC/MCSectionCOFF.h
@@ -82,7 +82,6 @@ class MCSectionCOFF final : public MCSection {
                             raw_ostream &OS,
                             uint32_t Subsection) const override;
   bool useCodeAlign() const override;
-  StringRef getVirtualSectionKind() const override;
 
   unsigned getOrAssignWinCFISectionID(unsigned *NextID) const {
     if (WinCFISectionID == ~0U)
diff --git a/llvm/include/llvm/MC/MCSectionELF.h b/llvm/include/llvm/MC/MCSectionELF.h
index f09d30591a3cf..a01f95f44c954 100644
--- a/llvm/include/llvm/MC/MCSectionELF.h
+++ b/llvm/include/llvm/MC/MCSectionELF.h
@@ -88,7 +88,6 @@ class MCSectionELF final : public MCSection {
                             raw_ostream &OS,
                             uint32_t Subsection) const override;
   bool useCodeAlign() const override;
-  StringRef getVirtualSectionKind() const override;
 
   bool isUnique() const { return UniqueID != NonUniqueID; }
   unsigned getUniqueID() const { return UniqueID; }
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 0f87cf8f13a59..07566707424f9 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -427,8 +427,12 @@ class LLVM_ABI MCStreamer {
   }
 
   MCFragment *getCurrentFragment() const {
+    // Ensure consistency with the section stack.
     assert(!getCurrentSection().first ||
            CurFrag->getParent() == getCurrentSection().first);
+    // Ensure we eagerly allocate an empty fragment when adding fragment with a
+    // variable-size tail.
+    assert(!CurFrag || CurFrag->getKind() == MCFragment::FT_Data);
     return CurFrag;
   }
   /// Save the current and previous section on the section stack.
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 674adc92257c9..9943d56d2deaa 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -567,15 +567,37 @@ void MCAssembler::writeSectionData(raw_ostream &OS,
     // not tracked for efficiency.
     auto Fn = [](char c) { return c != 0; };
     for (const MCFragment &F : *Sec) {
-      if (any_of(F.getContents(), Fn) || any_of(F.getVarContents(), Fn)) {
-        reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
-                                 Sec->getName() +
+      bool HasNonZero = false;
+      switch (F.getKind()) {
+      default:
+        reportFatalInternalError("BSS section '" + Sec->getName() +
+                                 "' contains invalid fragment");
+        break;
+      case MCFragment::FT_Data:
+      case MCFragment::FT_Relaxable:
+        HasNonZero =
+            any_of(F.getContents(), Fn) || any_of(F.getVarContents(), Fn);
+        break;
+      case MCFragment::FT_Align:
+        // Disallowed for API usage. AsmParser changes non-zero fill values to
+        // 0.
+        assert(F.getAlignFill() == 0 && "Invalid align in virtual section!");
+        break;
+      case MCFragment::FT_Fill:
+        HasNonZero = cast<MCFillFragment>(F).getValue() != 0;
+        break;
+      case MCFragment::FT_Org:
+        HasNonZero = cast<MCOrgFragment>(F).getValue() != 0;
+        break;
+      }
+      if (HasNonZero) {
+        reportError(SMLoc(), "BSS section '" + Sec->getName() +
                                  "' cannot have non-zero bytes");
         break;
       }
       if (F.getFixups().size() || F.getVarFixups().size()) {
-        reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
-                                 Sec->getName() + "' cannot have fixups");
+        reportError(SMLoc(),
+                    "BSS section '" + Sec->getName() + "' cannot have fixups");
         break;
       }
     }
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 83e447abb7e06..4c99010700fc7 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -119,7 +119,7 @@ void MCObjectStreamer::emitCFISections(bool EH, bool Debug) {
 void MCObjectStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
                                      SMLoc Loc) {
   MCStreamer::emitValueImpl(Value, Size, Loc);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
 
   MCDwarfLineEntry::make(this, getCurrentSectionOnly());
 
@@ -168,7 +168,7 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
   // 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 = getOrCreateDataFragment();
+  MCFragment *F = getCurrentFragment();
   Symbol->setFragment(F);
   Symbol->setOffset(F->getContents().size());
 
@@ -202,7 +202,7 @@ void MCObjectStreamer::emitULEB128Value(const MCExpr *Value) {
     emitULEB128IntValue(IntValue);
     return;
   }
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->makeLEB(false, Value);
   newFragment();
 }
@@ -213,7 +213,7 @@ void MCObjectStreamer::emitSLEB128Value(const MCExpr *Value) {
     emitSLEB128IntValue(IntValue);
     return;
   }
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->makeLEB(true, Value);
   newFragment();
 }
@@ -312,7 +312,7 @@ void MCObjectStreamer::emitInstruction(const MCInst &Inst,
 
 void MCObjectStreamer::emitInstToData(const MCInst &Inst,
                                       const MCSubtargetInfo &STI) {
-  MCFragment *F = getOrCreateDataFragment();
+  MCFragment *F = getCurrentFragment();
 
   // Append the instruction to the data fragment.
   size_t FixupStartIndex = F->getFixups().size();
@@ -344,7 +344,7 @@ void MCObjectStreamer::emitInstToData(const MCInst &Inst,
 
 void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
                                           const MCSubtargetInfo &STI) {
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   SmallVector<char, 16> Data;
   SmallVector<MCFixup, 1> Fixups;
   getAssembler().getEmitter().encodeInstruction(Inst, Data, Fixups, STI);
@@ -417,7 +417,7 @@ void MCObjectStreamer::emitDwarfAdvanceLineAddr(int64_t LineDelta,
     return;
   }
 
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->Kind = MCFragment::FT_Dwarf;
   F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, SMLoc()));
   F->setDwarfLineDelta(LineDelta);
@@ -449,7 +449,7 @@ void MCObjectStreamer::emitDwarfLineEndEntry(MCSection *Section,
 void MCObjectStreamer::emitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
                                                  const MCSymbol *Label,
                                                  SMLoc Loc) {
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->Kind = MCFragment::FT_DwarfFrame;
   F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, Loc));
   newFragment();
@@ -511,7 +511,7 @@ void MCObjectStreamer::emitCVFileChecksumOffsetDirective(unsigned FileNo) {
 
 void MCObjectStreamer::emitBytes(StringRef Data) {
   MCDwarfLineEntry::make(this, getCurrentSectionOnly());
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   DF->appendContents(ArrayRef(Data.data(), Data.size()));
 }
 
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index 77bf84364c5a3..6f07bd553bba0 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -3406,9 +3406,8 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, uint8_t ValueSize) {
 
   if (HasFillExpr && FillExpr != 0 && Section->isVirtualSection()) {
     ReturnVal |=
-        Warning(FillExprLoc, "ignoring non-zero fill value in " +
-                                 Section->getVirtualSectionKind() +
-                                 " section '" + Section->getName() + "'");
+        Warning(FillExprLoc, "ignoring non-zero fill value in BSS section '" +
+                                 Section->getName() + "'");
     FillExpr = 0;
   }
 
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 93671450c0c2f..251412d99efbf 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -34,8 +34,6 @@ MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
 
 bool MCSection::hasEnded() const { return End && End->isInSection(); }
 
-StringRef MCSection::getVirtualSectionKind() const { return "virtual"; }
-
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void MCSection::dump(
     DenseMap<const MCFragment *, SmallVector<const MCSymbol *, 0>> *FragToSyms)
diff --git a/llvm/lib/MC/MCSectionCOFF.cpp b/llvm/lib/MC/MCSectionCOFF.cpp
index 94e29ce27d881..5bf14735eda9b 100644
--- a/llvm/lib/MC/MCSectionCOFF.cpp
+++ b/llvm/lib/MC/MCSectionCOFF.cpp
@@ -115,7 +115,3 @@ void MCSectionCOFF::printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
 }
 
 bool MCSectionCOFF::useCodeAlign() const { return isText(); }
-
-StringRef MCSectionCOFF::getVirtualSectionKind() const {
-  return "IMAGE_SCN_CNT_UNINITIALIZED_DATA";
-}
diff --git a/llvm/lib/MC/MCSectionELF.cpp b/llvm/lib/MC/MCSectionELF.cpp
index 299fe40706e3a..ef33f9c314579 100644
--- a/llvm/lib/MC/MCSectionELF.cpp
+++ b/llvm/lib/MC/MCSectionELF.cpp
@@ -215,5 +215,3 @@ void MCSectionELF::printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
 bool MCSectionELF::useCodeAlign() const {
   return getFlags() & ELF::SHF_EXECINSTR;
 }
-
-StringRef MCSectionELF::getVirtualSectionKind() const { return "SHT_NOBITS"; }
diff --git a/llvm/lib/MC/MCWin64EH.cpp b/llvm/lib/MC/MCWin64EH.cpp
index e8b26bf291ee4..a1c049aaed9cb 100644
--- a/llvm/lib/MC/MCWin64EH.cpp
+++ b/llvm/lib/MC/MCWin64EH.cpp
@@ -318,7 +318,7 @@ static void EmitUnwindInfo(MCStreamer &streamer, WinEH::FrameInfo *info) {
 
   // Emit the epilog instructions.
   if (EnableUnwindV2) {
-    MCFragment *DF = OS->getOrCreateDataFragment();
+    MCFragment *DF = OS->getCurrentFragment();
 
     bool IsLast = true;
     for (const auto &Epilog : llvm::reverse(info->EpilogMap)) {
diff --git a/llvm/lib/MC/MCWinCOFFStreamer.cpp b/llvm/lib/MC/MCWinCOFFStreamer.cpp
index 3398775df3f91..370280eb87cfd 100644
--- a/llvm/lib/MC/MCWinCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCWinCOFFStreamer.cpp
@@ -278,7 +278,7 @@ void MCWinCOFFStreamer::emitCOFFSymbolIndex(MCSymbol const *Symbol) {
 
 void MCWinCOFFStreamer::emitCOFFSectionIndex(const MCSymbol *Symbol) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   const MCSymbolRefExpr *SRE = MCSymbolRefExpr::create(Symbol, getContext());
   MCFixup Fixup = MCFixup::create(DF->getContents().size(), SRE, FK_SecRel_2);
   DF->addFixup(Fixup);
@@ -288,7 +288,7 @@ void MCWinCOFFStreamer::emitCOFFSectionIndex(const MCSymbol *Symbol) {
 void MCWinCOFFStreamer::emitCOFFSecRel32(const MCSymbol *Symbol,
                                          uint64_t Offset) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol A for the relocation relative reference.
   const MCExpr *MCE = MCSymbolRefExpr::create(Symbol, getContext());
   // Add the constant offset, if given.
@@ -306,7 +306,7 @@ void MCWinCOFFStreamer::emitCOFFSecRel32(const MCSymbol *Symbol,
 void MCWinCOFFStreamer::emitCOFFImgRel32(const MCSymbol *Symbol,
                                          int64_t Offset) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol A for the relocation relative reference.
   const MCExpr *MCE = MCSymbolRefExpr::create(
       Symbol, MCSymbolRefExpr::VK_COFF_IMGREL32, getContext());
@@ -324,7 +324,7 @@ void MCWinCOFFStreamer::emitCOFFImgRel32(const MCSymbol *Symbol,
 
 void MCWinCOFFStreamer::emitCOFFSecNumber(MCSymbol const *Symbol) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol for section number.
   const MCExpr *MCE = MCCOFFSectionNumberTargetExpr::create(
       *Symbol, this->getWriter(), getContext());
@@ -338,7 +338,7 @@ void MCWinCOFFStreamer::emitCOFFSecNumber(MCSymbol const *Symbol) {
 
 void MCWinCOFFStreamer::emitCOFFSecOffset(MCSymbol const *Symbol) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol for section offset.
   const MCExpr *MCE =
       MCCOFFSectionOffsetTargetExpr::create(*Symbol, getContext());
diff --git a/llvm/lib/MC/MCXCOFFStreamer.cpp b/llvm/lib/MC/MCXCOFFStreamer.cpp
index 4d4529653aba9..63381b4f81859 100644
--- a/llvm/lib/MC/MCXCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCXCOFFStreamer.cpp
@@ -89,7 +89,7 @@ void MCXCOFFStreamer::emitXCOFFSymbolLinkageWithVisibility(
 void MCXCOFFStreamer::emitXCOFFRefDirective(const MCSymbol *Symbol) {
   // Add a Fixup here to later record a relocation of type R_REF to prevent the
   // ref symbol from being garbage collected (by the binder).
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   const MCSymbolRefExpr *SRE = MCSymbolRefExpr::create(Symbol, getContext());
   std::optional<MCFixupKind> MaybeKind =
       getAssembler().getBackend().getFixupKind("R_REF");
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
index eaba6fe5bfcb7..a7a9911de2d04 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
@@ -593,7 +593,7 @@ class ARMELFStreamer : public MCELFStreamer {
         getContext().reportError(Loc, "relocated expression must be 32-bit");
         return;
       }
-      getOrCreateDataFragment();
+      getCurrentFragment();
     }
 
     emitDataMappingSymbol();
@@ -1207,7 +1207,7 @@ inline void ARMELFStreamer::SwitchToExIdxSection(const MCSymbol &FnStart) {
 }
 
 void ARMELFStreamer::EmitFixup(const MCExpr *Expr, MCFixupKind Kind) {
-  MCFragment *Frag = getOrCreateDataFragment();
+  MCFragment *Frag = getCurrentFragment();
   Frag->addFixup(MCFixup::create(Frag->getContents().size(), Expr, Kind));
 }
 
@@ -1295,7 +1295,7 @@ void ARMELFStreamer::EmitPersonalityFixup(StringRef Name) {
       MCSymbolRefExpr::create(PersonalitySym, ARM::S_ARM_NONE, getContext());
 
   visitUsedExpr(*PersonalityRef);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   DF->addFixup(
       MCFixup::create(DF->getContents().size(), PersonalityRef, FK_Data_4));
 }
diff --git a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
index b89d6890903dd..5c7f34b870847 100644
--- a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
+++ b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
@@ -1033,42 +1033,42 @@ MCELFStreamer &MipsTargetELFStreamer::getStreamer() {
 }
 
 void MipsTargetELFStreamer::emitGPRel32Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_GPREL32));
   DF->appendContents(4, 0);
 }
 
 void MipsTargetELFStreamer::emitGPRel64Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_GPREL32));
   DF->appendContents(8, 0);
 }
 
 void MipsTargetELFStreamer::emitDTPRel32Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_DTPREL32));
   DF->appendContents(4, 0);
 }
 
 void MipsTargetELFStreamer::emitDTPRel64Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_DTPREL64));
   DF->appendContents(8, 0);
 }
 
 void MipsTargetELFStreamer::emitTPRel32Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_TPREL32));
   DF->appendContents(4, 0);
 }
 
 void MipsTargetELFStreamer::emitTPRel64Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_TPREL64));
   DF->appendContents(8, 0);
diff --git a/llvm/test/MC/COFF/bss-text.s b/llvm/test/MC/COFF/bss-text.s
index 439bd789dff2c..cedbb2f032236 100644
--- a/llvm/test/MC/COFF/bss-text.s
+++ b/llvm/test/MC/COFF/bss-text.s
@@ -4,11 +4,11 @@
 # RUN: llvm-mc -triple=x86_64-pc-win32 %s
 
 .bss
-# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section '.bss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section '.bss' cannot have non-zero bytes
   addb %bl,(%rax)
 
 .section uninitialized,"b"
-# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section 'uninitialized' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section 'uninitialized' cannot have non-zero bytes
   jmp foo
 
 .section bss0,"b"
diff --git a/llvm/test/MC/ELF/nobits-non-zero-value.s b/llvm/test/MC/ELF/nobits-non-zero-value.s
index e9516aa3e835a..ea95ec97ac8d2 100644
--- a/llvm/test/MC/ELF/nobits-non-zero-value.s
+++ b/llvm/test/MC/ELF/nobits-non-zero-value.s
@@ -9,20 +9,37 @@
 .bss
   addb %al,(%rax)
 
-# CHECK: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in SHT_NOBITS section '.bss'
+# CHECK: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in BSS section '.bss'
 .align 4, 42
 
-.align 4, 0
-
   .long 1
 
 .section .bss0,"aw",%nobits
 addb %al,(%rax)
 
-.section .bss1,"aw",%nobits
+.section data_fixup,"aw",%nobits
 .quad foo
 
+.section fill,"aw",%nobits
+.fill b-a,1,1
+
+.section org,"aw",%nobits
+.org 1,1
+
+.section ok,"aw",%nobits
+.org 1
+.fill 1
+.fill b-a,1,0
+.align 4, 0
+.long 0
+
+.text
+a: nop
+b:
+
 ## Location is not tracked for efficiency.
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.tbss' cannot have non-zero bytes
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss' cannot have non-zero bytes
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss1' cannot have fixups
+# CHECK: <unknown>:0: error: BSS section '.tbss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section '.bss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section 'data_fixup' cannot have fixups
+#...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jul 20, 2025

@llvm/pr-subscribers-backend-mips

Author: Fangrui Song (MaskRay)

Changes

Replace getOrCreateDataFragment with getCurrentFragment, adding an assert to ensure CurFrag is either null or an FT_Data fragment.

Fix fragment-in-BSS check to handle non-zero fill values for .fill and .org directives.
Restore the fragment type check
(5ee34ff removed a reachable llvm_unreachable) to detect unintended API usage.

Remove virtual functions getVirtualSectionKind (added in
https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics.
The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, and ELF object file formats.

The two changes are unrelated. Will land them separately.


Patch is 20.15 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149721.diff

18 Files Affected:

  • (modified) llvm/include/llvm/MC/MCObjectStreamer.h (-3)
  • (modified) llvm/include/llvm/MC/MCSection.h (-2)
  • (modified) llvm/include/llvm/MC/MCSectionCOFF.h (-1)
  • (modified) llvm/include/llvm/MC/MCSectionELF.h (-1)
  • (modified) llvm/include/llvm/MC/MCStreamer.h (+4)
  • (modified) llvm/lib/MC/MCAssembler.cpp (+27-5)
  • (modified) llvm/lib/MC/MCObjectStreamer.cpp (+9-9)
  • (modified) llvm/lib/MC/MCParser/AsmParser.cpp (+2-3)
  • (modified) llvm/lib/MC/MCSection.cpp (-2)
  • (modified) llvm/lib/MC/MCSectionCOFF.cpp (-4)
  • (modified) llvm/lib/MC/MCSectionELF.cpp (-2)
  • (modified) llvm/lib/MC/MCWin64EH.cpp (+1-1)
  • (modified) llvm/lib/MC/MCWinCOFFStreamer.cpp (+5-5)
  • (modified) llvm/lib/MC/MCXCOFFStreamer.cpp (+1-1)
  • (modified) llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp (+3-3)
  • (modified) llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp (+6-6)
  • (modified) llvm/test/MC/COFF/bss-text.s (+2-2)
  • (modified) llvm/test/MC/ELF/nobits-non-zero-value.s (+24-7)
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index 633334ee8a33f..429a1bb3321f6 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -72,9 +72,6 @@ class MCObjectStreamer : public MCStreamer {
   MCSymbol *emitCFILabel() override;
   void emitCFISections(bool EH, bool Debug) override;
 
-  // TODO: Change callers to use getCurrentFragment instead.
-  MCFragment *getOrCreateDataFragment() { return getCurrentFragment(); }
-
 protected:
   bool changeSectionImpl(MCSection *Section, uint32_t Subsection);
 
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index fe8c13d108c2c..72751f08e497b 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -179,8 +179,6 @@ class LLVM_ABI MCSection {
   /// Check whether this section is "virtual", that is has no actual object
   /// file contents.
   bool isVirtualSection() const { return IsVirtual; }
-
-  virtual StringRef getVirtualSectionKind() const;
 };
 
 // Represents a contiguous piece of code or data within a section. Its size is
diff --git a/llvm/include/llvm/MC/MCSectionCOFF.h b/llvm/include/llvm/MC/MCSectionCOFF.h
index 4472a128caa6b..f979413a3791e 100644
--- a/llvm/include/llvm/MC/MCSectionCOFF.h
+++ b/llvm/include/llvm/MC/MCSectionCOFF.h
@@ -82,7 +82,6 @@ class MCSectionCOFF final : public MCSection {
                             raw_ostream &OS,
                             uint32_t Subsection) const override;
   bool useCodeAlign() const override;
-  StringRef getVirtualSectionKind() const override;
 
   unsigned getOrAssignWinCFISectionID(unsigned *NextID) const {
     if (WinCFISectionID == ~0U)
diff --git a/llvm/include/llvm/MC/MCSectionELF.h b/llvm/include/llvm/MC/MCSectionELF.h
index f09d30591a3cf..a01f95f44c954 100644
--- a/llvm/include/llvm/MC/MCSectionELF.h
+++ b/llvm/include/llvm/MC/MCSectionELF.h
@@ -88,7 +88,6 @@ class MCSectionELF final : public MCSection {
                             raw_ostream &OS,
                             uint32_t Subsection) const override;
   bool useCodeAlign() const override;
-  StringRef getVirtualSectionKind() const override;
 
   bool isUnique() const { return UniqueID != NonUniqueID; }
   unsigned getUniqueID() const { return UniqueID; }
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 0f87cf8f13a59..07566707424f9 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -427,8 +427,12 @@ class LLVM_ABI MCStreamer {
   }
 
   MCFragment *getCurrentFragment() const {
+    // Ensure consistency with the section stack.
     assert(!getCurrentSection().first ||
            CurFrag->getParent() == getCurrentSection().first);
+    // Ensure we eagerly allocate an empty fragment when adding fragment with a
+    // variable-size tail.
+    assert(!CurFrag || CurFrag->getKind() == MCFragment::FT_Data);
     return CurFrag;
   }
   /// Save the current and previous section on the section stack.
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 674adc92257c9..9943d56d2deaa 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -567,15 +567,37 @@ void MCAssembler::writeSectionData(raw_ostream &OS,
     // not tracked for efficiency.
     auto Fn = [](char c) { return c != 0; };
     for (const MCFragment &F : *Sec) {
-      if (any_of(F.getContents(), Fn) || any_of(F.getVarContents(), Fn)) {
-        reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
-                                 Sec->getName() +
+      bool HasNonZero = false;
+      switch (F.getKind()) {
+      default:
+        reportFatalInternalError("BSS section '" + Sec->getName() +
+                                 "' contains invalid fragment");
+        break;
+      case MCFragment::FT_Data:
+      case MCFragment::FT_Relaxable:
+        HasNonZero =
+            any_of(F.getContents(), Fn) || any_of(F.getVarContents(), Fn);
+        break;
+      case MCFragment::FT_Align:
+        // Disallowed for API usage. AsmParser changes non-zero fill values to
+        // 0.
+        assert(F.getAlignFill() == 0 && "Invalid align in virtual section!");
+        break;
+      case MCFragment::FT_Fill:
+        HasNonZero = cast<MCFillFragment>(F).getValue() != 0;
+        break;
+      case MCFragment::FT_Org:
+        HasNonZero = cast<MCOrgFragment>(F).getValue() != 0;
+        break;
+      }
+      if (HasNonZero) {
+        reportError(SMLoc(), "BSS section '" + Sec->getName() +
                                  "' cannot have non-zero bytes");
         break;
       }
       if (F.getFixups().size() || F.getVarFixups().size()) {
-        reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
-                                 Sec->getName() + "' cannot have fixups");
+        reportError(SMLoc(),
+                    "BSS section '" + Sec->getName() + "' cannot have fixups");
         break;
       }
     }
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 83e447abb7e06..4c99010700fc7 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -119,7 +119,7 @@ void MCObjectStreamer::emitCFISections(bool EH, bool Debug) {
 void MCObjectStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
                                      SMLoc Loc) {
   MCStreamer::emitValueImpl(Value, Size, Loc);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
 
   MCDwarfLineEntry::make(this, getCurrentSectionOnly());
 
@@ -168,7 +168,7 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
   // 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 = getOrCreateDataFragment();
+  MCFragment *F = getCurrentFragment();
   Symbol->setFragment(F);
   Symbol->setOffset(F->getContents().size());
 
@@ -202,7 +202,7 @@ void MCObjectStreamer::emitULEB128Value(const MCExpr *Value) {
     emitULEB128IntValue(IntValue);
     return;
   }
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->makeLEB(false, Value);
   newFragment();
 }
@@ -213,7 +213,7 @@ void MCObjectStreamer::emitSLEB128Value(const MCExpr *Value) {
     emitSLEB128IntValue(IntValue);
     return;
   }
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->makeLEB(true, Value);
   newFragment();
 }
@@ -312,7 +312,7 @@ void MCObjectStreamer::emitInstruction(const MCInst &Inst,
 
 void MCObjectStreamer::emitInstToData(const MCInst &Inst,
                                       const MCSubtargetInfo &STI) {
-  MCFragment *F = getOrCreateDataFragment();
+  MCFragment *F = getCurrentFragment();
 
   // Append the instruction to the data fragment.
   size_t FixupStartIndex = F->getFixups().size();
@@ -344,7 +344,7 @@ void MCObjectStreamer::emitInstToData(const MCInst &Inst,
 
 void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
                                           const MCSubtargetInfo &STI) {
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   SmallVector<char, 16> Data;
   SmallVector<MCFixup, 1> Fixups;
   getAssembler().getEmitter().encodeInstruction(Inst, Data, Fixups, STI);
@@ -417,7 +417,7 @@ void MCObjectStreamer::emitDwarfAdvanceLineAddr(int64_t LineDelta,
     return;
   }
 
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->Kind = MCFragment::FT_Dwarf;
   F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, SMLoc()));
   F->setDwarfLineDelta(LineDelta);
@@ -449,7 +449,7 @@ void MCObjectStreamer::emitDwarfLineEndEntry(MCSection *Section,
 void MCObjectStreamer::emitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
                                                  const MCSymbol *Label,
                                                  SMLoc Loc) {
-  auto *F = getOrCreateDataFragment();
+  auto *F = getCurrentFragment();
   F->Kind = MCFragment::FT_DwarfFrame;
   F->setDwarfAddrDelta(buildSymbolDiff(*this, Label, LastLabel, Loc));
   newFragment();
@@ -511,7 +511,7 @@ void MCObjectStreamer::emitCVFileChecksumOffsetDirective(unsigned FileNo) {
 
 void MCObjectStreamer::emitBytes(StringRef Data) {
   MCDwarfLineEntry::make(this, getCurrentSectionOnly());
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   DF->appendContents(ArrayRef(Data.data(), Data.size()));
 }
 
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index 77bf84364c5a3..6f07bd553bba0 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -3406,9 +3406,8 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, uint8_t ValueSize) {
 
   if (HasFillExpr && FillExpr != 0 && Section->isVirtualSection()) {
     ReturnVal |=
-        Warning(FillExprLoc, "ignoring non-zero fill value in " +
-                                 Section->getVirtualSectionKind() +
-                                 " section '" + Section->getName() + "'");
+        Warning(FillExprLoc, "ignoring non-zero fill value in BSS section '" +
+                                 Section->getName() + "'");
     FillExpr = 0;
   }
 
diff --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 93671450c0c2f..251412d99efbf 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -34,8 +34,6 @@ MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
 
 bool MCSection::hasEnded() const { return End && End->isInSection(); }
 
-StringRef MCSection::getVirtualSectionKind() const { return "virtual"; }
-
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void MCSection::dump(
     DenseMap<const MCFragment *, SmallVector<const MCSymbol *, 0>> *FragToSyms)
diff --git a/llvm/lib/MC/MCSectionCOFF.cpp b/llvm/lib/MC/MCSectionCOFF.cpp
index 94e29ce27d881..5bf14735eda9b 100644
--- a/llvm/lib/MC/MCSectionCOFF.cpp
+++ b/llvm/lib/MC/MCSectionCOFF.cpp
@@ -115,7 +115,3 @@ void MCSectionCOFF::printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
 }
 
 bool MCSectionCOFF::useCodeAlign() const { return isText(); }
-
-StringRef MCSectionCOFF::getVirtualSectionKind() const {
-  return "IMAGE_SCN_CNT_UNINITIALIZED_DATA";
-}
diff --git a/llvm/lib/MC/MCSectionELF.cpp b/llvm/lib/MC/MCSectionELF.cpp
index 299fe40706e3a..ef33f9c314579 100644
--- a/llvm/lib/MC/MCSectionELF.cpp
+++ b/llvm/lib/MC/MCSectionELF.cpp
@@ -215,5 +215,3 @@ void MCSectionELF::printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
 bool MCSectionELF::useCodeAlign() const {
   return getFlags() & ELF::SHF_EXECINSTR;
 }
-
-StringRef MCSectionELF::getVirtualSectionKind() const { return "SHT_NOBITS"; }
diff --git a/llvm/lib/MC/MCWin64EH.cpp b/llvm/lib/MC/MCWin64EH.cpp
index e8b26bf291ee4..a1c049aaed9cb 100644
--- a/llvm/lib/MC/MCWin64EH.cpp
+++ b/llvm/lib/MC/MCWin64EH.cpp
@@ -318,7 +318,7 @@ static void EmitUnwindInfo(MCStreamer &streamer, WinEH::FrameInfo *info) {
 
   // Emit the epilog instructions.
   if (EnableUnwindV2) {
-    MCFragment *DF = OS->getOrCreateDataFragment();
+    MCFragment *DF = OS->getCurrentFragment();
 
     bool IsLast = true;
     for (const auto &Epilog : llvm::reverse(info->EpilogMap)) {
diff --git a/llvm/lib/MC/MCWinCOFFStreamer.cpp b/llvm/lib/MC/MCWinCOFFStreamer.cpp
index 3398775df3f91..370280eb87cfd 100644
--- a/llvm/lib/MC/MCWinCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCWinCOFFStreamer.cpp
@@ -278,7 +278,7 @@ void MCWinCOFFStreamer::emitCOFFSymbolIndex(MCSymbol const *Symbol) {
 
 void MCWinCOFFStreamer::emitCOFFSectionIndex(const MCSymbol *Symbol) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   const MCSymbolRefExpr *SRE = MCSymbolRefExpr::create(Symbol, getContext());
   MCFixup Fixup = MCFixup::create(DF->getContents().size(), SRE, FK_SecRel_2);
   DF->addFixup(Fixup);
@@ -288,7 +288,7 @@ void MCWinCOFFStreamer::emitCOFFSectionIndex(const MCSymbol *Symbol) {
 void MCWinCOFFStreamer::emitCOFFSecRel32(const MCSymbol *Symbol,
                                          uint64_t Offset) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol A for the relocation relative reference.
   const MCExpr *MCE = MCSymbolRefExpr::create(Symbol, getContext());
   // Add the constant offset, if given.
@@ -306,7 +306,7 @@ void MCWinCOFFStreamer::emitCOFFSecRel32(const MCSymbol *Symbol,
 void MCWinCOFFStreamer::emitCOFFImgRel32(const MCSymbol *Symbol,
                                          int64_t Offset) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol A for the relocation relative reference.
   const MCExpr *MCE = MCSymbolRefExpr::create(
       Symbol, MCSymbolRefExpr::VK_COFF_IMGREL32, getContext());
@@ -324,7 +324,7 @@ void MCWinCOFFStreamer::emitCOFFImgRel32(const MCSymbol *Symbol,
 
 void MCWinCOFFStreamer::emitCOFFSecNumber(MCSymbol const *Symbol) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol for section number.
   const MCExpr *MCE = MCCOFFSectionNumberTargetExpr::create(
       *Symbol, this->getWriter(), getContext());
@@ -338,7 +338,7 @@ void MCWinCOFFStreamer::emitCOFFSecNumber(MCSymbol const *Symbol) {
 
 void MCWinCOFFStreamer::emitCOFFSecOffset(MCSymbol const *Symbol) {
   visitUsedSymbol(*Symbol);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   // Create Symbol for section offset.
   const MCExpr *MCE =
       MCCOFFSectionOffsetTargetExpr::create(*Symbol, getContext());
diff --git a/llvm/lib/MC/MCXCOFFStreamer.cpp b/llvm/lib/MC/MCXCOFFStreamer.cpp
index 4d4529653aba9..63381b4f81859 100644
--- a/llvm/lib/MC/MCXCOFFStreamer.cpp
+++ b/llvm/lib/MC/MCXCOFFStreamer.cpp
@@ -89,7 +89,7 @@ void MCXCOFFStreamer::emitXCOFFSymbolLinkageWithVisibility(
 void MCXCOFFStreamer::emitXCOFFRefDirective(const MCSymbol *Symbol) {
   // Add a Fixup here to later record a relocation of type R_REF to prevent the
   // ref symbol from being garbage collected (by the binder).
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   const MCSymbolRefExpr *SRE = MCSymbolRefExpr::create(Symbol, getContext());
   std::optional<MCFixupKind> MaybeKind =
       getAssembler().getBackend().getFixupKind("R_REF");
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
index eaba6fe5bfcb7..a7a9911de2d04 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
@@ -593,7 +593,7 @@ class ARMELFStreamer : public MCELFStreamer {
         getContext().reportError(Loc, "relocated expression must be 32-bit");
         return;
       }
-      getOrCreateDataFragment();
+      getCurrentFragment();
     }
 
     emitDataMappingSymbol();
@@ -1207,7 +1207,7 @@ inline void ARMELFStreamer::SwitchToExIdxSection(const MCSymbol &FnStart) {
 }
 
 void ARMELFStreamer::EmitFixup(const MCExpr *Expr, MCFixupKind Kind) {
-  MCFragment *Frag = getOrCreateDataFragment();
+  MCFragment *Frag = getCurrentFragment();
   Frag->addFixup(MCFixup::create(Frag->getContents().size(), Expr, Kind));
 }
 
@@ -1295,7 +1295,7 @@ void ARMELFStreamer::EmitPersonalityFixup(StringRef Name) {
       MCSymbolRefExpr::create(PersonalitySym, ARM::S_ARM_NONE, getContext());
 
   visitUsedExpr(*PersonalityRef);
-  MCFragment *DF = getOrCreateDataFragment();
+  MCFragment *DF = getCurrentFragment();
   DF->addFixup(
       MCFixup::create(DF->getContents().size(), PersonalityRef, FK_Data_4));
 }
diff --git a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
index b89d6890903dd..5c7f34b870847 100644
--- a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
+++ b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
@@ -1033,42 +1033,42 @@ MCELFStreamer &MipsTargetELFStreamer::getStreamer() {
 }
 
 void MipsTargetELFStreamer::emitGPRel32Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_GPREL32));
   DF->appendContents(4, 0);
 }
 
 void MipsTargetELFStreamer::emitGPRel64Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_GPREL32));
   DF->appendContents(8, 0);
 }
 
 void MipsTargetELFStreamer::emitDTPRel32Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_DTPREL32));
   DF->appendContents(4, 0);
 }
 
 void MipsTargetELFStreamer::emitDTPRel64Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_DTPREL64));
   DF->appendContents(8, 0);
 }
 
 void MipsTargetELFStreamer::emitTPRel32Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_TPREL32));
   DF->appendContents(4, 0);
 }
 
 void MipsTargetELFStreamer::emitTPRel64Value(const MCExpr *Value) {
-  MCFragment *DF = getStreamer().getOrCreateDataFragment();
+  MCFragment *DF = getStreamer().getCurrentFragment();
   DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
                                Mips::fixup_Mips_TPREL64));
   DF->appendContents(8, 0);
diff --git a/llvm/test/MC/COFF/bss-text.s b/llvm/test/MC/COFF/bss-text.s
index 439bd789dff2c..cedbb2f032236 100644
--- a/llvm/test/MC/COFF/bss-text.s
+++ b/llvm/test/MC/COFF/bss-text.s
@@ -4,11 +4,11 @@
 # RUN: llvm-mc -triple=x86_64-pc-win32 %s
 
 .bss
-# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section '.bss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section '.bss' cannot have non-zero bytes
   addb %bl,(%rax)
 
 .section uninitialized,"b"
-# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section 'uninitialized' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section 'uninitialized' cannot have non-zero bytes
   jmp foo
 
 .section bss0,"b"
diff --git a/llvm/test/MC/ELF/nobits-non-zero-value.s b/llvm/test/MC/ELF/nobits-non-zero-value.s
index e9516aa3e835a..ea95ec97ac8d2 100644
--- a/llvm/test/MC/ELF/nobits-non-zero-value.s
+++ b/llvm/test/MC/ELF/nobits-non-zero-value.s
@@ -9,20 +9,37 @@
 .bss
   addb %al,(%rax)
 
-# CHECK: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in SHT_NOBITS section '.bss'
+# CHECK: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in BSS section '.bss'
 .align 4, 42
 
-.align 4, 0
-
   .long 1
 
 .section .bss0,"aw",%nobits
 addb %al,(%rax)
 
-.section .bss1,"aw",%nobits
+.section data_fixup,"aw",%nobits
 .quad foo
 
+.section fill,"aw",%nobits
+.fill b-a,1,1
+
+.section org,"aw",%nobits
+.org 1,1
+
+.section ok,"aw",%nobits
+.org 1
+.fill 1
+.fill b-a,1,0
+.align 4, 0
+.long 0
+
+.text
+a: nop
+b:
+
 ## Location is not tracked for efficiency.
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.tbss' cannot have non-zero bytes
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss' cannot have non-zero bytes
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss1' cannot have fixups
+# CHECK: <unknown>:0: error: BSS section '.tbss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section '.bss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: BSS section 'data_fixup' cannot have fixups
+#...
[truncated]

MaskRay referenced this pull request Jul 20, 2025
... by eagerly allocating an empty fragment when adding a fragment
with a variable-size tail.

X86AsmBackend, The JCC erratum mitigation and x86-pad-for-align set a
flag for FT_Relaxable, which needs to be moved to emitInstructionBegin.
```
  if (CF->getKind() == MCFragment::FT_Relaxable)
    CF->setAllowAutoPadding(canPadInst(Inst, OS));
```

Follow-up to #148544
MaskRay referenced this pull request Jul 20, 2025
Move the FT_Relaxable-in-BSS check from frequently-called
MCObjectStreamer::emitInstruction to MCAssembler::writeSectionData,
along with existing checks for other fragment types. For the uncommon
diagnostics, losing the location information is acceptable.
@MaskRay
Copy link
Member Author

MaskRay commented Jul 20, 2025

I'll rename isVirtualSection IsVirtual separately. The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, ELF, and WebAssembly object file formats. LLVM should not create a term "virtual" that is confusing and not used elsewhere.

Copy link
Contributor

@aengelke aengelke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

MaskRay added a commit that referenced this pull request Jul 20, 2025
Add an assert to ensure `CurFrag` is either null or an `FT_Data` fragment.

Follow-up to 39c8cfb.
Extracted from #149721
@MaskRay MaskRay changed the title MC: Clean up code related to fragments MC: Fix fragment-in-BSS check Jul 20, 2025
@MaskRay MaskRay merged commit 673e542 into main Jul 20, 2025
7 of 9 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/mc-clean-up-code-related-to-fragments branch July 20, 2025 18:11
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 20, 2025
* Handle non-zero fill values for `.fill` and `.org` directives.
* Restore the fragment type check
  (5ee34ff removed a reachable
  `llvm_unreachable`) to detect unintended API usage.

Remove virtual functions `getVirtualSectionKind` (added in
https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics.
The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, and ELF object file formats.

Pull Request: llvm/llvm-project#149721
cachemeifyoucan pushed a commit to cachemeifyoucan/llvm-project that referenced this pull request Jul 25, 2025
Add an assert to ensure `CurFrag` is either null or an `FT_Data` fragment.

Follow-up to 39c8cfb.
Extracted from llvm#149721
cachemeifyoucan pushed a commit to cachemeifyoucan/llvm-project that referenced this pull request Jul 25, 2025
* Handle non-zero fill values for `.fill` and `.org` directives.
* Restore the fragment type check
  (5ee34ff removed a reachable
  `llvm_unreachable`) to detect unintended API usage.

Remove virtual functions `getVirtualSectionKind` (added in
https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics.
The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, and ELF object file formats.

Pull Request: llvm#149721
cachemeifyoucan pushed a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2025
Add an assert to ensure `CurFrag` is either null or an `FT_Data` fragment.

Follow-up to 39c8cfb.
Extracted from llvm#149721
cachemeifyoucan pushed a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2025
* Handle non-zero fill values for `.fill` and `.org` directives.
* Restore the fragment type check
  (5ee34ff removed a reachable
  `llvm_unreachable`) to detect unintended API usage.

Remove virtual functions `getVirtualSectionKind` (added in
https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics.
The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, and ELF object file formats.

Pull Request: llvm#149721
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
Add an assert to ensure `CurFrag` is either null or an `FT_Data` fragment.

Follow-up to 39c8cfb.
Extracted from llvm#149721
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
* Handle non-zero fill values for `.fill` and `.org` directives.
* Restore the fragment type check
  (5ee34ff removed a reachable
  `llvm_unreachable`) to detect unintended API usage.

Remove virtual functions `getVirtualSectionKind` (added in
https://reviews.llvm.org/D78138) as they are unnecessary in diagnostics.
The a.out object file format has the BSS concept, which has been
inherited by COFF, XCOFF, Mach-O, and ELF object file formats.

Pull Request: llvm#149721
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants