Skip to content

Commit

Permalink
Reland "[lldb][DWARFASTParserClang] Fetch constant value from variabl…
Browse files Browse the repository at this point in the history
…e defintion if available (#71004)"

 #70639 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.

 **Testing**

 * If #70639 were to land
 without this patch then most of the `TestConstStaticIntegralMember.py`
 would start failing
  • Loading branch information
Michael137 committed Nov 7, 2023
1 parent 7c3707a commit 470de2b
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 6 deletions.
63 changes: 62 additions & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 11 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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);

Expand Down
7 changes: 7 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
5 changes: 5 additions & 0 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,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")
20 changes: 20 additions & 0 deletions lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
}

0 comments on commit 470de2b

Please sign in to comment.