[COFF][LLD] Add link support for precompiled headers .objs

[aganea]

Summary

This change allows for link-time merging of debugging information from
Microsoft precompiled types .objs compiled with cl.exe /Z7 /Yc and /Yu.

Previously, LLD used to crash when encountering these .objs on the command line.

Precompiled types use two specific records:

• LF_PRECOMP at the begining of a type stream, to signify a dependence on a

external precomp .obj

• LF_ENDPRECOMP in a precomp .obj, which marks the end of the type stream which

should be used by the dependent .obj.

When a .obj with a LF_PRECOMP reference is found, we preempt the processing of
that .obj, and attempt to first load the precompiled .obj. Once the precompiled
.obj is loaded and remapped, we simply take its (remapped) type IndexMap and
paste it in the TypeStreamMerger, just before merging in the dependent .obj's
type indices. This allows for back-referencing precomp type records which have
already merged.

LF_PRECOMP and LF_ENDPRECOMP records are ultimately dropped from the target PDB.

This fixes https://bugs.llvm.org/show_bug.cgi?id=34278

[rnk]

Can you split this into a patch that adds the basic YAML serialization and dumping functionality to LLVM, and a patch that adds the type merging capabilities across LLVM and LLD? The first one should take care of a lot of the boilerplate and we can stamp it quickly and then move on to review the complex type merging code.

[aganea]

Following https://reviews.llvm.org/D45283, I've updated the review only with the type merging and LLD support.
Some extra support would be needed for llvm-readobj -codeview-merged-types, but I prefer to do that later.

lld/COFF/PDB.cpp

303

I choose to preempt loading of the current .obj, and load the precomp .obj here. However if (future) multithreading is a concern, I could instead push the processing of the current .obj at the end of the stack. Let me know whichever you prefer.

312

The magic happens here.

[aganea]

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

488

I'm not very happy about this - I think I'll move it to lld/COFF/PDB.cpp instead.

[aganea]

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

488

After thought, I'll leave it the way it is - I can't find a better way to prevent the LF_ENDPRECOMP from being added to the output TypeTable.

[zturner]

Sorry for the delay, been OOO for 3 days.

BTW, When you upload the next version, can you make sure it has full context?

lld/COFF/PDB.cpp

258

Variable names should be CamelCase.

293

Would it be possible to write this as something like:

if (isExternalTypeStreamRecord(Types))
  return handleExternalTypeStreamRecord(Types);

Then this new handle function could handle both LF_TYPESERVER2 and LF_PRECOMP, and we wouldn't need the weird function with the boolean anymore

500–501

This doesn't look clang formatted.

506

Upper case F

507–510

Same here. Just fix all variable names actually.

542–544

Eww. How about returning an Expected<const CVIndexMap&> instead?

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

489–492

I noticed that when precompiled headers are used, the S_OBJNAME symbol also has a matching signature. Can / should this be used somehow?

[ruiu]

lld/COFF/PDB.cpp

258

Generally we avoid optional parameters. Please always explicitly pass true or false.

464

Please add a function comment explaining what this function is for.

This function seems a bit too long to me. Please consider splitting into smaller functions.

[aganea]

Adressed @zturner's and @ruiu's comments.

[aganea]

lld/COFF/PDB.cpp

293

Unfortunately we can't. In one case (PDB), we must return the full TPI/IPI map; in the other case (PCH) we must load the TPI map and fallthrough.

542–544

Please unsee this.

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

489–492

