[MC] Replace MCSection*::getName() with MCSection::getName(). NFC

[MaskRay]

Summary

I plan to use MCSection::getName() in D78138. Having the function in the base class is also convenient for debugging.

[rnk]

llvm/include/llvm/MC/MCSectionMachO.h

25–26

This data structure choice seems like the only thing preventing us from moving SectionName to the MCSection base class, and having a vanilla, non-virtual getName accessor. Want to try that instead? The only complication is that you have to stabilize the StringRef by using a substring of the string key in the MachOUniquingMap.

[MaskRay]

Fix MCSectionMachO as suggested by @rnk

[rnk]

lgtm, just comments on comments.

llvm/include/llvm/MC/MCSection.h

104

Are we sure we are dropping .zdebug_* support? If we're not confident, maybe just say "TODO: Make this private when possible."

llvm/include/llvm/MC/MCSectionELF.h

50–51

Isn't it owned by MCContext::ELFUniquingMap? That's much less surprising, since MCContext is the class responsible for creating these objects.

63

O_O exciting

llvm/include/llvm/MC/MCSectionWasm.h

44

I think it's MCContext again, I guess the ownership changed.

[MaskRay]

Address comments

Thanks for review!

llvm/include/llvm/MC/MCSection.h

104

Thanks, I'll reword it.

grimar asked whether we can drop accepting .zdebug_* as input.

I mentioned D61689 (which was reverted because we are relying on it... Eventually it is because of some integrated assembler difficulties)

llvm/include/llvm/MC/MCSectionELF.h

50–51

Thanks! Looks like the original comment was wrong..

63

For your amusement, this is a hack only used by the .debug_* -> .zdebug_* renaming.

llvm/include/llvm/MC/MCSectionWasm.h

44

It is a WasmSectionKey element in WasmUniquingMap (which is in MCContext)

llvm/include/llvm/MC/MCSection.h

struct PendingLabel {
     MCSymbol* Sym;
     unsigned Subsection;
     PendingLabel(MCSymbol* Sym, unsigned Subsection = 0)
       : Sym(Sym), Subsection(Subsection) {}
   };
   SmallVector<PendingLabel, 2> PendingLabels;
 
 protected:
+  // TODO Make Name private when possible.
+  StringRef Name;
   SectionVariant Variant;
   SectionKind Kind;
 
-  MCSection(SectionVariant V, SectionKind K, MCSymbol *Begin);
+  MCSection(SectionVariant V, StringRef Name, SectionKind K, MCSymbol *Begin);
   ~MCSection();
 
 public:
   MCSection(const MCSection &) = delete;
   MCSection &operator=(const MCSection &) = delete;
 
+  StringRef getName() const { return Name; }
   SectionKind getKind() const { return Kind; }
 
   SectionVariant getVariant() const { return Variant; }
 
   MCSymbol *getBeginSymbol() { return Begin; }
   const MCSymbol *getBeginSymbol() const {
     return const_cast<MCSection *>(this)->getBeginSymbol();
   }

