[llvm] a2fef66 - Revert "MCFragment: Use trailing data for fixed-size part"
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 3 23:18:56 PDT 2025
Author: Fangrui Song
Date: 2025-08-03T23:18:51-07:00
New Revision: a2fef664c29a53bfa8a66927fcf8b2e5c9da4876
URL: https://github.com/llvm/llvm-project/commit/a2fef664c29a53bfa8a66927fcf8b2e5c9da4876
DIFF: https://github.com/llvm/llvm-project/commit/a2fef664c29a53bfa8a66927fcf8b2e5c9da4876.diff
LOG: Revert "MCFragment: Use trailing data for fixed-size part"
This reverts commit f1aa6050bd90f8ec4273da55d362e23905ad3a81 (reland of #150846),
fixing conflicts.
It caused https://github.com/ClangBuiltLinux/linux/issues/2116 ,
which surfaced after a subsequent commit faa931b717c02d57f0814caa9133219040e6a85b 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.
Added:
Modified:
llvm/include/llvm/MC/MCObjectStreamer.h
llvm/include/llvm/MC/MCSection.h
llvm/lib/MC/MCObjectStreamer.cpp
llvm/lib/MC/MCWin64EH.cpp
llvm/lib/MC/MCWinCOFFStreamer.cpp
llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
Removed:
llvm/test/MC/ELF/many-instructions.s
################################################################################
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index b9e813b9b0d28..23d422ee7260f 100644
--- 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
index 4022ea7763e50..493bd1167a3a4 100644
--- 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
index bcc77c06d1fa1..db63f198ade16 100644
--- 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,21 +261,6 @@ 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)
@@ -340,16 +268,13 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
// 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
index a87648afde7d6..72a8dd7031198 100644
--- 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
index a45936bebf0c1..67baba75c3118 100644
--- 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
index 5df70c4675c0c..4056724ff189a 100644
--- a/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
+++ b/llvm/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp
@@ -1034,14 +1034,12 @@ MCELFStreamer &MipsTargetELFStreamer::getStreamer() {
void MipsTargetELFStreamer::emitGPRel32Value(const MCExpr *Value) {
auto &S = getStreamer();
- S.ensureHeadroom(4);
S.addFixup(Value, Mips::fixup_Mips_GPREL32);
S.appendContents(4, 0);
}
void MipsTargetELFStreamer::emitGPRel64Value(const MCExpr *Value) {
auto &S = getStreamer();
- S.ensureHeadroom(8);
// fixup_Mips_GPREL32 desginates R_MIPS_GPREL32+R_MIPS_64 on MIPS64.
S.addFixup(Value, Mips::fixup_Mips_GPREL32);
S.appendContents(8, 0);
@@ -1049,28 +1047,24 @@ void MipsTargetELFStreamer::emitGPRel64Value(const MCExpr *Value) {
void MipsTargetELFStreamer::emitDTPRel32Value(const MCExpr *Value) {
auto &S = getStreamer();
- S.ensureHeadroom(4);
S.addFixup(Value, Mips::fixup_Mips_DTPREL32);
S.appendContents(4, 0);
}
void MipsTargetELFStreamer::emitDTPRel64Value(const MCExpr *Value) {
auto &S = getStreamer();
- S.ensureHeadroom(8);
S.addFixup(Value, Mips::fixup_Mips_DTPREL64);
S.appendContents(8, 0);
}
void MipsTargetELFStreamer::emitTPRel32Value(const MCExpr *Value) {
auto &S = getStreamer();
- S.ensureHeadroom(4);
S.addFixup(Value, Mips::fixup_Mips_TPREL32);
S.appendContents(4, 0);
}
void MipsTargetELFStreamer::emitTPRel64Value(const MCExpr *Value) {
auto &S = getStreamer();
- S.ensureHeadroom(8);
S.addFixup(Value, Mips::fixup_Mips_TPREL64);
S.appendContents(8, 0);
}
diff --git a/llvm/test/MC/ELF/many-instructions.s b/llvm/test/MC/ELF/many-instructions.s
deleted file mode 100644
index cbdb2a71680d4..0000000000000
--- a/llvm/test/MC/ELF/many-instructions.s
+++ /dev/null
@@ -1,10 +0,0 @@
-# REQUIRES: asserts
-# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null -debug-only=mc-dump
-
-## Test that encodeInstruction may cause a new fragment to be created.
-# CHECK: 0 Data Size:16200
-# CHECK: 16200 Data Size:180
-
-.rept 16384/10
-movabsq $foo, %rax
-.endr
More information about the llvm-commits
mailing list