[LLD][COFF] When using PCH.OBJ, ensure func_id records indices are remapped under /DEBUG:GHASH

[aganea]

Summary

Before this patch, when using LLD with /DEBUG:GHASH and MSVC precomp.OBJ files, we had a bunch of:

lld-link: warning: S_[GL]PROC32ID record in blabla.obj refers to PDB item index 0x206ED1 which is not a LF[M]FUNC_ID record

This was caused by LF_FUNC_ID and LF_MFUNC_ID which didn't have correct mapping to the corresponding TPI records. The root issue was that the indexMapStorage was improperly re-assembled in UsePrecompSource::remapTpiWithGHashes.

After this patch, /DEBUG and /DEBUG:GHASH produce exactly the same debug infos in the PDB.

[rnk]

lgtm

The Harbormaster precommit test stuff is complaining, but maybe it's just because of the binary files in the patch.

lld/COFF/DebugTypes.cpp

1150–1153

I think this is ripe for some follow-on simplifications. I believe mapToFill is always just indexMapStorage. This routine is really rewriting ghash cell indices to PDB type indices.

[aganea]

Thanks for looking at this!

lld/COFF/DebugTypes.cpp

1150–1153

Yes indeed, I made that change locally, I could commit a NFC patch after landing this.

lld/COFF/DebugTypes.cpp

Error UsePrecompSource::mergeInPrecompHeaderObj() {
   PrecompSource *precompSrc = *e;
   if (precompSrc->tpiMap.empty())
     return Error::success();
 
   assert(precompDependency.getStartTypeIndex() ==
          TypeIndex::FirstNonSimpleIndex);
   assert(precompDependency.getTypesCount() <= precompSrc->tpiMap.size());
   // Use the previously remapped index map from the precompiled headers.
-  indexMapStorage.append(precompSrc->tpiMap.begin(),
+  indexMapStorage.insert(indexMapStorage.begin(), precompSrc->tpiMap.begin(),
                          precompSrc->tpiMap.begin() +
                              precompDependency.getTypesCount());
 
   return Error::success();
 }
 
 Error UsePrecompSource::mergeDebugT(TypeMerger *m) {
   // This object was compiled with /Yu, so process the corresponding
forEachTypeChecked(file->debugTypes, [&](const CVType &ty) {
     isItemIndex.push_back(isIdRecord(ty.kind()));
   });
   hashVec.erase(hashVec.begin(),
                 hashVec.begin() + precompDependency.getTypesCount());
   assignGHashesFromVector(std::move(hashVec));
 }
 
 void UsePrecompSource::remapTpiWithGHashes(GHashState *g) {
+  fillMapFromGHashes(g, indexMapStorage);
   // This object was compiled with /Yu, so process the corresponding
   // precompiled headers object (/Yc) first. Some type indices in the current
   // object are referencing data in the precompiled headers object, so we need
   // both to be loaded.
   if (Error e = mergeInPrecompHeaderObj()) {
     typeMergingError = joinErrors(std::move(typeMergingError), std::move(e));
     return;
   }
 
-  fillMapFromGHashes(g, indexMapStorage);
   tpiMap = indexMapStorage;
   ipiMap = indexMapStorage;
   mergeUniqueTypeRecords(file->debugTypes,
                          TypeIndex(precompDependency.getStartTypeIndex() +
                                    precompDependency.getTypesCount()));
   if (config->showSummary) {
     nbTypeRecords = ghashes.size();
     nbTypeRecordsBytes = file->debugTypes.size();

 /// Given the index into the ghash table for a particular type, return the type
 /// index for that type in the output PDB.
 static TypeIndex loadPdbTypeIndexFromCell(GHashState *g,
                                           uint32_t ghashCellIdx) {
   GHashCell cell = g->table.table[ghashCellIdx];
   return TypeIndex::fromArrayIndex(cell.getGHashIdx());
 }
 
 // Fill in a TPI or IPI index map using ghashes. For each source type, use its
 // ghash to lookup its final type index in the PDB, and store that in the map.
 void TpiSource::fillMapFromGHashes(GHashState *g,
                                    SmallVectorImpl<TypeIndex> &mapToFill) {
   for (size_t i = 0, e = ghashes.size(); i < e; ++i) {
     TypeIndex fakeCellIndex = indexMapStorage[i];
     if (fakeCellIndex.isSimple())
       mapToFill[i] = fakeCellIndex;
     else
       mapToFill[i] = loadPdbTypeIndexFromCell(g, fakeCellIndex.toArrayIndex());
   }
 }

lld/test/COFF/precomp-ghash.test

+
+# This test ensures that under /DEBUG:GHASH, IPI records LF_FUNC_ID/LF_MFUNC_ID
+# have properly remapped indices to corresponding TPI records.
+
+RUN: lld-link %p/Inputs/precomp-ghash-precomp.obj \
+RUN:    %p/Inputs/precomp-ghash-obj1.obj\
+RUN:    %p/Inputs/precomp-ghash-obj2.obj /debug:ghash /out:%t.exe /pdb:%t.pdb
+RUN: llvm-pdbutil dump -types -ids %t.pdb | FileCheck %s
+
+; These object files were generated via the following inputs and commands:
+; ----------------------------------------------
+; // precomp-ghash-obj.h
+; namespace NS {
+;   struct Foo {
+;     explicit Foo(int x) : X(x) {}
+;     int X;
+;   };
+;
+;   int func(const Foo &f);
+; }
+; ----------------------------------------------
+; // precomp-ghash-obj1.cpp
+; #include "precomp-ghash-obj.h"
+;
+; int main(int argc, char **argv) {
+;   NS::Foo f(argc);
+;   return NS::func(f);
+; }
+; ----------------------------------------------
+; // precomp-ghash-obj2.cpp
+; #include "precomp-ghash-obj.h"
+;
+; int NS::func(const Foo &f) {
+;   return 2 * f.X;
+; }
+; ----------------------------------------------
+; // precomp-ghash-precomp.cpp
+; #include "precomp-ghash-obj.h"
+; ----------------------------------------------
+; $ cl /c /Z7 /GS- precomp-ghash-precomp.cpp /Ycprecomp-ghash-obj.h
+; $ cl /c /Z7 /GS- precomp-ghash-obj1.cpp /Yuprecomp-ghash-obj.h
+; $ cl /c /Z7 /GS- precomp-ghash-obj2.cpp /Yuprecomp-ghash-obj.h
+
+CHECK:                           Types (TPI Stream)
+CHECK-NEXT: ============================================================
+CHECK:   0x1003 | LF_MFUNCTION
+CHECK:   0x274F | LF_PROCEDURE
+CHECK:                     Types (IPI Stream)                     
+CHECK-NEXT: ============================================================
+CHECK:   0x189D | LF_FUNC_ID [size = 20]
+CHECK-NEXT:       name = main, type = 0x274F, parent scope = <no type>
+CHECK-NEXT:  0x189E | LF_MFUNC_ID [size = 20]
+CHECK-NEXT:           name = {ctor}, type = 0x1003, class type = 0x1000