Revert "MC: Allocate initial fragment and define section symbol in changeSection" (#150736)

Reverts llvm/llvm-project#150574

This is causing a test failure on AArch64 MacOS bots:
https://lab.llvm.org/buildbot/#/builders/190/builds/24187

llvm/include/llvm/MC/MCContext.h

@@ -333,6 +333,8 @@ private:
   void reportCommon(SMLoc Loc,
                     std::function<void(SMDiagnostic &, const SourceMgr *)>);
 
+  MCFragment *allocInitialFragment(MCSection &Sec);
+
   MCSymbolTableEntry &getSymbolTableEntry(StringRef Name);
 
   MCSymbol *createSymbolImpl(const MCSymbolTableEntry *Name, bool IsTemporary);

llvm/include/llvm/MC/MCSection.h

@@ -589,8 +589,6 @@ private:
   /// offset between two locations may not be fully resolved.
   bool LinkerRelaxable : 1;
 
-  MCFragment DummyFragment;
-
   // Mapping from subsection number to fragment list. At layout time, the
   // subsection 0 list is replaced with concatenated fragments from all
   // subsections.
@@ -652,7 +650,7 @@ public:
   bool isLinkerRelaxable() const { return LinkerRelaxable; }
   void setLinkerRelaxable() { LinkerRelaxable = true; }
 
-  MCFragment &getDummyFragment() { return DummyFragment; }
+  MCFragment &getDummyFragment() { return *Subsections[0].second.Head; }
 
   FragList *curFragList() const { return CurFragList; }
   iterator begin() const { return iterator(CurFragList->Head); }

llvm/include/llvm/MC/MCXCOFFStreamer.h

@@ -22,7 +22,6 @@ public:
 
   XCOFFObjectWriter &getWriter();
 
-  void changeSection(MCSection *Section, uint32_t Subsection = 0) override;
   bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         Align ByteAlignment) override;

llvm/lib/MC/ELFObjectWriter.cpp

