Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" #71800

Michael137 · 2023-11-09T12:12:38Z

This patch relands #71004 which was reverted because the clang change it depends on was reverted.

In addition to the original patch, this PR includes a change to SymbolFileDWARF::ParseVariableDIE to support CU-level variable definitions that don't have locations, but represent a constant value. Previously, when debug-maps were available, we would assume that a variable with "static lifetime" (which in this case means "has a linkage name") has a valid address, which isn't the case for non-locationed constants. We could omit this additional change if we stopped attaching linkage names to global non-locationed constants.

Original commit message:
"""
#71780 proposes moving the DW_AT_const_value on inline static members from the declaration DIE to the definition DIE. This patch makes sure the LLDB's expression evaluator can continue to support static initialisers even if the declaration doesn't have a DW_AT_const_value anymore.

Previously the expression evaluator would find the constant for a VarDecl from its declaration DW_TAG_member DIE. In cases where the initialiser was specified out-of-class, LLDB could find it during symbol resolution.

However, neither of those will work for constants, since we don't have a constant attribute on the declaration anymore and we don't have constants in the symbol table.
"""

Depends on:

Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" #71780

llvmbot · 2023-11-09T12:13:09Z

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This patch relands #71004 which was reverted because the clang change it depends on was reverted.

In addition to the original patch, this PR includes a change to SymbolFileDWARF::ParseVariableDIE to support CU-level variable DIEs that don't have locations. Previously, when debug-maps were available, we would assume that a variable with "static lifetime" (which in this case means "has a linkage name") has a valid address, which isn't the case for non-locationed constants. We could omit this additional change if we stopped attaching linkage names to global non-locationed constants.

Original commit message:
"""
#71780 proposes moving the DW_AT_const_value on inline static members from the declaration DIE to the definition DIE. This patch makes sure the LLDB's expression evaluator can continue to support static initialisers even if the declaration doesn't have a DW_AT_const_value anymore.

Previously the expression evaluator would find the constant for a VarDecl from its declaration DW_TAG_member DIE. In cases where the initialiser was specified out-of-class, LLDB could find it during symbol resolution.

However, neither of those will work for constants, since we don't have a constant attribute on the declaration anymore and we don't have constants in the symbol table.
"""

Depends on:

Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" #71780

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

8 Files Affected:

(modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+62-1)
(modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (+11)
(modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+3-4)
(modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (+5-5)
(modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp (+7)
(modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h (+5)
(modified) lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py (+53)
(modified) lldb/test/API/lang/cpp/const_static_integral_member/main.cpp (+20)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3174c18c97d888c..4e41858674467a3 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"
@@ -133,6 +134,54 @@ static lldb::ModuleSP GetContainingClangModule(const DWARFDIE &die) {
   return lldb::ModuleSP();
 }
 
+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,
                                                      const DWARFDIE &die,
                                                      Log *log) {
@@ -2906,9 +2955,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 aabd83a194932ff..0f7e2c1efc58bee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3550,9 +3550,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();
 
@@ -3634,7 +3633,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 530191e8a37ba1b..126b6748e1fdf8a 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
@@ -113,6 +113,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(
@@ -138,3 +170,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
 }

…c data members with constant initializers" (#71780) This patch relands #70639 It was reverted because under certain conditions we triggered an assertion in `DIBuilder`. Specifically, in the original patch we called `EmitGlobalVariable` at the end of `CGDebugInfo::finalize`, after all the temporary `DIType`s have been uniqued. With limited debug-info such temporary nodes would be created more frequently, leaving us with non-uniqued nodes by the time we got to `DIBuilder::finalize`; this violated its pre-condition and caused assertions to trigger. To fix this, the latest iteration of the patch moves `EmitGlobalVariable` to the beginning of `CGDebugInfo::finalize`. Now, when we create a temporary `DIType` node as a result of emitting a variable definition, it will get uniqued in time. A test-case was added for this scenario. We also now don't emit a linkage name for non-locationed constants since LLDB doesn't make use of it anyway. Original commit message: """ When an LLDB user asks for the value of a static data member, LLDB starts by searching the Names accelerator table for the corresponding variable definition DIE. For static data members with out-of-class definitions that works fine, because those get represented as global variables with a location and making them eligible to be added to the Names table. However, in-class definitions won’t get indexed because we usually don't emit global variables for them. So in DWARF we end up with a single `DW_TAG_member` that usually holds the constant initializer. But we don't get a corresponding CU-level `DW_TAG_variable` like we do for out-of-class definitions. To make it more convenient for debuggers to get to the value of inline static data members, this patch makes sure we emit definitions for static variables with constant initializers the same way we do for other static variables. This also aligns Clang closer to GCC, which produces CU-level definitions for inline statics and also emits these into `.debug_pubnames`. The implementation keeps track of newly created static data members. Then in `CGDebugInfo::finalize`, we emit a global `DW_TAG_variable` with a `DW_AT_const_value` for any of those declarations that didn't end up with a definition in the `DeclCache`. The newly emitted `DW_TAG_variable` will look as follows: ``` 0x0000007b: DW_TAG_structure_type DW_AT_calling_convention (DW_CC_pass_by_value) DW_AT_name ("Foo") ... 0x0000008d: DW_TAG_member DW_AT_name ("i") DW_AT_type (0x00000062 "const int") DW_AT_external (true) DW_AT_declaration (true) DW_AT_const_value (4) Newly added vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv 0x0000009a: DW_TAG_variable DW_AT_specification (0x0000008d "i") DW_AT_const_value (4) DW_AT_linkage_name ("_ZN2t2IiE1iIfEE") ``` This patch also drops the `DW_AT_const_value` off of the declaration since we now always have it on the definition. This ensures that the `DWARFParallelLinker` can type-merge class with static members where we couldn't attach the constant on the declaration in some CUs. """ Dependent changes: * #71800

…e defintion if available"

… anymore

github-actions · 2023-11-13T06:11:47Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff d4912e80500706a9912725d45d822675cf9153d8 feba840daf34d0a8c0756d3cef34ec4585849c92 -- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h lldb/test/API/lang/cpp/const_static_integral_member/main.cpp

View the diff from clang-format here.

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 570cd5d469..63f0561b4e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -141,53 +141,53 @@ static bool ShouldIgnoreArtificialField(llvm::StringRef FieldName) {
          // 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);
+  std::optional<DWARFFormValue>
+  DWARFASTParserClang::FindConstantOnVariableDefinition(DWARFDIE die) {
+    assert(die.Tag() == llvm::dwarf::DW_TAG_member);
 
-  auto *dwarf = die.GetDWARF();
-  if (!dwarf)
-    return {};
+    auto *dwarf = die.GetDWARF();
+    if (!dwarf)
+      return {};
 
-  ConstString name{die.GetName()};
-  if (!name)
-    return {};
+    ConstString name{die.GetName()};
+    if (!name)
+      return {};
 
-  auto *CU = die.GetCU();
-  if (!CU)
-    return {};
+    auto *CU = die.GetCU();
+    if (!CU)
+      return {};
 
-  DWARFASTParser *dwarf_ast = dwarf->GetDWARFParser(*CU);
-  auto parent_decl_ctx = dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+    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);
+    // 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 {};
+    // 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);
+    auto var_sp = it->getSecond();
+    assert(var_sp != nullptr);
 
-  if (!var_sp->GetLocationIsConstantValueData())
-    return {};
+    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 {};
+    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;
-}
+    return form_value;
+  }
 
 TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
                                                      const DWARFDIE &die,

…c data members with constant initializers" (llvm#71780) This patch relands llvm#70639 It was reverted because under certain conditions we triggered an assertion in `DIBuilder`. Specifically, in the original patch we called `EmitGlobalVariable` at the end of `CGDebugInfo::finalize`, after all the temporary `DIType`s have been uniqued. With limited debug-info such temporary nodes would be created more frequently, leaving us with non-uniqued nodes by the time we got to `DIBuilder::finalize`; this violated its pre-condition and caused assertions to trigger. To fix this, the latest iteration of the patch moves `EmitGlobalVariable` to the beginning of `CGDebugInfo::finalize`. Now, when we create a temporary `DIType` node as a result of emitting a variable definition, it will get uniqued in time. A test-case was added for this scenario. We also now don't emit a linkage name for non-locationed constants since LLDB doesn't make use of it anyway. Original commit message: """ When an LLDB user asks for the value of a static data member, LLDB starts by searching the Names accelerator table for the corresponding variable definition DIE. For static data members with out-of-class definitions that works fine, because those get represented as global variables with a location and making them eligible to be added to the Names table. However, in-class definitions won’t get indexed because we usually don't emit global variables for them. So in DWARF we end up with a single `DW_TAG_member` that usually holds the constant initializer. But we don't get a corresponding CU-level `DW_TAG_variable` like we do for out-of-class definitions. To make it more convenient for debuggers to get to the value of inline static data members, this patch makes sure we emit definitions for static variables with constant initializers the same way we do for other static variables. This also aligns Clang closer to GCC, which produces CU-level definitions for inline statics and also emits these into `.debug_pubnames`. The implementation keeps track of newly created static data members. Then in `CGDebugInfo::finalize`, we emit a global `DW_TAG_variable` with a `DW_AT_const_value` for any of those declarations that didn't end up with a definition in the `DeclCache`. The newly emitted `DW_TAG_variable` will look as follows: ``` 0x0000007b: DW_TAG_structure_type DW_AT_calling_convention (DW_CC_pass_by_value) DW_AT_name ("Foo") ... 0x0000008d: DW_TAG_member DW_AT_name ("i") DW_AT_type (0x00000062 "const int") DW_AT_external (true) DW_AT_declaration (true) DW_AT_const_value (4) Newly added vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv 0x0000009a: DW_TAG_variable DW_AT_specification (0x0000008d "i") DW_AT_const_value (4) DW_AT_linkage_name ("_ZN2t2IiE1iIfEE") ``` This patch also drops the `DW_AT_const_value` off of the declaration since we now always have it on the definition. This ensures that the `DWARFParallelLinker` can type-merge class with static members where we couldn't attach the constant on the declaration in some CUs. """ Dependent changes: * llvm#71800

…e defintion if available" (llvm#71800) This patch relands llvm#71004 which was reverted because the clang change it depends on was reverted. In addition to the original patch, this PR includes a change to `SymbolFileDWARF::ParseVariableDIE` to support CU-level variable definitions that don't have locations, but represent a constant value. Previously, when debug-maps were available, we would assume that a variable with "static lifetime" (which in this case means "has a linkage name") has a valid address, which isn't the case for non-locationed constants. We could omit this additional change if we stopped attaching linkage names to global non-locationed constants. Original commit message: """ llvm#71780 proposes moving the `DW_AT_const_value` on inline static members from the declaration DIE to the definition DIE. This patch makes sure the LLDB's expression evaluator can continue to support static initialisers even if the declaration doesn't have a `DW_AT_const_value` anymore. Previously the expression evaluator would find the constant for a VarDecl from its declaration `DW_TAG_member` DIE. In cases where the initialiser was specified out-of-class, LLDB could find it during symbol resolution. However, neither of those will work for constants, since we don't have a constant attribute on the declaration anymore and we don't have constants in the symbol table. """ Depends on: * llvm#71780

Michael137 requested a review from JDevlieghere as a code owner November 9, 2023 12:12

Michael137 requested a review from felipepiovezan November 9, 2023 12:12

llvmbot added the lldb label Nov 9, 2023

Michael137 mentioned this pull request Nov 9, 2023

Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" #71780

Merged

adrian-prantl approved these changes Nov 10, 2023

View reviewed changes

Michael137 added 2 commits November 13, 2023 06:08

Reland "[lldb][DWARFASTParserClang] Fetch constant value from variabl…

21567b3

…e defintion if available"

fixup! don't need to change ParseVariableDIE to account for constants…

feba840

… anymore

Michael137 force-pushed the inline-statics-support/reland-lldb-find-constexpr-globals-clean branch from 381bbe3 to feba840 Compare November 13, 2023 06:09

Michael137 merged commit 15c8085 into llvm:main Nov 13, 2023
2 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" #71800

Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" #71800

Michael137 commented Nov 9, 2023 •

edited

Loading

llvmbot commented Nov 9, 2023

github-actions bot commented Nov 13, 2023

Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" #71800

Reland "[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available" #71800

Conversation

Michael137 commented Nov 9, 2023 • edited Loading

llvmbot commented Nov 9, 2023

github-actions bot commented Nov 13, 2023

Michael137 commented Nov 9, 2023 •

edited

Loading