[COFF] Drop unused comdat sections when GC is turned off

[rnk]

Summary

Adds a "Discarded" bool to SectionChunk to indicate if the section was
discarded by COMDAT deduplication. The Writer still just checks
isLive().

Fixes PR33446

[ruiu]

LGTM

lld/COFF/Chunks.cpp

236–239 ↗

(On Diff #102854)

Nice improvement!

lld/trunk/COFF/Chunks.h

public:
 
   // Adds COMDAT associative sections to this COMDAT section. A chunk
   // and its children are treated as a group by the garbage collector.
   void addAssociative(SectionChunk *Child);
 
   StringRef getDebugName() override;
   void setSymbol(DefinedRegular *S) { if (!Sym) Sym = S; }
 
+  // Returns true if the chunk was not dropped by GC or COMDAT deduplication.
+  bool isLive() { return Live && !Discarded; }
+
   // Used by the garbage collector.
-  bool isLive() { return !Config->DoGC || Live; }
   void markLive() {
+    assert(Config->DoGC && "should only mark things live from GC");
     assert(!isLive() && "Cannot mark an already live section!");
     Live = true;
   }
 
+  // Returns true if this chunk was dropped by COMDAT deduplication.
+  bool isDiscarded() const { return Discarded; }
+
+  // Used by the SymbolTable when discarding unused comdat sections. This is
+  // redundant when GC is enabled, as all comdat sections will start out dead.
+  void markDiscarded() { Discarded = true; }
+
   // Allow iteration over the bodies of this chunk's relocated symbols.
   llvm::iterator_range<symbol_iterator> symbols() const {
     return llvm::make_range(symbol_iterator(File, Relocs.begin()),
                             symbol_iterator(File, Relocs.end()));
   }
 
   // Allow iteration over the associated child chunks for this section.
   ArrayRef<SectionChunk *> children() const { return AssocChildren; }
public:
   ObjectFile *File;
 
 private:
   StringRef SectionName;
   std::vector<SectionChunk *> AssocChildren;
   llvm::iterator_range<const coff_relocation *> Relocs;
   size_t NumRelocs;
 
+  // True if this chunk was discarded because it was a duplicate comdat section.
+  bool Discarded;
+
   // Used by the garbage collector.
   bool Live;
 
   // Used for ICF (Identical COMDAT Folding)
   void replace(SectionChunk *Other);
   uint32_t Class[2] = {0, 0};
 
   // Sym points to a section symbol if this is a COMDAT chunk.

lld/trunk/COFF/Chunks.cpp

SectionChunk::SectionChunk(ObjectFile *F, const coff_section *H)
     : Chunk(SectionKind), Repl(this), Header(H), File(F),
       Relocs(File->getCOFFObj()->getRelocations(Header)),
       NumRelocs(std::distance(Relocs.begin(), Relocs.end())) {
   // Initialize SectionName.
   File->getCOFFObj()->getSectionName(Header, SectionName);
 
   Align = Header->getAlignment();
 
-  // Only COMDAT sections are subject of dead-stripping.
-  Live = !isCOMDAT();
+  // Chunks may be discarded during comdat merging.
+  Discarded = false;
+
+  // If linker GC is disabled, every chunk starts out alive.  If linker GC is
+  // enabled, treat non-comdat sections as roots. Generally optimized object
+  // files will be built with -ffunction-sections or /Gy, so most things worth
+  // stripping will be in a comdat.
+  Live = !Config->DoGC || !isCOMDAT();
 }
 
 static void add16(uint8_t *P, int16_t V) { write16le(P, read16le(P) + V); }
 static void add32(uint8_t *P, int32_t V) { write32le(P, read32le(P) + V); }
 static void add64(uint8_t *P, int64_t V) { write64le(P, read64le(P) + V); }
 static void or16(uint8_t *P, uint16_t V) { write16le(P, read16le(P) | V); }
 
 void SectionChunk::applyRelX64(uint8_t *Off, uint16_t Type, Defined *Sym,

 
 bool SectionChunk::isCOMDAT() const {
   return Header->Characteristics & IMAGE_SCN_LNK_COMDAT;
 }
 
 void SectionChunk::printDiscardedMessage() const {
   // Removed by dead-stripping. If it's removed by ICF, ICF already
   // printed out the name, so don't repeat that here.
-  if (Sym && this == Repl)
+  if (Sym && this == Repl) {
+    if (Discarded)
+      message("Discarded comdat symbol " + Sym->getName());
+    else if (!Live)
       message("Discarded " + Sym->getName());
   }
+}
 
 StringRef SectionChunk::getDebugName() {
   if (Sym)
     return Sym->getName();
   return "";
 }
 
 ArrayRef<uint8_t> SectionChunk::getContents() const {

lld/trunk/COFF/InputFiles.cpp

SymbolBody *ObjectFile::createDefined(COFFSymbolRef Sym, const void *AuxP,
   if (!SC)
     return nullptr;
 
   // Handle section definitions
   if (IsFirst && AuxP) {
     auto *Aux = reinterpret_cast<const coff_aux_section_definition *>(AuxP);
     if (Aux->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
       if (auto *ParentSC = cast_or_null<SectionChunk>(
-              SparseChunks[Aux->getNumber(Sym.isBigObj())]))
+              SparseChunks[Aux->getNumber(Sym.isBigObj())])) {
         ParentSC->addAssociative(SC);
+        // If we already discarded the parent, discard the child.
+        if (ParentSC->isDiscarded())
+          SC->markDiscarded();
+      }
     SC->Checksum = Aux->CheckSum;
   }
 
   DefinedRegular *B;
   if (Sym.isExternal()) {
     COFFObj->getSymbolName(Sym, Name);
     Symbol *S =
         Symtab->addRegular(this, Name, SC->isCOMDAT(), Sym.getGeneric(), SC);

lld/trunk/COFF/SymbolTable.cpp

Symbol *SymbolTable::addRegular(InputFile *F, StringRef N, bool IsCOMDAT,
   std::tie(S, WasInserted) = insert(N);
   if (!isa<BitcodeFile>(F))
     S->IsUsedInRegularObj = true;
   SymbolPreference SP = compareDefined(S, WasInserted, IsCOMDAT);
   if (SP == SP_CONFLICT) {
     reportDuplicate(S, F);
   } else if (SP == SP_NEW) {
     replaceBody<DefinedRegular>(S, F, N, IsCOMDAT, /*IsExternal*/ true, Sym, C);
+  } else if (SP == SP_EXISTING && IsCOMDAT && C) {
+    C->markDiscarded();
+    // Discard associative chunks that we've parsed so far. No need to recurse
+    // because an associative section cannot have children.
+    for (SectionChunk *Child : C->children())
+      Child->markDiscarded();
   }
   return S;
 }
 
 Symbol *SymbolTable::addCommon(InputFile *F, StringRef N, uint64_t Size,
                                const coff_symbol_generic *Sym, CommonChunk *C) {
   Symbol *S;
   bool WasInserted;

lld/trunk/test/COFF/Inputs/associative-comdat-2.s

+# Defines foo and foo_assoc globals. foo is comdat, and foo_assoc is comdat
+# associative with it. foo_assoc should be discarded iff foo is discarded,
+# either by linker GC or normal comdat merging.
+
+        .section        .rdata,"dr",associative,foo
+        .p2align        3
+        .quad   foo
+
+        .section        .data,"dw",discard,foo
+        .globl  foo                     # @foo
+        .p2align        2
+foo:
+        .long   42

lld/trunk/test/COFF/associative-comdat.s

+# RUN: llvm-mc %s -filetype=obj -o %t1.obj
+# RUN: llvm-mc %S/Inputs/associative-comdat-2.s -filetype=obj -o %t2.obj
+
+# RUN: lld-link -entry:main %t1.obj %t2.obj -out:%t.gc.exe
+# RUN: llvm-readobj -sections %t.gc.exe | FileCheck %s
+
+# RUN: lld-link -entry:main %t1.obj %t2.obj -opt:noref -out:%t.nogc.exe
+# RUN: llvm-readobj -sections %t.nogc.exe | FileCheck %s
+
+# CHECK: Sections [
+# CHECK:   Section {
+# CHECK:     Number: 1
+# CHECK-LABEL:     Name: .data (2E 64 61 74 61 00 00 00)
+# CHECK-NEXT:     VirtualSize: 0x4
+# CHECK:   Section {
+# CHECK-LABEL:     Name: .rdata (2E 72 64 61 74 61 00 00)
+#             This is the critical check to show that only *one* definition of
+#             foo_assoc was retained. This *must* be 8, not 16.
+# CHECK-NEXT:     VirtualSize: 0x8
+
+        .text
+        .def     main;
+        .scl    2;
+        .type   32;
+        .endef
+        .globl  main                    # -- Begin function main
+        .p2align        4, 0x90
+main:                                   # @main
+# BB#0:
+        movl    foo(%rip), %eax
+        retq
+                                        # -- End function
+
+# Defines foo and foo_assoc globals. foo is comdat, and foo_assoc is comdat
+# associative with it. foo_assoc should be discarded iff foo is discarded,
+# either by linker GC or normal comdat merging.
+
+        .section        .rdata,"dr",associative,foo
+        .p2align        3
+        .quad   foo
+
+        .section        .data,"dw",discard,foo
+        .globl  foo                     # @foo
+        .p2align        2
+foo:
+        .long   42