[PDB] Fix type server handling for archives

[rnk]

Summary

This fixes type indices for SDK or CRT static archives. Previously we'd
try to look next to the archive object file path, which would not exist
on the local machine.

Also error out if we can't resolve a type server record. Hypothetically
we can recover from this error by discarding debug info for this object,
but that is not yet implemented.

[ruiu]

LGTM

lld/trunk/COFF/PDB.cpp

static void mergeDebugT(ObjectFile *File,
                         TypeTableBuilder &IDTable,
                         TypeTableBuilder &TypeTable,
                         SmallVectorImpl<TypeIndex> &TypeIndexMap,
                         pdb::PDBTypeServerHandler &Handler) {
   ArrayRef<uint8_t> Data = getDebugSection(File, ".debug$T");
   if (Data.empty())
     return;
 
+  // Look for type server PDBs next to the input file. If this file has a parent
+  // archive, look next to the archive path.
+  StringRef LocalPath =
+      !File->ParentName.empty() ? File->ParentName : File->getName();
+  Handler.addSearchPath(sys::path::parent_path(LocalPath));
+
   BinaryByteStream Stream(Data, support::little);
   CVTypeArray Types;
   BinaryStreamReader Reader(Stream);
-  Handler.addSearchPath(sys::path::parent_path(File->getName()));
   if (auto EC = Reader.readArray(Types, Reader.getLength()))
     fatal(EC, "Reader::readArray failed");
-  if (auto Err = mergeTypeAndIdRecords(IDTable, TypeTable,
-                                                 TypeIndexMap, &Handler, Types))
+  if (auto Err = mergeTypeAndIdRecords(IDTable, TypeTable, TypeIndexMap,
+                                       &Handler, Types))
     fatal(Err, "codeview::mergeTypeStreams failed");
 }
 
 static bool remapTypeIndex(TypeIndex &TI, ArrayRef<TypeIndex> TypeIndexMap) {
   if (TI.isSimple())
     return true;
   if (TI.toArrayIndex() >= TypeIndexMap.size())
     return false;

lld/trunk/test/COFF/pdb-type-server-missing.yaml

+# This is an object compiled with /Zi (see the LF_TYPESERVER2 record) without an
+# adjacent type server PDB. Test that LLD fails gracefully on it.
+
+# FIXME: Ideally we'd do what MSVC does, which is to warn and drop all debug
+# info in the object with the missing PDB.
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: not lld-link %t.obj -out:%t.exe -debug -pdb:%t.pdb -nodefaultlib -entry:main 2>&1 | FileCheck %s
+
+# CHECK: error: {{.*}} Type server PDB was not found
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    Subsections:
+      - !Symbols
+        Records:
+          - Kind:            S_GPROC32_ID
+            ProcSym:
+              CodeSize:        3
+              DbgStart:        0
+              DbgEnd:          2
+              FunctionType:    4199
+              Flags:           [  ]
+              DisplayName:     main
+          - Kind:            S_FRAMEPROC
+            FrameProcSym:
+              TotalFrameBytes: 0
+              PaddingFrameBytes: 0
+              OffsetToPadding: 0
+              BytesOfCalleeSavedRegisters: 0
+              OffsetOfExceptionHandler: 0
+              SectionIdOfExceptionHandler: 0
+              Flags:           [ AsynchronousExceptionHandling, OptimizedForSpeed ]
+          - Kind:            S_PROC_ID_END
+            ScopeEndSym:
+      - !Lines
+        CodeSize:        3
+        Flags:           [  ]
+        RelocOffset:     0
+        RelocSegment:    0
+        Blocks:
+          - FileName:        'c:\src\llvm-project\build\t.c'
+            Lines:
+              - Offset:          0
+                LineStart:       1
+                IsStatement:     true
+                EndDelta:        0
+            Columns:
+      - !FileChecksums
+        Checksums:
+          - FileName:        'c:\src\llvm-project\build\t.c'
+            Kind:            MD5
+            Checksum:        270A878DCC1B845655B162F56C4F5020
+      - !StringTable
+        Strings:
+          - 'c:\src\llvm-project\build\t.c'
+    Relocations:
+      - VirtualAddress:  44
+        SymbolName:      main
+        Type:            IMAGE_REL_AMD64_SECREL
+      - VirtualAddress:  48
+        SymbolName:      main
+        Type:            IMAGE_REL_AMD64_SECTION
+      - VirtualAddress:  100
+        SymbolName:      main
+        Type:            IMAGE_REL_AMD64_SECREL
+      - VirtualAddress:  104
+        SymbolName:      main
+        Type:            IMAGE_REL_AMD64_SECTION
+  - Name:            '.debug$T'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    Types:
+      - Kind:            LF_TYPESERVER2
+        TypeServer2:
+          Guid:            ' ?  "?kB??RX?2??' # FIXME: yaml-ize this as hex.
+          Age:             18
+          Name:            'C:\src\llvm-project\build\definitely_not_found_for_sure.pdb'
+  - Name:            '.text$mn'
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     33C0C3
+symbols:
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          328
+      NumberOfRelocations: 4
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            '.debug$T'
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          564
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            '.text$mn'
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          3
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        4021952397
+      Number:          0
+  - Name:            main
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...

llvm/trunk/include/llvm/DebugInfo/PDB/GenericError.h

 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace pdb {
 
 enum class generic_error_code {
   invalid_path = 1,
   dia_sdk_not_present,
+  type_server_not_found,
   unspecified,
 };
 
 /// Base class for errors originating when parsing raw PDB files
 class GenericError : public ErrorInfo<GenericError> {
 public:
   static char ID;
   GenericError(generic_error_code C);

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

Error TypeStreamMerger::mergeIdRecords(TypeTableBuilder &Dest,
   DestIdStream = &Dest;
   TypeLookup = TypeSourceToDest;
 
   return doit(Ids);
 }
 
 Error TypeStreamMerger::mergeTypesAndIds(TypeTableBuilder &DestIds,
                                          TypeTableBuilder &DestTypes,
                                          const CVTypeArray &IdsAndTypes) {
   DestIdStream = &DestIds;
   DestTypeStream = &DestTypes;
-
   return doit(IdsAndTypes);
 }
 
 Error TypeStreamMerger::doit(const CVTypeArray &Types) {
-  LastError = Error::success();
-
   // We don't want to deserialize records.  I guess this flag is poorly named,
   // but it really means "Don't deserialize records before switching on the
   // concrete type.
   // FIXME: We can probably get even more speed here if we don't use the visitor
   // pipeline here, but instead write the switch ourselves.  I don't think it
   // would buy us much since it's already pretty fast, but it's probably worth
   // a few cycles.
   if (auto EC =
           codeview::visitTypeStream(Types, *this, VDS_BytesExternal, Handler))
     return EC;
 
   // If we found bad indices but no other errors, try doing another pass and see
   // if we can resolve the indices that weren't in the map on the first pass.
   // This may require multiple passes, but we should always make progress. MASM
   // is the only known CodeView producer that makes type streams that aren't
   // topologically sorted. The standard library contains MASM-produced objects,
   // so this is important to handle correctly, but we don't have to be too
   // efficient. MASM type streams are usually very small.
-  while (!*LastError && NumBadIndices > 0) {
+  while (!LastError && NumBadIndices > 0) {
     unsigned BadIndicesRemaining = NumBadIndices;
     IsSecondPass = true;
     NumBadIndices = 0;
     CurIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex);
 
     if (auto EC =
             codeview::visitTypeStream(Types, *this, VDS_BytesExternal, Handler))
       return EC;
 
     assert(NumBadIndices <= BadIndicesRemaining &&
            "second pass found more bad indices");
-    if (!*LastError && NumBadIndices == BadIndicesRemaining) {
+    if (!LastError && NumBadIndices == BadIndicesRemaining) {
       return llvm::make_error<CodeViewError>(
           cv_error_code::corrupt_record, "input type graph contains cycles");
     }
   }
 
-  Error Ret = std::move(*LastError);
-  LastError.reset();
-  return Ret;
+  if (LastError)
+    return std::move(*LastError);
+  return Error::success();
 }
 
 Error llvm::codeview::mergeTypeRecords(TypeTableBuilder &Dest,
                                        SmallVectorImpl<TypeIndex> &SourceToDest,
                                        TypeServerHandler *Handler,
   const CVTypeArray &Types) {
   TypeStreamMerger M(SourceToDest, Handler);
   return M.mergeTypeRecords(Dest, Types);
 }
 
 Error llvm::codeview::mergeIdRecords(TypeTableBuilder &Dest,
                                      ArrayRef<TypeIndex> TypeSourceToDest,
                                      SmallVectorImpl<TypeIndex> &SourceToDest,
   const CVTypeArray &Ids) {
   TypeStreamMerger M(SourceToDest, nullptr);
   return M.mergeIdRecords(Dest, TypeSourceToDest, Ids);
 }
 
 Error llvm::codeview::mergeTypeAndIdRecords(
     TypeTableBuilder &DestIds, TypeTableBuilder &DestTypes,
     SmallVectorImpl<TypeIndex> &SourceToDest, TypeServerHandler *Handler,
     const CVTypeArray &IdsAndTypes) {
-
   TypeStreamMerger M(SourceToDest, Handler);
   return M.mergeTypesAndIds(DestIds, DestTypes, IdsAndTypes);
 }

llvm/trunk/lib/DebugInfo/PDB/GenericError.cpp

 class GenericErrorCategory : public std::error_category {
 public:
   const char *name() const noexcept override { return "llvm.pdb"; }
 
   std::string message(int Condition) const override {
     switch (static_cast<generic_error_code>(Condition)) {
     case generic_error_code::unspecified:
       return "An unknown error has occurred.";
+    case generic_error_code::type_server_not_found:
+      return "Type server PDB was not found.";
     case generic_error_code::dia_sdk_not_present:
       return "LLVM was not compiled with support for DIA.  This usually means "
              "that you are are not using MSVC, or your Visual Studio "
              "installation "
              "is corrupt.";
     case generic_error_code::invalid_path:
       return "Unable to load PDB.  Make sure the file exists and is readable.";
     }

llvm/trunk/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp

 Expected<bool>
 PDBTypeServerHandler::handleInternal(PDBFile &File,
                                      TypeVisitorCallbacks &Callbacks) {
   auto ExpectedTpi = File.getPDBTpiStream();
   if (!ExpectedTpi)
     return ExpectedTpi.takeError();
 
   // For handling a type server, we should be using whatever the callback array
-  // was
-  // that is being used for the original file.  We shouldn't allow the visitor
-  // to
-  // arbitrarily stick a deserializer in there.
+  // was that is being used for the original file.  We shouldn't allow the
+  // visitor to arbitrarily stick a deserializer in there.
   if (auto EC = codeview::visitTypeStream(ExpectedTpi->typeArray(), Callbacks,
                                           VDS_BytesExternal))
     return std::move(EC);
 
   return true;
 }
 
 Expected<bool> PDBTypeServerHandler::handle(TypeServer2Record &TS,
for (auto &Path : SearchPaths) {
     if (GuidStr != TS.Guid)
       continue;
 
     Session = std::move(NS);
     return handleInternal(File, Callbacks);
   }
 
   // We couldn't find a matching PDB, so let it be handled by someone else.
-  return false;
+  return make_error<GenericError>(generic_error_code::type_server_not_found,
+                                  File);
 }