Revert "MCFragment: Use trailing data for fixed-size part"

MaskRay · MaskRay · commit a2fef664c29a · 2025-08-03T23:18:51.000-07:00
This reverts commit f1aa605 (reland of #150846), fixing conflicts. It caused ClangBuiltLinux/linux#2116 , which surfaced after a subsequent commit faa931b decreased sizeof(MCFragment). ``` % /tmp/Debug/bin/clang "-cc1as" "-triple" "aarch64" "-filetype" "obj" "-main-file-name" "a.s" "-o" "a.o" "a.s" clang: /home/ray/llvm/llvm/lib/MC/MCAssembler.cpp:615: void llvm::MCAssembler::writeSectionData(raw_ostream &, const MCSection *) const: Assertion `getContext().hadError() || OS.tell() - Start == getSectionAddressSize(*Sec)' failed. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump: 0. Program arguments: /tmp/Debug/bin/clang -cc1as -triple aarch64 -filetype obj -main-file-name a.s -o a.o a.s Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 libLLVMSupport.so.22.0git 0x00007cf91eb753cd llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 61 fish: Job 1, '/tmp/Debug/bin/clang "-cc1as" "…' terminated by signal SIGABRT (Abort) ``` The test is sensitive to precise fragment offsets. Using llvm-mc -filetype=obj -triple=aarch64 a.s does not replicate the issue. However, clang -cc1as includes an unnecessary `initSection` (adding an extra FT_Align), which causes the problem.
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -52,9 +52,6 @@ class MCObjectStreamer : public MCStreamer {
   DenseMap<const MCSymbol *, SmallVector<PendingAssignment, 1>>
       pendingAssignments;
 
-  SmallVector<std::unique_ptr<uint8_t[]>, 0> FragStorage;
-  // Available bytes in the current block for trailing data or new fragments.
-  size_t FragSpace = 0;
   // Used to allocate special fragments that do not use MCFragment's fixed-size
   // part.
   BumpPtrAllocator SpecialFragAllocator;
@@ -89,28 +86,25 @@ class MCObjectStreamer : public MCStreamer {
   /// \name MCStreamer Interface
   /// @{
 
-  uint8_t *getCurFragEnd() const {
-    return reinterpret_cast<uint8_t *>(CurFrag + 1) + CurFrag->getFixedSize();
-  }
-  MCFragment *allocFragSpace(size_t Headroom);
   // Add a new fragment to the current section without a variable-size tail.
   void newFragment();
 
+  template <typename FT, typename... Args> FT *allocFragment(Args &&...args) {
+    auto *F = new (SpecialFragAllocator.Allocate(sizeof(FT), alignof(FT)))
+        FT(std::forward<Args>(args)...);
+    return F;
+  }
   // Add a new special fragment to the current section and start a new empty
   // fragment.
   template <typename FT, typename... Args>
   FT *newSpecialFragment(Args &&...args) {
-    auto *F = new (SpecialFragAllocator.Allocate(sizeof(FT), alignof(FT)))
-        FT(std::forward<Args>(args)...);
+    auto *F = allocFragment<FT, Args...>(std::forward<Args>(args)...);
     addSpecialFragment(F);
     return F;
   }
 
-  void ensureHeadroom(size_t Headroom);
   void appendContents(ArrayRef<char> Contents);
-  void appendContents(size_t Num, uint8_t Elt);
-  // Add a fixup to the current fragment. Call ensureHeadroom beforehand to
-  // ensure the fixup and appended content apply to the same fragment.
+  void appendContents(size_t Num, char Elt);
   void addFixup(const MCExpr *Value, MCFixupKind Kind);
 
   void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
@@ -93,10 +93,10 @@ class MCFragment {
   bool AllowAutoPadding : 1;
 
   // Track content and fixups for the fixed-size part as fragments are
-  // appended to the section. The content is stored as trailing data of the
-  // MCFragment. The content remains immutable, except when modified by
-  // applyFixup.
-  uint32_t FixedSize = 0;
+  // appended to the section. The content remains immutable, except when
+  // modified by applyFixup.
+  uint32_t ContentStart = 0;
+  uint32_t ContentEnd = 0;
   uint32_t FixupStart = 0;
   uint32_t FixupEnd = 0;
 
@@ -189,6 +189,18 @@ class MCFragment {
   //== Content-related functions manage parent's storage using ContentStart and
   // ContentSize.
 
+  // Get a SmallVector reference. The caller should call doneAppending to update
+  // `ContentEnd`.
+  SmallVectorImpl<char> &getContentsForAppending();
+  void doneAppending();
+  void appendContents(ArrayRef<char> Contents) {
+    getContentsForAppending().append(Contents.begin(), Contents.end());
+    doneAppending();
+  }
+  void appendContents(size_t Num, char Elt) {
+    getContentsForAppending().append(Num, Elt);
+    doneAppending();
+  }
   MutableArrayRef<char> getContents();
   ArrayRef<char> getContents() const;
 
@@ -197,10 +209,10 @@ class MCFragment {
   MutableArrayRef<char> getVarContents();
   ArrayRef<char> getVarContents() const;
 
-  size_t getFixedSize() const { return FixedSize; }
+  size_t getFixedSize() const { return ContentEnd - ContentStart; }
   size_t getVarSize() const { return VarContentEnd - VarContentStart; }
   size_t getSize() const {
-    return FixedSize + (VarContentEnd - VarContentStart);
+    return ContentEnd - ContentStart + (VarContentEnd - VarContentStart);
   }
 
   //== Fixup-related functions manage parent's storage using FixupStart and
@@ -615,11 +627,28 @@ class LLVM_ABI MCSection {
   bool isBssSection() const { return IsBss; }
 };
 
+inline SmallVectorImpl<char> &MCFragment::getContentsForAppending() {
+  SmallVectorImpl<char> &S = getParent()->ContentStorage;
+  if (LLVM_UNLIKELY(ContentEnd != S.size())) {
+    // Move the elements to the end. Reserve space to avoid invalidating
+    // S.begin()+I for `append`.
+    auto Size = ContentEnd - ContentStart;
+    auto I = std::exchange(ContentStart, S.size());
+    S.reserve(S.size() + Size);
+    S.append(S.begin() + I, S.begin() + I + Size);
+  }
+  return S;
+}
+inline void MCFragment::doneAppending() {
+  ContentEnd = getParent()->ContentStorage.size();
+}
 inline MutableArrayRef<char> MCFragment::getContents() {
-  return {reinterpret_cast<char *>(this + 1), FixedSize};
+  return MutableArrayRef(getParent()->ContentStorage)
+      .slice(ContentStart, ContentEnd - ContentStart);
 }
 inline ArrayRef<char> MCFragment::getContents() const {
-  return {reinterpret_cast<const char *>(this + 1), FixedSize};
+  return ArrayRef(getParent()->ContentStorage)
+      .slice(ContentStart, ContentEnd - ContentStart);
 }
 
 inline MutableArrayRef<char> MCFragment::getVarContents() {
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -46,79 +46,23 @@ MCAssembler *MCObjectStreamer::getAssemblerPtr() {
   return nullptr;
 }
 
-constexpr size_t FragBlockSize = 16384;
-// Ensure the new fragment can at least store a few bytes.
-constexpr size_t NewFragHeadroom = 8;
-
-static_assert(NewFragHeadroom >= alignof(MCFragment));
-static_assert(FragBlockSize >= sizeof(MCFragment) + NewFragHeadroom);
-
-MCFragment *MCObjectStreamer::allocFragSpace(size_t Headroom) {
-  auto Size = std::max(FragBlockSize, sizeof(MCFragment) + Headroom);
-  FragSpace = Size - sizeof(MCFragment);
-  auto Block = std::unique_ptr<uint8_t[]>(new uint8_t[Size]);
-  auto *F = reinterpret_cast<MCFragment *>(Block.get());
-  FragStorage.push_back(std::move(Block));
-  return F;
-}
-
 void MCObjectStreamer::newFragment() {
-  MCFragment *F;
-  if (LLVM_LIKELY(sizeof(MCFragment) + NewFragHeadroom <= FragSpace)) {
-    auto End = reinterpret_cast<size_t>(getCurFragEnd());
-    F = reinterpret_cast<MCFragment *>(
-        alignToPowerOf2(End, alignof(MCFragment)));
-    FragSpace -= size_t(F) - End + sizeof(MCFragment);
-  } else {
-    F = allocFragSpace(0);
-  }
-  new (F) MCFragment();
-  addFragment(F);
-}
-
-void MCObjectStreamer::ensureHeadroom(size_t Headroom) {
-  if (Headroom <= FragSpace)
-    return;
-  auto *F = allocFragSpace(Headroom);
-  new (F) MCFragment();
-  addFragment(F);
+  addFragment(allocFragment<MCFragment>());
 }
 
 void MCObjectStreamer::addSpecialFragment(MCFragment *Frag) {
   assert(Frag->getKind() != MCFragment::FT_Data &&
-         "F should have a variable-size tail");
-  // Frag is not connected to FragSpace. Before modifying CurFrag with
-  // addFragment(Frag), allocate an empty fragment to maintain FragSpace
-  // connectivity, potentially reusing CurFrag's associated space.
-  MCFragment *F;
-  if (LLVM_LIKELY(sizeof(MCFragment) + NewFragHeadroom <= FragSpace)) {
-    auto End = reinterpret_cast<size_t>(getCurFragEnd());
-    F = reinterpret_cast<MCFragment *>(
-        alignToPowerOf2(End, alignof(MCFragment)));
-    FragSpace -= size_t(F) - End + sizeof(MCFragment);
-  } else {
-    F = allocFragSpace(0);
-  }
-  new (F) MCFragment();
-
+         "Frag should have a variable-size tail");
   addFragment(Frag);
-  addFragment(F);
+  newFragment();
 }
 
 void MCObjectStreamer::appendContents(ArrayRef<char> Contents) {
-  ensureHeadroom(Contents.size());
-  assert(FragSpace >= Contents.size());
-  llvm::copy(Contents, getCurFragEnd());
-  CurFrag->FixedSize += Contents.size();
-  FragSpace -= Contents.size();
+  CurFrag->appendContents(Contents);
 }
 
-void MCObjectStreamer::appendContents(size_t Num, uint8_t Elt) {
-  ensureHeadroom(Num);
-  MutableArrayRef<uint8_t> Data(getCurFragEnd(), Num);
-  llvm::fill(Data, Elt);
-  CurFrag->FixedSize += Num;
-  FragSpace -= Num;
+void MCObjectStreamer::appendContents(size_t Num, char Elt) {
+  CurFrag->appendContents(Num, Elt);
 }
 
 void MCObjectStreamer::addFixup(const MCExpr *Value, MCFixupKind Kind) {
@@ -171,8 +115,6 @@ void MCObjectStreamer::reset() {
   }
   EmitEHFrame = true;
   EmitDebugFrame = false;
-  FragStorage.clear();
-  FragSpace = 0;
   SpecialFragAllocator.Reset();
   MCStreamer::reset();
 }
@@ -202,6 +144,7 @@ void MCObjectStreamer::emitCFISections(bool EH, bool Debug, bool SFrame) {
 void MCObjectStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
                                      SMLoc Loc) {
   MCStreamer::emitValueImpl(Value, Size, Loc);
+  MCFragment *DF = getCurrentFragment();
 
   MCDwarfLineEntry::make(this, getCurrentSectionOnly());
 
@@ -216,9 +159,9 @@ void MCObjectStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
     emitIntValue(AbsValue, Size);
     return;
   }
-  ensureHeadroom(Size);
-  addFixup(Value, MCFixup::getDataKindForSize(Size));
-  appendContents(Size, 0);
+  DF->addFixup(MCFixup::create(DF->getContents().size(), Value,
+                               MCFixup::getDataKindForSize(Size)));
+  DF->appendContents(Size, 0);
 }
 
 MCSymbol *MCObjectStreamer::emitCFILabel() {
@@ -252,7 +195,7 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
   // section.
   MCFragment *F = CurFrag;
   Symbol->setFragment(F);
-  Symbol->setOffset(F->getFixedSize());
+  Symbol->setOffset(F->getContents().size());
 
   emitPendingAssignments(Symbol);
 }
@@ -318,38 +261,20 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
     F0 = CurFrag;
   }
 
-  // To maintain connectivity between CurFrag and FragSpace when CurFrag is
-  // modified, allocate an empty fragment and append it to the fragment list.
-  // (Subsections[I].second.Tail is not connected to FragSpace.)
-  MCFragment *F;
-  if (LLVM_LIKELY(sizeof(MCFragment) + NewFragHeadroom <= FragSpace)) {
-    auto End = reinterpret_cast<size_t>(getCurFragEnd());
-    F = reinterpret_cast<MCFragment *>(
-        alignToPowerOf2(End, alignof(MCFragment)));
-    FragSpace -= size_t(F) - End + sizeof(MCFragment);
-  } else {
-    F = allocFragSpace(0);
-  }
-  new (F) MCFragment();
-  F->setParent(Section);
-
   auto &Subsections = Section->Subsections;
   size_t I = 0, E = Subsections.size();
   while (I != E && Subsections[I].first < Subsection)
     ++I;
   // If the subsection number is not in the sorted Subsections list, create a
   // new fragment list.
   if (I == E || Subsections[I].first != Subsection) {
+    auto *F = allocFragment<MCFragment>();
+    F->setParent(Section);
     Subsections.insert(Subsections.begin() + I,
                        {Subsection, MCSection::FragList{F, F}});
-    Section->CurFragList = &Subsections[I].second;
-    CurFrag = F;
-  } else {
-    Section->CurFragList = &Subsections[I].second;
-    CurFrag = Subsections[I].second.Tail;
-    // Ensure CurFrag is associated with FragSpace.
-    addFragment(F);
   }
+  Section->CurFragList = &Subsections[I].second;
+  CurFrag = Section->CurFragList->Tail;
 
   // Define the section symbol at subsection 0's initial fragment if required.
   if (!NewSec)
@@ -420,15 +345,11 @@ void MCObjectStreamer::emitInstToData(const MCInst &Inst,
   MCFragment *F = getCurrentFragment();
 
   // Append the instruction to the data fragment.
-  size_t CodeOffset = getCurFragSize();
-  SmallString<16> Content;
+  size_t CodeOffset = F->getContents().size();
   SmallVector<MCFixup, 1> Fixups;
-  getAssembler().getEmitter().encodeInstruction(Inst, Content, Fixups, STI);
-  appendContents(Content);
-  if (CurFrag != F) {
-    F = CurFrag;
-    CodeOffset = 0;
-  }
+  getAssembler().getEmitter().encodeInstruction(
+      Inst, F->getContentsForAppending(), Fixups, STI);
+  F->doneAppending();
   F->setHasInstructions(STI);
 
   if (Fixups.empty())
diff --git a/llvm/lib/MC/MCWin64EH.cpp b/llvm/lib/MC/MCWin64EH.cpp
@@ -318,9 +318,6 @@ static void EmitUnwindInfo(MCStreamer &streamer, WinEH::FrameInfo *info) {
 
   // Emit the epilog instructions.
   if (EnableUnwindV2) {
-    // Ensure the fixups and appended content apply to the same fragment.
-    OS->ensureHeadroom(info->EpilogMap.size() * 2);
-
     bool IsLast = true;
     for (const auto &Epilog : llvm::reverse(info->EpilogMap)) {
       if (IsLast) {
diff --git a/llvm/lib/MC/MCWinCOFFStreamer.cpp b/llvm/lib/MC/MCWinCOFFStreamer.cpp
@@ -279,7 +279,6 @@ void MCWinCOFFStreamer::emitCOFFSymbolIndex(MCSymbol const *Symbol) {
 void MCWinCOFFStreamer::emitCOFFSectionIndex(const MCSymbol *Symbol) {
   visitUsedSymbol(*Symbol);
   const MCSymbolRefExpr *SRE = MCSymbolRefExpr::create(Symbol, getContext());
-  ensureHeadroom(2);
   addFixup(SRE, FK_SecRel_2);
   appendContents(2, 0);
 }
@@ -293,7 +292,6 @@ void MCWinCOFFStreamer::emitCOFFSecRel32(const MCSymbol *Symbol,
   if (Offset)
     MCE = MCBinaryExpr::createAdd(
         MCE, MCConstantExpr::create(Offset, getContext()), getContext());
-  ensureHeadroom(4);
   addFixup(MCE, FK_SecRel_4);
   // Emit 4 bytes (zeros) to the object file.
   appendContents(4, 0);
@@ -309,7 +307,6 @@ void MCWinCOFFStreamer::emitCOFFImgRel32(const MCSymbol *Symbol,
   if (Offset)
     MCE = MCBinaryExpr::createAdd(
         MCE, MCConstantExpr::create(Offset, getContext()), getContext());
-  ensureHeadroom(4);
   addFixup(MCE, FK_Data_4);
   // Emit 4 bytes (zeros) to the object file.
   appendContents(4, 0);
@@ -320,7 +317,6 @@ void MCWinCOFFStreamer::emitCOFFSecNumber(MCSymbol const *Symbol) {
   // Create Symbol for section number.
   const MCExpr *MCE = MCCOFFSectionNumberTargetExpr::create(
       *Symbol, this->getWriter(), getContext());
-  ensureHeadroom(4);
   addFixup(MCE, FK_Data_4);
   // Emit 4 bytes (zeros) to the object file.
   appendContents(4, 0);
@@ -331,7 +327,6 @@ void MCWinCOFFStreamer::emitCOFFSecOffset(MCSymbol const *Symbol) {
   // Create Symbol for section offset.
   const MCExpr *MCE =
       MCCOFFSectionOffsetTargetExpr::create(*Symbol, getContext());
-  ensureHeadroom(4);
   addFixup(MCE, FK_Data_4);
   // Emit 4 bytes (zeros) to the object file.
   appendContents(4, 0);
diff --git a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
diff --git a/llvm/test/MC/ELF/many-instructions.s b/llvm/test/MC/ELF/many-instructions.s
