[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the declaration … (PR #120569)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

<details>
<summary>Changes</summary>

…DIE's SymbolFile

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 surrounding code used to assume we don't call `CompleteTypeFromDWARF` with a `nullptr` `Type*`. This used to be more of an issue when `CompleteRecordType` actually made use of the `Type*` parameter (see https://github.com/llvm/llvm-project/pull/120456).
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 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.

---
Full diff: https://github.com/llvm/llvm-project/pull/120569.diff


6 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+3-1) 
- (added) lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile (+3) 
- (added) lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py (+32) 
- (added) lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp (+15) 
- (added) lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h (+10) 
- (added) lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp (+8) 


``````````diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 68e50902d641a2..936a44865b4a7e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1593,7 +1593,9 @@ bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) {
   DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU());
   if (!dwarf_ast)
     return false;
-  Type *type = GetDIEToType().lookup(decl_die.GetDIE());
+  Type *type = decl_die.GetDWARF()->GetDIEToType().lookup(decl_die.GetDIE());
+  assert (type);
+
   if (decl_die != def_die) {
     GetDIEToType()[def_die.GetDIE()] = type;
     DWARFASTParserClang *ast_parser =
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..ad26d503c545b2
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
@@ -0,0 +1,32 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCaseTypedefToOuterFwd(TestBase):
+    '''
+    We are stopped in main.o, which only sees a forward declaration
+    of FooImpl. We then try to get the FooImpl::Ref typedef (whose
+    definition is in lib.o). Make sure we correctly resolve this
+    typedef.
+    '''
+    def test(self):
+        self.build()
+        (_, _, thread, _) = lldbutil.run_to_source_breakpoint(
+            self, "return", lldb.SBFileSpec("main.cpp")
+        )
+
+        foo = thread.frames[0].FindVariable('foo')
+        self.assertSuccess(foo.GetError(), "Found foo")
+
+        foo_type = foo.GetType()
+        self.assertTrue(foo_type)
+
+        impl = foo_type.GetPointeeType()
+        self.assertTrue(impl)
+
+        ref = impl.FindDirectNestedType('Ref')
+        self.assertTrue(ref)
+
+        self.assertEqual(ref.GetCanonicalType(), foo_type)
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..2bc38f6503396d
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp
@@ -0,0 +1,15 @@
+#include "lib.h"
+
+template <typename T> struct FooImpl {
+  using Ref = FooImpl<T> *;
+
+  Ref Create() { return new FooImpl<T>(); }
+};
+
+Foo getString() {
+  FooImpl<char> impl;
+  Foo ret;
+  ret.impl = impl.Create();
+
+  return ret;
+}
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..d5dfddf2246209
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h
@@ -0,0 +1,10 @@
+#ifndef LIB_H_IN
+#define LIB_H_IN
+
+template <typename T> struct FooImpl;
+
+struct Foo {
+  FooImpl<char> *impl = nullptr;
+};
+
+#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..61c819ca0d31a1
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp
@@ -0,0 +1,8 @@
+#include "lib.h"
+
+extern Foo getString();
+
+int main() {
+  FooImpl<char> *foo = getString().impl;
+  return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/120569