[Lldb-commits] [lldb] Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" (PR #71800)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Sun Nov 12 22:09:32 PST 2023


https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/71800

>From 21567b31b5f11c7ed17fc49139e58cef570c4dac Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 6 Nov 2023 11:34:09 +0000
Subject: [PATCH 1/2] Reland "[lldb][DWARFASTParserClang] Fetch constant value
 from variable defintion if available"

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 62 ++++++++++++++++++-
 .../SymbolFile/DWARF/DWARFASTParserClang.h    | 11 ++++
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      |  7 +--
 .../SymbolFile/DWARF/SymbolFileDWARF.h        | 10 +--
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp   |  7 +++
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.h     |  5 ++
 .../TestConstStaticIntegralMember.py          | 53 ++++++++++++++++
 .../cpp/const_static_integral_member/main.cpp | 20 ++++++
 8 files changed, 165 insertions(+), 10 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a70dc9832f425c6..570cd5d46952681 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -31,6 +31,7 @@
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/TypeMap.h"
+#include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
@@ -139,6 +140,53 @@ static bool ShouldIgnoreArtificialField(llvm::StringRef FieldName) {
   return FieldName.starts_with("_vptr$")
          // gdb emit vtable pointer as "_vptr.classname"
          || FieldName.starts_with("_vptr.");
+
+std::optional<DWARFFormValue>
+DWARFASTParserClang::FindConstantOnVariableDefinition(DWARFDIE die) {
+  assert(die.Tag() == llvm::dwarf::DW_TAG_member);
+
+  auto *dwarf = die.GetDWARF();
+  if (!dwarf)
+    return {};
+
+  ConstString name{die.GetName()};
+  if (!name)
+    return {};
+
+  auto *CU = die.GetCU();
+  if (!CU)
+    return {};
+
+  DWARFASTParser *dwarf_ast = dwarf->GetDWARFParser(*CU);
+  auto parent_decl_ctx = dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+
+  // Make sure we populate the GetDieToVariable cache.
+  VariableList variables;
+  dwarf->FindGlobalVariables(name, parent_decl_ctx, UINT_MAX, variables);
+
+  // The cache contains the variable definition whose DW_AT_specification
+  // points to our declaration DIE. Look up that definition using our
+  // declaration.
+  auto const &die_to_var = dwarf->GetDIEToVariable();
+  auto it = die_to_var.find(die.GetDIE());
+  if (it == die_to_var.end())
+    return {};
+
+  auto var_sp = it->getSecond();
+  assert(var_sp != nullptr);
+
+  if (!var_sp->GetLocationIsConstantValueData())
+    return {};
+
+  auto def = dwarf->GetDIE(var_sp->GetID());
+  auto def_attrs = def.GetAttributes();
+  DWARFFormValue form_value;
+  if (!def_attrs.ExtractFormValueAtIndex(
+          def_attrs.FindAttributeIndex(llvm::dwarf::DW_AT_const_value),
+          form_value))
+    return {};
+
+  return form_value;
 }
 
 TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
@@ -2914,9 +2962,21 @@ void DWARFASTParserClang::ParseSingleMember(
 
       bool unused;
       // TODO: Support float/double static members as well.
-      if (!attrs.const_value_form || !ct.IsIntegerOrEnumerationType(unused))
+      if (!ct.IsIntegerOrEnumerationType(unused))
         return;
 
+      // Newer versions of Clang don't emit the DW_AT_const_value
+      // on the declaration of an inline static data member. Instead
+      // it's attached to the definition DIE. If that's the case,
+      // try and fetch it.
+      if (!attrs.const_value_form) {
+        auto maybe_form_value = FindConstantOnVariableDefinition(die);
+        if (!maybe_form_value)
+          return;
+
+        attrs.const_value_form = *maybe_form_value;
+      }
+
       llvm::Expected<llvm::APInt> const_value_or_err =
           ExtractIntFromFormValue(ct, *attrs.const_value_form);
       if (!const_value_or_err) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index c381c58fba74263..21fd6f9d7980efc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -373,6 +373,17 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
                        lldb_private::CompilerType &class_clang_type,
                        const lldb::AccessType default_accesibility,
                        lldb_private::ClangASTImporter::LayoutInfo &layout_info);
+
+  /// Tries to find the definition DW_TAG_variable DIE of the the specified
+  /// DW_TAG_member 'die'. If such definition exists, returns the
+  /// DW_AT_const_value of that definition if available. Returns std::nullopt
+  /// otherwise.
+  ///
+  /// In newer versions of clang, DW_AT_const_value attributes are not attached
+  /// to the declaration of a inline static data-member anymore, but rather on
+  /// its definition. This function is used to locate said constant.
+  std::optional<lldb_private::plugin::dwarf::DWARFFormValue>
+  FindConstantOnVariableDefinition(lldb_private::plugin::dwarf::DWARFDIE die);
 };
 
 /// Parsed form of all attributes that are relevant for type reconstruction.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 5bd4665dff7da7f..4cf113d93120980 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3564,9 +3564,8 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
   }
 
   // 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
