COFF: Devirtualize mark(), markLive() and isCOMDAT().

Only SectionChunk can be dead-stripped. Previously,
all types of chunks implemented these functions,
but their functions were blank.

Likewise, only DefinedRegular and DefinedCOMDAT symbols
can be dead-stripped. markLive() function was implemented
for other symbol types, but they were blank.

I started thinking that the change I made in r240319 was
a mistake. I separated DefinedCOMDAT from DefinedRegular
because I thought that would make the code cleaner, but now
we want to handle them as the same type here. Maybe we
should roll it back.

This change should improve readability a bit as this removes
some dubious uses of reinterpret_cast. Previously, we
assumed that all COMDAT chunks are actually SectionChunks,
which was not very obvious.

llvm-svn: 240675

lld/COFF/Chunks.cpp

@@ -28,7 +28,7 @@ namespace lld {
 namespace coff {
 
 SectionChunk::SectionChunk(ObjectFile *F, const coff_section *H)
-    : File(F), Ptr(this), Header(H),
+    : Chunk(SectionKind), File(F), Ptr(this), Header(H),
       Relocs(File->getCOFFObj()->getRelocations(Header)),
       NumRelocs(std::distance(Relocs.begin(), Relocs.end())) {
   // Initialize SectionName.
@@ -39,16 +39,9 @@ SectionChunk::SectionChunk(ObjectFile *F, const coff_section *H)
   if (Shift > 0)
     Align = uint32_t(1) << (Shift - 1);
 
-  // When a new chunk is created, we don't if if it's going to make it
-  // to the final output. Initially all sections are unmarked in terms
-  // of garbage collection. The writer will call markLive() to mark
-  // all reachable section chunks.
-  Live = false;
-
   // COMDAT sections are not GC root. Non-text sections are not
   // subject of garbage collection (thus they are root).
-  if (!isCOMDAT() && !(Header->Characteristics & IMAGE_SCN_CNT_CODE))
+  Root = !isCOMDAT() && !(Header->Characteristics & IMAGE_SCN_CNT_CODE);
-    Root = true;
 }
 
 static void add16(uint8_t *P, int16_t V) { write16le(P, read16le(P) + V); }
@@ -93,13 +86,17 @@ void SectionChunk::mark() {
   // Mark all symbols listed in the relocation table for this section.
   for (const coff_relocation &Rel : Relocs) {
     SymbolBody *B = File->getSymbolBody(Rel.SymbolTableIndex);
-    if (auto *Def = dyn_cast<Defined>(B))
+    if (auto *D = dyn_cast<DefinedRegular>(B)) {
-      Def->markLive();
+      D->markLive();
+    } else if (auto *D = dyn_cast<DefinedCOMDAT>(B)) {
+      D->markLive();
+    }
   }
 
   // Mark associative sections if any.
   for (Chunk *C : AssocChildren)
-    C->markLive();
+    if (auto *SC = dyn_cast<SectionChunk>(C))
+      SC->markLive();
 }
 
 void SectionChunk::addAssociative(SectionChunk *Child) {
@@ -139,7 +136,7 @@ bool SectionChunk::isCOMDAT() const {
   return Header->Characteristics & IMAGE_SCN_LNK_COMDAT;
 }
 
-void SectionChunk::printDiscardedMessage() {
+void SectionChunk::printDiscardedMessage() const {
   if (this == Ptr) {
     // Removed by dead-stripping.
     llvm::dbgs() << "Discarded " << Sym->getName() << "\n";

lld/COFF/Chunks.h

@@ -39,6 +39,8 @@ class OutputSection;
 // doesn't even have actual data (if common or bss).
 class Chunk {
 public:
+  enum Kind { SectionKind, OtherKind };
+  Kind kind() const { return ChunkKind; }
   virtual ~Chunk() = default;
 
   // Returns the size of this chunk (even if this is a common or BSS.)
@@ -71,24 +73,6 @@ public:
     llvm_unreachable("unimplemented getSectionName");
   }
 
-  // Called if the garbage collector decides to not include this chunk
-  // in a final output. It's supposed to print out a log message to stdout.
-  // It is illegal to call this function on non-section chunks because
-  // only section chunks are subject of garbage collection.
-  virtual void printDiscardedMessage() {
-    llvm_unreachable("unimplemented printDiscardedMessage");
-  }
-
-  // Returns true if this is a COMDAT section. Usually, it is an error
-  // if there are more than one defined symbols having the same name,
-  // but symbols at begining of COMDAT sections allowed to duplicate.
-  virtual bool isCOMDAT() const { return false; }
-
-  // Used by the garbage collector.
-  bool isRoot() { return Root; }
-  bool isLive() { return Live; }
-  void markLive() { if (!Live) mark(); }
-
   // An output section has pointers to chunks in the section, and each
   // chunk has a back pointer to an output section.
   void setOutputSection(OutputSection *O) { Out = O; }
@@ -103,6 +87,9 @@ public:
   virtual StringRef getDebugName() { return ""; }
 
 protected:
+  Chunk(Kind K = OtherKind) : ChunkKind(K) {}
+  const Kind ChunkKind;
+
   // The RVA of this chunk in the output. The writer sets a value.
   uint64_t RVA = 0;
 
@@ -114,25 +101,24 @@ protected:
 
   // The alignment of this chunk. The writer uses the value.
   uint32_t Align = 1;
-
-  // Used by the garbage collector.
-  virtual void mark() {}
-  bool Live = true;
-  bool Root = false;
 };
 
 // A chunk corresponding a section of an input file.
 class SectionChunk : public Chunk {
 public:
   SectionChunk(ObjectFile *File, const coff_section *Header);
+  static bool classof(const Chunk *C) { return C->kind() == SectionKind; }
   size_t getSize() const override { return Header->SizeOfRawData; }
   void writeTo(uint8_t *Buf) override;
   bool hasData() const override;
   uint32_t getPermissions() const override;
   StringRef getSectionName() const override { return SectionName; }
-  void printDiscardedMessage() override;
-  bool isCOMDAT() const override;
   void getBaserels(std::vector<uint32_t> *Res, Defined *ImageBase) override;
+  bool isCOMDAT() const;
+
+  // Called if the garbage collector decides to not include this chunk
+  // in a final output. It's supposed to print out a log message to stdout.
+  void printDiscardedMessage() const;
 
   // Adds COMDAT associative sections to this COMDAT section. A chunk
   // and its children are treated as a group by the garbage collector.
@@ -141,15 +127,18 @@ public:
   StringRef getDebugName() override;
   void setSymbol(DefinedCOMDAT *S) { if (!Sym) Sym = S; }
 
-  uint64_t getHash() const;
+  // Used by the garbage collector.
-  bool equals(const SectionChunk *Other) const;
+  bool isRoot() { return Root; }
+  bool isLive() { return Live; }
+  void markLive() { if (!Live) mark(); }
 
   // Used for ICF (Identical COMDAT Folding)
   SectionChunk *repl();
   void replaceWith(SectionChunk *Other);
+  uint64_t getHash() const;
+  bool equals(const SectionChunk *Other) const;
 
 private:
-  void mark() override;
   ArrayRef<uint8_t> getContents() const;
 
   // A file this chunk was created from.
@@ -167,6 +156,11 @@ private:
   llvm::iterator_range<const coff_relocation *> Relocs;
   size_t NumRelocs;
 
+  // Used by the garbage collector.
+  void mark();
+  bool Live = false;
+  bool Root;
+
   // Chunks are basically unnamed chunks of bytes.
   // Symbols are associated for debugging and logging purposs only.
   DefinedCOMDAT *Sym = nullptr;

lld/COFF/ICF.cpp

@@ -39,9 +39,9 @@ void doICF(const std::vector<Chunk *> &Chunks) {
   std::unordered_set<SectionChunk *, Hasher, Equals> Set;
   bool removed = false;
   for (Chunk *C : Chunks) {
-    if (!C->isCOMDAT() || !C->isLive())
+    auto *SC = dyn_cast<SectionChunk>(C);
+    if (!SC || !SC->isCOMDAT() || !SC->isLive())
       continue;
-    auto *SC = reinterpret_cast<SectionChunk *>(C);
     auto P = Set.insert(SC);
     bool Inserted = P.second;
     if (Inserted)

lld/COFF/InputFiles.cpp

@@ -182,7 +182,7 @@ SymbolBody *ObjectFile::createSymbolBody(COFFSymbolRef Sym, const void *AuxP,
     return new (Alloc) Undefined(Name);
   }
   if (Sym.isCommon()) {
-    Chunk *C = new (Alloc) CommonChunk(Sym);
+    auto *C = new (Alloc) CommonChunk(Sym);
     Chunks.push_back(C);
     return new (Alloc) DefinedCommon(COFFObj.get(), Sym, C);
   }
@@ -213,10 +213,10 @@ SymbolBody *ObjectFile::createSymbolBody(COFFSymbolRef Sym, const void *AuxP,
       }
     }
   }
-  if (Chunk *C = SparseChunks[Sym.getSectionNumber()]) {
+  Chunk *C = SparseChunks[Sym.getSectionNumber()];
-    if (!C->isCOMDAT())
+  if (auto *SC = cast_or_null<SectionChunk>(C)) {
-      return new (Alloc) DefinedRegular(COFFObj.get(), Sym, C);
+    if (!SC->isCOMDAT())
-    auto *SC = reinterpret_cast<SectionChunk *>(C);
+      return new (Alloc) DefinedRegular(COFFObj.get(), Sym, SC);
     auto *B = new (Alloc) DefinedCOMDAT(COFFObj.get(), Sym, SC);
     if (Sym.getValue() == 0 && !AuxP)
       SC->setSymbol(B);

lld/COFF/Symbols.h

@@ -108,17 +108,13 @@ public:
   // The writer uses this information to apply relocations.
   virtual uint64_t getFileOff() = 0;
 
-  // Called by the garbage collector. All Defined subclasses should
-  // know how to call depending symbols' markLive functions.
-  virtual void markLive() {}
-
   int compare(SymbolBody *Other) override;
 };
 
 // Regular defined symbols read from object file symbol tables.
 class DefinedRegular : public Defined {
 public:
-  DefinedRegular(COFFObjectFile *F, COFFSymbolRef S, Chunk *C)
+  DefinedRegular(COFFObjectFile *F, COFFSymbolRef S, SectionChunk *C)
       : Defined(DefinedRegularKind), COFFFile(F), Sym(S), Data(C) {}
 
   static bool classof(const SymbolBody *S) {
@@ -128,15 +124,15 @@ public:
   StringRef getName() override;
   uint64_t getRVA() override { return Data->getRVA() + Sym.getValue(); }
   bool isExternal() override { return Sym.isExternal(); }
-  void markLive() override { Data->markLive(); }
   uint64_t getFileOff() override { return Data->getFileOff() + Sym.getValue(); }
   int compare(SymbolBody *Other) override;
+  void markLive() { Data->markLive(); }
 
 private:
   StringRef Name;
   COFFObjectFile *COFFFile;
   COFFSymbolRef Sym;
-  Chunk *Data;
+  SectionChunk *Data;
 };
 
 class DefinedCOMDAT : public Defined {
@@ -155,8 +151,8 @@ public:
   StringRef getName() override;
   uint64_t getRVA() override { return Data->repl()->getRVA() + Sym.getValue(); }
   bool isExternal() override { return Sym.isExternal(); }
-  void markLive() override { Data->repl()->markLive(); }
   int compare(SymbolBody *Other) override;
+  void markLive() { Data->repl()->markLive(); }
   Chunk *getChunk() { return Data->repl(); }
 
 private:
@@ -168,7 +164,7 @@ private:
 
 class DefinedCommon : public Defined {
 public:
-  DefinedCommon(COFFObjectFile *F, COFFSymbolRef S, Chunk *C)
+  DefinedCommon(COFFObjectFile *F, COFFSymbolRef S, CommonChunk *C)
       : Defined(DefinedCommonKind), COFFFile(F), Sym(S), Data(C) {}
 
   static bool classof(const SymbolBody *S) {
@@ -178,7 +174,6 @@ public:
   StringRef getName() override;
   uint64_t getRVA() override { return Data->getRVA(); }
   bool isExternal() override { return Sym.isExternal(); }
-  void markLive() override { Data->markLive(); }
   uint64_t getFileOff() override { return Data->getFileOff(); }
   int compare(SymbolBody *Other) override;
 
@@ -188,7 +183,7 @@ private:
   StringRef Name;
   COFFObjectFile *COFFFile;
   COFFSymbolRef Sym;
-  Chunk *Data;
+  CommonChunk *Data;
 };
 
 // Absolute symbols.

lld/COFF/Writer.cpp

@@ -111,11 +111,18 @@ void OutputSection::writeHeaderTo(uint8_t *Buf) {
 void Writer::markLive() {
   if (!Config->DoGC)
     return;
-  for (StringRef Name : Config->GCRoots)
+  for (StringRef Name : Config->GCRoots) {
-    cast<Defined>(Symtab->find(Name))->markLive();
+    SymbolBody *B = Symtab->find(Name);
+    if (auto *D = dyn_cast<DefinedRegular>(B)) {
+      D->markLive();
+    } else if (auto *D = dyn_cast<DefinedCOMDAT>(B)) {
+      D->markLive();
+    }
+  }
   for (Chunk *C : Symtab->getChunks())
-    if (C->isRoot())
+    if (auto *SC = dyn_cast<SectionChunk>(C))
-      C->markLive();
+      if (SC->isRoot())
+        SC->markLive();
 }
 
 struct Hasher {
@@ -139,11 +146,14 @@ void Writer::createSections() {
   // First, bin chunks by name.
   std::map<StringRef, std::vector<Chunk *>> Map;
   for (Chunk *C : Symtab->getChunks()) {
-    if (Config->DoGC && !C->isLive()) {
+    if (Config->DoGC) {
+      auto *SC = dyn_cast<SectionChunk>(C);
+      if (SC && !SC->isLive()) {
         if (Config->Verbose)
-        C->printDiscardedMessage();
+          SC->printDiscardedMessage();
         continue;
       }
+    }
     Map[C->getSectionName()].push_back(C);
   }