[Lldb-commits] [lldb] e42edb5 - [lldb] Fix expressions that involve nested structs/classes/unions. (#77029)

via lldb-commits lldb-commits at lists.llvm.org
Fri Jan 5 09:26:57 PST 2024


Author: Greg Clayton
Date: 2024-01-05T09:26:54-08:00
New Revision: e42edb5547618c172abe25914000bb61f5278c4c

URL: https://github.com/llvm/llvm-project/commit/e42edb5547618c172abe25914000bb61f5278c4c
DIFF: https://github.com/llvm/llvm-project/commit/e42edb5547618c172abe25914000bb61f5278c4c.diff

LOG: [lldb] Fix expressions that involve nested structs/classes/unions. (#77029)

The LLDB expression parser relies on using the external AST source
support in LLDB. This allows us to find a class at the root namespace
level, but it wouldn't allow us to find nested classes all of the time.
When LLDB finds a class via this mechanism, it would be able to complete
this class when needed, but during completion, we wouldn't populate
nested types within this class which would prevent us from finding
contained types when needed as clang would expect them to be present if
a class was completed. When we parse a type for a class, struct or
union, we make a forward declaration to the class which can be
completed. Now when the class is completed, we also add any contained
types to the class' declaration context which now allows these types to
be found. If we have a struct that contains a struct, we will add the
forward declaration of the contained structure which can be c ompleted
later. Having this forward declaration makes it possible for LLDB to
find everything it needs now.

This should fix an existing issue:
https://github.com/llvm/llvm-project/issues/53904

Previously, contained types could be parsed by accident and allow
expression to complete successfully. Other times we would have to run an
expression multiple times because our old type lookup from our
expressions would cau se a type to be parsed, but not used in the
current expression, but this would have parsed a type into the
containing decl context and the expression might succeed if it is run
again.

Added: 
    lldb/test/API/commands/expression/nested/Makefile
    lldb/test/API/commands/expression/nested/TestNestedExpressions.py
    lldb/test/API/commands/expression/nested/main.cpp

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3e08f2550081f2..009722b85aa186 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2150,6 +2150,7 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
   SymbolFileDWARF *dwarf = die.GetDWARF();
 
   ClangASTImporter::LayoutInfo layout_info;
+  std::vector<DWARFDIE> contained_type_dies;
 
   if (die.HasChildren()) {
     const bool type_is_objc_object_or_interface =
@@ -2175,7 +2176,8 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
 
     DelayedPropertyList delayed_properties;
     ParseChildMembers(die, clang_type, bases, member_function_dies,
-                      delayed_properties, default_accessibility, layout_info);
+                      contained_type_dies, delayed_properties,
+                      default_accessibility, layout_info);
 
     // Now parse any methods if there were any...
     for (const DWARFDIE &die : member_function_dies)
@@ -2231,6 +2233,12 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
     if (record_decl)
       GetClangASTImporter().SetRecordLayout(record_decl, layout_info);
   }
+  // Now parse all contained types inside of the class. We make forward
+  // declarations to all classes, but we need the CXXRecordDecl to have decls
+  // for all contained types because we don't get asked for them via the
+  // external AST support.
+  for (const DWARFDIE &die : contained_type_dies)
+    dwarf->ResolveType(die);
 
   return (bool)clang_type;
 }
@@ -3110,6 +3118,7 @@ bool DWARFASTParserClang::ParseChildMembers(
     const DWARFDIE &parent_die, CompilerType &class_clang_type,
     std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> &base_classes,
     std::vector<DWARFDIE> &member_function_dies,
+    std::vector<DWARFDIE> &contained_type_dies,
     DelayedPropertyList &delayed_properties,
     const AccessType default_accessibility,
     ClangASTImporter::LayoutInfo &layout_info) {
@@ -3159,6 +3168,8 @@ bool DWARFASTParserClang::ParseChildMembers(
       break;
 
     default:
+      if (llvm::dwarf::isType(tag))
+        contained_type_dies.push_back(die);
       break;
     }
   }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 3e28e54d6220db..8d4af203bb2871 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -175,6 +175,7 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
       lldb_private::CompilerType &class_compiler_type,
       std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> &base_classes,
       std::vector<lldb_private::plugin::dwarf::DWARFDIE> &member_function_dies,
