[LLD][COFF] Separate module descriptors creation from type/symbol merging

[aganea]

Summary

Take module DBI creation out of PDBLinker::addObjFile() into its own function.

This patch is point 4. in the "Proposed commit strategy" in D59226

[aganea]

Simplify.

[rnk]

lgtm

COFF/InputFiles.h

162

I would suggest MergedIntoPDB or something more descriptive. This is only used in one place, so it doesn't need to be short. "Merged" could mean a lot of things, and someone using DWARF might assume this was relating to PDBs.

COFF/InputFiles.h

public:
   // underlying object file.
   Symbol *getSymbol(uint32_t SymbolIndex) {
     return Symbols[SymbolIndex];
   }
 
   // Returns the underlying COFF file.
   COFFObjectFile *getCOFFObj() { return COFFObj.get(); }
 
-  // Whether the object was already merged into the final PDB or not
-  bool wasProcessedForPDB() const { return !!ModuleDBI; }
-
   static std::vector<ObjFile *> Instances;
 
   // Flags in the absolute @feat.00 symbol if it is present. These usually
   // indicate if an object was compiled with certain security features enabled
   // like stack guard, safeseh, /guard:cf, or other things.
   uint32_t Feat00Flags = 0;
 
   // True if this object file is compatible with SEH.  COFF-specific and
public:
   // When using Microsoft precompiled headers, this is the PCH's key.
   // The same key is used by both the precompiled object, and objects using the
   // precompiled object. Any difference indicates out-of-date objects.
   llvm::Optional<uint32_t> PCHSignature;
 
   // Tells whether this file was compiled with /hotpatch
   bool HotPatchable = false;
 
+  // Whether the object was already merged into the final PDB or not
+  bool Merged = false;
+
 private:
   const coff_section* getSection(uint32_t I);
   const coff_section *getSection(COFFSymbolRef Sym) {
     return getSection(Sym.getSectionNumber());
   }
 
   void initializeChunks();
   void initializeSymbols();

COFF/PDB.cpp

