[PATCH] D36426: [PDB] Fix linking of function symbols and local variables

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 7 14:09:30 PDT 2017

rnk added inline comments.

Comment at: lld/COFF/PDB.cpp:334
+/// MSVC translates S_PROC_ID_END to S_END, and S_[LG]PROC32_ID to S_[LG]PROC32
+static SymbolKind canonicalizeSymbolKind(MutableArrayRef<uint8_t> &RecordData,
+                                         const TypeTableBuilder &IDTable) {
This thing isn't really canonicalizing the symbol kind anymore, it's rewriting the symbol record. Maybe name it something like `rewriteSymbolForPDB`?

Comment at: lld/COFF/PDB.cpp:361
+    if (!TI->isSimple() && !TI->isNoneType()) {
+      ArrayRef<uint8_t> FuncIdData = IDTable.records()[TI->toArrayIndex()];
+      FuncIdRecord FID;
We need an error check for OOB IPI index

Comment at: lld/COFF/PDB.cpp:362-365
+      FuncIdRecord FID;
+      CVType FuncId(TypeLeafKind::LF_FUNC_ID, FuncIdData);
+      cantFail(TypeDeserializer::deserializeAs<FuncIdRecord>(FuncId, FID));
+      *TI = FID.FunctionType;
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?

Comment at: lld/COFF/PDB.cpp:488
+    remapTypesInSymbolRecord(File, Contents, IndexMap, IDTable, TypeRefs);
+    SymbolKind NewKind = canonicalizeSymbolKind(NewData, IDTable);
+    assert(NewKind == symbolKind(NewData));
Let's keep this as part of the copySymbolForPdb.

Comment at: llvm/lib/DebugInfo/CodeView/TypeIndexDiscovery.cpp:401
   case SymbolKind::S_REGREL32:
     Refs.push_back({TiRefKind::TypeRef, 4, 1}); // Type
Maybe add S_BPREL32 before this? They're related, BPRELSYM32 vs REGREL32.

Comment at: llvm/tools/llvm-pdbutil/MinimalSymbolDumper.cpp:740
   // FIXME: It seems FunctionType is sometimes an id and sometimes a type.
+  bool IsType = true;
Remove the FIXME, it's done now. :)


More information about the llvm-commits mailing list