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

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


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


================
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;
----------------
rnk wrote:
> 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?
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.


https://reviews.llvm.org/D36426