[LLD] [COFF] When doing automatic dll imports, replace whole .refptr.<var> chunks with __imp_<var>

[mstorsjo]

Summary

After fixing up the runtime pseudo relocation, the .refptr.<var> will be a plain pointer with the same value as the IAT entry itself. To save a little binary size and reduce the number of runtime pseudo relocations, redirect references to the IAT entry (via the __imp_<var> symbol) itself and discard the .refptr.<var> chunk (as long as the same section chunk doesn't contain anything else than the single pointer).

[rnk]

lgtm

test/COFF/autoimport-refptr.s

35 ↗

(On Diff #163169)

Hm, they really should put a colon after the address to make this easier to read. =/

[ruiu]

COFF/Chunks.h

189 ↗

(On Diff #163169)

Now Live member has both read and write accessors, so I'd make it a public member and remove this function and isLive.

COFF/SymbolTable.cpp

173 ↗

(On Diff #163169)

This probably needs comment as I believe this is hard to understand for those who don't know what the intention of this code is.

[mstorsjo]

COFF/Chunks.h

189 ↗

(On Diff #163169)

Ok - what about the markLive function, should I remove that one as well? Or do we want to keep it for the sake of the asserts?

[ruiu]

COFF/Chunks.h

189 ↗

(On Diff #163169)

Looks like the only use of markLive is in MarkLive.cpp, and if you remove that function and inline it, it is obvious that you don't really need these asserts. So, yes, we should remove markLive function.

[mstorsjo]

Dropped the isLive() and discard() accessors and made Live public instead. Kept the markLive accessor since it has got useful asserts.

COFF/SymbolTable.cpp

173 ↗

(On Diff #163169)

I added some kind of summary in a comment here.

[mstorsjo]

COFF/Chunks.h

189 ↗

(On Diff #163169)

Sorry, I missed this comment, as it was added right before I updated the diff yesterday.

Will remove that method as well, and commit this.

[mstorsjo]

COFF/SymbolTable.cpp

182 ↗

(On Diff #163254)

I added another extra sanity check that the Refptr chunk only contains 1 relocation and that it points at this specific symbol that we're importing, before I committed this.

lld/trunk/COFF/Chunks.h

public:
   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.
   void addAssociative(SectionChunk *Child);
 
   StringRef getDebugName() override;
 
-  // Returns true if the chunk was not dropped by GC.
-  bool isLive() { return Live; }
-
-  // Used by the garbage collector.
-  void markLive() {
-    assert(Config->DoGC && "should only mark things live from GC");
-    assert(!isLive() && "Cannot mark an already live section!");
-    Live = true;
-  }
-
   // True if this is a codeview debug info chunk. These will not be laid out in
   // the image. Instead they will end up in the PDB, if one is requested.
   bool isCodeView() const {
     return SectionName == ".debug" || SectionName.startswith(".debug$");
   }
 
   // True if this is a DWARF debug info or exception handling chunk.
   bool isDWARF() const {
public:
   // The file that this chunk was created from.
   ObjFile *File;
 
   // The COMDAT leader symbol if this is a COMDAT chunk.
   DefinedRegular *Sym = nullptr;
 
   ArrayRef<coff_relocation> Relocs;
 
+  // Used by the garbage collector.
+  bool Live;
+
 private:
   StringRef SectionName;
   std::vector<SectionChunk *> AssocChildren;
 
-  // Used by the garbage collector.
-  bool Live;
-
   // Used for ICF (Identical COMDAT Folding)
   void replace(SectionChunk *Other);
   uint32_t Class[2] = {0, 0};
 };
 
 // This class is used to implement an lld-specific feature (not implemented in
 // MSVC) that minimizes the output size by finding string literals sharing tail
 // parts and merging them.

lld/trunk/COFF/Chunks.cpp

void MergeChunk::addSection(SectionChunk *C) {
   auto *&MC = Instances[C->Alignment];
   if (!MC)
     MC = make<MergeChunk>(C->Alignment);
   MC->Sections.push_back(C);
 }
 
 void MergeChunk::finalizeContents() {
   for (SectionChunk *C : Sections)
-    if (C->isLive())
+    if (C->Live)
       Builder.add(toStringRef(C->getContents()));
   Builder.finalize();
 
   for (SectionChunk *C : Sections) {
-    if (!C->isLive())
+    if (!C->Live)
       continue;
     size_t Off = Builder.getOffset(toStringRef(C->getContents()));
     C->setOutputSection(Out);
     C->setRVA(RVA + Off);
     C->OutputSectionOff = OutputSectionOff + Off;
   }
 }
 

lld/trunk/COFF/ICF.cpp

 // a few instances of programs that are not safe for data merging.
 // Therefore, we merge only functions just like the MSVC tool. However, we also
 // merge read-only sections in a couple of cases where the address of the
 // section is insignificant to the user program and the behaviour matches that
 // of the Visual C++ linker.
 bool ICF::isEligible(SectionChunk *C) {
   // Non-comdat chunks, dead chunks, and writable chunks are not elegible.
   bool Writable = C->getOutputCharacteristics() & llvm::COFF::IMAGE_SCN_MEM_WRITE;
-  if (!C->isCOMDAT() || !C->isLive() || Writable)
+  if (!C->isCOMDAT() || !C->Live || Writable)
     return false;
 
   // Code sections are eligible.
   if (C->getOutputCharacteristics() & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
     return true;
 
   // .pdata and .xdata unwind info sections are eligible.
   StringRef OutSecName = C->getSectionName().split('$').first;

lld/trunk/COFF/MarkLive.cpp

void markLive(ArrayRef<Chunk *> Chunks) {
   // We build up a worklist of sections which have been marked as live. We only
   // push into the worklist when we discover an unmarked section, and we mark
   // as we push, so sections never appear twice in the list.
   SmallVector<SectionChunk *, 256> Worklist;
 
   // COMDAT section chunks are dead by default. Add non-COMDAT chunks.
   for (Chunk *C : Chunks)
     if (auto *SC = dyn_cast<SectionChunk>(C))
-      if (SC->isLive())
+      if (SC->Live)
         Worklist.push_back(SC);
 
   auto Enqueue = [&](SectionChunk *C) {
-    if (C->isLive())
+    if (C->Live)
       return;
-    C->markLive();
+    C->Live = true;
     Worklist.push_back(C);
   };
 
   auto AddSym = [&](Symbol *B) {
     if (auto *Sym = dyn_cast<DefinedRegular>(B))
       Enqueue(Sym->getChunk());
     else if (auto *Sym = dyn_cast<DefinedImportData>(B))
       Sym->File->Live = true;
     else if (auto *Sym = dyn_cast<DefinedImportThunk>(B))
       Sym->WrappedSym->File->Live = Sym->WrappedSym->File->ThunkLive = true;
   };
 
   // Add GC root chunks.
   for (Symbol *B : Config->GCRoot)
     AddSym(B);
 
   while (!Worklist.empty()) {
     SectionChunk *SC = Worklist.pop_back_val();
-    assert(SC->isLive() && "We mark as live when pushing onto the worklist!");
+    assert(SC->Live && "We mark as live when pushing onto the worklist!");
 
     // Mark all symbols listed in the relocation table for this section.
     for (Symbol *B : SC->symbols())
       if (B)
         AddSym(B);
 
     // Mark associative sections if any.
     for (SectionChunk *C : SC->children())
       Enqueue(C);
   }
 }
 
 }
 }

lld/trunk/COFF/PDB.cpp

void PDBLinker::addObjFile(ObjFile *File) {
 
   File->ModuleDBI = &ExitOnErr(Builder.getDbiBuilder().addModuleInfo(Name));
   File->ModuleDBI->setObjFileName(Path);
 
   auto Chunks = File->getChunks();
   uint32_t Modi = File->ModuleDBI->getModuleIndex();
   for (Chunk *C : Chunks) {
     auto *SecChunk = dyn_cast<SectionChunk>(C);
-    if (!SecChunk || !SecChunk->isLive())
+    if (!SecChunk || !SecChunk->Live)
       continue;
     pdb::SectionContrib SC = createSectionContrib(SecChunk, Modi);
     File->ModuleDBI->setFirstSectionContrib(SC);
     break;
   }
 
   // Before we can process symbol substreams from .debug$S, we need to process
   // type information, file checksums, and the string table.  Add type info to
void PDBLinker::addObjFile(ObjFile *File) {
 
   ScopedTimer T(SymbolMergingTimer);
 
   // Now do all live .debug$S sections.
   DebugStringTableSubsectionRef CVStrTab;
   DebugChecksumsSubsectionRef Checksums;
   std::vector<ulittle32_t *> StringTableReferences;
   for (SectionChunk *DebugChunk : File->getDebugChunks()) {
-    if (!DebugChunk->isLive() || DebugChunk->getSectionName() != ".debug$S")
+    if (!DebugChunk->Live || DebugChunk->getSectionName() != ".debug$S")
       continue;
 
     ArrayRef<uint8_t> RelocatedDebugContents =
         relocateDebugChunk(Alloc, DebugChunk);
     if (RelocatedDebugContents.empty())
       continue;
 
     DebugSubsectionArray Subsections;

lld/trunk/COFF/SymbolTable.cpp

bool SymbolTable::handleMinGWAutomaticImport(Symbol *Sym, StringRef Name) {
   // Replace the reference directly to a variable with a reference
   // to the import address table instead. This obviously isn't right,
   // but we mark the symbol as IsRuntimePseudoReloc, and a later pass
   // will add runtime pseudo relocations for every relocation against
   // this Symbol. The runtime pseudo relocation framework expects the
   // reference itself to point at the IAT entry.
   Sym->replaceKeepingName(Imp, sizeof(DefinedImportData));
   cast<DefinedImportData>(Sym)->IsRuntimePseudoReloc = true;
+
+  // There may exist symbols named .refptr.<name> which only consist
+  // of a single pointer to <name>. If it turns out <name> is
+  // automatically imported, we don't need to keep the .refptr.<name>
+  // pointer at all, but redirect all accesses to it to the IAT entry
+  // for __imp_<name> instead, and drop the whole .refptr.<name> chunk.
+  DefinedRegular *Refptr =
+      dyn_cast_or_null<DefinedRegular>(find((".refptr." + Name).str()));
+  size_t PtrSize = Config->is64() ? 8 : 4;
+  if (Refptr && Refptr->getChunk()->getSize() == PtrSize) {
+    SectionChunk *SC = dyn_cast_or_null<SectionChunk>(Refptr->getChunk());
+    if (SC && SC->Relocs.size() == 1 && *SC->symbols().begin() == Sym) {
+        log("Replacing .refptr." + Name + " with " + Imp->getName());
+        Refptr->getChunk()->Live = false;
+        Refptr->replaceKeepingName(Imp, sizeof(DefinedImportData));
+    }
+  }
   return true;
 }
 
 void SymbolTable::reportRemainingUndefines() {
   SmallPtrSet<Symbol *, 8> Undefs;
   DenseMap<Symbol *, Symbol *> LocalImports;
 
   for (auto &I : SymMap) {

lld/trunk/COFF/Symbols.cpp

if (auto *Sym = dyn_cast<DefinedCOFF>(this))
     return Sym->File;
   if (auto *Sym = dyn_cast<Lazy>(this))
     return Sym->File;
   return nullptr;
 }
 
 bool Symbol::isLive() const {
   if (auto *R = dyn_cast<DefinedRegular>(this))
-    return R->getChunk()->isLive();
+    return R->getChunk()->Live;
   if (auto *Imp = dyn_cast<DefinedImportData>(this))
     return Imp->File->Live;
   if (auto *Imp = dyn_cast<DefinedImportThunk>(this))
     return Imp->WrappedSym->File->ThunkLive;
   // Assume any other kind of symbol is live.
   return true;
 }
 

lld/trunk/COFF/Writer.cpp

void Writer::createSections() {
   DidatSec = CreateSection(".didat", DATA | R);
   RsrcSec = CreateSection(".rsrc", DATA | R);
   RelocSec = CreateSection(".reloc", DATA | DISCARDABLE | R);
 
   // Then bin chunks by name and output characteristics.
   std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> Map;
   for (Chunk *C : Symtab->getChunks()) {
     auto *SC = dyn_cast<SectionChunk>(C);
-    if (SC && !SC->isLive()) {
+    if (SC && !SC->Live) {
       if (Config->Verbose)
         SC->printDiscardedMessage();
       continue;
     }
     Map[{C->getSectionName(), C->getOutputCharacteristics()}].push_back(C);
   }
 
   // Process an /order option.

 // Visit all relocations from all section contributions of this object file and
 // mark the relocation target as address-taken.
 static void markSymbolsWithRelocations(ObjFile *File,
                                        SymbolRVASet &UsedSymbols) {
   for (Chunk *C : File->getChunks()) {
     // We only care about live section chunks. Common chunks and other chunks
     // don't generally contain relocations.
     SectionChunk *SC = dyn_cast<SectionChunk>(C);
-    if (!SC || !SC->isLive())
+    if (!SC || !SC->Live)
       continue;
 
     for (const coff_relocation &Reloc : SC->Relocs) {
       if (Config->Machine == I386 && Reloc.Type == COFF::IMAGE_REL_I386_REL32)
         // Ignore relative relocations on x86. On x86_64 they can't be ignored
         // since they're also used to compute absolute addresses.
         continue;
 

 void Writer::markSymbolsForRVATable(ObjFile *File,
                                     ArrayRef<SectionChunk *> SymIdxChunks,
                                     SymbolRVASet &TableSymbols) {
   for (SectionChunk *C : SymIdxChunks) {
     // Skip sections discarded by linker GC. This comes up when a .gfids section
     // is associated with something like a vtable and the vtable is discarded.
     // In this case, the associated gfids section is discarded, and we don't
     // mark the virtual member functions as address-taken by the vtable.
-    if (!C->isLive())
+    if (!C->Live)
       continue;
 
     // Validate that the contents look like symbol table indices.
     ArrayRef<uint8_t> Data = C->getContents();
     if (Data.size() % 4 != 0) {
       warn("ignoring " + C->getSectionName() +
            " symbol table index section in object " + toString(File));
       continue;

 // though the code didn't expect it to, produce the table that the runtime
 // uses for fixing them up, and provide the synthetic symbols that the
 // runtime uses for finding the table.
 void Writer::createRuntimePseudoRelocs() {
   std::vector<RuntimePseudoReloc> Rels;
 
   for (Chunk *C : Symtab->getChunks()) {
     auto *SC = dyn_cast<SectionChunk>(C);
-    if (!SC || !SC->isLive())
+    if (!SC || !SC->Live)
       continue;
     SC->getRuntimePseudoRelocs(Rels);
   }
 
   if (!Rels.empty())
     log("Writing " + Twine(Rels.size()) + " runtime pseudo relocations");
   PseudoRelocTableChunk *Table = make<PseudoRelocTableChunk>(Rels);
   RdataSec->addChunk(Table);

lld/trunk/test/COFF/autoimport-refptr.s

+# REQUIRES: x86
+
+# RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\n.data\nvariable:\n.long 42" > %t-lib.s
+# RUN: llvm-mc -triple=x86_64-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
+# RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
+
+# RUN: llvm-mc -triple=x86_64-windows-gnu %s -filetype=obj -o %t.obj
+# RUN: lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-lib.lib -verbose
+
+# RUN: llvm-readobj -coff-imports %t.exe | FileCheck -check-prefix=IMPORTS %s
+# RUN: llvm-objdump -d %t.exe | FileCheck -check-prefix=DISASM %s
+# RUN: llvm-objdump -s %t.exe | FileCheck -check-prefix=CONTENTS %s
+
+# IMPORTS: Import {
+# IMPORTS-NEXT: Name: autoimport-refptr.s.tmp-lib.dll
+# IMPORTS-NEXT: ImportLookupTableRVA: 0x2030
+# IMPORTS-NEXT: ImportAddressTableRVA: 0x2040
+# IMPORTS-NEXT: Symbol: variable (0)
+# IMPORTS-NEXT: }
+
+# DISASM: Disassembly of section .text:
+# DISASM: .text:
+# Relative offset at 0x1002 pointing at the IAT at 0x2040
+# DISASM: 140001000:      48 8b 05 39 10 00 00    movq    4153(%rip), %rax
+# DISASM: 140001007:      8b 00   movl    (%rax), %eax
+# Relative offset at 0x100b pointing at the .refptr.localvar stub at
+# 0x2000
+# DISASM: 140001009:      48 8b 0d f0 0f 00 00    movq    4080(%rip), %rcx
+# DISASM: 140001010:      03 01   addl    (%rcx), %eax
+# DISASM: 140001012:      c3      retq
+
+# relocs: pointing at an empty list of runtime pseudo relocs.
+# localvar: 42
+# CONTENTS: Contents of section .data:
+# CONTENTS:  140003000 08200040 01000000 08200040 01000000
+# CONTENTS:  140003010 2a000000
+
+    .global main
+    .global localvar
+    .text
+main:
+    movq .refptr.variable(%rip), %rax
+    movl (%rax), %eax
+    movq .refptr.localvar(%rip), %rcx
+    addl (%rcx), %eax
+    ret
+
+    .data
+relocs:
+    .quad __RUNTIME_PSEUDO_RELOC_LIST__
+    .quad __RUNTIME_PSEUDO_RELOC_LIST_END__
+localvar:
+    .int 42
+
+# Normally the compiler wouldn't emit a stub for a variable that is
+# emitted in the same translation unit.
+    .section .rdata$.refptr.localvar,"dr",discard,.refptr.localvar
+    .global .refptr.localvar
+.refptr.localvar:
+    .quad localvar
+
+    .section .rdata$.refptr.variable,"dr",discard,.refptr.variable
+    .global .refptr.variable
+.refptr.variable:
+    .quad variable