[LLDB][NativePDB] Ignore functions with no type in name lookup #153382
Conversation
|
CC @slydiman |
|
@llvm/pr-subscribers-lldb Author: nerix (Nerixyz) ChangesSome functions don't have the This does fix the failures caused by #153160 mentioned in #153160 (comment). However, in Full diff: https://github.com/llvm/llvm-project/pull/153382.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 986d647b4de2d..5b2e12101d26e 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1675,7 +1675,7 @@ void SymbolFileNativePDB::CacheFunctionNames() {
llvm::cantFail(SymbolDeserializer::deserializeAs<ProcSym>(*iter));
if ((proc.Flags & ProcSymFlags::IsUnreachable) != ProcSymFlags::None)
continue;
- if (proc.Name.empty())
+ if (proc.Name.empty() || proc.FunctionType.isNoneType())
continue;
// The function/procedure symbol only contains the demangled name.
|
|
Could you elaborate in the PR description why these "no prototype functions" would cause issues if we cached them? |
A bit later in the function, we look up the function type: auto type = m_index->tpi().getType(proc.FunctionType);In PDB, types with an index lower than |
Ah gotcha. Thanks for the context. It would be more obvious if we guarded this on |
| @@ -1675,7 +1675,7 @@ void SymbolFileNativePDB::CacheFunctionNames() { | |||
| llvm::cantFail(SymbolDeserializer::deserializeAs<ProcSym>(*iter)); | |||
| if ((proc.Flags & ProcSymFlags::IsUnreachable) != ProcSymFlags::None) | |||
| continue; | |||
| if (proc.Name.empty()) | |||
| if (proc.Name.empty() || proc.FunctionType.isNoneType()) | |||
There was a problem hiding this comment.
Would proc.FunctionType.isSimple() be more robust? Or can FunctionType never be "simple"?
There was a problem hiding this comment.
Yea, it would be more robust, let's do that. I couldn't think of a case where the type would be simple and not none (i.e. a primitive type). But in that case, we should still skip the function.
Some functions don't have the
FunctionTypeset. We can't look these up and won't be able to call them, so ignore them when caching the function names.This does fix the failures caused by #153160 mentioned in #153160 (comment). However, in
lldb-shell::msstl_smoke.cppthere's another failure not introduced by #153160 (fixed with #153386).