[lldb][NativePDB] Fix crash in debugger when PDB has bad type index value #166455
Conversation
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-platform-windows Author: Vladimir Gorsunov (gv) ChangesFix crash when an inline site record in the PDB file contains type index which is out of bounds Full diff: https://github.com/llvm/llvm-project/pull/166455.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index aaec1600dacff..8be6dd196c07c 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1719,19 +1719,23 @@ void SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
}
// Get the inlined function name.
- CVType inlinee_cvt = m_index->ipi().getType(inline_site.Inlinee);
std::string inlinee_name;
- if (inlinee_cvt.kind() == LF_MFUNC_ID) {
+ llvm::Expected<CVType> inlinee_cvt =
+ m_index->ipi().typeCollection().getTypeOrError(inline_site.Inlinee);
+ if (!inlinee_cvt) {
+ inlinee_name = "[error reading function name: " +
+ llvm::toString(inlinee_cvt.takeError()) + "]";
+ } else if (inlinee_cvt->kind() == LF_MFUNC_ID) {
MemberFuncIdRecord mfr;
cantFail(
- TypeDeserializer::deserializeAs<MemberFuncIdRecord>(inlinee_cvt, mfr));
+ TypeDeserializer::deserializeAs<MemberFuncIdRecord>(*inlinee_cvt, mfr));
LazyRandomTypeCollection &types = m_index->tpi().typeCollection();
inlinee_name.append(std::string(types.getTypeName(mfr.ClassType)));
inlinee_name.append("::");
inlinee_name.append(mfr.getName().str());
- } else if (inlinee_cvt.kind() == LF_FUNC_ID) {
+ } else if (inlinee_cvt->kind() == LF_FUNC_ID) {
FuncIdRecord fir;
- cantFail(TypeDeserializer::deserializeAs<FuncIdRecord>(inlinee_cvt, fir));
+ cantFail(TypeDeserializer::deserializeAs<FuncIdRecord>(*inlinee_cvt, fir));
TypeIndex parent_idx = fir.getParentScope();
if (!parent_idx.isNoneType()) {
LazyRandomTypeCollection &ids = m_index->ipi().typeCollection();
diff --git a/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h b/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h
index 5b23ac9f862a0..da18339b8662b 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h
@@ -70,7 +70,7 @@ class LLVM_ABI LazyRandomTypeCollection : public TypeCollection {
uint32_t getOffsetOfType(TypeIndex Index);
std::optional<CVType> tryGetType(TypeIndex Index);
-
+ llvm::Expected<CVType> getTypeOrError(TypeIndex Index);
CVType getType(TypeIndex Index) override;
StringRef getTypeName(TypeIndex Index) override;
bool contains(TypeIndex Index) override;
diff --git a/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp b/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp
index 23ab5344df1ed..760fe0e709ba4 100644
--- a/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp
+++ b/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp
@@ -93,20 +93,28 @@ CVType LazyRandomTypeCollection::getType(TypeIndex Index) {
return Records[Index.toArrayIndex()].Type;
}
-std::optional<CVType> LazyRandomTypeCollection::tryGetType(TypeIndex Index) {
+llvm::Expected<CVType>
+LazyRandomTypeCollection::getTypeOrError(TypeIndex Index) {
if (Index.isSimple())
- return std::nullopt;
+ return llvm::createStringError("Type index too low (%d)", Index.getIndex());
if (auto EC = ensureTypeExists(Index)) {
- consumeError(std::move(EC));
- return std::nullopt;
+ return std::move(EC);
}
if (!contains(Index))
- return std::nullopt;
+ return llvm::createStringError("Type index too high (%d)",
+ Index.getIndex());
return Records[Index.toArrayIndex()].Type;
}
+std::optional<CVType> LazyRandomTypeCollection::tryGetType(TypeIndex Index) {
+ llvm::Expected<CVType> res = getTypeOrError(Index);
+ if (!res)
+ return std::nullopt;
+ return std::move(*res);
+}
+
StringRef LazyRandomTypeCollection::getTypeName(TypeIndex Index) {
if (Index.isNoneType() || Index.isSimple())
return TypeIndex::simpleTypeName(Index);
|
|
@llvm/pr-subscribers-debuginfo Author: Vladimir Gorsunov (gv) ChangesFix crash when an inline site record in the PDB file contains type index which is out of bounds Full diff: https://github.com/llvm/llvm-project/pull/166455.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index aaec1600dacff..8be6dd196c07c 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1719,19 +1719,23 @@ void SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
}
// Get the inlined function name.
- CVType inlinee_cvt = m_index->ipi().getType(inline_site.Inlinee);
std::string inlinee_name;
- if (inlinee_cvt.kind() == LF_MFUNC_ID) {
+ llvm::Expected<CVType> inlinee_cvt =
+ m_index->ipi().typeCollection().getTypeOrError(inline_site.Inlinee);
+ if (!inlinee_cvt) {
+ inlinee_name = "[error reading function name: " +
+ llvm::toString(inlinee_cvt.takeError()) + "]";
+ } else if (inlinee_cvt->kind() == LF_MFUNC_ID) {
MemberFuncIdRecord mfr;
cantFail(
- TypeDeserializer::deserializeAs<MemberFuncIdRecord>(inlinee_cvt, mfr));
+ TypeDeserializer::deserializeAs<MemberFuncIdRecord>(*inlinee_cvt, mfr));
LazyRandomTypeCollection &types = m_index->tpi().typeCollection();
inlinee_name.append(std::string(types.getTypeName(mfr.ClassType)));
inlinee_name.append("::");
inlinee_name.append(mfr.getName().str());
- } else if (inlinee_cvt.kind() == LF_FUNC_ID) {
+ } else if (inlinee_cvt->kind() == LF_FUNC_ID) {
FuncIdRecord fir;
- cantFail(TypeDeserializer::deserializeAs<FuncIdRecord>(inlinee_cvt, fir));
+ cantFail(TypeDeserializer::deserializeAs<FuncIdRecord>(*inlinee_cvt, fir));
TypeIndex parent_idx = fir.getParentScope();
if (!parent_idx.isNoneType()) {
LazyRandomTypeCollection &ids = m_index->ipi().typeCollection();
diff --git a/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h b/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h
index 5b23ac9f862a0..da18339b8662b 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h
@@ -70,7 +70,7 @@ class LLVM_ABI LazyRandomTypeCollection : public TypeCollection {
uint32_t getOffsetOfType(TypeIndex Index);
std::optional<CVType> tryGetType(TypeIndex Index);
-
+ llvm::Expected<CVType> getTypeOrError(TypeIndex Index);
CVType getType(TypeIndex Index) override;
StringRef getTypeName(TypeIndex Index) override;
bool contains(TypeIndex Index) override;
diff --git a/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp b/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp
index 23ab5344df1ed..760fe0e709ba4 100644
--- a/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp
+++ b/llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp
@@ -93,20 +93,28 @@ CVType LazyRandomTypeCollection::getType(TypeIndex Index) {
return Records[Index.toArrayIndex()].Type;
}
-std::optional<CVType> LazyRandomTypeCollection::tryGetType(TypeIndex Index) {
+llvm::Expected<CVType>
+LazyRandomTypeCollection::getTypeOrError(TypeIndex Index) {
if (Index.isSimple())
- return std::nullopt;
+ return llvm::createStringError("Type index too low (%d)", Index.getIndex());
if (auto EC = ensureTypeExists(Index)) {
- consumeError(std::move(EC));
- return std::nullopt;
+ return std::move(EC);
}
if (!contains(Index))
- return std::nullopt;
+ return llvm::createStringError("Type index too high (%d)",
+ Index.getIndex());
return Records[Index.toArrayIndex()].Type;
}
+std::optional<CVType> LazyRandomTypeCollection::tryGetType(TypeIndex Index) {
+ llvm::Expected<CVType> res = getTypeOrError(Index);
+ if (!res)
+ return std::nullopt;
+ return std::move(*res);
+}
+
StringRef LazyRandomTypeCollection::getTypeName(TypeIndex Index) {
if (Index.isNoneType() || Index.isSimple())
return TypeIndex::simpleTypeName(Index);
|
|
Can you add a test that (used to) reproduce(s) the crash? Presumably this isn't covered when running the API tests with PDB support? |
I'll try, although I'm not sure yet how. It will need a PDB file which is broken in a certain way. Right now I have 200MB PDB produced by MSVS that triggered a crash for me, but I don't think there's any way to use that. There probably must be a way to use |
There was a problem hiding this comment.
The LLDB changes look good to me. Though, it looks like the LLVM tests are failing. It might be easier if you land the LLVM changes first.
Can you add a test that (used to) reproduce(s) the crash? Presumably this isn't covered when running the API tests with PDB support?
This feels similar to #166090 (i.e. an artifact of incremental linking), so testing is a bit more complex. yaml2pdb is probably the way to go here. Currently, that won't work, because the generated PDB is missing the section headers. I'll try to add this, so both PRs can have a test.
Opened #166566 for this.
| llvm::Expected<CVType> res = getTypeOrError(Index); | ||
| if (!res) | ||
| return std::nullopt; | ||
| return *res; |
There was a problem hiding this comment.
You can use llvm::expectedToOptional here.
…alue Fix crash when an inline site record in the PDB file contains type index which is out of bounds
As #166566 is merged I tried to load a PDB generated by AFAICS there is no support for converting section contributions to or from YAML. Until that is changed, Thread 1 "lldb" received signal SIGSEGV, Segmentation fault. |
I'm surprised the debugger can even load the PDB. In my testing, section contributions weren't necessary. I forgot to mention that this needs another patch - #168211. Arguably, we should also handle this gracefully. |
|
Should work now. You can do something similar to this: https://github.com/llvm/llvm-project/pull/166090/files#diff-d4185e28c08f72d5ae39d59054bcd6cea460ff7a95706bbc0809c37cc5409a2e If you change the |
4 participants
Fix crash when an inline site record in the PDB file contains type index which is out of bounds