[PDB] Fix linking of function symbols and local variables

[zturner]

Summary

[PDB] Fix linking of function symbols and local variables.

The compiler outputs PROC32_ID symbols into the object files
for functions, and these symbols have an embedded type index
which, when copied to the PDB, refer to the IPI stream.  However,
the symbols themselves are also converted into regular symbols
(e.g. S_GPROC32_ID -> S_GPROC32), and type indices in the regular
symbol records refer to the TPI stream.  So this patch applies
two fixes to function records.
  1. It converts ID symbols to the proper non-ID record type.
  2. After remapping the type index from the object file's index
     space to the PDB file/IPI stream's index space, it then
     remaps that index to the TPI stream's index space by.

Besides functions, during the remapping process we were also
discarding symbol record types which we did not recognize.  In
particular, we were discarding S_BPREL32 records, which is what
MSVC uses to describe local variables on the stack.  So this
patch fixes that as well by copying them to the PDB.

[rnk]

lld/COFF/PDB.cpp

334 ↗

(On Diff #110078)

This thing isn't really canonicalizing the symbol kind anymore, it's rewriting the symbol record. Maybe name it something like rewriteSymbolForPDB?

361 ↗

(On Diff #110078)

We need an error check for OOB IPI index

362–365 ↗

(On Diff #110078)

In theory, we could do the same discoverTypeIndices trick that we do above to find the type index reference. Alternatively, if we know it's an LF_FUNC_ID, we should load the appropriate offset. Are these ever LF_MFUNC_IDs, though?

488 ↗

(On Diff #110078)

Let's keep this as part of the copySymbolForPdb.

llvm/lib/DebugInfo/CodeView/TypeIndexDiscovery.cpp

401 ↗

(On Diff #110078)

Maybe add S_BPREL32 before this? They're related, BPRELSYM32 vs REGREL32.

llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp

740 ↗

(On Diff #110078)

Remove the FIXME, it's done now. :)

[zturner]

lld/COFF/PDB.cpp

361 ↗

(On Diff #110078)

That should have already happened when we merged the type streams. Wouldn't we have discarded any records that have invalid type index references, rather than emitting them with bad indices?

362–365 ↗

(On Diff #110078)

We could just load the appropriate offset, but that would require hardcoding the offset, which is duplication of magic numbers that I'd like to avoid. I chose to do it this way because it makes the code more readable and is ultimately just copying some fields.

[zturner]

Update

[zturner]

lld/COFF/PDB.cpp

488 ↗

(On Diff #110078)

I actually think it should be here. I tried moving it into copySymbolForPdb and the code becomes really ugly. You end up having to duplicate a lot of the logic from remapTypesInSymbolRecord but forcing it to remap into the TPI stream's index space. By doing it this way, each step is logically independent. copy symbol literally just copies some memory like the comment says. remap types does exactly what it says, and so does canonicalize. I can call it remapSymbolKind or something like that if you prefer, but I kind of like the idea of keeping it as a sequence of 3 independent steps like this.

[rnk]

lld/COFF/PDB.cpp

334 ↗

(On Diff #110078)

I still like rewriteSymbolForPDB

361 ↗

(On Diff #110078)

Yeah, that makes sense. Any invalid item index would've become the simple "not translated" type index.

488 ↗

(On Diff #110078)

Right, I agree, it should happen after remapping. Let's call it something else, though. We're doing a lot more than canonicalizing the symbol kind. Ultimately, we're going to need logic here for filtering out S_UDT records.

365 ↗

(On Diff #110121)

These can be LF_MFUNC_ID records, we need to handle that.

489 ↗

(On Diff #110121)

Rather than having this assert, let's remove the return type from canonicalizeSymbolKind and do SymbolKind NewKind = symbolKind(NewData);. It's one load.

490–492 ↗

(On Diff #110121)

These != assertions are trivial, IMO. They also don't represent internal errors, we emitted those records for a while, and cvdump still accepted our PDBs.

llvm/lib/DebugInfo/CodeView/TypeIndexDiscovery.cpp

401 ↗

(On Diff #110078)

ping?

[zturner]

I think I uploaded the wrong diff last time. Hopefully this one is right.

[rnk]

Only one comment, but it does need to be addressed before committing. I'm surprised we don't hit this case anywhere in the LLD test suite. I guess we mostly have C test case inputs, not C++.

lld/COFF/PDB.cpp

365 ↗

(On Diff #110127)

This would fail if it were an LF_MFUNC_ID

lld/trunk/COFF/PDB.cpp

 #include "Config.h"
 #include "Error.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "Writer.h"
 #include "llvm/DebugInfo/CodeView/CVDebugRecord.h"
 #include "llvm/DebugInfo/CodeView/DebugSubsectionRecord.h"
 #include "llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h"
+#include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolSerializer.h"
 #include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeDumpVisitor.h"
 #include "llvm/DebugInfo/CodeView/TypeIndexDiscovery.h"
 #include "llvm/DebugInfo/CodeView/TypeStreamMerger.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
 #include "llvm/DebugInfo/MSF/MSFCommon.h"
if (TI.toArrayIndex() >= TypeIndexMap.size())
     return false;
   TI = TypeIndexMap[TI.toArrayIndex()];
   return true;
 }
 
 static void remapTypesInSymbolRecord(ObjFile *File,
                                      MutableArrayRef<uint8_t> Contents,
                                      const CVIndexMap &IndexMap,
+                                     const TypeTableBuilder &IDTable,
                                      ArrayRef<TiReference> TypeRefs) {
   for (const TiReference &Ref : TypeRefs) {
     unsigned ByteSize = Ref.Count * sizeof(TypeIndex);
     if (Contents.size() < Ref.Offset + ByteSize)
       fatal("symbol record too short");
 
     // This can be an item index or a type index. Choose the appropriate map.
     ArrayRef<TypeIndex> TypeOrItemMap = IndexMap.TPIMap;
     if (Ref.Kind == TiRefKind::IndexRef && IndexMap.IsTypeServerMap)
       TypeOrItemMap = IndexMap.IPIMap;
 
     MutableArrayRef<TypeIndex> TIs(
         reinterpret_cast<TypeIndex *>(Contents.data() + Ref.Offset), Ref.Count);
     for (TypeIndex &TI : TIs) {
       if (!remapTypeIndex(TI, TypeOrItemMap)) {
         TI = TypeIndex(SimpleTypeKind::NotTranslated);
         log("ignoring symbol record in " + File->getName() +
             " with bad type index 0x" + utohexstr(TI.getIndex()));
         continue;
       }
     }
   }
 }
 
-/// MSVC translates S_PROC_ID_END to S_END.
-uint16_t canonicalizeSymbolKind(SymbolKind Kind) {
-  if (Kind == SymbolKind::S_PROC_ID_END)
-    return SymbolKind::S_END;
-  return Kind;
+static SymbolKind symbolKind(ArrayRef<uint8_t> RecordData) {
+  const RecordPrefix *Prefix =
+      reinterpret_cast<const RecordPrefix *>(RecordData.data());
+  return static_cast<SymbolKind>(uint16_t(Prefix->RecordKind));
+}
+
+/// MSVC translates S_PROC_ID_END to S_END, and S_[LG]PROC32_ID to S_[LG]PROC32
+static void translateIdSymbols(MutableArrayRef<uint8_t> &RecordData,
+                               const TypeTableBuilder &IDTable) {
+  RecordPrefix *Prefix = reinterpret_cast<RecordPrefix *>(RecordData.data());
+
+  SymbolKind Kind = symbolKind(RecordData);
+
+  if (Kind == SymbolKind::S_PROC_ID_END) {
+    Prefix->RecordKind = SymbolKind::S_END;
+    return;
+  }
+
+  // In an object file, GPROC32_ID has an embedded reference which refers to the
+  // single object file type index namespace.  This has already been translated
+  // to the PDB file's ID stream index space, but we need to convert this to a
+  // symbol that refers to the type stream index space.  So we remap again from
+  // ID index space to type index space.
+  if (Kind == SymbolKind::S_GPROC32_ID || Kind == SymbolKind::S_LPROC32_ID) {
+    SmallVector<TiReference, 1> Refs;
+    auto Content = RecordData.drop_front(sizeof(RecordPrefix));
+    CVSymbol Sym(Kind, RecordData);
+    discoverTypeIndicesInSymbol(Sym, Refs);
+    assert(Refs.size() == 1);
+    assert(Refs.front().Count == 1);
+
+    TypeIndex *TI =
+        reinterpret_cast<TypeIndex *>(Content.data() + Refs[0].Offset);
+    // `TI` is the index of a FuncIdRecord or MemberFuncIdRecord which lives in
+    // the IPI stream, whose `FunctionType` member refers to the TPI stream.
+    // Note that LF_FUNC_ID and LF_MEMFUNC_ID have the same record layout, and
+    // in both cases we just need the second type index.
+    if (!TI->isSimple() && !TI->isNoneType()) {
+      ArrayRef<uint8_t> FuncIdData = IDTable.records()[TI->toArrayIndex()];
+      SmallVector<TypeIndex, 2> Indices;
+      discoverTypeIndices(FuncIdData, Indices);
+      assert(Indices.size() == 2);
+      *TI = Indices[1];
+    }
+
+    Kind = (Kind == SymbolKind::S_GPROC32_ID) ? SymbolKind::S_GPROC32
+                                              : SymbolKind::S_LPROC32;
+    Prefix->RecordKind = uint16_t(Kind);
+  }
 }
 
 /// Copy the symbol record. In a PDB, symbol records must be 4 byte aligned.
 /// The object file may not be aligned.
 static MutableArrayRef<uint8_t> copySymbolForPdb(const CVSymbol &Sym,
                                                  BumpPtrAllocator &Alloc) {
   size_t Size = alignTo(Sym.length(), alignOf(CodeViewContainer::Pdb));
   assert(Size >= 4 && "record too short");
   assert(Size <= MaxRecordLength && "record too long");
   void *Mem = Alloc.Allocate(Size, 4);
 
   // Copy the symbol record and zero out any padding bytes.
   MutableArrayRef<uint8_t> NewData(reinterpret_cast<uint8_t *>(Mem), Size);
   memcpy(NewData.data(), Sym.data().data(), Sym.length());
   memset(NewData.data() + Sym.length(), 0, Size - Sym.length());
 
   // Update the record prefix length. It should point to the beginning of the
-  // next record. MSVC does some canonicalization of the record kind, so we do
-  // that as well.
+  // next record.
   auto *Prefix = reinterpret_cast<RecordPrefix *>(Mem);
-  Prefix->RecordKind = canonicalizeSymbolKind(Sym.kind());
   Prefix->RecordLen = Size - 2;
   return NewData;
 }
 
 /// Return true if this symbol opens a scope. This implies that the symbol has
 /// "parent" and "end" fields, which contain the offset of the S_END or
 /// S_INLINESITE_END record.
 static bool symbolOpensScope(SymbolKind Kind) {
if (Stack.empty()) {
     return;
   }
   SymbolScope S = Stack.pop_back_val();
   S.OpeningRecord->PtrEnd = CurOffset;
 }
 
 static void mergeSymbolRecords(BumpPtrAllocator &Alloc, ObjFile *File,
                                const CVIndexMap &IndexMap,
+                               const TypeTableBuilder &IDTable,
                                BinaryStreamRef SymData) {
   // FIXME: Improve error recovery by warning and skipping records when
   // possible.
   CVSymbolArray Syms;
   BinaryStreamReader Reader(SymData);
   ExitOnErr(Reader.readArray(Syms, Reader.getLength()));
   SmallVector<SymbolScope, 4> Scopes;
-  for (const CVSymbol &Sym : Syms) {
+  for (CVSymbol Sym : Syms) {
     // Discover type index references in the record. Skip it if we don't know
     // where they are.
     SmallVector<TiReference, 32> TypeRefs;
-    if (!discoverTypeIndices(Sym, TypeRefs)) {
+    if (!discoverTypeIndicesInSymbol(Sym, TypeRefs)) {
       log("ignoring unknown symbol record with kind 0x" + utohexstr(Sym.kind()));
       continue;
     }
 
     // Copy the symbol record so we can mutate it.
     MutableArrayRef<uint8_t> NewData = copySymbolForPdb(Sym, Alloc);
 
     // Re-map all the type index references.
     MutableArrayRef<uint8_t> Contents =
         NewData.drop_front(sizeof(RecordPrefix));
-    remapTypesInSymbolRecord(File, Contents, IndexMap, TypeRefs);
+    remapTypesInSymbolRecord(File, Contents, IndexMap, IDTable, TypeRefs);
+
+    // An object file may have S_xxx_ID symbols, but these get converted to
+    // "real" symbols in a PDB.
+    translateIdSymbols(NewData, IDTable);
+
+    SymbolKind NewKind = symbolKind(NewData);
 
     // Fill in "Parent" and "End" fields by maintaining a stack of scopes.
-    CVSymbol NewSym(Sym.kind(), NewData);
-    if (symbolOpensScope(Sym.kind()))
+    CVSymbol NewSym(NewKind, NewData);
+    if (symbolOpensScope(NewKind))
       scopeStackOpen(Scopes, File->ModuleDBI->getNextSymbolOffset(), NewSym);
-    else if (symbolEndsScope(Sym.kind()))
+    else if (symbolEndsScope(NewKind))
       scopeStackClose(Scopes, File->ModuleDBI->getNextSymbolOffset(), File);
 
     // Add the symbol to the module.
     File->ModuleDBI->addSymbol(NewSym);
   }
 }
 
 // Allocate memory for a .debug$S section and relocate it.
for (const DebugSubsectionRecord &SS : Subsections) {
         ExitOnErr(Checksums.initialize(SS.getRecordData()));
         break;
       case DebugSubsectionKind::Lines:
         // We can add the relocated line table directly to the PDB without
         // modification because the file checksum offsets will stay the same.
         File->ModuleDBI->addDebugSubsection(SS);
         break;
       case DebugSubsectionKind::Symbols:
-        mergeSymbolRecords(Alloc, File, IndexMap, SS.getRecordData());
+        mergeSymbolRecords(Alloc, File, IndexMap, IDTable, SS.getRecordData());
         break;
       default:
         // FIXME: Process the rest of the subsections.
         break;
       }
     }
 
     if (Checksums.valid()) {

lld/trunk/test/COFF/pdb-comdat.test

 CHECK:                           Symbols
 CHECK: ============================================================
 CHECK-LABEL:   Mod 0000 | `{{.*}}pdb_comdat_main.obj`:
 CHECK:     4 | S_OBJNAME [size = 56] sig=0, `C:\src\llvm-project\build\pdb_comdat_main.obj`
 CHECK:    60 | S_COMPILE3 [size = 60]
 CHECK:         machine = intel x86-x64, Ver = Microsoft (R) Optimizing Compiler, language = c
 CHECK:         frontend = 19.0.24215.1, backend = 19.0.24215.1
 CHECK:         flags = security checks | hot patchable
-CHECK:   120 | S_GPROC32_ID [size = 44] `main`
+CHECK:   120 | S_GPROC32 [size = 44] `main`
 CHECK:         parent = 0, end = 196, addr = 0002:0000, code size = 24
 CHECK:         debug start = 4, debug end = 19, flags = none
 CHECK:   164 | S_FRAMEPROC [size = 32]
 CHECK:         size = 40, padding size = 0, offset to padding = 0
 CHECK:         bytes of callee saved registers = 0, exception handler addr = 0000:0000
 CHECK:         flags = has async eh | opt speed
 CHECK:   196 | S_END [size = 4]
 CHECK:   200 | S_GDATA32 [size = 24] `global`
 CHECK:         type = 0x0074 (int), addr = 0000:0000
 CHECK:   224 | S_BUILDINFO [size = 8] BuildId = `0x100A`
-CHECK:   232 | S_GPROC32_ID [size = 44] `foo`
+CHECK:   232 | S_GPROC32 [size = 44] `foo`
 CHECK:         parent = 0, end = 308, addr = 0002:0032, code size = 15
 CHECK:         debug start = 0, debug end = 14, flags = none
 CHECK:   276 | S_FRAMEPROC [size = 32]
 CHECK:         size = 0, padding size = 0, offset to padding = 0
 CHECK:         bytes of callee saved registers = 0, exception handler addr = 0000:0000
 CHECK:         flags = marked inline | has async eh | opt speed
 CHECK:   308 | S_END [size = 4]
 CHECK-LABEL:   Mod 0001 | `{{.*}}pdb_comdat_bar.obj`:
 CHECK:     4 | S_OBJNAME [size = 56] sig=0, `C:\src\llvm-project\build\pdb_comdat_bar.obj`
 CHECK:    60 | S_COMPILE3 [size = 60]
 CHECK:       machine = intel x86-x64, Ver = Microsoft (R) Optimizing Compiler, language = c
 CHECK:       frontend = 19.0.24215.1, backend = 19.0.24215.1
 CHECK:       flags = security checks | hot patchable
-CHECK:   120 | S_GPROC32_ID [size = 44] `bar`
+CHECK:   120 | S_GPROC32 [size = 44] `bar`
 CHECK:       parent = 0, end = 196, addr = 0002:0048, code size = 14
 CHECK:       debug start = 4, debug end = 9, flags = none
 CHECK:   164 | S_FRAMEPROC [size = 32]
 CHECK:       size = 40, padding size = 0, offset to padding = 0
 CHECK:       bytes of callee saved registers = 0, exception handler addr = 0000:0000
 CHECK:       flags = has async eh | opt speed
 CHECK:   196 | S_END [size = 4]
 CHECK:   200 | S_GDATA32 [size = 24] `global`
 CHECK:       type = 0x0074 (int), addr = 0000:0000
 CHECK:   224 | S_BUILDINFO [size = 8] BuildId = `0x100D`
-CHECK-NOT:   S_GPROC32_ID {{.*}} `foo`
+CHECK-NOT:   S_GPROC32 {{.*}} `foo`
 CHECK-LABEL:   Mod 0002 | `* Linker *`:
 
 Reorder the object files and verify that the other table is selected.
 
 RUN: lld-link pdb_comdat_bar.obj pdb_comdat_main.obj -out:t.exe -debug -pdb:t.pdb -nodefaultlib -entry:main
 RUN: llvm-pdbutil dump -l t.pdb | FileCheck %s --check-prefix=REORDER
 
 REORDER-LABEL:   Mod 0000 | `{{.*}}pdb_comdat_bar.obj`:
 REORDER:       c:\src\llvm-project\build\pdb_comdat_bar.c (MD5: 365279DB4FCBEDD721BBFC3B14A953C2)
 REORDER:       c:\src\llvm-project\build\foo.h (MD5: D74D834EFAC3AE2B45E606A8320B1D5C)
 REORDER-LABEL:   Mod 0001 | `{{.*}}pdb_comdat_main.obj`:
 REORDER:       c:\src\llvm-project\build\pdb_comdat_main.c
 REORDER-NOT:       c:\src\llvm-project\build\foo.h
 REORDER-LABEL:   Mod 0002 | `* Linker *`:

lld/trunk/test/COFF/pdb-invalid-func-type.yaml

 # This test has an S_GPROC32_ID symbol with an invalid type index. Make sure we
 # keep the record, or we'll have unbalanced scopes, which is bad. This situation
 # can arise when we can't find the type server PDB.
 
 # RUN: yaml2obj %s -o %t.obj
 # RUN: lld-link %t.obj -out:%t.exe -debug -pdb:%t.pdb -nodefaultlib -entry:main
 # RUN: llvm-pdbutil dump -symbols %t.pdb | FileCheck %s
 
 # CHECK: Mod 0000 | `{{.*}}pdb-invalid-func-type.yaml.tmp.obj`:
-# CHECK:      4 | S_GPROC32_ID [size = 44] `main`
+# CHECK:      4 | S_GPROC32 [size = 44] `main`
 # CHECK:          parent = 0, end = 80, addr = 0001:0000, code size = 3
 # CHECK:     48 | S_FRAMEPROC [size = 32]
 # CHECK:     80 | S_END [size = 4]
 
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_AMD64
   Characteristics: [  ]

lld/trunk/test/COFF/pdb-procid-remapping.test

+# RUN: yaml2obj < %p/Inputs/pdb1.yaml > %t1.obj
+# RUN: yaml2obj < %p/Inputs/pdb2.yaml > %t2.obj
+# RUN: lld-link /debug /pdb:%t.pdb /dll /out:%t.dll /entry:main /nodefaultlib \
+# RUN:   %t1.obj %t2.obj
+
+# RUN: llvm-pdbutil dump -symbols %t.pdb | FileCheck %s
+
+CHECK:                                Symbols
+CHECK-NEXT: ============================================================
+CHECK-LABEL:    Mod 0000 |
+CHECK:                92 | S_GPROC32 [size = 44] `main`
+CHECK-NEXT:                parent = 0, end = 168, addr = 0002:0000, code size = 14
+CHECK-NEXT:                type = `0x1004 (int (<no type>))`, debug start = 4, debug end = 9, flags = none
+CHECK-NEXT:          136 | S_FRAMEPROC [size = 32]
+CHECK-NEXT:                size = 40, padding size = 0, offset to padding = 0
+CHECK-NEXT:                bytes of callee saved registers = 0, exception handler addr = 0000:0000
+CHECK-NEXT:                flags = has async eh | opt speed
+CHECK-NEXT:          168 | S_END [size = 4]
+CHECK-LABEL:    Mod 0001 |
+CHECK:                92 | S_GPROC32 [size = 44] `foo`
+CHECK-NEXT:                parent = 0, end = 168, addr = 0002:0016, code size = 6
+CHECK-NEXT:                type = `0x1001 (int ())`, debug start = 0, debug end = 5, flags = none
+CHECK-NEXT:          136 | S_FRAMEPROC [size = 32]
+CHECK-NEXT:                size = 0, padding size = 0, offset to padding = 0
+CHECK-NEXT:                bytes of callee saved registers = 0, exception handler addr = 0000:0000
+CHECK-NEXT:                flags = has async eh | opt speed
+CHECK-NEXT:          168 | S_END [size = 4]
+CHECK-LABEL:    Mod 0002 |
+CHECK:                 4 | S_OBJNAME [size = 20] sig=0, `* Linker *`

lld/trunk/test/COFF/pdb-scopes.test

 scopes. Make sure we get the right parent and end offsets.
 
 RUN: yaml2obj %S/Inputs/pdb-scopes-a.yaml -o %t-a.obj
 RUN: yaml2obj %S/Inputs/pdb-scopes-b.yaml -o %t-b.obj
 RUN: lld-link %t-a.obj %t-b.obj -debug -entry:main -nodefaultlib -out:%t.exe -pdb:%t.pdb
 RUN: llvm-pdbutil dump -symbols %t.pdb | FileCheck %s
 
 CHECK-LABEL: Mod 0000 | `{{.*}}pdb-scopes.test.tmp-a.obj`:
-CHECK: 104 | S_GPROC32_ID [size = 44] `g`
+CHECK: 104 | S_GPROC32 [size = 44] `g`
 CHECK:       parent = 0, end = 196, addr = 0002:0000, code size = 5
 CHECK:       debug start = 4, debug end = 4, flags = none
 CHECK: 180 | S_REGREL32 [size = 16] `x`
 CHECK: 196 | S_END [size = 4]
-CHECK: 200 | S_GPROC32_ID [size = 44] `main`
+CHECK: 200 | S_GPROC32 [size = 44] `main`
 CHECK:       parent = 0, end = 384, addr = 0002:0016, code size = 58
 CHECK:       debug start = 8, debug end = 53, flags = none
 CHECK: 276 | S_REGREL32 [size = 20] `argc`
 CHECK: 296 | S_BLOCK32 [size = 24] ``
 CHECK:       parent = 200, end = 336
 CHECK:       code size = 17, addr = 0002:0031
 CHECK: 320 | S_REGREL32 [size = 16] `x`
 CHECK: 336 | S_END [size = 4]
 CHECK: 340 | S_BLOCK32 [size = 24] ``
 CHECK:       parent = 200, end = 380
 CHECK:       code size = 17, addr = 0002:0050
 CHECK: 364 | S_REGREL32 [size = 16] `y`
 CHECK: 380 | S_END [size = 4]
 CHECK: 384 | S_END [size = 4]
 
 CHECK-LABEL: Mod 0001 | `{{.*}}pdb-scopes.test.tmp-b.obj`:
-CHECK: 104 | S_GPROC32_ID [size = 44] `f`
+CHECK: 104 | S_GPROC32 [size = 44] `f`
 CHECK:       parent = 0, end = 284, addr = 0002:0080, code size = 62
 CHECK:       debug start = 8, debug end = 57, flags = none
 CHECK: 180 | S_REGREL32 [size = 16] `x`
 CHECK: 196 | S_BLOCK32 [size = 24] ``
 CHECK:       parent = 104, end = 236
 CHECK:       code size = 20, addr = 0002:0095
 CHECK: 220 | S_REGREL32 [size = 16] `y`
 CHECK: 236 | S_END [size = 4]
 CHECK: 240 | S_BLOCK32 [size = 24] ``
 CHECK:       parent = 104, end = 280
 CHECK:       code size = 20, addr = 0002:0117
 CHECK: 264 | S_REGREL32 [size = 16] `w`
 CHECK: 280 | S_END [size = 4]
 CHECK: 284 | S_END [size = 4]
 
 CHECK-LABEL: Mod 0002 | `* Linker *`:

lld/trunk/test/COFF/pdb-symbol-types.yaml

 # CHECK:                           Symbols
 # CHECK: ============================================================
 # CHECK-LABEL:   Mod 0000 | `{{.*}}pdb-symbol-types.yaml.tmp.obj`:
 # CHECK:     4 | S_OBJNAME [size = 52] sig=0, `C:\src\llvm-project\build\symbol-types.obj`
 # CHECK:    56 | S_COMPILE3 [size = 60]
 # CHECK:         machine = intel x86-x64, Ver = Microsoft (R) Optimizing Compiler, language = c
 # CHECK:         frontend = 19.0.24215.1, backend = 19.0.24215.1
 # CHECK:         flags = security checks | hot patchable
-# CHECK:   116 | S_GPROC32_ID [size = 44] `main`
+# CHECK:   116 | S_GPROC32 [size = 44] `main`
 # CHECK:         parent = 0, end = 192, addr = 0002:0000, code size = 7
 # CHECK:         debug start = 0, debug end = 6, flags = none
 # CHECK:   160 | S_FRAMEPROC [size = 32]
 # CHECK:         size = 0, padding size = 0, offset to padding = 0
 # CHECK:         bytes of callee saved registers = 0, exception handler addr = 0000:0000
 # CHECK:         flags = has async eh | opt speed
 # CHECK:   192 | S_END [size = 4]
 # CHECK:   196 | S_GDATA32 [size = 28] `global_foo`

lld/trunk/test/COFF/pdb-type-server-simple.test

 CHECK:            {{.*}}: `a.c`
 CHECK:   [[B_BUILD:[^ ]*]] | LF_BUILDINFO [size = 28]
 CHECK:            {{.*}}: `b.c`
 
 CHECK-LABEL:                           Symbols
 CHECK: ============================================================
 CHECK-LABEL:   Mod 0000 | `{{.*}}a.obj`:
 CHECK:        4 | S_OBJNAME [size = 40] sig=0, `C:\src\llvm-project\build\a.obj`
-CHECK:      104 | S_GPROC32_ID [size = 44] `main`
+CHECK:      104 | S_GPROC32 [size = 44] `main`
 CHECK:            parent = 0, end = 196, addr = 0002:0000, code size = 27
 CHECK:            type = {{.*}}, debug start = 4, debug end = 22, flags = none
 CHECK:      200 | S_UDT [size = 12] `Foo`
 CHECK:            original type = [[FOO_COMPLETE]]
 CHECK:      212 | S_BUILDINFO [size = 8] BuildId = `[[A_BUILD]]`
 CHECK-LABEL:   Mod 0001 | `{{.*}}b.obj`:
 CHECK:        4 | S_OBJNAME [size = 40] sig=0, `C:\src\llvm-project\build\b.obj`
 CHECK:       44 | S_COMPILE3 [size = 60]
 CHECK:            machine = intel x86-x64, Ver = Microsoft (R) Optimizing Compiler, language = c
 CHECK:            frontend = 19.0.24215.1, backend = 19.0.24215.1
 CHECK:            flags = security checks | hot patchable
-CHECK:      104 | S_GPROC32_ID [size = 44] `g`
+CHECK:      104 | S_GPROC32 [size = 44] `g`
 CHECK:            parent = 0, end = 196, addr = 0002:0032, code size = 13
 CHECK:            type = {{.*}}, debug start = 5, debug end = 12, flags = none
 CHECK:      148 | S_FRAMEPROC [size = 32]
 CHECK:            size = 0, padding size = 0, offset to padding = 0
 CHECK:            bytes of callee saved registers = 0, exception handler addr = 0000:0000
 CHECK:            flags = has async eh | opt speed
 CHECK:      180 | S_REGREL32 [size = 16] `p`
 CHECK:            type = [[FOO_PTR]] (Foo*), register = rsp, offset = 8
 CHECK:      196 | S_END [size = 4]
 CHECK:      200 | S_UDT [size = 12] `Foo`
 CHECK:            original type = [[FOO_COMPLETE]]
 CHECK:      212 | S_BUILDINFO [size = 8] BuildId = `[[B_BUILD]]`
 CHECK-LABEL:   Mod 0002 | `* Linker *`:

llvm/trunk/include/llvm/DebugInfo/CodeView/TypeDeserializer.h

if (auto EC = I.Mapping.visitTypeBegin(CVT))
       return EC;
     if (auto EC = I.Mapping.visitKnownRecord(CVT, Record))
       return EC;
     if (auto EC = I.Mapping.visitTypeEnd(CVT))
       return EC;
     return Error::success();
   }
 
+  template <typename T>
+  static Expected<T> deserializeAs(ArrayRef<uint8_t> Data) {
+    CVType CVT;
+    CVT.RecordData = Data;
+    MappingInfo I(CVT.content());
+    const RecordPrefix *Prefix =
+        reinterpret_cast<const RecordPrefix *>(Data.data());
+    TypeRecordKind K =
+        static_cast<TypeRecordKind>(uint16_t(Prefix->RecordKind));
+    T Record(K);
+    if (auto EC = deserializeAs<T>(CVT, Record))
+      return std::move(EC);
+    return Record;
+  }
+
   Error visitTypeBegin(CVType &Record) override {
     assert(!Mapping && "Already in a type mapping!");
     Mapping = llvm::make_unique<MappingInfo>(Record.content());
     return Mapping->Mapping.visitTypeBegin(Record);
   }
 
   Error visitTypeBegin(CVType &Record, TypeIndex Index) override {
     return visitTypeBegin(Record);

llvm/trunk/include/llvm/DebugInfo/CodeView/TypeIndexDiscovery.h

 };
 
 void discoverTypeIndices(ArrayRef<uint8_t> RecordData,
                          SmallVectorImpl<TiReference> &Refs);
 void discoverTypeIndices(const CVType &Type,
                          SmallVectorImpl<TiReference> &Refs);
 void discoverTypeIndices(const CVType &Type,
                          SmallVectorImpl<TypeIndex> &Indices);
+void discoverTypeIndices(ArrayRef<uint8_t> RecordData,
+                         SmallVectorImpl<TypeIndex> &Indices);
 
 /// Discover type indices in symbol records. Returns false if this is an unknown
 /// record.
-bool discoverTypeIndices(const CVSymbol &Symbol,
+bool discoverTypeIndicesInSymbol(const CVSymbol &Symbol,
                                  SmallVectorImpl<TiReference> &Refs);
+bool discoverTypeIndicesInSymbol(ArrayRef<uint8_t> RecordData,
+                                 SmallVectorImpl<TiReference> &Refs);
+bool discoverTypeIndicesInSymbol(ArrayRef<uint8_t> RecordData,
+                                 SmallVectorImpl<TypeIndex> &Indices);
 }
 }
 
 #endif

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

case SymbolKind::S_FILESTATIC:
     Refs.push_back({TiRefKind::TypeRef, 0, 1}); // Type
     break;
   case SymbolKind::S_LOCAL:
     Refs.push_back({TiRefKind::TypeRef, 0, 1}); // Type
     break;
   case SymbolKind::S_CONSTANT:
     Refs.push_back({TiRefKind::TypeRef, 0, 1}); // Type
     break;
+  case SymbolKind::S_BPREL32:
   case SymbolKind::S_REGREL32:
     Refs.push_back({TiRefKind::TypeRef, 4, 1}); // Type
     break;
   case SymbolKind::S_CALLSITEINFO:
     Refs.push_back({TiRefKind::TypeRef, 8, 1}); // Call signature
     break;
   case SymbolKind::S_CALLERS:
   case SymbolKind::S_CALLEES:
static bool discoverTypeIndices(ArrayRef<uint8_t> Content, SymbolKind Kind,
   return true;
 }
 
 void llvm::codeview::discoverTypeIndices(const CVType &Type,
                                          SmallVectorImpl<TiReference> &Refs) {
   ::discoverTypeIndices(Type.content(), Type.kind(), Refs);
 }
 
-void llvm::codeview::discoverTypeIndices(const CVType &Type,
-                                         SmallVectorImpl<TypeIndex> &Indices) {
-
+static void resolveTypeIndexReferences(ArrayRef<uint8_t> RecordData,
+                                       ArrayRef<TiReference> Refs,
   Indices.clear();
 
-  SmallVector<TiReference, 4> Refs;
-  discoverTypeIndices(Type, Refs);
   if (Refs.empty())
     return;
 
-  BinaryStreamReader Reader(Type.content(), support::little);
+  RecordData = RecordData.drop_front(sizeof(RecordPrefix));
+
+  BinaryStreamReader Reader(RecordData, support::little);
   for (const auto &Ref : Refs) {
     Reader.setOffset(Ref.Offset);
     FixedStreamArray<TypeIndex> Run;
     cantFail(Reader.readArray(Run, Ref.Count));
     Indices.append(Run.begin(), Run.end());
   }
 }
 
+void llvm::codeview::discoverTypeIndices(const CVType &Type,
+                                         SmallVectorImpl<TypeIndex> &Indices) {
+  return discoverTypeIndices(Type.RecordData, Indices);
+}
+
+void llvm::codeview::discoverTypeIndices(ArrayRef<uint8_t> RecordData,
+                                         SmallVectorImpl<TypeIndex> &Indices) {
+  SmallVector<TiReference, 4> Refs;
+  discoverTypeIndices(RecordData, Refs);
+  resolveTypeIndexReferences(RecordData, Refs, Indices);
+}
+
 void llvm::codeview::discoverTypeIndices(ArrayRef<uint8_t> RecordData,
                                          SmallVectorImpl<TiReference> &Refs) {
   const RecordPrefix *P =
       reinterpret_cast<const RecordPrefix *>(RecordData.data());
   TypeLeafKind K = static_cast<TypeLeafKind>(uint16_t(P->RecordKind));
   ::discoverTypeIndices(RecordData.drop_front(sizeof(RecordPrefix)), K, Refs);
 }
 
-bool llvm::codeview::discoverTypeIndices(const CVSymbol &Sym,
-                                         SmallVectorImpl<TiReference> &Refs) {
+bool llvm::codeview::discoverTypeIndicesInSymbol(
+    const CVSymbol &Sym, SmallVectorImpl<TiReference> &Refs) {
   SymbolKind K = Sym.kind();
   return ::discoverTypeIndices(Sym.content(), K, Refs);
 }
+
+bool llvm::codeview::discoverTypeIndicesInSymbol(
+    ArrayRef<uint8_t> RecordData, SmallVectorImpl<TiReference> &Refs) {
+  const RecordPrefix *P =
+      reinterpret_cast<const RecordPrefix *>(RecordData.data());
+  SymbolKind K = static_cast<SymbolKind>(uint16_t(P->RecordKind));
+  return ::discoverTypeIndices(RecordData.drop_front(sizeof(RecordPrefix)), K,
+                               Refs);
+}
+
+bool llvm::codeview::discoverTypeIndicesInSymbol(
+    ArrayRef<uint8_t> RecordData, SmallVectorImpl<TypeIndex> &Indices) {
+  SmallVector<TiReference, 2> Refs;
+  if (!discoverTypeIndicesInSymbol(RecordData, Refs))
+    return false;
+  resolveTypeIndexReferences(RecordData, Refs, Indices);
+  return true;
+}

llvm/trunk/tools/llvm-pdbutil/DumpOutputStyle.cpp

Error DumpOutputStyle::dumpModuleSyms() {
   AutoIndent Indent(P);
   if (!File.hasPDBDbiStream()) {
     P.formatLine("DBI Stream not present");
     return Error::success();
   }
 
   ExitOnError Err("Unexpected error processing symbols: ");
 
+  auto &Ids = Err(initializeTypes(StreamIPI));
   auto &Types = Err(initializeTypes(StreamTPI));
 
   iterateModules(
       File, P, 2, [&](uint32_t I, StringsAndChecksumsPrinter &Strings) {
         auto ExpectedModS = getModuleDebugStream(File, I);
         if (!ExpectedModS) {
           P.formatLine("Error loading module stream {0}.  {1}", I,
                        toString(ExpectedModS.takeError()));
           return;
         }
 
         ModuleDebugStreamRef &ModS = *ExpectedModS;
 
         SymbolVisitorCallbackPipeline Pipeline;
         SymbolDeserializer Deserializer(nullptr, CodeViewContainer::Pdb);
-        MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, Types);
+        MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, Ids,
+                                   Types);
 
         Pipeline.addCallbackToPipeline(Deserializer);
         Pipeline.addCallbackToPipeline(Dumper);
         CVSymbolVisitor Visitor(Pipeline);
         auto SS = ModS.getSymbolsSubstream();
         if (auto EC =
                 Visitor.visitSymbolStream(ModS.getSymbolArray(), SS.Offset)) {
           P.formatLine("Error while processing symbol records.  {0}",

 Error DumpOutputStyle::dumpSymbolsFromGSI(const GSIHashTable &Table,
                                           bool HashExtras) {
   auto ExpectedSyms = File.getPDBSymbolStream();
   if (!ExpectedSyms)
     return ExpectedSyms.takeError();
   auto ExpectedTypes = initializeTypes(StreamTPI);
   if (!ExpectedTypes)
     return ExpectedTypes.takeError();
+  auto ExpectedIds = initializeTypes(StreamIPI);
+  if (!ExpectedIds)
+    return ExpectedIds.takeError();
   SymbolVisitorCallbackPipeline Pipeline;
   SymbolDeserializer Deserializer(nullptr, CodeViewContainer::Pdb);
-  MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, *ExpectedTypes);
+  MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, *ExpectedIds,
+                             *ExpectedTypes);
 
   Pipeline.addCallbackToPipeline(Deserializer);
   Pipeline.addCallbackToPipeline(Dumper);
   CVSymbolVisitor Visitor(Pipeline);
 
   BinaryStreamRef SymStream =
       ExpectedSyms->getSymbolArray().getUnderlyingStream();
   for (uint32_t PubSymOff : Table) {

llvm/trunk/tools/llvm-pdbutil/MinimalSymbolDumper.h

 }
 
 namespace pdb {
 class LinePrinter;
 
 class MinimalSymbolDumper : public codeview::SymbolVisitorCallbacks {
 public:
   MinimalSymbolDumper(LinePrinter &P, bool RecordBytes,
+                      codeview::LazyRandomTypeCollection &Ids,
                       codeview::LazyRandomTypeCollection &Types)
-      : P(P), Types(Types) {}
+      : P(P), Ids(Ids), Types(Types) {}
 
   Error visitSymbolBegin(codeview::CVSymbol &Record) override;
   Error visitSymbolBegin(codeview::CVSymbol &Record, uint32_t Offset) override;
   Error visitSymbolEnd(codeview::CVSymbol &Record) override;
 
 #define SYMBOL_RECORD(EnumName, EnumVal, Name)                                 \
   virtual Error visitKnownRecord(codeview::CVSymbol &CVR,                      \
                                  codeview::Name &Record) override;
 #define SYMBOL_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
 #include "llvm/DebugInfo/CodeView/CodeViewSymbols.def"
 
 private:
+  std::string typeOrIdIndex(codeview::TypeIndex TI, bool IsType) const;
+
   std::string typeIndex(codeview::TypeIndex TI) const;
+  std::string idIndex(codeview::TypeIndex TI) const;
 
   LinePrinter &P;
+  codeview::LazyRandomTypeCollection &Ids;
   codeview::LazyRandomTypeCollection &Types;
 };
 } // namespace pdb
 } // namespace llvm
 
 #endif
 No newline at end of file

llvm/trunk/tools/llvm-pdbutil/MinimalSymbolDumper.cpp

Error MinimalSymbolDumper::visitSymbolBegin(codeview::CVSymbol &Record,
   return Error::success();
 }
 
 Error MinimalSymbolDumper::visitSymbolEnd(CVSymbol &Record) {
   P.Unindent();
   return Error::success();
 }
 
-std::string MinimalSymbolDumper::typeIndex(TypeIndex TI) const {
+std::string MinimalSymbolDumper::typeOrIdIndex(codeview::TypeIndex TI,
+                                               bool IsType) const {
   if (TI.isSimple())
     return formatv("{0}", TI).str();
-  StringRef Name = Types.getTypeName(TI);
+  auto &Container = IsType ? Types : Ids;
+  StringRef Name = Container.getTypeName(TI);
   if (Name.size() > 32) {
     Name = Name.take_front(32);
     return formatv("{0} ({1}...)", TI, Name);
   } else
     return formatv("{0} ({1})", TI, Name);
 }
 
+std::string MinimalSymbolDumper::idIndex(codeview::TypeIndex TI) const {
+  return typeOrIdIndex(TI, false);
+}
+
+std::string MinimalSymbolDumper::typeIndex(TypeIndex TI) const {
+  return typeOrIdIndex(TI, true);
+}
+
 Error MinimalSymbolDumper::visitKnownRecord(CVSymbol &CVR, BlockSym &Block) {
   P.format(" `{0}`", Block.Name);
   AutoIndent Indent(P, 7);
   P.formatLine("parent = {0}, end = {1}", Block.Parent, Block.End);
   P.formatLine("code size = {0}, addr = {1}", Block.CodeSize,
                formatSegmentOffset(Block.Segment, Block.CodeOffset));
   return Error::success();
 }

 
 Error MinimalSymbolDumper::visitKnownRecord(CVSymbol &CVR, ProcSym &Proc) {
   P.format(" `{0}`", Proc.Name);
   AutoIndent Indent(P, 7);
   P.formatLine("parent = {0}, end = {1}, addr = {2}, code size = {3}",
                Proc.Parent, Proc.End,
                formatSegmentOffset(Proc.Segment, Proc.CodeOffset),
                Proc.CodeSize);
-  // FIXME: It seems FunctionType is sometimes an id and sometimes a type.
+  bool IsType = true;
+  switch (Proc.getKind()) {
+  case SymbolRecordKind::GlobalProcIdSym:
+  case SymbolRecordKind::ProcIdSym:
+  case SymbolRecordKind::DPCProcIdSym:
+    IsType = false;
+    break;
+  default:
+    break;
+  }
   P.formatLine("type = `{0}`, debug start = {1}, debug end = {2}, flags = {3}",
-               typeIndex(Proc.FunctionType), Proc.DbgStart, Proc.DbgEnd,
+               typeOrIdIndex(Proc.FunctionType, IsType), Proc.DbgStart,
+               Proc.DbgEnd,
                formatProcSymFlags(P.getIndentLevel() + 9, Proc.Flags));
   return Error::success();
 }
 
 Error MinimalSymbolDumper::visitKnownRecord(CVSymbol &CVR,
                                             ScopeEndSym &ScopeEnd) {
   return Error::success();
 }

llvm/trunk/unittests/DebugInfo/CodeView/TypeIndexDiscoveryTest.cpp

for (uint32_t I = 0; I < TTB->records().size(); ++I) {
       ArrayRef<uint8_t> Data = TTB->records()[I];
       discoverTypeIndices(Data, Refs[I]);
     }
   }
 
   void discoverTypeIndicesInSymbols() {
     Refs.resize(Symbols.size());
     for (uint32_t I = 0; I < Symbols.size(); ++I)
-      discoverTypeIndices(Symbols[I], Refs[I]);
+      discoverTypeIndicesInSymbol(Symbols[I], Refs[I]);
   }
 
   // Helper function to write out a field list record with the given list
   // of member records.
   void writeFieldListImpl() {}
 
   template <typename RecType, typename... Rest>
   void writeFieldListImpl(RecType &&Record, Rest &&... Records) {