Yes, and the new revision (https://reviews.llvm.org/D45213?id=154301) is using it. It was probably meant there at the begining of the stream to bail out quickly, if the associated signature doesn't match the PCH session.

[aganea]

Currently splitting this in many smaller changes.

[aganea]

I've corrected the issues raised. Anything that wasn't related to this change was already commited separately.

This patch is now ready for review again.

[aganea]

Ping! Could anybody please take a look at this? Thank you!

[zturner]

How big are the object files?

lld/trunk/COFF/PDB.cpp

326–327 ↗

(On Diff #167531)

FWIW, all object files always start with an S_OBJNAME, even if there is no PCH support. But if there is no PCH support, the signature field will just be 0.

355 ↗

(On Diff #167531)

I think we should just return 0; here. As you said, if it's there, it will be first, so if it's not first, there's no point to continue looking in subsequent .debug$S sections.

425–426 ↗

(On Diff #167531)

I almost wonder if we should add CVTypeArray::drop_front(). You don't have to do it in this patch, just something I thought of.

626–627 ↗

(On Diff #167531)

Can you add static to this function?

635 ↗

(On Diff #167531)

In another patch we were discussing whether we should stop using path::Style::windows. When cross-compiling you might be on non-Windows, and these path comparisons would be incorrect in that case. So I think we should probably just use the default native style here.

638–639 ↗

(On Diff #167531)

It's hard to know exactly what to do here. Is equals_lower correct or equals? It seems like we need a sys::path::equals_path() that uses case-insensitive comparison on Windows and case-sensitive on Linux. Can you add equals_path() as a global function in this file and use that here instead?

lld/trunk/test/COFF/precomp-link.test

10 ↗

(On Diff #167531)

The # signs are confusing. I don't think you need to put any marker in front of them. You can just write it as code. lit and FileCheck ignore comment markers at the beginning of lines. and only match or run lines that have a RUN or CHECK line after any comments. So just putting raw code directly in the file is fine.

[zturner]

BTW, sorry this took so long for anyone to review. I think partly it's because nobody here really taken the time to understand in detail how MS precompiled headers works, so we were all hoping someone else would review it. :-/

Anyway, at least from my point of view (unless someone else chimes in) this looks good provided it works (which you say it does) and it has tests (which it does) and the last remaining issues I outlined are fixed.

[neerajksingh]

I just want to mention as a heads-up: there will later be some complexities in this PCH finding code when the /deterministic flags are added to public builds of cl.exe. PCH files paths might be replaced with environment variables that will need to be expanded when acquiring a PDB.

FYI, in the MS linker, we find out early on that a file contains precompiled type information from the presence of the debug$P section and mostly process those files before processing the non-PCT files. That avoids some need to search when acquiring the PCH data.

[rnk]

In D45213#1269836, @neerajksingh wrote:

FYI, in the MS linker, we find out early on that a file contains precompiled type information from the presence of the debug$P section and mostly process those files before processing the non-PCT files. That avoids some need to search when acquiring the PCH data.

Thanks, that's interesting.

llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

100 ↗

(On Diff #167531)

I was hoping to come up with some way to avoid the extra out parameter, but that's my only real concern. Structurally, everything else looks like type servers, and it's pretty straightforward.

[rnk]

I meant to add more before hitting submit...

I looked at the linker part, and things make sense to me. I trust that Zach looked more at the mechanics. I think we should fix the remaining issues he highlighted, and otherwise this is ready.

[aganea]

Please see the updated diff which adresses comments.

In D45213#1259147, @zturner wrote:

How big are the object files?

65.6 KB. I want to ensure things work smoothly with cl-generated OBJs.

lld/trunk/COFF/PDB.cpp

425–426 ↗

(On Diff #167531)

I went ahead and added VarStreamArray::drop_front(), please let me know if that sounds good.

llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

100 ↗

(On Diff #167531)

I agree - in a subsequent chage we could fold the "merge" behavior into a single function, add a initializer struct, and call the function with a initializer list instead, ie. mergeRecords({Dest, Types, Hashes, EP}). That way we could accomodate different merging scenarios, and have only one codepath.

[aganea]

Update diff again:

• Added more coverage tests.
• Added comment to clarify the sys::path::Style::windows and precomp OBJs situation when cross-compiling.

[aganea]

In D45213#1259147, @zturner wrote:

FWIW, all object files always start with an S_OBJNAME, even if there is no PCH support. But if there is no PCH support, the signature field will just be 0.

Just wanted to point out that it's only true for cl-generated OBJs. clang-cl does not generate the S_OBJNAME record.

lld/trunk/COFF/PDB.cpp

686 ↗

(On Diff #171104)

@zturner I've added this comment.

[rnk]

In D45213#1275808, @aganea wrote:

In D45213#1259147, @zturner wrote:

FWIW, all object files always start with an S_OBJNAME, even if there is no PCH support. But if there is no PCH support, the signature field will just be 0.

Just wanted to point out that it's only true for cl-generated OBJs. clang-cl does not generate the S_OBJNAME record.

True. I added S_BUILDINFO the other week.

I think @zturner had a stalled patch to add S_OBJNAME. Yeah, this: https://reviews.llvm.org/D43002. I think the lesson is that it might be better not to stuff more things into DICompileUnit than absolutely necessary.

Is there anything else that needs to be done here? I'm happy with this.

lld/COFF/InputFiles.h

 #include "lld/Common/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/COFF.h"

   // Returns the underying COFF file.
   COFFObjectFile *getCOFFObj() { return COFFObj.get(); }
 
+  // Whether the .obj 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

   // if we are not producing a PDB.
   llvm::pdb::DbiModuleDescriptorBuilder *ModuleDBI = nullptr;
 
+  // If the .obj represents a precompiled types type map, we need the signature
+  // through the following record
+  llvm::Optional<llvm::codeview::EndPrecompRecord> EndPrecomp;
+
 private:
   void initializeChunks();
   void initializeSymbols();

lld/COFF/PDB.cpp

 #include "llvm/Object/CVDebugRecord.h"
 #include "llvm/Support/BinaryByteStream.h"
 #include "llvm/Support/Endian.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JamCRC.h"
 #include "llvm/Support/Path.h"

   SmallVector<TypeIndex, 0> TPIMap;
   SmallVector<TypeIndex, 0> IPIMap;
   bool IsTypeServerMap = false;
+  bool IsPrecompiledTypeMap = false;
 };
 
 class PDBLinker {

   void addObjectsToPDB();
 
   /// Link CodeView from a single object file into the PDB.
-  void addObjFile(ObjFile *File);
+  void addObjFile(ObjFile *File, CVIndexMap *CV = nullptr);
 
   /// Produce a mapping from the type and item indices used in the object
   /// file to those in the destination PDB.

   Expected<const CVIndexMap&> maybeMergeTypeServerPDB(ObjFile *File,
                                                       TypeServer2Record &TS);
 
+  const CVIndexMap &aquirePrecompObj(ObjFile *File, PrecompRecord &Precomp);
+
   /// Add the section map and section contributions to the PDB.
   void addSections(ArrayRef<OutputSection *> OutputSections,
                    ArrayRef<uint8_t> SectionTable);

   /// Type index mappings of type server PDBs that we've loaded so far.
   std::map<GUID, CVIndexMap> TypeServerIndexMappings;
 
+  /// Type index mappings of precompiled type type map that we've loaded so far.
+  std::map<uint32_t, CVIndexMap> PrecompTypeIndexMappings;
+
   /// List of TypeServer PDBs which cannot be loaded.
   /// Cached to prevent repeated load attempts.
   std::set<GUID> MissingTypeServerPDBs;

   });
 }
 
-static Optional<TypeServer2Record>
-maybeReadTypeServerRecord(CVTypeArray &Types) {
+template <typename RecordType>
+static Optional<RecordType>
+maybeReadRecord(CVTypeArray &Types, const uint32_t RecordTypeValue,
+                const bool drop = false) {
   auto I = Types.begin();
   if (I == Types.end())
     return None;
   const CVType &Type = *I;
-  if (Type.kind() != LF_TYPESERVER2)
+  if (Type.kind() != RecordTypeValue)
     return None;
-  TypeServer2Record TS;
-  if (auto EC = TypeDeserializer::deserializeAs(const_cast<CVType &>(Type), TS))
-    fatal("error reading type server record: " + toString(std::move(EC)));
-  return std::move(TS);
+  RecordType R;
+  if (auto EC = TypeDeserializer::deserializeAs(const_cast<CVType &>(Type), R))
+    fatal("error reading record: " + toString(std::move(EC)));
+  if (drop)
+    Types.setUnderlyingStream(
+                        Types.getUnderlyingStream().drop_front(Type.length()));
+  return std::move(R);
 }
 
 Expected<const CVIndexMap&> PDBLinker::mergeDebugT(ObjFile *File,

   ScopedTimer T(TypeMergingTimer);
 
   ArrayRef<uint8_t> Data = getDebugSection(File, ".debug$T");
+  // Try again, precompiled headers use .debug$P instead of .debug$T
   if (Data.empty())
+    Data = getDebugSection(File, ".debug$P");
+  if (Data.empty())
     return ObjectIndexMap;
 
   BinaryByteStream Stream(Data, support::little);

 
   // Look through type servers. If we've already seen this type server, don't
   // merge any type information.
-  if (Optional<TypeServer2Record> TS = maybeReadTypeServerRecord(Types))
-    return maybeMergeTypeServerPDB(File, *TS);
+  if (Optional<TypeServer2Record> TS =
+                      maybeReadRecord<TypeServer2Record>(Types, LF_TYPESERVER2))
+      return maybeMergeTypeServerPDB(File, *TS);
 
-  // This is a /Z7 object. Fill in the temporary, caller-provided
-  // ObjectIndexMap.
+  // This is a /Z7 object.
+  // If the current .obj was compiled with /Yu, then process the corresponding
+  // precompiled .obj (/Yc) first. Some type indices in the current .obj are
+  // referencing data in the precompiled .obj, so we need both to be loaded.
+  if (Optional<PrecompRecord> Precomp =
+                      maybeReadRecord<PrecompRecord>(Types, LF_PRECOMP, true)) {
+    const CVIndexMap &PrecompIndexMap = aquirePrecompObj(File, *Precomp);
+    assert(PrecompIndexMap.IsPrecompiledTypeMap);
+
+    if (PrecompIndexMap.TPIMap.empty())
+      return PrecompIndexMap;
+
+    assert(Precomp->getStartTypeIndex() == TypeIndex::FirstNonSimpleIndex);
+    assert(Precomp->getTypesCount() <= PrecompIndexMap.TPIMap.size());
+    // Use the precomp's previously remapped index map
+    ObjectIndexMap.TPIMap.append(PrecompIndexMap.TPIMap.begin(),
+               PrecompIndexMap.TPIMap.begin() + Precomp->getTypesCount());
+  }
+  
+  // Fill in the temporary, caller-provided ObjectIndexMap.
   if (Config->DebugGHashes) {
     ArrayRef<GloballyHashedType> Hashes;
     std::vector<GloballyHashedType> OwnedHashes;

     }
 
     if (auto Err = mergeTypeAndIdRecords(GlobalIDTable, GlobalTypeTable,
-                                         ObjectIndexMap.TPIMap, Types, Hashes))
+                                         ObjectIndexMap.TPIMap, Types, Hashes,
+                                         File->EndPrecomp))
       fatal("codeview::mergeTypeAndIdRecords failed: " +
             toString(std::move(Err)));
   } else {
     if (auto Err = mergeTypeAndIdRecords(IDTable, TypeTable,
-                                         ObjectIndexMap.TPIMap, Types))
+                                         ObjectIndexMap.TPIMap, Types,
+                                         File->EndPrecomp))
       fatal("codeview::mergeTypeAndIdRecords failed: " +
             toString(std::move(Err)));
   }
+
   return ObjectIndexMap;
 }
 

     auto IpiHashes =
         GloballyHashedType::hashIds(ExpectedIpi->typeArray(), TpiHashes);
 
+    Optional<EndPrecompRecord> EndPrecomp;
     // Merge TPI first, because the IPI stream will reference type indices.
     if (auto Err = mergeTypeRecords(GlobalTypeTable, IndexMap.TPIMap,
-                                    ExpectedTpi->typeArray(), TpiHashes))
+                                    ExpectedTpi->typeArray(), TpiHashes,
+                                    EndPrecomp))
       fatal("codeview::mergeTypeRecords failed: " + toString(std::move(Err)));
 
     // Merge IPI.

   return IndexMap;
 }
 
+const CVIndexMap &PDBLinker::aquirePrecompObj(ObjFile *File,
+                                              PrecompRecord &Precomp) {
+
+  // First, check if we already loaded the precompiled types .obj with this
+  // signature. Return the type index mapping if we have it.
+  auto Insertion = PrecompTypeIndexMappings.insert(
+                                       {Precomp.getSignature(), CVIndexMap()});
+  CVIndexMap &IndexMap = Insertion.first->second;
+  if (!Insertion.second)
+    return IndexMap;
+
+  // Mark this map as a precompiled types map.
+  IndexMap.IsPrecompiledTypeMap = true;
+
+  SmallString<128> PrecompPath = Precomp.getPrecompFilePath();
+  sys::fs::make_absolute(PrecompPath);
+  sys::path::native(PrecompPath, sys::path::Style::windows);
+
+  ObjFile *CurrentFile = nullptr;
+  SmallString<128> CurrentPath;
+
+  // link.exe requires that a precompiled object must always be provided
+  // on the command-line
+  // First, try comparing the whole path if possible
+  for (ObjFile *f : ObjFile::Instances) {
+    CurrentPath = f->getName();
+    sys::fs::make_absolute(CurrentPath);
+    sys::path::native(CurrentPath, sys::path::Style::windows);
+
+    if (CurrentPath.equals_lower(PrecompPath)) {
+      CurrentFile = f;
+      break;
+    }
+    CurrentPath.clear();
+  }
+
+  if (CurrentPath.empty())
+  {
+    // If the precomp object was on a different path, we can still use it,
+    // although only if its unique on the command-line
+    StringRef PrecompName = sys::path::filename(PrecompPath);
+    CurrentPath.clear();
+    for (ObjFile *f : ObjFile::Instances) {
+      SmallString<128> path = f->getName();
+      sys::fs::make_absolute(path);
+      sys::path::native(path, sys::path::Style::windows);
+      StringRef filename = sys::path::filename(path);
+
+      if (!filename.equals_lower(PrecompName))
+          continue;
+
+      if (!CurrentPath.empty())
+          fatal("the precompiled object name must appear once on the "
+                "command-line: " + PrecompName);
+
+      CurrentPath = filename;
+      CurrentFile = f;
+    }
+  }
+
+  if (!CurrentPath.empty())
+  {
+    addObjFile(CurrentFile, &IndexMap);
+
+    if (!CurrentFile->EndPrecomp)
+      fatal(CurrentPath + " is not a precompiled .obj");
+
+    if (Precomp.getSignature() != CurrentFile->EndPrecomp->getSignature())
+      warn("the signature of the precompiled file " + CurrentPath + " does not "
+           "match; the file(s) might be out of date. Linking as if no debug "
+           "info.");
+
+    return IndexMap;
+  }
+
+  warn("dependent precompiled object not found: " + PrecompPath +
+       ", linking as if no debug info (" + File->getName() + ")");
+
+  static CVIndexMap emptyMap;
+  emptyMap.IsPrecompiledTypeMap = true;
+  return emptyMap;
+}
+
 static bool remapTypeIndex(TypeIndex &TI, ArrayRef<TypeIndex> TypeIndexMap) {
   if (TI.isSimple())
     return true;

                            ".debug$S");
 }
 
-void PDBLinker::addObjFile(ObjFile *File) {
+void PDBLinker::addObjFile(ObjFile *File, CVIndexMap *CV) {
+  if (File->wasProcessedForPDB())
+    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

   // 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, ObjectIndexMap);
+  auto IndexMapResult = mergeDebugT(File, CV ? *CV : ObjectIndexMap);
 
   // If the .debug$T sections fail to merge, assume there is no debug info.
   if (!IndexMapResult) {

lld/test/COFF/precomp-link.test

+
+RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj \
+RUN:          %S/Inputs/precomp.obj /nodefaultlib /entry:main \
+RUN:          /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf
+RUN: llvm-pdbutil dump -types %t.pdb | FileCheck %s
+
+CHECK: Types (TPI Stream)
+CHECK-NOT: LF_PRECOMP
+CHECK-NOT: LF_ENDPRECOMP
+
+# // precomp.h
+# #pragma once
+# int Function(char A);
+
+# // precomp.cpp
+# #include "precomp.h"
+
+# // a.cpp
+# #include "precomp.h"
+# int main(void) {
+#   Function('a');
+#   return 0;
+# }
+
+# // b.cpp
+# #include "precomp.h"
+# int Function(char a) {
+#   return (int)a;
+# }
+
+# // cl.exe precomp.cpp /Z7 /Ycprecomp.h /c
+# // cl.exe a.cpp b.cpp /Z7 /Yuprecomp.h /c

llvm/include/llvm/DebugInfo/CodeView/TypeStreamMerger.h

 Error mergeTypeAndIdRecords(MergingTypeTableBuilder &DestIds,
                             MergingTypeTableBuilder &DestTypes,
                             SmallVectorImpl<TypeIndex> &SourceToDest,
-                            const CVTypeArray &IdsAndTypes);
+                            const CVTypeArray &IdsAndTypes,
+                            Optional<EndPrecompRecord> &EndPrecomp);
 
 Error mergeTypeAndIdRecords(GlobalTypeTableBuilder &DestIds,
                             GlobalTypeTableBuilder &DestTypes,
                             SmallVectorImpl<TypeIndex> &SourceToDest,
                             const CVTypeArray &IdsAndTypes,
-                            ArrayRef<GloballyHashedType> Hashes);
+                            ArrayRef<GloballyHashedType> Hashes,
+                            Optional<EndPrecompRecord> &EndPrecomp);
 
 Error mergeTypeRecords(GlobalTypeTableBuilder &Dest,
                        SmallVectorImpl<TypeIndex> &SourceToDest,
                        const CVTypeArray &Types,
-                       ArrayRef<GloballyHashedType> Hashes);
+                       ArrayRef<GloballyHashedType> Hashes,
+                       Optional<EndPrecompRecord> &EndPrecomp);
 
 Error mergeIdRecords(GlobalTypeTableBuilder &Dest, ArrayRef<TypeIndex> Types,
                      SmallVectorImpl<TypeIndex> &SourceToDest,

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

 #include "llvm/ADT/StringExtras.h"
 #include "llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h"
 #include "llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h"
+#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeIndexDiscovery.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"

 public:
   explicit TypeStreamMerger(SmallVectorImpl<TypeIndex> &SourceToDest)
       : IndexMap(SourceToDest) {
-    SourceToDest.clear();
+    // A precompiled obj might be referenced here:
+    // All data in SourceToDest is assumed to belong to the precompiled obj,
+    // and is already remapped to the target PDB.
+    // Every type that will be merged in needs to back-reference this data.
+    // We also don't want to resolve twice the types in the precompiled obj.
+    CurIndex += SourceToDest.size();
   }
 
   static const TypeIndex Untranslated;

   // Local hashing entry points
   Error mergeTypesAndIds(MergingTypeTableBuilder &DestIds,
                          MergingTypeTableBuilder &DestTypes,
-                         const CVTypeArray &IdsAndTypes);
+                         const CVTypeArray &IdsAndTypes,
+                         Optional<EndPrecompRecord> &EP);
   Error mergeIdRecords(MergingTypeTableBuilder &Dest,
                        ArrayRef<TypeIndex> TypeSourceToDest,
                        const CVTypeArray &Ids);

   Error mergeTypesAndIds(GlobalTypeTableBuilder &DestIds,
                          GlobalTypeTableBuilder &DestTypes,
                          const CVTypeArray &IdsAndTypes,
-                         ArrayRef<GloballyHashedType> Hashes);
+                         ArrayRef<GloballyHashedType> Hashes,
+                         Optional<EndPrecompRecord> &EP);
   Error mergeIdRecords(GlobalTypeTableBuilder &Dest,
                        ArrayRef<TypeIndex> TypeSourceToDest,
                        const CVTypeArray &Ids,
                        ArrayRef<GloballyHashedType> Hashes);