@@ -559,7 +559,20 @@ void ELFWriter::computeSymbolTable(const RevGroupMapTy &RevGroupMap) {
     } else {
       const MCSectionELF &Section =
           static_cast<const MCSectionELF &>(Symbol.getSection());
-      assert(Section.isRegistered());
+
+      // We may end up with a situation when section symbol is technically
+      // defined, but should not be. That happens because we explicitly
+      // pre-create few .debug_* sections to have accessors.
+      // And if these sections were not really defined in the code, but were
+      // referenced, we simply error out.
+      if (!Section.isRegistered()) {
+        assert(static_cast<const MCSymbolELF &>(Symbol).getType() ==
+               ELF::STT_SECTION);
+        Ctx.reportError(SMLoc(),
+                        "Undefined section reference: " + Symbol.getName());
+        continue;
+      }
+
       if (Mode == NonDwoOnly && isDwoSection(Section))
         continue;
       MSD.SectionIndex = Section.getOrdinal();

llvm/lib/MC/MCAssembler.cpp

@@ -106,6 +106,7 @@ void MCAssembler::reset() {
 bool MCAssembler::registerSection(MCSection &Section) {
   if (Section.isRegistered())
     return false;
+  assert(Section.curFragList()->Head && "allocInitialFragment not called");
   Sections.push_back(&Section);
   Section.setIsRegistered(true);
   return true;

llvm/lib/MC/MCContext.cpp

@@ -200,6 +200,16 @@ MCInst *MCContext::createMCInst() {
   return new (MCInstAllocator.Allocate()) MCInst;
 }
 
+// Allocate the initial MCFragment for the begin symbol.
+MCFragment *MCContext::allocInitialFragment(MCSection &Sec) {
+  assert(!Sec.curFragList()->Head);
+  auto *F = allocFragment<MCFragment>();
+  F->setParent(&Sec);
+  Sec.curFragList()->Head = F;
+  Sec.curFragList()->Tail = F;
+  return F;
+}
+
 //===----------------------------------------------------------------------===//
 // Symbol Manipulation
 //===----------------------------------------------------------------------===//
@@ -433,19 +443,17 @@ MCSymbol *MCContext::getDirectionalLocalSymbol(unsigned LocalLabelVal,
   return getOrCreateDirectionalLocalSymbol(LocalLabelVal, Instance);
 }
 
-// Create a section symbol, with a distinct one for each section of the same.
-// The first symbol is used for assembly code references.
 template <typename Symbol>
 Symbol *MCContext::getOrCreateSectionSymbol(StringRef Section) {
   Symbol *R;
   auto &SymEntry = getSymbolTableEntry(Section);
   MCSymbol *Sym = SymEntry.second.Symbol;
+  // A section symbol can not redefine regular symbols. There may be multiple
+  // sections with the same name, in which case the first such section wins.
   if (Sym && Sym->isDefined() &&
       (!Sym->isInSection() || Sym->getSection().getBeginSymbol() != Sym))
     reportError(SMLoc(), "invalid symbol redefinition");
-  // Use the symbol's index to track if it has been used as a section symbol.
-  // Set to -1 to catch potential bugs if misused as a symbol index.
-  if (Sym && Sym->getIndex() != -1u) {
+  if (Sym && Sym->isUndefined()) {
     R = cast<Symbol>(Sym);
   } else {
     SymEntry.second.Used = true;
@@ -453,8 +461,6 @@ Symbol *MCContext::getOrCreateSectionSymbol(StringRef Section) {
     if (!Sym)
       SymEntry.second.Symbol = R;
   }
-  // Mark as section symbol.
-  R->setIndex(-1u);
   return R;
 }
 
@@ -562,6 +568,7 @@ MCSectionMachO *MCContext::getMachOSection(StringRef Segment, StringRef Section,
       MCSectionMachO(Segment, Name.substr(Name.size() - Section.size()),
                      TypeAndAttributes, Reserved2, Kind, Begin);
   R.first->second = Ret;
+  allocInitialFragment(*Ret);
   return Ret;
 }
 
@@ -572,8 +579,15 @@ MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type,
                                               bool Comdat, unsigned UniqueID,
                                               const MCSymbolELF *LinkedToSym) {
   auto *R = getOrCreateSectionSymbol<MCSymbolELF>(Section);
-  return new (ELFAllocator.Allocate()) MCSectionELF(
+  R->setBinding(ELF::STB_LOCAL);
+  R->setType(ELF::STT_SECTION);
+
+  auto *Ret = new (ELFAllocator.Allocate()) MCSectionELF(
       Section, Type, Flags, EntrySize, Group, Comdat, UniqueID, R, LinkedToSym);
+
+  auto *F = allocInitialFragment(*Ret);
+  R->setFragment(F);
+  return Ret;
 }
 
 MCSectionELF *
@@ -729,6 +743,7 @@ MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
       MCSectionGOFF(CachedName, Kind, IsVirtual, Attributes,
                     static_cast<MCSectionGOFF *>(Parent));
   Iter->second = GOFFSection;
+  allocInitialFragment(*GOFFSection);
   return GOFFSection;
 }
 
@@ -783,7 +798,8 @@ MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
   MCSectionCOFF *Result = new (COFFAllocator.Allocate()) MCSectionCOFF(
       CachedName, Characteristics, COMDATSymbol, Selection, UniqueID, Begin);
   Iter->second = Result;
-  Begin->setFragment(&Result->getDummyFragment());
+  auto *F = allocInitialFragment(*Result);
+  Begin->setFragment(F);
   return Result;
 }
 
@@ -854,6 +870,8 @@ MCSectionWasm *MCContext::getWasmSection(const Twine &Section, SectionKind Kind,
       MCSectionWasm(CachedName, Kind, Flags, GroupSym, UniqueID, Begin);
   Entry.second = Result;
 
+  auto *F = allocInitialFragment(*Result);
+  Begin->setFragment(F);
   return Result;
 }
 
@@ -909,11 +927,24 @@ MCSectionXCOFF *MCContext::getXCOFFSection(
                        MultiSymbolsAllowed);
 
   Entry.second = Result;
+
+  auto *F = allocInitialFragment(*Result);
+
+  // We might miss calculating the symbols difference as absolute value before
+  // adding fixups when symbol_A without the fragment set is the csect itself
+  // and symbol_B is in it.
+  // TODO: Currently we only set the fragment for XMC_PR csects and DWARF
+  // sections because we don't have other cases that hit this problem yet.
+  if (IsDwarfSec || CsectProp->MappingClass == XCOFF::XMC_PR)
+    QualName->setFragment(F);
+
   return Result;
 }
 
 MCSectionSPIRV *MCContext::getSPIRVSection() {
   MCSectionSPIRV *Result = new (SPIRVAllocator.Allocate()) MCSectionSPIRV();
+
+  allocInitialFragment(*Result);
   return Result;
 }
 
@@ -933,6 +964,7 @@ MCSectionDXContainer *MCContext::getDXContainerSection(StringRef Section,
       new (DXCAllocator.Allocate()) MCSectionDXContainer(Name, K, nullptr);
 
   // The first fragment will store the header
+  allocInitialFragment(*MapIt->second);
   return MapIt->second;
 }
 

llvm/lib/MC/MCELFStreamer.cpp

@@ -89,9 +89,7 @@ void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
     getWriter().markGnuAbi();
 
   MCObjectStreamer::changeSection(Section, Subsection);
-  auto *Sym = static_cast<MCSymbolELF *>(Section->getBeginSymbol());
-  Sym->setBinding(ELF::STB_LOCAL);
-  Sym->setType(ELF::STT_SECTION);
+  Asm.registerSymbol(*Section->getBeginSymbol());
 }
 
 void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) {

llvm/lib/MC/MCMachOStreamer.cpp

@@ -140,8 +140,6 @@ void MCMachOStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
     MCSymbol *Label = getContext().createLinkerPrivateTempSymbol();
     Section->setBeginSymbol(Label);
     HasSectionLabel[Section] = true;
-    if (!Label->isInSection())
-      emitLabel(Label);
   }
 }
 