-  // the member.
+  // for static constexpr member variables -- DW_AT_const_value and
+  // DW_AT_location will both be present in the DIE defining the member.
   bool location_is_const_value_data =
       const_value_form.IsValid() && !location_form.IsValid();
 
@@ -3648,7 +3647,7 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
       // found there.
       location_list.SetModule(debug_map_symfile->GetObjectFile()->GetModule());
 
-    if (is_static_lifetime) {
+    if (is_static_lifetime && !location_is_const_value_data) {
       if (is_external)
         scope = eValueTypeVariableGlobal;
       else
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 28430ccb87924b5..e6efbba7e24990a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -343,6 +343,11 @@ class SymbolFileDWARF : public SymbolFileCommon {
     return m_forward_decl_compiler_type_to_die;
   }
 
+  typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
+      DIEToVariableSP;
+
+  virtual DIEToVariableSP &GetDIEToVariable() { return m_die_to_variable_sp; }
+
   virtual UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap();
 
   bool ClassOrStructIsVirtual(const DWARFDIE &die);
@@ -362,9 +367,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
   Type *ResolveTypeUID(const DIERef &die_ref);
 
 protected:
-  typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb::VariableSP>
-      DIEToVariableSP;
-
   SymbolFileDWARF(const SymbolFileDWARF &) = delete;
   const SymbolFileDWARF &operator=(const SymbolFileDWARF &) = delete;
 
@@ -488,8 +490,6 @@ class SymbolFileDWARF : public SymbolFileCommon {
 
   void UpdateExternalModuleListIfNeeded();
 
-  virtual DIEToVariableSP &GetDIEToVariable() { return m_die_to_variable_sp; }
-
   void BuildCuTranslationTable();
   std::optional<uint32_t> GetDWARFUnitIndex(uint32_t cu_idx);
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index 223af281cd57db7..ca698a84a9146da 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -142,3 +142,10 @@ SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
     return DebugInfo().GetDIE(die_ref);
   return GetBaseSymbolFile().GetDIE(die_ref);
 }
+
+void SymbolFileDWARFDwo::FindGlobalVariables(
+    ConstString name, const CompilerDeclContext &parent_decl_ctx,
+    uint32_t max_matches, VariableList &variables) {
+  GetBaseSymbolFile().FindGlobalVariables(name, parent_decl_ctx, max_matches,
+                                          variables);
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
index fa8376507d6a89b..9f5950e51b0c180 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -51,6 +51,11 @@ class SymbolFileDWARFDwo : public SymbolFileDWARF {
                               lldb::offset_t &offset,
                               std::vector<Value> &stack) const override;
 
+  void FindGlobalVariables(ConstString name,
+                           const CompilerDeclContext &parent_decl_ctx,
+                           uint32_t max_matches,
+                           VariableList &variables) override;
+
 protected:
   DIEToTypePtr &GetDIEToType() override;
 
diff --git a/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py b/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
index 78ea23ac8f70610..b84f376a3e4400d 100644
--- a/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ b/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -114,6 +114,38 @@ def test_class_with_only_const_static(self):
 
         self.expect_expr("ClassWithOnlyConstStatic::member", result_value="3")
 
+    def check_global_var(self, name: str, expect_type, expect_val):
+        var_list = self.target().FindGlobalVariables(name, lldb.UINT32_MAX)
+        self.assertEqual(len(var_list), 1)
+        varobj = var_list[0]
+        self.assertEqual(varobj.type.name, expect_type)
+        self.assertEqual(varobj.value, expect_val)
+
+    # For debug-info produced by older versions of clang, inline static data members
+    # wouldn't get indexed into the Names accelerator table preventing LLDB from finding
+    # them.
+    @expectedFailureAll(compiler=["clang"], compiler_version=["<", "18.0"])
+    def test_inline_static_members(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.cpp")
+        )
+
+        self.check_global_var("A::int_val", "const int", "1")
+        self.check_global_var("A::int_val_with_address", "const int", "2")
+        self.check_global_var("A::bool_val", "const bool", "true")
+        self.check_global_var("A::enum_val", "Enum", "enum_case2")
+        self.check_global_var("A::enum_bool_val", "EnumBool", "enum_bool_case1")
+        self.check_global_var("A::scoped_enum_val", "ScopedEnum", "scoped_enum_case2")
+
+        self.check_global_var("ClassWithOnlyConstStatic::member", "const int", "3")
+
+        self.check_global_var("ClassWithConstexprs::member", "const int", "2")
+        self.check_global_var("ClassWithConstexprs::enum_val", "Enum", "enum_case2")
+        self.check_global_var(
+            "ClassWithConstexprs::scoped_enum_val", "ScopedEnum", "scoped_enum_case2"
+        )
+
     # With older versions of Clang, LLDB fails to evaluate classes with only
     # constexpr members when dsymutil is enabled
     @expectedFailureAll(
@@ -139,3 +171,24 @@ def test_class_with_only_constexpr_static(self):
         self.expect_expr(
             "ClassWithEnumAlias::enum_alias_alias", result_value="scoped_enum_case1"
         )
+
+    def test_shadowed_static_inline_members(self):
+        """Tests that the expression evaluator and SBAPI can both
+        correctly determine the requested inline static variable
+        in the presence of multiple variables of the same name."""
+
+        self.build()
+        lldbutil.run_to_name_breakpoint(self, "bar")
+
+        self.check_global_var("ns::Foo::mem", "const int", "10")
+
+        self.expect_expr("mem", result_value="10")
+        self.expect_expr("Foo::mem", result_value="10")
+        self.expect_expr("ns::Foo::mem", result_value="10")
+        self.expect_expr("::Foo::mem", result_value="-29")
+
+    @expectedFailureAll(bugnumber="target var doesn't honour global namespace")
+    def test_shadowed_static_inline_members_xfail(self):
+        self.build()
+        lldbutil.run_to_name_breakpoint(self, "bar")
+        self.check_global_var("::Foo::mem", "const int", "-29")
diff --git a/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp b/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
index 4275f471df6aed1..fb0b49e2d7aad7a 100644
--- a/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ b/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -89,6 +89,25 @@ struct ClassWithEnumAlias {
       ScopedEnum::scoped_enum_case1;
 };
 
+namespace ns {
+struct Foo {
+  constexpr static int mem = 10;
+
+  void bar() { return; }
+};
+} // namespace ns
+
+struct Foo {
+  constexpr static int mem = -29;
+};
+
+int func() {
+  Foo f1;
+  ns::Foo f2;
+  f2.bar();
+  return ns::Foo::mem + Foo::mem;
+}
+
 int main() {
   A a;
 
@@ -124,6 +143,7 @@ int main() {
 
   auto enum_alias_val = ClassWithEnumAlias::enum_alias;
   auto enum_alias_alias_val = ClassWithEnumAlias::enum_alias_alias;
+  auto ret = func();
 
   return 0; // break here
 }

>From feba840daf34d0a8c0756d3cef34ec4585849c92 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 9 Nov 2023 21:28:32 +0000
Subject: [PATCH 2/2] fixup! don't need to change ParseVariableDIE to account
 for constants anymore

---
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 4cf113d93120980..b8b2eb58a8bd85c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3647,7 +3647,7 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
       // found there.
       location_list.SetModule(debug_map_symfile->GetObjectFile()->GetModule());
 
-    if (is_static_lifetime && !location_is_const_value_data) {
+    if (is_static_lifetime) {
       if (is_external)
         scope = eValueTypeVariableGlobal;
       else



More information about the lldb-commits mailing list