llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) <details> <summary>Changes</summary> A scripted frame's variable exists as two objects: a Variable holding the scope the frame assigned it, and a `ValueObject` holding the value the frame built. `SBValue::GetValueType()` reports the `ValueObject`'s, and `GetValueObjectForFrameVariable` handed the value back unwrapped, so the scope never reached a client. A value built in Python is a `ValueObjectConstResult`, which describes how it was produced rather than which variable it stands for, leaving anything that groups a frame's variables by scope unable to place it. `frame variable` was unaffected because it reads the `Variables` directly. Pair the two in a small delegating `ValueObject` that takes its value type from the variable and everything else from the value. `ValueObjectVariable` already does this for a variable read from debug info, but cannot be reused: it recovers the value from a DWARF location expression, and a frame that builds its own values supplies none. Wrapping rather than annotating keeps the correction out of the value, so a value a frame passes through from elsewhere is reported as that frame presents it without changing what the same value reports anywhere else. It is also independent of which kind of `ValueObject` the value is, unlike a value type override on `ValueObjectConstResult`, which would stop working as soon as a frame can supply a value that is not a const result. Reaching a variable by name goes through `Variable::NameMatches`, which dereferences `m_owner_scope` for every entry whose name does not match. A null owner scope is a legal state, `CalculateSymbolContext` handles it explicitly and a frame that makes up its own variables has none, so drop that lookup. The `SymbolContext` it filled in has had no reader since 22b044877d23, which dropped the language argument to `Mangled::NameMatches` along with the only use of it. Also document that the synthetic flag in `GetValueTypeForVariable`'s return is what exempts a variable from the scope rules a declared variable is subject to. A value with no storage behind it is filtered out of an in-scope-only listing without it. --- Full diff: https://github.com/llvm/llvm-project/pull/221708.diff 4 Files Affected: - (modified) lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameInterface.h (+11) - (modified) lldb/source/Plugins/Process/scripted/ScriptedFrame.cpp (+103-2) - (modified) lldb/source/Symbol/Variable.cpp (-3) - (modified) lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py (+17) ``````````diff diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameInterface.h index b2a37bf497504..86be7649cfe4b 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameInterface.h @@ -52,6 +52,17 @@ class ScriptedFrameInterface : virtual public ScriptedInterface { virtual lldb::ValueObjectListSP GetVariables() { return nullptr; } + /// Choose which kind of variable \a value is presented as, for instance + /// \a eValueTypeVariableLocal to have it listed among the frame's locals. + /// + /// Include \a eValueTypeSyntheticFlag for a value the implementation built + /// itself. That flag is what exempts a variable from the scope rules a + /// declared variable is subject to, so a value with no storage behind it will + /// be filtered out of an in-scope-only listing without it. Leave it off to + /// present a variable that really does exist in the frame. + /// + /// Returning std::nullopt keeps the kind the value already reports, with the + /// synthetic flag added. virtual std::optional<lldb::ValueType> GetValueTypeForVariable(lldb::ValueObjectSP value) { return std::nullopt; diff --git a/lldb/source/Plugins/Process/scripted/ScriptedFrame.cpp b/lldb/source/Plugins/Process/scripted/ScriptedFrame.cpp index cac9f0e44bb73..2ff674e98ea2d 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedFrame.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedFrame.cpp @@ -42,6 +42,90 @@ using namespace lldb; using namespace lldb_private; +namespace { + +/// Presents a value as one of a frame's variables. +/// +/// A value describes how it was produced, not which of a frame's variables it +/// stands for; only the variable records that. A frame that builds its own +/// values pairs the two here, taking the value type from the variable and +/// everything else from the value. +/// +/// ValueObjectVariable does the same for a variable read from debug info, but +/// requires the value to be recoverable from a DWARF location expression. +class ValueObjectProvidedVariable : public ValueObject { +public: + static lldb::ValueObjectSP Create(ValueObject &parent, + const lldb::VariableSP &variable_sp) { + return (new ValueObjectProvidedVariable(parent, variable_sp))->GetSP(); + } + + ~ValueObjectProvidedVariable() override = default; + + lldb::ValueType GetValueType() const override { + return m_variable_sp->GetScope(); + } + + llvm::Expected<uint64_t> GetByteSize() override { + return m_parent->GetByteSize(); + } + + llvm::Expected<uint32_t> CalculateNumChildren(uint32_t max) override { + return m_parent->GetNumChildren(max); + } + + bool IsInScope() override { return m_parent->IsInScope(); } + + // This wrapper is transparent: consumers that walk parents, such as + // expression path construction, must not see an extra level. + ValueObject *GetParent() override { + return m_parent ? m_parent->GetParent() : nullptr; + } + + const ValueObject *GetParent() const override { + return m_parent ? m_parent->GetParent() : nullptr; + } + +protected: + ValueObjectProvidedVariable(ValueObject &parent, + const lldb::VariableSP &variable_sp) + : ValueObject(parent), m_variable_sp(variable_sp) { + SetName(parent.GetName()); + } + + bool UpdateValue() override { + SetValueIsValid(false); + m_error.Clear(); + + if (!m_parent->UpdateValueIfNeeded(false)) { + if (m_error.Success() && m_parent->GetError().Fail()) + m_error = m_parent->GetError().Clone(); + return false; + } + + m_update_point.SetUpdated(); + m_value = m_parent->GetValue(); + SetAddressTypeOfChildren(m_parent->GetAddressTypeOfChildren()); + ExecutionContext exe_ctx(GetExecutionContextRef()); + m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get()); + SetValueDidChange(m_parent->GetValueDidChange()); + return true; + } + + CompilerType GetCompilerTypeImpl() override { + return m_parent->GetCompilerType(); + } + + lldb::VariableSP m_variable_sp; + +private: + ValueObjectProvidedVariable(const ValueObjectProvidedVariable &) = delete; + const ValueObjectProvidedVariable & + operator=(const ValueObjectProvidedVariable &) = delete; +}; + +} // namespace + char ScriptedFrame::ID; void ScriptedFrame::CheckInterpreterAndScriptObject() const { @@ -305,8 +389,12 @@ lldb::ValueObjectSP ScriptedFrame::GetValueObjectForFrameVariable( if (!values) return {}; - return values->FindValueObjectByValueName( + lldb::ValueObjectSP valobj_sp = values->FindValueObjectByValueName( variable_sp->GetName().AsCString(nullptr)); + if (!valobj_sp) + return {}; + + return ValueObjectProvidedVariable::Create(*valobj_sp, variable_sp); } lldb::ValueObjectSP ScriptedFrame::FindVariable(ConstString name) { @@ -315,7 +403,20 @@ lldb::ValueObjectSP ScriptedFrame::FindVariable(ConstString name) { if (!values) return {}; - return values->FindValueObjectByValueName(name.AsCString(nullptr)); + lldb::ValueObjectSP valobj_sp = + values->FindValueObjectByValueName(name.AsCString(nullptr)); + if (!valobj_sp) + return {}; + + // Classify the value the same way the frame's variable list would, so a value + // reached by name is not reported differently from the same value reached by + // enumeration. + if (m_variable_list_sp) { + if (VariableSP variable_sp = m_variable_list_sp->FindVariable(name)) + return ValueObjectProvidedVariable::Create(*valobj_sp, variable_sp); + } + + return valobj_sp; } lldb::ValueObjectSP ScriptedFrame::GetValueForVariableExpressionPath( diff --git a/lldb/source/Symbol/Variable.cpp b/lldb/source/Symbol/Variable.cpp index 3fc5c1cc38408..8ada6f6f4d2c7 100644 --- a/lldb/source/Symbol/Variable.cpp +++ b/lldb/source/Symbol/Variable.cpp @@ -92,9 +92,6 @@ ConstString Variable::GetUnqualifiedName() const { return m_name; } bool Variable::NameMatches(ConstString name) const { if (m_name == name) return true; - SymbolContext variable_sc; - m_owner_scope->CalculateSymbolContext(&variable_sc); - return m_mangled.NameMatches(name); } bool Variable::NameMatches(const RegularExpression ®ex) const { diff --git a/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py b/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py index eb327d2a34c90..6e9ecd925d759 100644 --- a/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py +++ b/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py @@ -866,6 +866,23 @@ def test_get_values(self): self.assertEqual(variables.GetSize(), 1) self.assertEqual(variables.GetValueAtIndex(0).name, "_handler_one") + # A variable reports the value type the frame classified it as, not the + # one its underlying value has. `_handler_one` is built from an + # expression, so these differ. + self.assertEqual( + variables.GetValueAtIndex(0).GetValueType(), + lldb.eValueTypeVariableLocal | lldb.eValueTypeSyntheticFlag, + ) + # Classifying a variable must not cost its value or its type. + self.assertEqual(variables.GetValueAtIndex(0).GetValueAsUnsigned(), 1) + self.assertEqual(variables.GetValueAtIndex(0).GetTypeName(), "uint32_t") + # Reaching the same value by name has to agree with the enumeration. + self.assertEqual( + frame0.FindVariable("_handler_one").GetValueType(), + lldb.eValueTypeVariableLocal | lldb.eValueTypeSyntheticFlag, + ) + self.assertEqual(frame0.FindVariable("_handler_one").GetValueAsUnsigned(), 1) + # Check the `frame variable` command(s) handle synthetic variables the # way we expect by printing them. self.expect("frame var", substrs=["variable_in_main", "_handler_one"]) `````````` </details> https://github.com/llvm/llvm-project/pull/221708 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
