[Lldb-commits] [lldb] Fix type lookup bug where wrong decl context was being used for a DIE. (PR #94846)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

<details>
<summary>Changes</summary>

The function that calculated the declaration context for a DIE was incorrectly transparently traversing acrosss DW_TAG_subprogram dies when climbing the parent DIE chain. This meant that types defined in functions would appear to have the declaration context of anything above the function. I fixed the GetTypeLookupContextImpl(...) function in DWARFDIE.cpp to not transparently skip over functions, lexical blocks and inlined functions and compile and type units. Added a test to verify things are working.

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


4 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (+12) 
- (added) lldb/test/API/functionalities/type_types/Makefile (+2) 
- (added) lldb/test/API/functionalities/type_types/TestFindTypes.py (+69) 
- (added) lldb/test/API/functionalities/type_types/main.cpp (+16) 


``````````diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 7cf92adc6ef57..c10174e8848ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -491,6 +491,18 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
     case DW_TAG_base_type:
       push_ctx(CompilerContextKind::Builtin, name);
       break;
+    // If any of the tags below appear in the parent chain, stop the decl
+    // context and return. Prior to these being in here, if a type existed in a
+    // namespace "a" like "a::my_struct", but we also have a function in that
+    // same namespace "a" which contained a type named "my_struct", both would
+    // return "a::my_struct" as the declaration context since the
+    // DW_TAG_subprogram would be skipped and its parent would be found.
+    case DW_TAG_compile_unit:
+    case DW_TAG_type_unit:
+    case DW_TAG_subprogram:
+    case DW_TAG_lexical_block:
+    case DW_TAG_inlined_subroutine:
+      return;
     default:
       break;
     }
diff --git a/lldb/test/API/functionalities/type_types/Makefile b/lldb/test/API/functionalities/type_types/Makefile
new file mode 100644
index 0000000000000..3d0b98f13f3d7
--- /dev/null
+++ b/lldb/test/API/functionalities/type_types/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/type_types/TestFindTypes.py b/lldb/test/API/functionalities/type_types/TestFindTypes.py
new file mode 100644
index 0000000000000..adbaaba51d080
--- /dev/null
+++ b/lldb/test/API/functionalities/type_types/TestFindTypes.py
@@ -0,0 +1,69 @@
+"""
+Test the SBModule and SBTarget type lookup APIs to find multiple types.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TypeFindFirstTestCase(TestBase):
+    def test_find_first_type(self):
+        """
+        Test SBTarget::FindTypes() and SBModule::FindTypes() APIs.
+
+        We had issues where our declaration context when finding types was
+        incorrectly calculated where a type in a namepace, and a type in a
+        function that was also in the same namespace would match a lookup. For
+        example:
+
+            namespace a {
+              struct Foo {
+                int foo;
+              };
+
+              unsigned foo() {
+                typedef unsigned Foo;
+                Foo foo = 12;
+                return foo;
+              }
+            } // namespace a
+
+
+        Previously LLDB would calculate the declaration context of "a::Foo"
+        correctly, but incorrectly calculate the declaration context of "Foo"
+        from within the foo() function as "a::Foo". Adding tests to ensure this
+        works correctly.
+        """
+        self.build()
+        target = self.createTestTarget()
+        exe_module = target.GetModuleAtIndex(0)
+        self.assertTrue(exe_module.IsValid())
+        # Test the SBTarget and SBModule APIs for FindFirstType
+        for api in [target, exe_module]:
+            # We should find the "a::Foo" but not the "Foo" type in the function
+            types = api.FindTypes("a::Foo")
+            self.assertEqual(types.GetSize(), 1)
+            type_str0 = str(types.GetTypeAtIndex(0))
+            self.assertIn('struct Foo', type_str0)
+
+            # When we search by type basename, we should find any type whose
+            # basename matches "Foo", so "a::Foo" and the "Foo" type in the
+            # function.
+            types = api.FindTypes("Foo")
+            self.assertEqual(types.GetSize(), 2)
+            type_str0 = str(types.GetTypeAtIndex(0))
+            type_str1 = str(types.GetTypeAtIndex(1))
+            # We don't know which order the types will come back as, so
+            if 'struct Foo' in type_str0:
+                self.assertIn('typedef Foo', type_str1)
+            else:
+                self.assertIn('struct Foo', type_str1)
+
+            # When we search by type basename with "::" prepended, we should
+            # only types in the root namespace which means only "Foo" type in
+            # the function.
+            types = api.FindTypes("::Foo")
+            self.assertEqual(types.GetSize(), 1)
+            type_str0 = str(types.GetTypeAtIndex(0))
+            self.assertIn('typedef Foo', type_str0)
diff --git a/lldb/test/API/functionalities/type_types/main.cpp b/lldb/test/API/functionalities/type_types/main.cpp
new file mode 100644
index 0000000000000..57eecd81f0ce7
--- /dev/null
+++ b/lldb/test/API/functionalities/type_types/main.cpp
@@ -0,0 +1,16 @@
+namespace a {
+  struct Foo {};
+
+  unsigned foo() {
+    typedef unsigned Foo;
+    Foo foo = 12;
+    return foo;
+  }
+} // namespace a
+
+
+int main() {
+  a::Foo f = {};
+  a::foo();
+  return 0;
+}

``````````

</details>


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