[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
Thu Nov 9 04:12:44 PST 2023
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/71800
This patch relands https://github.com/llvm/llvm-project/pull/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:
"""
https://github.com/llvm/llvm-project/pull/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:
* https://github.com/llvm/llvm-project/pull/71780
>From d3e0391685605491383114f82d906c811f899b5d 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] Reland "[lldb][DWARFASTParserClang] Fetch constant value from
variable defintion if available"
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 63 ++++++++++++++++++-
.../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, 166 insertions(+), 10 deletions(-)
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
}
More information about the lldb-commits
mailing list