-  Error mergeTypeRecords(GlobalTypeTableBuilder &Dest, const CVTypeArray &Types,
-                         ArrayRef<GloballyHashedType> Hashes);
+  Error mergeTypeRecords(GlobalTypeTableBuilder &Dest,
+                         const CVTypeArray &Types,
+                         ArrayRef<GloballyHashedType> Hashes,
+                         Optional<EndPrecompRecord> &EP);
 
 private:
   Error doit(const CVTypeArray &Types);

     return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);
   }
 
+  Expected<bool> shouldRemapType(const CVType &Type);
+
   Optional<Error> LastError;
 
   bool UseGlobalHashes = false;

   /// Temporary storage that we use to copy a record's data while re-writing
   /// its type indices.
   SmallVector<uint8_t, 256> RemapStorage;
+
+  Optional<EndPrecompRecord> EndPrecomp;
 };
 
 } // end anonymous namespace

   if (IsSecondPass && MapPos >= Map.size()) {
     // FIXME: Print a more useful error. We can give the current record and the
     // index that we think its pointing to.
-    LastError = joinErrors(std::move(*LastError), errorCorruptRecord());
+    if (LastError)
+      LastError = joinErrors(std::move(*LastError), errorCorruptRecord());
+    else
+      LastError = errorCorruptRecord();
   }
 
   ++NumBadIndices;

 
 Error TypeStreamMerger::mergeTypesAndIds(MergingTypeTableBuilder &DestIds,
                                          MergingTypeTableBuilder &DestTypes,
-                                         const CVTypeArray &IdsAndTypes) {
+                                         const CVTypeArray &IdsAndTypes,
+                                         Optional<EndPrecompRecord> &EP) {
   DestIdStream = &DestIds;
   DestTypeStream = &DestTypes;
   UseGlobalHashes = false;
-  return doit(IdsAndTypes);
+
+  auto e = doit(IdsAndTypes);
+
+  EP = EndPrecomp;
+  return e;
 }
 
 // Global hashing entry points
 Error TypeStreamMerger::mergeTypeRecords(GlobalTypeTableBuilder &Dest,
                                          const CVTypeArray &Types,
-                                         ArrayRef<GloballyHashedType> Hashes) {
+                                         ArrayRef<GloballyHashedType> Hashes,
+                                         Optional<EndPrecompRecord> &EP) {
   DestGlobalTypeStream = &Dest;
   UseGlobalHashes = true;
   GlobalHashes = Hashes;
 
-  return doit(Types);
+  auto e = doit(Types);
+
+  EP = EndPrecomp;
+  return e;
 }
 
 Error TypeStreamMerger::mergeIdRecords(GlobalTypeTableBuilder &Dest,

 Error TypeStreamMerger::mergeTypesAndIds(GlobalTypeTableBuilder &DestIds,
                                          GlobalTypeTableBuilder &DestTypes,
                                          const CVTypeArray &IdsAndTypes,
-                                         ArrayRef<GloballyHashedType> Hashes) {
+                                         ArrayRef<GloballyHashedType> Hashes,
+                                         Optional<EndPrecompRecord> &EP) {
   DestGlobalIdStream = &DestIds;
   DestGlobalTypeStream = &DestTypes;
   UseGlobalHashes = true;
   GlobalHashes = Hashes;
-  return doit(IdsAndTypes);
+  
+  auto e = doit(IdsAndTypes);
+  
+  EP = EndPrecomp;
+  return e;
 }
 
 Error TypeStreamMerger::doit(const CVTypeArray &Types) {

 }
 
 Error TypeStreamMerger::remapType(const CVType &Type) {
-  auto DoSerialize =
-      [this, Type](MutableArrayRef<uint8_t> Storage) -> ArrayRef<uint8_t> {
-    return remapIndices(Type, Storage);
-  };
+  auto R = shouldRemapType(Type);
+  if (!R)
+    return R.takeError();
 
   TypeIndex DestIdx = Untranslated;
-  if (LLVM_LIKELY(UseGlobalHashes)) {
-    GlobalTypeTableBuilder &Dest =
-        isIdRecord(Type.kind()) ? *DestGlobalIdStream : *DestGlobalTypeStream;
-    GloballyHashedType H = GlobalHashes[CurIndex.toArrayIndex()];
-    DestIdx = Dest.insertRecordAs(H, Type.RecordData.size(), DoSerialize);
-  } else {
-    MergingTypeTableBuilder &Dest =
-        isIdRecord(Type.kind()) ? *DestIdStream : *DestTypeStream;
+  if (*R) {
+    auto DoSerialize =
+        [this, Type](MutableArrayRef<uint8_t> Storage) -> ArrayRef<uint8_t> {
+      return remapIndices(Type, Storage);
+    };
+    if (LLVM_LIKELY(UseGlobalHashes)) {
+      GlobalTypeTableBuilder &Dest =
+          isIdRecord(Type.kind()) ? *DestGlobalIdStream : *DestGlobalTypeStream;
+      GloballyHashedType H = GlobalHashes[CurIndex.toArrayIndex()];
+      DestIdx = Dest.insertRecordAs(H, Type.RecordData.size(), DoSerialize);
+    } else {
+      MergingTypeTableBuilder &Dest =
+          isIdRecord(Type.kind()) ? *DestIdStream : *DestTypeStream;
 
       RemapStorage.resize(Type.RecordData.size());
       ArrayRef<uint8_t> Result = DoSerialize(RemapStorage);
       if (!Result.empty())
         DestIdx = Dest.insertRecordBytes(Result);
+    }
   }
   addMapping(DestIdx);
 

 
 Error llvm::codeview::mergeTypeAndIdRecords(
     MergingTypeTableBuilder &DestIds, MergingTypeTableBuilder &DestTypes,
-    SmallVectorImpl<TypeIndex> &SourceToDest, const CVTypeArray &IdsAndTypes) {
+    SmallVectorImpl<TypeIndex> &SourceToDest, const CVTypeArray &IdsAndTypes,
+    Optional<EndPrecompRecord> &EndPrecomp) {
   TypeStreamMerger M(SourceToDest);
-  return M.mergeTypesAndIds(DestIds, DestTypes, IdsAndTypes);
+  return M.mergeTypesAndIds(DestIds, DestTypes, IdsAndTypes, EndPrecomp);
 }
 
 Error llvm::codeview::mergeTypeAndIdRecords(
     GlobalTypeTableBuilder &DestIds, GlobalTypeTableBuilder &DestTypes,
     SmallVectorImpl<TypeIndex> &SourceToDest, const CVTypeArray &IdsAndTypes,
-    ArrayRef<GloballyHashedType> Hashes) {
+    ArrayRef<GloballyHashedType> Hashes,
+    Optional<EndPrecompRecord> &EndPrecomp) {
   TypeStreamMerger M(SourceToDest);
-  return M.mergeTypesAndIds(DestIds, DestTypes, IdsAndTypes, Hashes);
+  return M.mergeTypesAndIds(DestIds, DestTypes, IdsAndTypes, Hashes,
+                            EndPrecomp);
 }
 
 Error llvm::codeview::mergeTypeRecords(GlobalTypeTableBuilder &Dest,
                                        SmallVectorImpl<TypeIndex> &SourceToDest,
                                        const CVTypeArray &Types,
-                                       ArrayRef<GloballyHashedType> Hashes) {
+                                       ArrayRef<GloballyHashedType> Hashes,
+                                       Optional<EndPrecompRecord> &EndPrecomp) {
   TypeStreamMerger M(SourceToDest);
-  return M.mergeTypeRecords(Dest, Types, Hashes);
+  return M.mergeTypeRecords(Dest, Types, Hashes, EndPrecomp);
 }
 
 Error llvm::codeview::mergeIdRecords(GlobalTypeTableBuilder &Dest,

   TypeStreamMerger M(SourceToDest);
   return M.mergeIdRecords(Dest, Types, Ids, Hashes);
 }
+
+Expected<bool> TypeStreamMerger::shouldRemapType(const CVType &Type) {
+  // For .obj files containing precompiled types, we need to extract the
+  // signature, through EndPrecompRecord. This is done here for performance
+  // reasons, to avoid re-parsing the Types stream; in most cases, this .obj
+  // will not be a precompiled types file.
+  if (Type.kind() == LF_ENDPRECOMP) {
+    assert(!EndPrecomp);
+    EndPrecomp.emplace();
+    if (auto EC = TypeDeserializer::deserializeAs(const_cast<CVType &>(Type),
+                                                  EndPrecomp.getValue()))
+      return joinErrors(std::move(EC), errorCorruptRecord());
+    return false;
+  }
+  return true;
+}

llvm/tools/llvm-readobj/COFFDumper.cpp

         error(object_error::parse_failed);
       }
       SmallVector<TypeIndex, 128> SourceToDest;
-      if (auto EC = mergeTypeAndIdRecords(CVIDs, CVTypes, SourceToDest, Types))
+      Optional<EndPrecompRecord> EndPrecomp;
+      if (auto EC = mergeTypeAndIdRecords(CVIDs, CVTypes, SourceToDest, Types,
+                                          EndPrecomp))
         return error(std::move(EC));
     }
   }