llvm/lib/MC/MCObjectStreamer.cpp

@@ -185,10 +185,10 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
 
   getAssembler().registerSymbol(*Symbol);
 
-  // Set the fragment and offset. This function might be called by
-  // changeSection, when the section stack top hasn't been changed to the new
-  // section.
-  MCFragment *F = CurFrag;
+  // If there is a current fragment, mark the symbol as pointing into it.
+  // Otherwise queue the label and set its fragment pointer when we emit the
+  // next fragment.
+  MCFragment *F = getCurrentFragment();
   Symbol->setFragment(F);
   Symbol->setOffset(F->getContents().size());
 
@@ -247,15 +247,6 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   assert(Section && "Cannot switch to a null section!");
   getContext().clearDwarfLocSeen();
 
-  // Register the section and create an initial fragment for subsection 0
-  // if `Subsection` is non-zero.
-  bool NewSec = getAssembler().registerSection(*Section);
-  MCFragment *F0 = nullptr;
-  if (NewSec && Subsection) {
-    changeSection(Section, 0);
-    F0 = CurFrag;
-  }
-
   auto &Subsections = Section->Subsections;
   size_t I = 0, E = Subsections.size();
   while (I != E && Subsections[I].first < Subsection)
@@ -271,13 +262,7 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   Section->CurFragList = &Subsections[I].second;
   CurFrag = Section->CurFragList->Tail;
 