for (FileChecksumEntry &FC : Checksums) {
     ExitOnErr(Linker.Builder.getDbiBuilder().addModuleSourceFile(
         *File.ModuleDBI, FileName));
     NewChecksums->addChecksum(FileName, FC.Kind, FC.Checksum);
   }
   File.ModuleDBI->addDebugSubsection(std::move(NewChecksums));
 }
 
 void PDBLinker::addObjFile(ObjFile *File, CVIndexMap *ExternIndexMap) {
-  if (File->wasProcessedForPDB())
+  if (File->Merged)
     return;
-  // Add a module descriptor for every object file. We need to put an absolute
-  // path to the object into the PDB. If this is a plain object, we make its
-  // path absolute. If it's an object in an archive, we make the archive path
-  // absolute.
-  bool InArchive = !File->ParentName.empty();
-  SmallString<128> Path = InArchive ? File->ParentName : File->getName();
-  pdbMakeAbsolute(Path);
-  StringRef Name = InArchive ? File->getName() : StringRef(Path);
-
-  pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
-  File->ModuleDBI = &ExitOnErr(DbiBuilder.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->Live)
-      continue;
-    pdb::SectionContrib SC = createSectionContrib(SecChunk, Modi);
-    File->ModuleDBI->setFirstSectionContrib(SC);
-    break;
-  }
+  File->Merged = true;
 
   // 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
   // the PDB first, so that we can get the map from object file type and item
   // indices to PDB type and item indices.
   CVIndexMap ObjectIndexMap;
   auto IndexMapResult =
       mergeDebugT(File, ExternIndexMap ? ExternIndexMap : &ObjectIndexMap);
 
   // If the .debug$T sections fail to merge, assume there is no debug info.
   if (!IndexMapResult) {
     if (!Config->WarnDebugInfoUnusable) {
       consumeError(IndexMapResult.takeError());
       return;
     }
-    StringRef FileName = sys::path::filename(Path);
-    warn("Cannot use debug info for '" + FileName + "' [LNK4099]\n" +
+    warn("Cannot use debug info for '" + toString(File) + "' [LNK4099]\n" +
          ">>> failed to load reference " +
          StringRef(toString(IndexMapResult.takeError())));
     return;
   }
 
   ScopedTimer T(SymbolMergingTimer);
 
+  pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
   DebugSHandler DSH(*this, *File, *IndexMapResult);
   // Now do all live .debug$S and .debug$F sections.
   for (SectionChunk *DebugChunk : File->getDebugChunks()) {
     if (!DebugChunk->Live || DebugChunk->getSize() == 0)
       continue;
 
     if (DebugChunk->getSectionName() == ".debug$S") {
       DSH.handleDebugS(*DebugChunk);
if (DebugChunk->getSectionName() == ".debug$F") {
       continue;
     }
   }
 
   // Do any post-processing now that all .debug$S sections have been processed.
   DSH.finish();
 }
 
+// Add a module descriptor for every object file. We need to put an absolute
+// path to the object into the PDB. If this is a plain object, we make its
+// path absolute. If it's an object in an archive, we make the archive path
+// absolute.
+static void createModuleDBI(pdb::PDBFileBuilder &Builder) {
+  pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
+  SmallString<128> ObjName;
+
+  for (ObjFile *File : ObjFile::Instances) {
+
+    bool InArchive = !File->ParentName.empty();
+    ObjName = InArchive ? File->ParentName : File->getName();
+    pdbMakeAbsolute(ObjName);
+    StringRef ModName = InArchive ? File->getName() : StringRef(ObjName);
+
+    File->ModuleDBI = &ExitOnErr(DbiBuilder.addModuleInfo(ModName));
+    File->ModuleDBI->setObjFileName(ObjName);
+
+    ArrayRef<Chunk *> Chunks = File->getChunks();
+    uint32_t Modi = File->ModuleDBI->getModuleIndex();
+
+    for (Chunk *C : Chunks) {
+      auto *SecChunk = dyn_cast<SectionChunk>(C);
+      if (!SecChunk || !SecChunk->Live)
+        continue;
+      pdb::SectionContrib SC = createSectionContrib(SecChunk, Modi);
+      File->ModuleDBI->setFirstSectionContrib(SC);
+      break;
+    }
+  }
+}
+
 static PublicSym32 createPublic(Defined *Def) {
   PublicSym32 Pub(SymbolKind::S_PUB32);
   Pub.Name = Def->getName();
   if (auto *D = dyn_cast<DefinedCOFF>(Def)) {
     if (D->getCOFFSymbol().isFunctionDefinition())
       Pub.Flags = PublicSymFlags::Function;
   } else if (isa<DefinedImportThunk>(Def)) {
     Pub.Flags = PublicSymFlags::Function;
   }
 
   OutputSection *OS = Def->getChunk()->getOutputSection();
   assert(OS && "all publics should be in final image");
   Pub.Offset = Def->getRVA() - OS->getRVA();
   Pub.Segment = OS->SectionIndex;
   return Pub;
 }
 
 // Add all object files to the PDB. Merge .debug$T sections into IpiData and
 // TpiData.
 void PDBLinker::addObjectsToPDB() {
   ScopedTimer T1(AddObjectsTimer);
+
+  createModuleDBI(Builder);
+
   for (ObjFile *File : ObjFile::Instances)
     addObjFile(File);
 
   Builder.getStringTableBuilder().setStrings(PDBStrTab);
   T1.stop();
 
   // Construct TPI and IPI stream contents.
   ScopedTimer T2(TpiStreamLayoutTimer);

test/COFF/precomp-link.test

 RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf
 RUN: llvm-pdbutil dump -types %t.pdb | FileCheck %s
 
 RUN: lld-link %S/Inputs/precomp.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf
 RUN: llvm-pdbutil dump -types %t.pdb | FileCheck %s
 
 RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-invalid.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf 2>&1 | FileCheck %s -check-prefix FAILURE
 
 RUN: not lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf 2>&1 | FileCheck %s -check-prefix FAILURE-MISSING-PRECOMPOBJ
 
-FAILURE: warning: Cannot use debug info for 'precomp-invalid.obj' [LNK4099]
+FAILURE: warning: Cannot use debug info for '{{.*}}precomp-invalid.obj' [LNK4099]
 FAILURE-NEXT: failed to load reference '{{.*}}precomp.obj': The signature does not match; the file(s) might be out of date.
 
-FAILURE-MISSING-PRECOMPOBJ: warning: Cannot use debug info for 'precomp-a.obj' [LNK4099]
+FAILURE-MISSING-PRECOMPOBJ: warning: Cannot use debug info for '{{.*}}precomp-a.obj' [LNK4099]
 FAILURE-MISSING-PRECOMPOBJ-NEXT: failed to load reference '{{.*}}precomp.obj': The path to this file must be provided on the command-line
 
 CHECK: Types (TPI Stream)
 CHECK-NOT: LF_PRECOMP
 CHECK-NOT: LF_ENDPRECOMP
 
 // precomp.h
 #pragma once