[Lldb-commits] [lldb] 28d1490 - [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (#120569)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Dec 23 03:51:31 PST 2024
Author: Michael Buch
Date: 2024-12-23T11:51:28Z
New Revision: 28d14904c00b74154b8dfa71d5b062a7e590c44c
URL: https://github.com/llvm/llvm-project/commit/28d14904c00b74154b8dfa71d5b062a7e590c44c
DIFF: https://github.com/llvm/llvm-project/commit/28d14904c00b74154b8dfa71d5b062a7e590c44c.diff
LOG: [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (#120569)
The problem here manifests as follows:
1. We are stopped in main.o, so the first `ParseTypeFromDWARF` on
`FooImpl<char>` gets called on `main.o`'s SymbolFile. This adds a
mapping from *declaration die* -> `TypeSP` into `main.o`'s
`GetDIEToType` map.
2. We then `CompleteType(FooImpl<char>)`. Depending on the order of
entries in the debug-map, this might call `CompleteType` on `lib.o`'s
SymbolFile. In which case, `GetDIEToType().lookup(decl_die)` will return
a `nullptr`. This is already a bit iffy because some of the surrounding
code assumes we don't call `CompleteTypeFromDWARF` with a `nullptr`
`Type*`. E.g., `CompleteEnumType` blindly dereferences it (though enums
will never encounter this because their definition is fetched in
ParseEnum, unlike for structures).
3. While in `CompleteTypeFromDWARF`, we call `ParseTypeFromDWARF` again.
This will parse the member function `FooImpl::Create` and its return
type which is a typedef to `FooImpl*`. But now we're inside `lib.o`'s
SymbolFile, so we call it on the definition DIE. In step (2) we just
inserted a `nullptr` into `GetDIEToType` for the definition DIE, so we
trivially return a `nullptr` from `ParseTypeFromDWARF`. Instead of
reporting back this parse failure to the user LLDB trucks on and marks
`FooImpl::Ref` to be `void*`.
This test-case will trigger an assert in `TypeSystemClang::VerifyDecl`
even if we just `frame var` (but only in debug-builds). In release
builds where this function is a no-op, we'll create an incorrect Clang
AST node for the `Ref` typedef.
The proposed fix here is to share the `GetDIEToType` map between
SymbolFiles if a debug-map exists.
**Alternatives considered**
* Check the `GetDIEToType` map of the `SymbolFile` that the declaration
DIE belongs to. The assumption here being that if we called
`ParseTypeFromDWARF` on a declaration, the `GetDIEToType` map that the
result was inserted into was the one on that DIE's SymbolFile. This was
the first version of this patch, but that felt like a weaker version
sharing the map. It complicates the code in `CompleteType` and is less
consistent with the other bookkeeping structures we already share
between SymbolFiles
* Return from `SymbolFileDWARF::CompleteType` if there is no type in the
current `GetDIEToType`. Then `SymbolFileDWARFDebugMap::CompleteType`
could continue to the next `SymbolFile` which does own the type. But
that didn't quite work because we remove the
`GetForwardCompilerTypeToDie` entry in `SymbolFile::CompleteType`, which
`SymbolFileDWARFDebugMap::CompleteType` relies upon for iterating
Added:
lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp
lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h
lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 71a77a4ed7458f..28e7cceb397154 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3633,8 +3633,7 @@ bool DWARFASTParserClang::CopyUniqueClassMethodTypes(
static_cast<DWARFASTParserClang *>(
SymbolFileDWARF::GetDWARFParser(*dst_class_die.GetCU()));
auto link = [&](DWARFDIE src, DWARFDIE dst) {
- SymbolFileDWARF::DIEToTypePtr &die_to_type =
- dst_class_die.GetDWARF()->GetDIEToType();
+ auto &die_to_type = dst_class_die.GetDWARF()->GetDIEToType();
clang::DeclContext *dst_decl_ctx =
dst_dwarf_ast_parser->m_die_to_decl_ctx[dst.GetDIE()];
if (dst_decl_ctx)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 87517266fced5c..360dbaa1beb5eb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -482,6 +482,13 @@ static ConstString GetDWARFMachOSegmentName() {
return g_dwarf_section_name;
}
+llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &
+SymbolFileDWARF::GetDIEToType() {
+ if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile())
+ return debug_map_symfile->GetDIEToType();
+ return m_die_to_type;
+}
+
llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
SymbolFileDWARF::GetForwardDeclCompilerTypeToDIE() {
if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile())
@@ -1594,6 +1601,8 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) {
if (!dwarf_ast)
return false;
Type *type = GetDIEToType().lookup(decl_die.GetDIE());
+ assert(type);
+
if (decl_die != def_die) {
GetDIEToType()[def_die.GetDIE()] = type;
DWARFASTParserClang *ast_parser =
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 6ecc8855380411..7309f7a86b659c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -335,9 +335,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
m_file_index = file_index;
}
- typedef llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> DIEToTypePtr;
-
- virtual DIEToTypePtr &GetDIEToType() { return m_die_to_type; }
+ virtual llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &GetDIEToType();
virtual llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef> &
GetForwardDeclCompilerTypeToDIE();
@@ -529,7 +527,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
UniqueDWARFASTTypeMap m_unique_ast_type_map;
// A map from DIE to lldb_private::Type. For record type, the key might be
// either declaration DIE or definition DIE.
- DIEToTypePtr m_die_to_type;
+ llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> m_die_to_type;
DIEToVariableSP m_die_to_variable_sp;
// A map from CompilerType to the struct/class/union/enum DIE (might be a
// declaration or a definition) that is used to construct it.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
index 0ebcad2866a72e..df41d6a2a4e42f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -291,6 +291,10 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
return m_unique_ast_type_map;
}
+ llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &GetDIEToType() {
+ return m_die_to_type;
+ }
+
// OSOEntry
class OSOEntry {
public:
@@ -329,6 +333,8 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
llvm::DenseMap<lldb::opaque_compiler_type_t, DIERef>
m_forward_decl_compiler_type_to_die;
UniqueDWARFASTTypeMap m_unique_ast_type_map;
+ llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> m_die_to_type;
+
DebugMap m_debug_map;
// When an object file from the debug map gets parsed in
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index 49632e1d8911cb..c1829abe72933a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -102,7 +102,8 @@ bool SymbolFileDWARFDwo::ParseVendorDWARFOpcode(
return GetBaseSymbolFile().ParseVendorDWARFOpcode(op, opcodes, offset, stack);
}
-SymbolFileDWARF::DIEToTypePtr &SymbolFileDWARFDwo::GetDIEToType() {
+llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &
+SymbolFileDWARFDwo::GetDIEToType() {
return GetBaseSymbolFile().GetDIEToType();
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
index 15c28fefd81f9d..75f5986f14014c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -70,7 +70,7 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {
SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref) override;
protected:
- DIEToTypePtr &GetDIEToType() override;
+ llvm::DenseMap<const DWARFDebugInfoEntry *, Type *> &GetDIEToType() override;
DIEToVariableSP &GetDIEToVariable() override;
diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
new file mode 100644
index 00000000000000..d9db5666f9b6cd
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := lib.cpp main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
new file mode 100644
index 00000000000000..7e9f6967a1288c
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
@@ -0,0 +1,38 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCaseTypedefToOuterFwd(TestBase):
+ """
+ We find a global variable whose type is forward declared
+ (whose definition is in either main.o or lib.o). We then
+ try to get the 'Ref' typedef nested within that forward
+ declared type. This test makes sure we correctly resolve
+ this typedef.
+
+ We test this for two cases, where the definition lives
+ in main.o or lib.o.
+ """
+
+ def check_global_var(self, target, name: str):
+ var = target.FindFirstGlobalVariable(name)
+ self.assertSuccess(var.GetError(), f"Found {name}")
+
+ var_type = var.GetType()
+ self.assertTrue(var_type)
+
+ impl = var_type.GetPointeeType()
+ self.assertTrue(impl)
+
+ ref = impl.FindDirectNestedType("Ref")
+ self.assertTrue(ref)
+
+ self.assertEqual(ref.GetCanonicalType(), var_type)
+
+ def test(self):
+ self.build()
+ target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+ self.check_global_var(target, "gLibExternalDef")
+ self.check_global_var(target, "gMainExternalDef")
diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp
new file mode 100644
index 00000000000000..b50a13afe1acc7
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp
@@ -0,0 +1,10 @@
+#include "lib.h"
+
+template <typename T> struct FooImpl {
+ using Ref = FooImpl<T> *;
+
+ Ref Create() { return new FooImpl<T>(); }
+};
+
+FooImpl<char> gLibLocalDef;
+BarImpl<char> *gLibExternalDef = nullptr;
diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h
new file mode 100644
index 00000000000000..d909fe58df12b3
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h
@@ -0,0 +1,7 @@
+#ifndef LIB_H_IN
+#define LIB_H_IN
+
+template <typename T> struct FooImpl;
+template <typename T> struct BarImpl;
+
+#endif // LIB_H_IN
diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp
new file mode 100644
index 00000000000000..acf8337a69d0bb
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp
@@ -0,0 +1,12 @@
+#include "lib.h"
+
+template <typename T> struct BarImpl {
+ using Ref = BarImpl<T> *;
+
+ Ref Create() { return new BarImpl<T>(); }
+};
+
+BarImpl<char> gMainLocalDef;
+FooImpl<char> *gMainExternalDef = nullptr;
+
+int main() { return 0; }
More information about the lldb-commits
mailing list