Skip to content

Commit 7d272b5

Browse files
dyungmahesh-attarde
authored andcommitted
Revert "MC: Allocate initial fragment and define section symbol in changeSection" (llvm#150736)
Reverts llvm#150574 This is causing a test failure on AArch64 MacOS bots: https://lab.llvm.org/buildbot/#/builders/190/builds/24187
1 parent f1d800f commit 7d272b5

15 files changed

+89
-80
lines changed

llvm/include/llvm/MC/MCContext.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,8 @@ class MCContext {
333333
void reportCommon(SMLoc Loc,
334334
std::function<void(SMDiagnostic &, const SourceMgr *)>);
335335

336+
MCFragment *allocInitialFragment(MCSection &Sec);
337+
336338
MCSymbolTableEntry &getSymbolTableEntry(StringRef Name);
337339

338340
MCSymbol *createSymbolImpl(const MCSymbolTableEntry *Name, bool IsTemporary);

llvm/include/llvm/MC/MCSection.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,6 @@ class LLVM_ABI MCSection {
589589
/// offset between two locations may not be fully resolved.
590590
bool LinkerRelaxable : 1;
591591

592-
MCFragment DummyFragment;
593-
594592
// Mapping from subsection number to fragment list. At layout time, the
595593
// subsection 0 list is replaced with concatenated fragments from all
596594
// subsections.
@@ -652,7 +650,7 @@ class LLVM_ABI MCSection {
652650
bool isLinkerRelaxable() const { return LinkerRelaxable; }
653651
void setLinkerRelaxable() { LinkerRelaxable = true; }
654652

655-
MCFragment &getDummyFragment() { return DummyFragment; }
653+
MCFragment &getDummyFragment() { return *Subsections[0].second.Head; }
656654

657655
FragList *curFragList() const { return CurFragList; }
658656
iterator begin() const { return iterator(CurFragList->Head); }

llvm/include/llvm/MC/MCXCOFFStreamer.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ class MCXCOFFStreamer : public MCObjectStreamer {
2222

2323
XCOFFObjectWriter &getWriter();
2424

25-
void changeSection(MCSection *Section, uint32_t Subsection = 0) override;
2625
bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
2726
void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
2827
Align ByteAlignment) override;

llvm/lib/MC/ELFObjectWriter.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,20 @@ void ELFWriter::computeSymbolTable(const RevGroupMapTy &RevGroupMap) {
559559
} else {
560560
const MCSectionELF &Section =
561561
static_cast<const MCSectionELF &>(Symbol.getSection());
562-
assert(Section.isRegistered());
562+
563+
// We may end up with a situation when section symbol is technically
564+
// defined, but should not be. That happens because we explicitly
565+
// pre-create few .debug_* sections to have accessors.
566+
// And if these sections were not really defined in the code, but were
567+
// referenced, we simply error out.
568+
if (!Section.isRegistered()) {
569+
assert(static_cast<const MCSymbolELF &>(Symbol).getType() ==
570+
ELF::STT_SECTION);
571+
Ctx.reportError(SMLoc(),
572+
"Undefined section reference: " + Symbol.getName());
573+
continue;
574+
}
575+
563576
if (Mode == NonDwoOnly && isDwoSection(Section))
564577
continue;
565578
MSD.SectionIndex = Section.getOrdinal();

llvm/lib/MC/MCAssembler.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ void MCAssembler::reset() {
106106
bool MCAssembler::registerSection(MCSection &Section) {
107107
if (Section.isRegistered())
108108
return false;
109+
assert(Section.curFragList()->Head && "allocInitialFragment not called");
109110
Sections.push_back(&Section);
110111
Section.setIsRegistered(true);
111112
return true;

llvm/lib/MC/MCContext.cpp

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,16 @@ MCInst *MCContext::createMCInst() {
200200
return new (MCInstAllocator.Allocate()) MCInst;
201201
}
202202

203+
// Allocate the initial MCFragment for the begin symbol.
204+
MCFragment *MCContext::allocInitialFragment(MCSection &Sec) {
205+
assert(!Sec.curFragList()->Head);
206+
auto *F = allocFragment<MCFragment>();
207+
F->setParent(&Sec);
208+
Sec.curFragList()->Head = F;
209+
Sec.curFragList()->Tail = F;
210+
return F;
211+
}
212+
203213
//===----------------------------------------------------------------------===//
204214
// Symbol Manipulation
205215
//===----------------------------------------------------------------------===//
@@ -433,28 +443,24 @@ MCSymbol *MCContext::getDirectionalLocalSymbol(unsigned LocalLabelVal,
433443
return getOrCreateDirectionalLocalSymbol(LocalLabelVal, Instance);
434444
}
435445

436-
// Create a section symbol, with a distinct one for each section of the same.
437-
// The first symbol is used for assembly code references.
438446
template <typename Symbol>
439447
Symbol *MCContext::getOrCreateSectionSymbol(StringRef Section) {
440448
Symbol *R;
441449
auto &SymEntry = getSymbolTableEntry(Section);
442450
MCSymbol *Sym = SymEntry.second.Symbol;
451+
// A section symbol can not redefine regular symbols. There may be multiple
452+
// sections with the same name, in which case the first such section wins.
443453
if (Sym && Sym->isDefined() &&
444454
(!Sym->isInSection() || Sym->getSection().getBeginSymbol() != Sym))
445455
reportError(SMLoc(), "invalid symbol redefinition");
446-
// Use the symbol's index to track if it has been used as a section symbol.
447-
// Set to -1 to catch potential bugs if misused as a symbol index.
448-
if (Sym && Sym->getIndex() != -1u) {
456+
if (Sym && Sym->isUndefined()) {
449457
R = cast<Symbol>(Sym);
450458
} else {
451459
SymEntry.second.Used = true;
452460
R = new (&SymEntry, *this) Symbol(&SymEntry, /*isTemporary=*/false);
453461
if (!Sym)
454462
SymEntry.second.Symbol = R;
455463
}
456-
// Mark as section symbol.
457-
R->setIndex(-1u);
458464
return R;
459465
}
460466

@@ -562,6 +568,7 @@ MCSectionMachO *MCContext::getMachOSection(StringRef Segment, StringRef Section,
562568
MCSectionMachO(Segment, Name.substr(Name.size() - Section.size()),
563569
TypeAndAttributes, Reserved2, Kind, Begin);
564570
R.first->second = Ret;
571+
allocInitialFragment(*Ret);
565572
return Ret;
566573
}
567574

@@ -572,8 +579,15 @@ MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type,
572579
bool Comdat, unsigned UniqueID,
573580
const MCSymbolELF *LinkedToSym) {
574581
auto *R = getOrCreateSectionSymbol<MCSymbolELF>(Section);
575-
return new (ELFAllocator.Allocate()) MCSectionELF(
582+
R->setBinding(ELF::STB_LOCAL);
583+
R->setType(ELF::STT_SECTION);
584+
585+
auto *Ret = new (ELFAllocator.Allocate()) MCSectionELF(
576586
Section, Type, Flags, EntrySize, Group, Comdat, UniqueID, R, LinkedToSym);
587+
588+
auto *F = allocInitialFragment(*Ret);
589+
R->setFragment(F);
590+
return Ret;
577591
}
578592

579593
MCSectionELF *
@@ -729,6 +743,7 @@ MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
729743
MCSectionGOFF(CachedName, Kind, IsVirtual, Attributes,
730744
static_cast<MCSectionGOFF *>(Parent));
731745
Iter->second = GOFFSection;
746+
allocInitialFragment(*GOFFSection);
732747
return GOFFSection;
733748
}
734749

@@ -783,7 +798,8 @@ MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
783798
MCSectionCOFF *Result = new (COFFAllocator.Allocate()) MCSectionCOFF(
784799
CachedName, Characteristics, COMDATSymbol, Selection, UniqueID, Begin);
785800
Iter->second = Result;
786-
Begin->setFragment(&Result->getDummyFragment());
801+
auto *F = allocInitialFragment(*Result);
802+
Begin->setFragment(F);
787803
return Result;
788804
}
789805

@@ -854,6 +870,8 @@ MCSectionWasm *MCContext::getWasmSection(const Twine &Section, SectionKind Kind,
854870
MCSectionWasm(CachedName, Kind, Flags, GroupSym, UniqueID, Begin);
855871
Entry.second = Result;
856872

873+
auto *F = allocInitialFragment(*Result);
874+
Begin->setFragment(F);
857875
return Result;
858876
}
859877

@@ -909,11 +927,24 @@ MCSectionXCOFF *MCContext::getXCOFFSection(
909927
MultiSymbolsAllowed);
910928

911929
Entry.second = Result;
930+
931+
auto *F = allocInitialFragment(*Result);
932+
933+
// We might miss calculating the symbols difference as absolute value before
934+
// adding fixups when symbol_A without the fragment set is the csect itself
935+
// and symbol_B is in it.
936+
// TODO: Currently we only set the fragment for XMC_PR csects and DWARF
937+
// sections because we don't have other cases that hit this problem yet.
938+
if (IsDwarfSec || CsectProp->MappingClass == XCOFF::XMC_PR)
939+
QualName->setFragment(F);
940+
912941
return Result;
913942
}
914943

915944
MCSectionSPIRV *MCContext::getSPIRVSection() {
916945
MCSectionSPIRV *Result = new (SPIRVAllocator.Allocate()) MCSectionSPIRV();
946+
947+
allocInitialFragment(*Result);
917948
return Result;
918949
}
919950

@@ -933,6 +964,7 @@ MCSectionDXContainer *MCContext::getDXContainerSection(StringRef Section,
933964
new (DXCAllocator.Allocate()) MCSectionDXContainer(Name, K, nullptr);
934965

935966
// The first fragment will store the header
967+
allocInitialFragment(*MapIt->second);
936968
return MapIt->second;
937969
}
938970

llvm/lib/MC/MCELFStreamer.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,7 @@ void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
8989
getWriter().markGnuAbi();
9090

9191
MCObjectStreamer::changeSection(Section, Subsection);
92-
auto *Sym = static_cast<MCSymbolELF *>(Section->getBeginSymbol());
93-
Sym->setBinding(ELF::STB_LOCAL);
94-
Sym->setType(ELF::STT_SECTION);
92+
Asm.registerSymbol(*Section->getBeginSymbol());
9593
}
9694

9795
void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) {

llvm/lib/MC/MCMachOStreamer.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,6 @@ void MCMachOStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
140140
MCSymbol *Label = getContext().createLinkerPrivateTempSymbol();
141141
Section->setBeginSymbol(Label);
142142
HasSectionLabel[Section] = true;
143-
if (!Label->isInSection())
144-
emitLabel(Label);
145143
}
146144
}
147145

llvm/lib/MC/MCObjectStreamer.cpp

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,10 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
185185

186186
getAssembler().registerSymbol(*Symbol);
187187

188-
// Set the fragment and offset. This function might be called by
189-
// changeSection, when the section stack top hasn't been changed to the new
190-
// section.
191-
MCFragment *F = CurFrag;
188+
// If there is a current fragment, mark the symbol as pointing into it.
189+
// Otherwise queue the label and set its fragment pointer when we emit the
190+
// next fragment.
191+
MCFragment *F = getCurrentFragment();
192192
Symbol->setFragment(F);
193193
Symbol->setOffset(F->getContents().size());
194194

@@ -247,15 +247,6 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
247247
assert(Section && "Cannot switch to a null section!");
248248
getContext().clearDwarfLocSeen();
249249

250-
// Register the section and create an initial fragment for subsection 0
251-
// if `Subsection` is non-zero.
252-
bool NewSec = getAssembler().registerSection(*Section);
253-
MCFragment *F0 = nullptr;
254-
if (NewSec && Subsection) {
255-
changeSection(Section, 0);
256-
F0 = CurFrag;
257-
}
258-
259250
auto &Subsections = Section->Subsections;
260251
size_t I = 0, E = Subsections.size();
261252
while (I != E && Subsections[I].first < Subsection)
@@ -271,13 +262,7 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
271262
Section->CurFragList = &Subsections[I].second;
272263
CurFrag = Section->CurFragList->Tail;
273264

274-
// Define the section symbol at subsection 0's initial fragment if required.
275-
if (!NewSec)
276-
return;
277-
if (auto *Sym = Section->getBeginSymbol()) {
278-
Sym->setFragment(Subsection ? F0 : CurFrag);
279-
getAssembler().registerSymbol(*Sym);
280-
}
265+
getAssembler().registerSection(*Section);
281266
}
282267

283268
void MCObjectStreamer::switchSectionNoPrint(MCSection *Section) {

llvm/lib/MC/MCSection.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsBss,
2222
MCSymbol *Begin)
2323
: Begin(Begin), HasInstructions(false), IsRegistered(false), IsText(IsText),
2424
IsBss(IsBss), LinkerRelaxable(false), Name(Name), Variant(V) {
25-
DummyFragment.setParent(this);
25+
// The initial subsection number is 0. Create a fragment list.
26+
CurFragList = &Subsections.emplace_back(0u, FragList{}).second;
2627
}
2728

2829
MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {

0 commit comments

Comments
 (0)