llvm/include/llvm/MC/MCSectionCOFF.h

 #include <cassert>
 
 namespace llvm {
 
 class MCSymbol;
 
 /// This represents a section on Windows
 class MCSectionCOFF final : public MCSection {
-  // The memory for this string is stored in the same MCContext as *this.
-  StringRef SectionName;
-
   // FIXME: The following fields should not be mutable, but are for now so the
   // asm parser can honor the .linkonce directive.
 
   /// This is the Characteristics field of a section, drawn from the enums
   /// below.
   mutable unsigned Characteristics;
 
   /// The unique IDs used with the .pdata and .xdata sections created internally
   /// by the assembler. This ID is used to ensure that for every .text section,
   /// there is exactly one .pdata and one .xdata section, which is required by
   /// the Microsoft incremental linker. This data is mutable because this ID is
   /// not notionally part of the section.
   mutable unsigned WinCFISectionID = ~0U;
 
   /// The COMDAT symbol of this section. Only valid if this is a COMDAT section.
   /// Two COMDAT sections are merged if they have the same COMDAT symbol.
   MCSymbol *COMDATSymbol;
 
   /// This is the Selection field for the section symbol, if it is a COMDAT
   /// section (Characteristics & IMAGE_SCN_LNK_COMDAT) != 0
   mutable int Selection;
 
 private:
   friend class MCContext;
-  MCSectionCOFF(StringRef Section, unsigned Characteristics,
+  // The storage of Name is owned by MCContext's COFFUniquingMap.
+  MCSectionCOFF(StringRef Name, unsigned Characteristics,
                 MCSymbol *COMDATSymbol, int Selection, SectionKind K,
                 MCSymbol *Begin)
-      : MCSection(SV_COFF, K, Begin), SectionName(Section),
-        Characteristics(Characteristics), COMDATSymbol(COMDATSymbol),
-        Selection(Selection) {
+      : MCSection(SV_COFF, Name, K, Begin), Characteristics(Characteristics),
+        COMDATSymbol(COMDATSymbol), Selection(Selection) {
     assert((Characteristics & 0x00F00000) == 0 &&
            "alignment must not be set upon section creation");
   }
 
 public:
   /// Decides whether a '.section' directive should be printed before the
   /// section name
   bool ShouldOmitSectionDirective(StringRef Name, const MCAsmInfo &MAI) const;
 
-  StringRef getName() const { return SectionName; }
   unsigned getCharacteristics() const { return Characteristics; }
   MCSymbol *getCOMDATSymbol() const { return COMDATSymbol; }
   int getSelection() const { return Selection; }
 
   void setSelection(int Selection) const;
 
   void PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                             raw_ostream &OS,

llvm/include/llvm/MC/MCSectionELF.h

 
 namespace llvm {
 
 class MCSymbol;
 
 /// This represents a section on linux, lots of unix variants and some bare
 /// metal systems.
 class MCSectionELF final : public MCSection {
-  /// This is the name of the section.  The referenced memory is owned by
-  /// TargetLoweringObjectFileELF's ELFUniqueMap.
-  StringRef SectionName;
-
   /// This is the sh_type field of a section, drawn from the enums below.
   unsigned Type;
 
   /// This is the sh_flags field of a section, drawn from the enums below.
   unsigned Flags;
 
   unsigned UniqueID;
 
   /// The size of each entry in this section. This size only makes sense for
   /// sections that contain fixed-sized entries. If a section does not contain
   /// fixed-sized entries 'EntrySize' will be 0.
   unsigned EntrySize;
 
   const MCSymbolELF *Group;
 
   /// Used by SHF_LINK_ORDER. If non-null, the sh_link field will be set to the
   /// section header index of the section where LinkedToSym is defined.
   const MCSymbol *LinkedToSym;
 
 private:
   friend class MCContext;
 
-  MCSectionELF(StringRef Section, unsigned type, unsigned flags, SectionKind K,
+  // The storage of Name is owned by MCContext's ELFUniquingMap.
+  MCSectionELF(StringRef Name, unsigned type, unsigned flags, SectionKind K,
                unsigned entrySize, const MCSymbolELF *group, unsigned UniqueID,
                MCSymbol *Begin, const MCSymbolELF *LinkedToSym)
-      : MCSection(SV_ELF, K, Begin), SectionName(Section), Type(type),
-        Flags(flags), UniqueID(UniqueID), EntrySize(entrySize), Group(group),
+      : MCSection(SV_ELF, Name, K, Begin), Type(type), Flags(flags),
+        UniqueID(UniqueID), EntrySize(entrySize), Group(group),
         LinkedToSym(LinkedToSym) {
     if (Group)
       Group->setIsSignature();
   }
 
-  void setSectionName(StringRef Name) { SectionName = Name; }
+  // TODO Delete after we stop supporting generation of GNU-style .zdebug_*
+  // sections.
+  void setSectionName(StringRef Name) { this->Name = Name; }
 
 public:
   /// Decides whether a '.section' directive should be printed before the
   /// section name
   bool ShouldOmitSectionDirective(StringRef Name, const MCAsmInfo &MAI) const;
 
-  StringRef getName() const { return SectionName; }
   unsigned getType() const { return Type; }
   unsigned getFlags() const { return Flags; }
   unsigned getEntrySize() const { return EntrySize; }
   void setFlags(unsigned F) { Flags = F; }
   const MCSymbolELF *getGroup() const { return Group; }
 
   void PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                             raw_ostream &OS,

llvm/include/llvm/MC/MCSectionMachO.h

 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/MC/MCSection.h"
 
 namespace llvm {
 
 /// This represents a section on a Mach-O system (used by Mac OS X).  On a Mac
 /// system, these are also described in /usr/include/mach-o/loader.h.
 class MCSectionMachO final : public MCSection {
   char SegmentName[16];  // Not necessarily null terminated!
-  char SectionName[16];  // Not necessarily null terminated!
 
   /// This is the SECTION_TYPE and SECTION_ATTRIBUTES field of a section, drawn
   /// from the enums below.
   unsigned TypeAndAttributes;
 
   /// The 'reserved2' field of a section, used to represent the size of stubs,
   /// for example.
   unsigned Reserved2;
 
   MCSectionMachO(StringRef Segment, StringRef Section, unsigned TAA,
                  unsigned reserved2, SectionKind K, MCSymbol *Begin);
   friend class MCContext;
 public:
 
   StringRef getSegmentName() const {
     // SegmentName is not necessarily null terminated!
     if (SegmentName[15])
       return StringRef(SegmentName, 16);
     return StringRef(SegmentName);
   }
-  StringRef getName() const {
-    // SectionName is not necessarily null terminated!
-    if (SectionName[15])
-      return StringRef(SectionName, 16);
-    return StringRef(SectionName);
-  }
 
   unsigned getTypeAndAttributes() const { return TypeAndAttributes; }
   unsigned getStubSize() const { return Reserved2; }
 
   MachO::SectionType getType() const {
     return static_cast<MachO::SectionType>(TypeAndAttributes &
                                            MachO::SECTION_TYPE);
   }

llvm/include/llvm/MC/MCSectionWasm.h

 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 
 class MCSymbol;
 
 /// This represents a section on wasm.
 class MCSectionWasm final : public MCSection {
-  /// This is the name of the section.  The referenced memory is owned by
-  /// TargetLoweringObjectFileWasm's WasmUniqueMap.
-  StringRef SectionName;
-
   unsigned UniqueID;
 
   const MCSymbolWasm *Group;
 
   // The offset of the MC function/data section in the wasm code/data section.
   // For data relocations the offset is relative to start of the data payload
   // itself and does not include the size of the section header.
   uint64_t SectionOffset = 0;
 
   // For data sections, this is the index of of the corresponding wasm data
   // segment
   uint32_t SegmentIndex = 0;
 
   // Whether this data segment is passive
   bool IsPassive = false;
 
+  // The storage of Name is owned by MCContext's WasmUniquingMap.
   friend class MCContext;
-  MCSectionWasm(StringRef Section, SectionKind K, const MCSymbolWasm *group,
+  MCSectionWasm(StringRef Name, SectionKind K, const MCSymbolWasm *group,
                 unsigned UniqueID, MCSymbol *Begin)
-      : MCSection(SV_Wasm, K, Begin), SectionName(Section), UniqueID(UniqueID),
-        Group(group) {}
+      : MCSection(SV_Wasm, Name, K, Begin), UniqueID(UniqueID), Group(group) {}
 
 public:
   /// Decides whether a '.section' directive should be printed before the
   /// section name
   bool shouldOmitSectionDirective(StringRef Name, const MCAsmInfo &MAI) const;
 
-  StringRef getName() const { return SectionName; }
   const MCSymbolWasm *getGroup() const { return Group; }
 
   void PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                             raw_ostream &OS,
                             const MCExpr *Subsection) const override;
   bool UseCodeAlign() const override;
   bool isVirtualSection() const override;
 

llvm/include/llvm/MC/MCSectionXCOFF.h

 // 2) Uninitialized: The Type will be XTY_CM, it will contain a single symbol,
 //    and may not contain label definitions.
 // 3) An external reference providing a symbol table entry for a symbol
 //    contained in another XCOFF object file. External reference csects are not
 //    implemented yet.
 class MCSectionXCOFF final : public MCSection {
   friend class MCContext;
 
-  StringRef Name;
   XCOFF::StorageMappingClass MappingClass;
   XCOFF::SymbolType Type;
   XCOFF::StorageClass StorageClass;
   MCSymbolXCOFF *const QualName;
 
-  MCSectionXCOFF(StringRef Section, XCOFF::StorageMappingClass SMC,
+  MCSectionXCOFF(StringRef Name, XCOFF::StorageMappingClass SMC,
                  XCOFF::SymbolType ST, XCOFF::StorageClass SC, SectionKind K,
                  MCSymbolXCOFF *QualName, MCSymbol *Begin)
-      : MCSection(SV_XCOFF, K, Begin), Name(Section), MappingClass(SMC),
-        Type(ST), StorageClass(SC), QualName(QualName) {
+      : MCSection(SV_XCOFF, Name, K, Begin), MappingClass(SMC), Type(ST),
+        StorageClass(SC), QualName(QualName) {
     assert((ST == XCOFF::XTY_SD || ST == XCOFF::XTY_CM || ST == XCOFF::XTY_ER) &&
            "Invalid or unhandled type for csect.");
     assert(QualName != nullptr && "QualName is needed.");
     QualName->setStorageClass(SC);
     QualName->setRepresentedCsect(this);
   }
 
 public:
   ~MCSectionXCOFF();
 
   static bool classof(const MCSection *S) {
     return S->getVariant() == SV_XCOFF;
   }
 
-  StringRef getName() const { return Name; }
   XCOFF::StorageMappingClass getMappingClass() const { return MappingClass; }
   XCOFF::StorageClass getStorageClass() const { return StorageClass; }
   XCOFF::SymbolType getCSectType() const { return Type; }
   MCSymbolXCOFF *getQualNameSymbol() const { return QualName; }
 
   void PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                             raw_ostream &OS,
                             const MCExpr *Subsection) const override;
   bool UseCodeAlign() const override;
   bool isVirtualSection() const override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/MC/MCAssembler.cpp

for (const MCFragment &F : *Sec) {
         // Check that we aren't trying to write a non-zero contents (or fixups)
         // into a virtual section. This is to support clients which use standard
         // directives to fill the contents of virtual sections.
         const MCDataFragment &DF = cast<MCDataFragment>(F);
         if (DF.fixup_begin() != DF.fixup_end())
           report_fatal_error("cannot have fixups in virtual section!");
         for (unsigned i = 0, e = DF.getContents().size(); i != e; ++i)
           if (DF.getContents()[i]) {
-            if (auto *ELFSec = dyn_cast<const MCSectionELF>(Sec))
-              report_fatal_error("non-zero initializer found in section '" +
-                                 ELFSec->getName() + "'");
-            else
-              report_fatal_error("non-zero initializer found in virtual section");
+                               Sec->getName() + "'");
           }
         break;
       }
       case MCFragment::FT_Align:
         // Check that we aren't trying to write a non-zero value into a virtual
         // section.
         assert((cast<MCAlignFragment>(F).getValueSize() == 0 ||
                 cast<MCAlignFragment>(F).getValue() == 0) &&

llvm/lib/MC/MCContext.cpp

MCSectionMachO *MCContext::getMachOSection(StringRef Segment, StringRef Section,
                                            unsigned TypeAndAttributes,
                                            unsigned Reserved2, SectionKind Kind,
                                            const char *BeginSymName) {
   // We unique sections by their segment/section pair.  The returned section
   // may not have the same flags as the requested section, if so this should be
   // diagnosed by the client as an error.
 
   // Form the name to look up.
-  SmallString<64> Name;
-  Name += Segment;
-  Name.push_back(',');
-  Name += Section;
+  assert(Section.size() <= 16 && "section name is too long");
+  assert(!memchr(Section.data(), '\0', Section.size()) &&
+         "section name cannot contain NUL");
 
   // Do the lookup, if we have a hit, return it.
-  MCSectionMachO *&Entry = MachOUniquingMap[Name];
-  if (Entry)
-    return Entry;
+  auto R = MachOUniquingMap.try_emplace((Segment + Twine(',') + Section).str());
+  if (!R.second)
+    return R.first->second;
 
   MCSymbol *Begin = nullptr;
   if (BeginSymName)
     Begin = createTempSymbol(BeginSymName, false);
 
   // Otherwise, return a new section.
-  return Entry = new (MachOAllocator.Allocate()) MCSectionMachO(
-             Segment, Section, TypeAndAttributes, Reserved2, Kind, Begin);
+  StringRef Name = R.first->first();
+  R.first->second = new (MachOAllocator.Allocate())
+      MCSectionMachO(Segment, Name.substr(Name.size() - Section.size()),
+                     TypeAndAttributes, Reserved2, Kind, Begin);
+  return R.first->second;
 }
 
 void MCContext::renameELFSection(MCSectionELF *Section, StringRef Name) {
   StringRef GroupName;
   if (const MCSymbol *Group = Section->getGroup())
     GroupName = Group->getName();
 
   // This function is only used by .debug*, which should not have the

llvm/lib/MC/MCSection.cpp

 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <utility>
 
 using namespace llvm;
 
-MCSection::MCSection(SectionVariant V, SectionKind K, MCSymbol *Begin)
+MCSection::MCSection(SectionVariant V, StringRef Name, SectionKind K,
+                     MCSymbol *Begin)
     : Begin(Begin), BundleGroupBeforeFirstInst(false), HasInstructions(false),
-      IsRegistered(false), DummyFragment(this), Variant(V), Kind(K) {}
+      IsRegistered(false), DummyFragment(this), Name(Name), Variant(V),
+      Kind(K) {}
 
 MCSymbol *MCSection::getEndSymbol(MCContext &Ctx) {
   if (!End)
     End = Ctx.createTempSymbol("sec_end", true);
   return End;
 }
 
 bool MCSection::hasEnded() const { return End && End->isInSection(); }

llvm/lib/MC/MCSectionCOFF.cpp

void MCSectionCOFF::setSelection(int Selection) const {
   this->Selection = Selection;
   Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
 }
 
 void MCSectionCOFF::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                                          raw_ostream &OS,
                                          const MCExpr *Subsection) const {
   // standard sections don't require the '.section'
-  if (ShouldOmitSectionDirective(SectionName, MAI)) {
+  if (ShouldOmitSectionDirective(getName(), MAI)) {
     OS << '\t' << getName() << '\n';
     return;
   }
 
   OS << "\t.section\t" << getName() << ",\"";
   if (getCharacteristics() & COFF::IMAGE_SCN_CNT_INITIALIZED_DATA)
     OS << 'd';
   if (getCharacteristics() & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA)
     OS << 'b';
   if (getCharacteristics() & COFF::IMAGE_SCN_MEM_EXECUTE)
     OS << 'x';
   if (getCharacteristics() & COFF::IMAGE_SCN_MEM_WRITE)
     OS << 'w';
   else if (getCharacteristics() & COFF::IMAGE_SCN_MEM_READ)
     OS << 'r';
   else
     OS << 'y';
   if (getCharacteristics() & COFF::IMAGE_SCN_LNK_REMOVE)
     OS << 'n';
   if (getCharacteristics() & COFF::IMAGE_SCN_MEM_SHARED)
     OS << 's';
   if ((getCharacteristics() & COFF::IMAGE_SCN_MEM_DISCARDABLE) &&
-      !isImplicitlyDiscardable(SectionName))
+      !isImplicitlyDiscardable(getName()))
     OS << 'D';
   OS << '"';
 
   if (getCharacteristics() & COFF::IMAGE_SCN_LNK_COMDAT) {
     if (COMDATSymbol)
       OS << ",";
     else
       OS << "\n\t.linkonce\t";

llvm/lib/MC/MCSectionELF.cpp

for (const char *B = Name.begin(), *E = Name.end(); B < E; ++B) {
     }
   }
   OS << '"';
 }
 
 void MCSectionELF::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                                         raw_ostream &OS,
                                         const MCExpr *Subsection) const {
-  if (ShouldOmitSectionDirective(SectionName, MAI)) {
+  if (ShouldOmitSectionDirective(getName(), MAI)) {
     OS << '\t' << getName();
     if (Subsection) {
       OS << '\t';
       Subsection->print(OS, &MAI);
     }
     OS << '\n';
     return;
   }

llvm/lib/MC/MCSectionMachO.cpp

 ENTRY("" /*FIXME*/,          S_ATTR_LOC_RELOC)
 #undef ENTRY
   { 0, StringLiteral("none"), StringLiteral("") }, // used if section has no attributes but has a stub size
 };
 
 MCSectionMachO::MCSectionMachO(StringRef Segment, StringRef Section,
                                unsigned TAA, unsigned reserved2, SectionKind K,
                                MCSymbol *Begin)
-    : MCSection(SV_MachO, K, Begin), TypeAndAttributes(TAA),
+    : MCSection(SV_MachO, Section, K, Begin), TypeAndAttributes(TAA),
       Reserved2(reserved2) {
   assert(Segment.size() <= 16 && Section.size() <= 16 &&
          "Segment or section string too long");
   for (unsigned i = 0; i != 16; ++i) {
     if (i < Segment.size())
       SegmentName[i] = Segment[i];
     else
       SegmentName[i] = 0;
-
-    if (i < Section.size())
-      SectionName[i] = Section[i];
-    else
-      SectionName[i] = 0;
   }
 }
 
 void MCSectionMachO::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                                           raw_ostream &OS,
                                           const MCExpr *Subsection) const {
   OS << "\t.section\t" << getSegmentName() << ',' << getName();
 

llvm/lib/MC/MCSectionWasm.cpp

static void printName(raw_ostream &OS, StringRef Name) {
   }
   OS << '"';
 }
 
 void MCSectionWasm::PrintSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
                                          raw_ostream &OS,
                                          const MCExpr *Subsection) const {
 
-  if (shouldOmitSectionDirective(SectionName, MAI)) {
+  if (shouldOmitSectionDirective(getName(), MAI)) {
     OS << '\t' << getName();
     if (Subsection) {
       OS << '\t';
       Subsection->print(OS, &MAI);
     }
     OS << '\n';
     return;
   }