werat updated this revision to Diff 330445.
werat added a comment.

Address review comments:

- Don't create expensive `ConstString` objects
- Merge `ParseStaticConstMemberDIE` into `ParseVariableDIE`
- Add more test cases


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92643/new/

https://reviews.llvm.org/D92643

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/python_api/target/globals/Makefile
  lldb/test/API/python_api/target/globals/TestTargetGlobals.py
  lldb/test/API/python_api/target/globals/main.cpp

Index: lldb/test/API/python_api/target/globals/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/target/globals/main.cpp
@@ -0,0 +1,30 @@
+namespace ns1 {
+class Klass {
+public:
+  inline static int x = 1;
+};
+} // namespace ns1
+
+namespace ns2 {
+class Klass {
+public:
+  inline static int x = 2;
+};
+} // namespace ns2
+
+class Vars {
+public:
+  inline static double inline_static = 1.5;
+  static constexpr int static_constexpr = 2;
+  static const int static_const_out_out_class;
+};
+
+const int Vars::static_const_out_out_class = 3;
+
+char global_var_of_char_type = 'X';
+
+int main() {
+  ns1::Klass ns1;
+  ns2::Klass ns2;
+  Vars v;
+}
Index: lldb/test/API/python_api/target/globals/TestTargetGlobals.py
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/target/globals/TestTargetGlobals.py
@@ -0,0 +1,54 @@
+"""
+Test SBTarget::FindGlobalVariables API.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TargetAPITestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @add_test_categories(['pyapi'])
+    def test_find_global_variables(self):
+        """Exercise SBTarget.FindGlobalVariables() API."""
+        self.build()
+
+        # Don't need to launch a process, since we're only interested in
+        # looking up global variables.
+        target = self.dbg.CreateTarget(self.getBuildArtifact())
+
+        def test_global_var(query, name, type_name, value):
+            value_list = target.FindGlobalVariables(query, 100500)
+            self.assertEqual(value_list.GetSize(), 1)
+            var = value_list.GetValueAtIndex(0)
+            self.DebugSBValue(var)
+            self.assertTrue(var)
+            self.assertEqual(var.GetName(), name)
+            self.assertEqual(var.GetTypeName(), type_name)
+            self.assertEqual(var.GetValue(), value)
+
+        test_global_var(
+            "Vars::inline_static",
+            "Vars::inline_static", "double", "1.5")
+        test_global_var(
+            "Vars::static_constexpr",
+            "Vars::static_constexpr", "const int", "2")
+        test_global_var(
+            "Vars::static_const_out_out_class",
+            "Vars::static_const_out_out_class", "const int", "3")
+        test_global_var(
+            "global_var_of_char_type",
+            "::global_var_of_char_type", "char", "'X'")
+
+        # Looking for a specific scope should find the variable from that scope.
+        test_global_var("ns1::Klass::x", "ns1::Klass::x", "int", "1")
+        test_global_var("ns2::Klass::x", "ns2::Klass::x", "int", "2")
+        
+        # However the query for `Klass::x` should return both variables:
+        # `ns1::Klass::x` and `ns2::Klass::x`.
+        values = target.FindGlobalVariables("Klass::x", 100500)
+        self.assertEqual(values.GetSize(), 2)
+        self.assertEqual(
+            {values[0].GetName(), values[1].GetName()}, 
+            {"ns1::Klass::x", "ns2::Klass::x"})
Index: lldb/test/API/python_api/target/globals/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/target/globals/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2147,6 +2147,46 @@
     return variables.GetSize() - original_size < max_matches;
   });
 
+  // If we don't have enough matches and the variable context is not empty, try
+  // to resolve the context as a type and look for static const members.
+  if (variables.GetSize() - original_size < max_matches && !context.empty()) {
+    llvm::StringRef type_scope;
+    llvm::StringRef type_name;
+    TypeClass type_class;
+    if (!Type::GetTypeScopeAndBasename(context, type_scope, type_name,
+                                       type_class))
+      type_name = context;
+
+    m_index->GetTypes(ConstString(type_name), [&](DWARFDIE parent) {
+      DWARFDeclContext dwarf_decl_ctx = GetDWARFDeclContext(parent);
+      llvm::StringRef parent_type_name = dwarf_decl_ctx.GetQualifiedName();
+
+      // This type is from another scope, skip it.
+      if (!parent_type_name.endswith(context))
+        return true;
+
+      auto *dwarf_cu = llvm::dyn_cast<DWARFCompileUnit>(parent.GetCU());
+      if (!dwarf_cu)
+        return true;
+      sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
+
+      for (DWARFDIE die = parent.GetFirstChild(); die.IsValid();
+           die = die.GetSibling()) {
+        // Try parsing the entry as a static const member.
+        if (auto var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS)) {
+          if (var_sp->GetUnqualifiedName().GetStringRef() != basename)
+            continue;
+
+          // There can be only one member with a given name.
+          variables.AddVariableIfUnique(var_sp);
+          break;
+        }
+      }
+
+      return variables.GetSize() - original_size < max_matches;
+    });
+  }
+
   // Return the number of variable that were appended to the list
   const uint32_t num_matches = variables.GetSize() - original_size;
   if (log && num_matches > 0) {
@@ -3098,10 +3138,9 @@
     return var_sp; // Already been parsed!
 
   const dw_tag_t tag = die.Tag();
-  ModuleSP module = GetObjectFile()->GetModule();
 
   if (tag != DW_TAG_variable && tag != DW_TAG_constant &&
-      (tag != DW_TAG_formal_parameter || !sc.function))
+      tag != DW_TAG_member && (tag != DW_TAG_formal_parameter || !sc.function))
     return nullptr;
 
   DWARFAttributes attributes;
@@ -3175,6 +3214,14 @@
     }
   }
 
+  // Look only for static const members with const values.
+  if (tag == DW_TAG_member &&
+      !DWARFFormValue::IsDataForm(const_value_form.Form()))
+    return nullptr;
+
+  const dw_tag_t parent_tag = die.GetParent().Tag();
+  ModuleSP module = GetObjectFile()->GetModule();
+
   // Prefer DW_AT_location over DW_AT_const_value. Both can be emitted e.g.
   // for static constexpr member variables -- DW_AT_const_value will be
   // present in the class declaration and DW_AT_location in the DIE defining
@@ -3232,17 +3279,16 @@
     }
   }
 
-  const DWARFDIE parent_context_die = GetDeclContextDIEContainingDIE(die);
-  const dw_tag_t parent_tag = die.GetParent().Tag();
-  bool is_static_member = (parent_tag == DW_TAG_compile_unit ||
-                           parent_tag == DW_TAG_partial_unit) &&
-                          (parent_context_die.Tag() == DW_TAG_class_type ||
-                           parent_context_die.Tag() == DW_TAG_structure_type);
-
-  ValueType scope = eValueTypeInvalid;
-
-  const DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die);
-  SymbolContextScope *symbol_context_scope = nullptr;
+  // DW_TAG_member entries are obviously static members in this context.
+  bool is_static_member = tag == DW_TAG_member;
+  // Other entries can be static members too, look at their parent.
+  if (!is_static_member) {
+    const DWARFDIE parent_context_die = GetDeclContextDIEContainingDIE(die);
+    is_static_member = (parent_tag == DW_TAG_compile_unit ||
+                        parent_tag == DW_TAG_partial_unit) &&
+                       (parent_context_die.Tag() == DW_TAG_class_type ||
+                        parent_context_die.Tag() == DW_TAG_structure_type);
+  }
 
   bool has_explicit_mangled = mangled != nullptr;
   if (!mangled) {
@@ -3256,14 +3302,19 @@
     // If the compiler does not emit a linkage name, we should be
     // able to generate a fully qualified name from the
     // declaration context.
-    if ((parent_tag == DW_TAG_compile_unit ||
+    if ((tag == DW_TAG_member || parent_tag == DW_TAG_compile_unit ||
          parent_tag == DW_TAG_partial_unit) &&
         Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU())))
       mangled =
           GetDWARFDeclContext(die).GetQualifiedNameAsConstString().GetCString();
   }
 
-  if (tag == DW_TAG_formal_parameter)
+  ValueType scope = eValueTypeInvalid;
+  SymbolContextScope *symbol_context_scope = nullptr;
+
+  if (tag == DW_TAG_member)
+    scope = eValueTypeVariableGlobal;
+  else if (tag == DW_TAG_formal_parameter)
     scope = eValueTypeVariableArgument;
   else {
     // DWARF doesn't specify if a DW_TAG_variable is a local, global
@@ -3392,6 +3443,7 @@
     case DW_TAG_inlined_subroutine:
     case DW_TAG_lexical_block:
       if (sc.function) {
+        const DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die);
         symbol_context_scope =
             sc.function->GetBlock(true).FindBlockByID(sc_parent_die.GetID());
         if (symbol_context_scope == nullptr)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to