+      std::vector<lldb_private::plugin::dwarf::DWARFDIE> &contained_type_dies,
       DelayedPropertyList &delayed_properties,
       const lldb::AccessType default_accessibility,
       lldb_private::ClangASTImporter::LayoutInfo &layout_info);

diff  --git a/lldb/test/API/commands/expression/nested/Makefile b/lldb/test/API/commands/expression/nested/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/commands/expression/nested/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git a/lldb/test/API/commands/expression/nested/TestNestedExpressions.py b/lldb/test/API/commands/expression/nested/TestNestedExpressions.py
new file mode 100644
index 00000000000000..7f194e921e5628
--- /dev/null
+++ b/lldb/test/API/commands/expression/nested/TestNestedExpressions.py
@@ -0,0 +1,70 @@
+"""
+Test calling an expression with errors that a FixIt can fix.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class NestedExpressions(TestBase):
+
+    def test_enum_in_nested_structs(self):
+        """
+            Test expressions that references an enumeration in nested structs.
+        """
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        target = self.dbg.CreateTarget(exe_path)
+        self.assertTrue(target, "Target: %s is not valid." % (exe_path))
+        self.expect_expr("A::B::C::EnumType::Eleven",
+                         result_type="A::B::C::EnumType",
+                         result_value="Eleven")
+
+    def test_struct_in_nested_structs(self):
+        """
+            Test expressions that references a struct in nested structs.
+        """
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        target = self.dbg.CreateTarget(exe_path)
+        self.assertTrue(target, "Target: %s is not valid." % (exe_path))
+        self.expect_expr("sizeof(A::B::C)", result_value="1")
+        self.expect_expr("sizeof(A::B)", result_value="2")
+
+    def test_static_in_nested_structs(self):
+        """
+            Test expressions that references a static variable in nested structs.
+        """
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "Stop here to evaluate expressions", lldb.SBFileSpec("main.cpp")
+        )
+        self.expect_expr("A::B::C::enum_static",
+                         result_type="A::B::C::EnumType",
+                         result_value="Eleven")
+
+    def test_enum_in_nested_namespaces(self):
+        """
+            Test expressions that references an enumeration in nested namespaces.
+        """
+        self.build()
+        exe_path = self.getBuildArtifact("a.out")
+        target = self.dbg.CreateTarget(exe_path)
+        self.assertTrue(target, "Target: %s is not valid." % (exe_path))
+        self.expect_expr("a::b::c::Color::Blue",
+                         result_type="a::b::c::Color",
+                         result_value="Blue")
+
+    def test_static_in_nested_namespaces(self):
+        """
+            Test expressions that references an enumeration in nested namespaces.
+        """
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "Stop here to evaluate expressions", lldb.SBFileSpec("main.cpp")
+        )
+        self.expect_expr("a::b::c::d",
+                         result_type="int",
+                         result_value="12")

diff  --git a/lldb/test/API/commands/expression/nested/main.cpp b/lldb/test/API/commands/expression/nested/main.cpp
new file mode 100644
index 00000000000000..620535fa09534b
--- /dev/null
+++ b/lldb/test/API/commands/expression/nested/main.cpp
@@ -0,0 +1,31 @@
+namespace a {
+namespace b {
+namespace c {
+static int d = 12;
+enum Color { Red, Green, Blue };
+} // namespace c
+} // namespace b
+} // namespace a
+
+struct A {
+  int _a = 'a';
+  struct B {
+    short _b = 'b';
+    struct C {
+      char _c = 'c';
+      enum EnumType : int { Eleven = 11 };
+      static EnumType enum_static;
+    };
+  };
+};
+
+A::B::C::EnumType A::B::C::enum_static = A::B::C::Eleven;
+
+int foo() {
+  a::b::c::Color color = a::b::c::Blue;
+  return A::B::C::enum_static == a::b::c::d && ((int)color == 0);
+}
+
+int main() {
+  return foo(); // Stop here to evaluate expressions
+}


        


More information about the lldb-commits mailing list