-  // Define the section symbol at subsection 0's initial fragment if required.
-  if (!NewSec)
-    return;
-  if (auto *Sym = Section->getBeginSymbol()) {
-    Sym->setFragment(Subsection ? F0 : CurFrag);
-    getAssembler().registerSymbol(*Sym);
-  }
+  getAssembler().registerSection(*Section);
 }
 
 void MCObjectStreamer::switchSectionNoPrint(MCSection *Section) {

llvm/lib/MC/MCSection.cpp

@@ -22,7 +22,8 @@ MCSection::MCSection(SectionVariant V, StringRef Name, bool IsText, bool IsBss,
                      MCSymbol *Begin)
     : Begin(Begin), HasInstructions(false), IsRegistered(false), IsText(IsText),
       IsBss(IsBss), LinkerRelaxable(false), Name(Name), Variant(V) {
-  DummyFragment.setParent(this);
+  // The initial subsection number is 0. Create a fragment list.
+  CurFragList = &Subsections.emplace_back(0u, FragList{}).second;
 }
 
 MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {

llvm/lib/MC/MCStreamer.cpp

@@ -1314,10 +1314,8 @@ void MCStreamer::emitZerofill(MCSection *, MCSymbol *, uint64_t, Align, SMLoc) {
 }
 void MCStreamer::emitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
                                 uint64_t Size, Align ByteAlignment) {}
-void MCStreamer::changeSection(MCSection *Sec, uint32_t) {
-  CurFrag = &Sec->getDummyFragment();
-  if (auto *Sym = Sec->getBeginSymbol())
-    Sym->setFragment(&Sec->getDummyFragment());
+void MCStreamer::changeSection(MCSection *Section, uint32_t) {
+  CurFrag = &Section->getDummyFragment();
 }
 void MCStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {}
 void MCStreamer::emitBytes(StringRef Data) {}
@@ -1360,6 +1358,9 @@ void MCStreamer::switchSection(MCSection *Section, uint32_t Subsection) {
     changeSection(Section, Subsection);
     SectionStack.back().first = MCSectionSubPair(Section, Subsection);
     assert(!Section->hasEnded() && "Section already ended");
+    MCSymbol *Sym = Section->getBeginSymbol();
+    if (Sym && !Sym->isInSection())
+      emitLabel(Sym);
   }
 }
 
@@ -1386,6 +1387,9 @@ void MCStreamer::switchSectionNoPrint(MCSection *Section) {
   SectionStack.back().second = SectionStack.back().first;
   SectionStack.back().first = MCSectionSubPair(Section, 0);
   changeSection(Section, 0);
+  MCSymbol *Sym = Section->getBeginSymbol();
+  if (Sym && !Sym->isInSection())
+    emitLabel(Sym);
 }
 
 MCSymbol *MCStreamer::endSection(MCSection *Section) {

llvm/lib/MC/MCXCOFFStreamer.cpp

@@ -36,20 +36,6 @@ XCOFFObjectWriter &MCXCOFFStreamer::getWriter() {
   return static_cast<XCOFFObjectWriter &>(getAssembler().getWriter());
 }
 
-void MCXCOFFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
-  MCObjectStreamer::changeSection(Section, Subsection);
-  auto *Sec = cast<MCSectionXCOFF>(Section);
-  // We might miss calculating the symbols difference as absolute value before
-  // adding fixups when symbol_A without the fragment set is the csect itself
-  // and symbol_B is in it.
-  // TODO: Currently we only set the fragment for XMC_PR csects and DWARF
-  // sections because we don't have other cases that hit this problem yet.
-  // if (IsDwarfSec || CsectProp->MappingClass == XCOFF::XMC_PR)
-  //   QualName->setFragment(F);
-  if (Sec->isDwarfSect() || Sec->getMappingClass() == XCOFF::XMC_PR)
-    Sec->getQualNameSymbol()->setFragment(CurFrag);
-}
-
 bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym,
                                           MCSymbolAttr Attribute) {
   auto *Symbol = cast<MCSymbolXCOFF>(Sym);

llvm/test/CodeGen/XCore/section-name.ll

@@ -1,4 +1,4 @@
-; RUN: llc < %s -mtriple=xcore | FileCheck %s
+; RUN: not llc < %s -mtriple=xcore -o /dev/null 2>&1 | FileCheck %s
 
 @bar = internal global i32 zeroinitializer
 
@@ -6,4 +6,4 @@ define void @".dp.bss"() {
   ret void
 }
 
-; CHECK: .dp.bss:
+; CHECK: <unknown>:0: error: symbol '.dp.bss' is already defined

llvm/test/MC/ELF/section-sym2.s

@@ -1,40 +1,27 @@
 # RUN: llvm-mc -filetype=obj -triple x86_64 %s -o %t
-# RUN: llvm-readelf -SrsX %t | FileCheck %s
+# RUN: llvm-readelf -Srs %t | FileCheck %s
 
 ## Test that we can forward reference a section.
 
 mov .rodata, %rsi
-mov data, %rsi
 mov .debug_info, %rsi
-mov .debug_abbrev, %rsi
 
 .section .rodata,"a"
-.pushsection data, 2; .long 2; .popsection
-.section data; .long 1
 .section .debug_info,"G",@progbits,11,comdat; .long x1
 .section .debug_info,"G",@progbits,22,comdat; .long x2
 .section .debug_info,"",@progbits; .long x0
 
-.text
-mov data, %rdi
-
-# CHECK:      Relocation section '.rela.text'
-# CHECK:      R_X86_64_32S {{.*}} data + 0
-# CHECK:      R_X86_64_32S {{.*}} data + 0
-
 # CHECK:      Relocation section '.rela.debug_info' at offset {{.*}} contains 1
 # CHECK:      Relocation section '.rela.debug_info' at offset {{.*}} contains 1
 # CHECK:      Relocation section '.rela.debug_info' at offset {{.*}} contains 1
 
-# CHECK:      Symbol table '.symtab' contains 10 entries:
-# CHECK-NEXT:    Num:
+# CHECK:      Symbol table '.symtab' contains 8 entries:
+# CHECK-NEXT:    Num:    Value          Size Type    Bind   Vis       Ndx Name
 # CHECK-NEXT:  0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND
-# CHECK-NEXT:  0000000000000000     0 SECTION LOCAL  DEFAULT [[#]] (.rodata) .rodata
-# CHECK-NEXT:  0000000000000000     0 SECTION LOCAL  DEFAULT [[#]] (data) data
-# CHECK-NEXT:  0000000000000000     0 SECTION LOCAL  DEFAULT [[#]] (.debug_info) .debug_info
-# CHECK-NEXT:  0000000000000000     0 NOTYPE  LOCAL  DEFAULT [[#]] (.group) 11
-# CHECK-NEXT:  0000000000000000     0 NOTYPE  LOCAL  DEFAULT [[#]] (.group) 22
-# CHECK-NEXT:  0000000000000000     0 NOTYPE  GLOBAL DEFAULT   UND .debug_abbrev
+# CHECK-NEXT:  0000000000000000     0 SECTION LOCAL  DEFAULT     4 .rodata
+# CHECK-NEXT:  0000000000000000     0 SECTION LOCAL  DEFAULT    11 .debug_info
+# CHECK-NEXT:  0000000000000000     0 NOTYPE  LOCAL  DEFAULT     5 11
+# CHECK-NEXT:  0000000000000000     0 NOTYPE  LOCAL  DEFAULT     8 22
 # CHECK-NEXT:  0000000000000000     0 NOTYPE  GLOBAL DEFAULT   UND x1
 # CHECK-NEXT:  0000000000000000     0 NOTYPE  GLOBAL DEFAULT   UND x2
 # CHECK-NEXT:  0000000000000000     0 NOTYPE  GLOBAL DEFAULT   UND x0

llvm/test/MC/ELF/undefined-debug.s

@@ -0,0 +1,5 @@
+// RUN: not llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t 2>&1 | FileCheck %s
+// CHECK: error: Undefined section reference: .debug_pubnames
+
+.section .foo,"",@progbits
+  .long  .debug_pubnames