MC: Allocate initial fragment in changeSection

13a79bb (2017) introduced fragment
creation in MCContext for createELFSectionImpl, which was inappropriate.
Fragments should only be created when using MCSteramer, not during
`MCContext::get*Section` calls.
`initMachOMCObjectFileInfo` defines multiple sections, some of which may
not be used by the code generator. This caused symbol names matching
these sections to be incorrectly marked as undefined (see
https://reviews.llvm.org/D55173).

The fragment code was later replicated in other file formats, such as
WebAssembly (see https://reviews.llvm.org/D46561), XCOFF, and GOFF.

This patch fixes the problem by moving initial fragment allocation from
MCContext::createSection to MCStreamer::changeSection.
While MCContext still creates a section symbol, the symbol is not
attached to the initial fragment.

* test/CodeGen/XCore/section-name.ll now passes. XCore doesn't support
  MCObjectStreamer. I don't think the MCAsmStreamer output behavior
  change matters.

Author: MaskRay

llvm/include/llvm/MC/MCSection.h

@@ -589,6 +589,8 @@ class LLVM_ABI MCSection {
   /// 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.
@@ -650,7 +652,7 @@ class LLVM_ABI MCSection {
   bool isLinkerRelaxable() const { return LinkerRelaxable; }
   void setLinkerRelaxable() { LinkerRelaxable = true; }
 
-  MCFragment &getDummyFragment() { return *Subsections[0].second.Head; }
+  MCFragment &getDummyFragment() { return DummyFragment; }
 
   FragList *curFragList() const { return CurFragList; }
   iterator begin() const { return iterator(CurFragList->Head); }

llvm/include/llvm/MC/MCXCOFFStreamer.h

@@ -22,6 +22,7 @@ class MCXCOFFStreamer : public MCObjectStreamer {
 
   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

@@ -566,8 +566,6 @@ void ELFWriter::computeSymbolTable(const RevGroupMapTy &RevGroupMap) {
       // 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;

llvm/lib/MC/MCAsmStreamer.cpp

@@ -60,6 +60,7 @@ class MCAsmStreamer final : public MCStreamer {
   bool ShowInst = false;
   bool UseDwarfDirectory = false;
 
+  void changeSection(MCSection *, uint32_t) override;
   void EmitRegisterName(int64_t Register);
   void PrintQuotedString(StringRef Data, raw_ostream &OS) const;
   void printDwarfFileDirective(unsigned FileNo, StringRef Directory,
@@ -2429,6 +2430,12 @@ void MCAsmStreamer::AddEncodingComment(const MCInst &Inst,
   }
 }
 
+void MCAsmStreamer::changeSection(MCSection *Sec, uint32_t) {
+  CurFrag = &Sec->getDummyFragment();
+  if (auto *Sym = Sec->getBeginSymbol())
+    Sym->setFragment(&Sec->getDummyFragment());
+}
+
 void MCAsmStreamer::emitInstruction(const MCInst &Inst,
                                     const MCSubtargetInfo &STI) {
   if (MAI->isAIX() && CurFrag)

llvm/lib/MC/MCAssembler.cpp

@@ -106,7 +106,6 @@ 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

@@ -443,24 +443,28 @@ 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");
-  if (Sym && Sym->isUndefined()) {
+  // 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) {
     R = cast<Symbol>(Sym);
   } else {
     SymEntry.second.Used = true;
     R = new (&SymEntry, *this) Symbol(&SymEntry, /*isTemporary=*/false);
     if (!Sym)
       SymEntry.second.Symbol = R;
   }
+  // Mark as section symbol.
+  R->setIndex(-1u);
   return R;
 }
 
@@ -568,7 +572,6 @@ 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;
 }
 
@@ -579,15 +582,8 @@ MCSectionELF *MCContext::createELFSectionImpl(StringRef Section, unsigned Type,
                                               bool Comdat, unsigned UniqueID,
                                               const MCSymbolELF *LinkedToSym) {
   auto *R = getOrCreateSectionSymbol<MCSymbolELF>(Section);
-  R->setBinding(ELF::STB_LOCAL);
-  R->setType(ELF::STT_SECTION);
-
-  auto *Ret = new (ELFAllocator.Allocate()) MCSectionELF(
+  return new (ELFAllocator.Allocate()) MCSectionELF(
       Section, Type, Flags, EntrySize, Group, Comdat, UniqueID, R, LinkedToSym);
-
-  auto *F = allocInitialFragment(*Ret);
-  R->setFragment(F);
-  return Ret;
 }
 
 MCSectionELF *
@@ -743,7 +739,6 @@ MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
       MCSectionGOFF(CachedName, Kind, IsVirtual, Attributes,
                     static_cast<MCSectionGOFF *>(Parent));
   Iter->second = GOFFSection;
-  allocInitialFragment(*GOFFSection);
   return GOFFSection;
 }
 
@@ -798,8 +793,7 @@ MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
   MCSectionCOFF *Result = new (COFFAllocator.Allocate()) MCSectionCOFF(
       CachedName, Characteristics, COMDATSymbol, Selection, UniqueID, Begin);
   Iter->second = Result;
-  auto *F = allocInitialFragment(*Result);
-  Begin->setFragment(F);
+  Begin->setFragment(&Result->getDummyFragment());
   return Result;
 }
 
@@ -870,8 +864,6 @@ 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;
 }
 
@@ -927,24 +919,11 @@ 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;
 }
 
@@ -964,7 +943,6 @@ 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,7 +89,10 @@ void MCELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
     getWriter().markGnuAbi();
 
   MCObjectStreamer::changeSection(Section, Subsection);
-  Asm.registerSymbol(*Section->getBeginSymbol());
+  auto *Sym = static_cast<MCSymbolELF *>(Section->getBeginSymbol());
+  Sym->setBinding(ELF::STB_LOCAL);
+  Sym->setType(ELF::STT_SECTION);
+  Asm.registerSymbol(*Sym);
 }
 
 void MCELFStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Target) {

llvm/lib/MC/MCGOFFStreamer.cpp

@@ -29,11 +29,19 @@ GOFFObjectWriter &MCGOFFStreamer::getWriter() {
 void MCGOFFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   // Make sure that all section are registered in the correct order.
   SmallVector<MCSectionGOFF *> Sections;
-  for (auto *S = static_cast<MCSectionGOFF *>(Section); S; S = S->getParent())
+  for (auto *S = static_cast<MCSectionGOFF *>(Section); (S = S->getParent());)
     Sections.push_back(S);
-  while (!Sections.empty()) {
-    auto *S = Sections.pop_back_val();
-    MCObjectStreamer::changeSection(S, Sections.empty() ? Subsection : 0);
+  while (!Sections.empty())
+    MCObjectStreamer::changeSection(Sections.pop_back_val(), 0);
+
+  bool Registered = Section->isRegistered();
+  MCObjectStreamer::changeSection(Section, Subsection);
+  if (!Registered) {
+    MCSymbol *Sym = Section->getBeginSymbol();
+    if (Sym) {
+      Sym->setFragment(nullptr);
+      emitLabel(Sym);
+    }
   }
 }
 

llvm/lib/MC/MCMachOStreamer.cpp

@@ -140,6 +140,8 @@ 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);
 
-  // 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();
+  // 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;
   Symbol->setFragment(F);
   Symbol->setOffset(F->getContents().size());
 
@@ -247,6 +247,15 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   assert(Section && "Cannot switch to a null section!");
   getContext().clearDwarfLocSeen();
 
+  // If the section was not registered before and Subsection is not zero,
+  // allocate the initial fragment for subsection 0 for the section symbol.
+  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)
@@ -262,7 +271,12 @@ void MCObjectStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
   Section->CurFragList = &Subsections[I].second;
   CurFrag = Section->CurFragList->Tail;
 
-  getAssembler().registerSection(*Section);
+  if (NewSec) {
+    if (!Subsection)
+      F0 = CurFrag;
+    if (auto *Sym = Section->getBeginSymbol())
+      Sym->setFragment(F0);
+  }
 }
 
 void MCObjectStreamer::switchSectionNoPrint(MCSection *Section) {