[lldb][DWARFASTParserClang] Fetch constant value from variable defintion if available #71004
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThis is a draft patch that shows how LLDB could continue to support static initialisers even if the declaration didn't have a #70639 proposes moving the Previously the expression evaluator would find the constant for a VarDecl from its declaration 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. Full diff: https://github.com/llvm/llvm-project/pull/71004.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 182cc6764651747..39571dbcd30a57b 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,42 @@ static lldb::ModuleSP GetContainingClangModule(const DWARFDIE &die) {
return lldb::ModuleSP();
}
+static std::optional<DWARFFormValue>
+FindConstantOnVariableDefinition(DWARFDIE die) {
+ auto *dwarf = die.GetDWARF();
+ if (!dwarf)
+ return {};
+
+ ConstString name{die.GetName()};
+ if (!name)
+ return {};
+
+ // Make sure we populate the GetDieToVariable cache.
+ VariableList variables;
+ dwarf->FindGlobalVariables(name, {}, UINT_MAX, variables);
+
+ 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 +2943,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 a 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/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 069a2050f0eaadc..ba4532c48338b76 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -342,6 +342,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);
@@ -361,9 +366,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;
@@ -487,8 +489,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);
|
…tion if available
Michael137
force-pushed
the
inline-statics-support/lldb-find-constexpr-globals
branch
from
4f12660
to
a3d165a
Compare
Michael137
changed the title
|
@clayborg what are your thoughts on this? This could be a way to continue supporting static inline members in the expression evaluator without having the constant on the declaration DIE. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
adrian-prantl
approved these changes
Nov 2, 2023
| /// DW_AT_const_value of that definition if available. Returns std::nullopt | ||
| /// otherwise. | ||
| /// | ||
| /// In newer versions of clang, DW_AT_const_value's are not attached to the |
There was a problem hiding this comment.
DW_AT_const_value attributes
|
Looks like this caused an LLDB test failure on linux for And |
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Nov 6, 2023
…e defintion if available (llvm#71004)" This reverts commit ef3feba. This caused an LLDB test failure on Linux for `lang/cpp/symbols/TestSymbols.test_dwo`: ``` make: Leaving directory '/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/lang/cpp/symbols/TestSymbols.test_dwo' runCmd: expression -- D::i PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. HandleCommand(command = "expression -- D::i") 1. <user expression 0>:1:4: current parser token 'i' 2. <lldb wrapper prefix>:44:1: parsing function body '$__lldb_expr' 3. <lldb wrapper prefix>:44:1: in compound statement ('{}') Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 _lldb.cpython-39-x86_64-linux-gnu.so 0x00007fbcfcb08b87 1 _lldb.cpython-39-x86_64-linux-gnu.so 0x00007fbcfcb067ae 2 _lldb.cpython-39-x86_64-linux-gnu.so 0x00007fbcfcb0923f 3 libpthread.so.0 0x00007fbd07ab7140 ``` And a failure in `TestCallStdStringFunction.py` on Linux aarch64: ``` -- Exit Code: -11 Command Output (stdout): -- lldb version 18.0.0git (https://github.com/llvm/llvm-project.git revision ef3feba) clang revision ef3feba llvm revision ef3feba -- Command Output (stderr): -- PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. HandleCommand(command = "expression str") 1. <lldb wrapper prefix>:45:34: current parser token ';' 2. <lldb wrapper prefix>:44:1: parsing function body '$__lldb_expr' 3. <lldb wrapper prefix>:44:1: in compound statement ('{}') #0 0x0000ffffb72a149c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x58c749c) llvm#1 0x0000ffffb729f458 llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x58c5458) llvm#2 0x0000ffffb72a1bd0 SignalHandler(int) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x58c7bd0) llvm#3 0x0000ffffbdd9e7dc (linux-vdso.so.1+0x7dc) llvm#4 0x0000ffffb71799d8 lldb_private::plugin::dwarf::SymbolFileDWARF::FindGlobalVariables(lldb_private::ConstString, lldb_private::CompilerDeclContext const&, unsigned int, lldb_private::VariableList&) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x579f9d8) llvm#5 0x0000ffffb7197508 DWARFASTParserClang::FindConstantOnVariableDefinition(lldb_private::plugin::dwarf::DWARFDIE) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lib/python3.8/site-packages/lldb/_[lldb.cpython-38-aarch64-linux-gnu.so](http://lldb.cpython-38-aarch64-linux-gnu.so/)+0x57bd508) ```
Michael137
added a commit
that referenced
this pull request
Nov 7, 2023
…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
This was referenced Nov 8, 2023
Michael137
added a commit
that referenced
this pull request
Nov 13, 2023
…e defintion if available" (#71800) 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: * #71780
zahiraam
pushed a commit
to zahiraam/llvm-project
that referenced
this pull request
Nov 20, 2023
…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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#70639 proposes moving the
DW_AT_const_valueon 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 aDW_AT_const_valueanymore.Previously the expression evaluator would find the constant for a VarDecl from its declaration
DW_TAG_memberDIE. 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
TestConstStaticIntegralMember.pywould start failing