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 <michaelbuc...@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 <michaelbuc...@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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits