From f43ee18e988d923ef5529f1df3910bd2af09e825 Mon Sep 17 00:00:00 2001 From: cmtice Date: Tue, 31 Mar 2026 14:13:48 -0700 Subject: [PATCH] [LLDB] Add allow_var_updates to DIL and CanUpdateVar to SetValueFromInteger (#186421) In preparation for updating DIL to handle assignments, this adds a member variable to the DIL Interpreter indicating whether or not updating program variables is allowed. For invocations from the LLDB command prompt (through "frame variable") we want to allow it, but from other places we might not. Therefore we also add new StackFrame ExpressionPathOption, eExpressionPathOptionsAllowVarUpdates, which we add to calls from CommandObjectFrame, and which is checked in GetValueForVariableExpressionPath. Finally, we also add a parameter, can_update_vars, with a default value of true, to ValueObject::SetValueFromInteger, as that will be the main function used to by assignment in DIL. --- lldb/include/lldb/Target/StackFrame.h | 3 +- lldb/include/lldb/ValueObject/DILEval.h | 5 ++-- lldb/include/lldb/ValueObject/DILParser.h | 3 +- lldb/include/lldb/ValueObject/ValueObject.h | 18 +++++------ lldb/source/Commands/CommandObjectFrame.cpp | 3 +- lldb/source/Target/StackFrame.cpp | 16 +++------- lldb/source/ValueObject/DILEval.cpp | 20 ++++++++++--- lldb/source/ValueObject/DILParser.cpp | 23 +++++++++----- lldb/source/ValueObject/ValueObject.cpp | 33 +++++++++++---------- 9 files changed, 71 insertions(+), 53 deletions(-) diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h index fe42be358743..66a7cec5f66b 100644 --- a/lldb/include/lldb/Target/StackFrame.h +++ b/lldb/include/lldb/Target/StackFrame.h @@ -56,7 +56,8 @@ public: eExpressionPathOptionsNoSyntheticChildren = (1u << 2), eExpressionPathOptionsNoSyntheticArrayRange = (1u << 3), eExpressionPathOptionsAllowDirectIVarAccess = (1u << 4), - eExpressionPathOptionsInspectAnonymousUnions = (1u << 5) + eExpressionPathOptionsInspectAnonymousUnions = (1u << 5), + eExpressionPathOptionsAllowVarUpdates = (1u << 6) }; enum class Kind { diff --git a/lldb/include/lldb/ValueObject/DILEval.h b/lldb/include/lldb/ValueObject/DILEval.h index da4479036c17..323ac5eace2b 100644 --- a/lldb/include/lldb/ValueObject/DILEval.h +++ b/lldb/include/lldb/ValueObject/DILEval.h @@ -40,8 +40,7 @@ class Interpreter : Visitor { public: Interpreter(lldb::TargetSP target, llvm::StringRef expr, std::shared_ptr frame_sp, - lldb::DynamicValueType use_dynamic, bool use_synthetic, - bool fragile_ivar, bool check_ptr_vs_member); + lldb::DynamicValueType use_dynamic, uint32_t options); /// Evaluate an ASTNode. /// \returns A non-null lldb::ValueObjectSP or an Error. @@ -137,6 +136,8 @@ private: bool m_use_synthetic; bool m_fragile_ivar; bool m_check_ptr_vs_member; + // TODO: Remove 'maybe_unused' when next PR, using this, gets submitted. + [[maybe_unused]] bool m_allow_var_updates; }; } // namespace lldb_private::dil diff --git a/lldb/include/lldb/ValueObject/DILParser.h b/lldb/include/lldb/ValueObject/DILParser.h index d093f95cde84..4afedb735af8 100644 --- a/lldb/include/lldb/ValueObject/DILParser.h +++ b/lldb/include/lldb/ValueObject/DILParser.h @@ -71,8 +71,7 @@ public: DILLexer lexer, std::shared_ptr frame_sp, lldb::DynamicValueType use_dynamic, - bool use_synthetic, bool fragile_ivar, - bool check_ptr_vs_member); + uint32_t options); ~DILParser() = default; diff --git a/lldb/include/lldb/ValueObject/ValueObject.h b/lldb/include/lldb/ValueObject/ValueObject.h index 69b9b0a65000..cf69a713f136 100644 --- a/lldb/include/lldb/ValueObject/ValueObject.h +++ b/lldb/include/lldb/ValueObject/ValueObject.h @@ -453,17 +453,17 @@ public: /// value to a boolean and return that. Otherwise return an error. llvm::Expected GetValueAsBool(); - /// Update an existing integer ValueObject with a new integer value. This - /// is only intended to be used with 'temporary' ValueObjects, i.e. ones that - /// are not associated with program variables. It does not update program - /// memory, registers, stack, etc. - void SetValueFromInteger(const llvm::APInt &value, Status &error); + /// Update an existing integer ValueObject with a new integer value. If + /// can_update_var is true, will allow updating objects associated with + /// program variables; otherwise not. + void SetValueFromInteger(const llvm::APInt &value, Status &error, + bool can_update_var = true); /// Update an existing integer ValueObject with an integer value created - /// frome 'new_val_sp'. This is only intended to be used with 'temporary' - /// ValueObjects, i.e. ones that are not associated with program variables. - /// It does not update program memory, registers, stack, etc. - void SetValueFromInteger(lldb::ValueObjectSP new_val_sp, Status &error); + /// frome 'new_val_sp'. If can_update_var is true, will allow updating objects + /// associated with program variables; otherwise not. + void SetValueFromInteger(lldb::ValueObjectSP new_val_sp, Status &error, + bool can_update_var = true); virtual bool SetValueFromCString(const char *value_str, Status &error); diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 9133359fbf53..1488e3bfe589 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -610,7 +610,8 @@ protected: uint32_t expr_path_options = StackFrame::eExpressionPathOptionCheckPtrVsMember | StackFrame::eExpressionPathOptionsAllowDirectIVarAccess | - StackFrame::eExpressionPathOptionsInspectAnonymousUnions; + StackFrame::eExpressionPathOptionsInspectAnonymousUnions | + StackFrame::eExpressionPathOptionsAllowVarUpdates; lldb::VariableSP var_sp; valobj_sp = frame->GetValueForVariableExpressionPath( entry.ref(), m_varobj_options.use_dynamic, expr_path_options, diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index e76b4ad6735a..89435e20cc49 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -541,13 +541,6 @@ ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath( uint32_t options, lldb::VariableSP &var_sp, Status &error, lldb::DILMode mode) { - const bool check_ptr_vs_member = - (options & eExpressionPathOptionCheckPtrVsMember) != 0; - const bool no_fragile_ivar = - (options & eExpressionPathOptionsNoFragileObjcIvar) != 0; - const bool no_synth_child = - (options & eExpressionPathOptionsNoSyntheticChildren) != 0; - // Lex the expression. auto lex_or_err = dil::DILLexer::Create(var_expr, mode); if (!lex_or_err) { @@ -556,9 +549,9 @@ ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath( } // Parse the expression. - auto tree_or_error = dil::DILParser::Parse( - var_expr, std::move(*lex_or_err), shared_from_this(), use_dynamic, - !no_synth_child, !no_fragile_ivar, check_ptr_vs_member); + auto tree_or_error = + dil::DILParser::Parse(var_expr, std::move(*lex_or_err), + shared_from_this(), use_dynamic, options); if (!tree_or_error) { error = Status::FromError(tree_or_error.takeError()); return ValueObjectConstResult::Create(nullptr, std::move(error)); @@ -567,8 +560,7 @@ ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath( // Evaluate the parsed expression. lldb::TargetSP target = this->CalculateTarget(); dil::Interpreter interpreter(target, var_expr, shared_from_this(), - use_dynamic, !no_synth_child, !no_fragile_ivar, - check_ptr_vs_member); + use_dynamic, options); auto valobj_or_error = interpreter.Evaluate(**tree_or_error); if (!valobj_or_error) { diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp index e1736e1dc008..80b31be8e7c2 100644 --- a/lldb/source/ValueObject/DILEval.cpp +++ b/lldb/source/ValueObject/DILEval.cpp @@ -388,11 +388,23 @@ lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref, Interpreter::Interpreter(lldb::TargetSP target, llvm::StringRef expr, std::shared_ptr frame_sp, - lldb::DynamicValueType use_dynamic, bool use_synthetic, - bool fragile_ivar, bool check_ptr_vs_member) + lldb::DynamicValueType use_dynamic, uint32_t options) : m_target(std::move(target)), m_expr(expr), m_exe_ctx_scope(frame_sp), - m_use_dynamic(use_dynamic), m_use_synthetic(use_synthetic), - m_fragile_ivar(fragile_ivar), m_check_ptr_vs_member(check_ptr_vs_member) { + m_use_dynamic(use_dynamic) { + + const bool check_ptr_vs_member = + (options & StackFrame::eExpressionPathOptionCheckPtrVsMember) != 0; + const bool no_fragile_ivar = + (options & StackFrame::eExpressionPathOptionsNoFragileObjcIvar) != 0; + const bool no_synth_child = + (options & StackFrame::eExpressionPathOptionsNoSyntheticChildren) != 0; + const bool allow_var_updates = + (options & StackFrame::eExpressionPathOptionsAllowVarUpdates) != 0; + + m_use_synthetic = !no_synth_child; + m_fragile_ivar = !no_fragile_ivar; + m_check_ptr_vs_member = check_ptr_vs_member; + m_allow_var_updates = allow_var_updates; } llvm::Expected Interpreter::Evaluate(const ASTNode &node) { diff --git a/lldb/source/ValueObject/DILParser.cpp b/lldb/source/ValueObject/DILParser.cpp index bb5a86bc7168..919acd4645f7 100644 --- a/lldb/source/ValueObject/DILParser.cpp +++ b/lldb/source/ValueObject/DILParser.cpp @@ -86,14 +86,23 @@ CompilerType ResolveTypeByName(const std::string &name, return {}; } -llvm::Expected -DILParser::Parse(llvm::StringRef dil_input_expr, DILLexer lexer, - std::shared_ptr frame_sp, - lldb::DynamicValueType use_dynamic, bool use_synthetic, - bool fragile_ivar, bool check_ptr_vs_member) { +llvm::Expected DILParser::Parse(llvm::StringRef dil_input_expr, + DILLexer lexer, + std::shared_ptr frame_sp, + + lldb::DynamicValueType use_dynamic, + uint32_t options) { + const bool check_ptr_vs_member = + (options & StackFrame::eExpressionPathOptionCheckPtrVsMember) != 0; + const bool no_fragile_ivar = + (options & StackFrame::eExpressionPathOptionsNoFragileObjcIvar) != 0; + const bool no_synth_child = + (options & StackFrame::eExpressionPathOptionsNoSyntheticChildren) != 0; + llvm::Error error = llvm::Error::success(); - DILParser parser(dil_input_expr, lexer, frame_sp, use_dynamic, use_synthetic, - fragile_ivar, check_ptr_vs_member, error); + DILParser parser(dil_input_expr, lexer, frame_sp, use_dynamic, + !no_synth_child, !no_fragile_ivar, check_ptr_vs_member, + error); ASTNodeUP node_up = parser.Run(); assert(node_up && "ASTNodeUP must not contain a nullptr"); diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp index 5b343848dda6..edad5aa4d490 100644 --- a/lldb/source/ValueObject/ValueObject.cpp +++ b/lldb/source/ValueObject/ValueObject.cpp @@ -1203,22 +1203,23 @@ llvm::Expected ValueObject::GetValueAsBool() { return llvm::createStringError("type cannot be converted to bool"); } -void ValueObject::SetValueFromInteger(const llvm::APInt &value, Status &error) { +void ValueObject::SetValueFromInteger(const llvm::APInt &value, Status &error, + bool can_update_var) { // Verify the current object is an integer object CompilerType val_type = GetCompilerType(); if (!val_type.IsInteger() && !val_type.IsUnscopedEnumerationType() && !HasFloatingRepresentation(val_type) && !val_type.IsPointerType() && !val_type.IsScalarType()) { error = - Status::FromErrorString("current value object is not an integer objet"); + Status::FromErrorString("current value object is not an scalar object"); return; } - // Verify the current object is not actually associated with any program - // variable. - if (GetVariable()) { + // Verify, if current object is associated with a program variable, that + // we are allowing updating program variables in this case. + if (GetVariable() && !can_update_var) { error = Status::FromErrorString( - "current value object is not a temporary object"); + "Not allowed to update program variables in this case."); return; } @@ -1242,22 +1243,22 @@ void ValueObject::SetValueFromInteger(const llvm::APInt &value, Status &error) { } void ValueObject::SetValueFromInteger(lldb::ValueObjectSP new_val_sp, - Status &error) { + Status &error, bool can_update_var) { // Verify the current object is an integer object CompilerType val_type = GetCompilerType(); if (!val_type.IsInteger() && !val_type.IsUnscopedEnumerationType() && !HasFloatingRepresentation(val_type) && !val_type.IsPointerType() && !val_type.IsScalarType()) { error = - Status::FromErrorString("current value object is not an integer objet"); + Status::FromErrorString("current value object is not an scalar object"); return; } - // Verify the current object is not actually associated with any program - // variable. - if (GetVariable()) { + // Verify, if current object is associated with a program variable, that + // we are allowing updating program variables in this case. + if (GetVariable() && !can_update_var) { error = Status::FromErrorString( - "current value object is not a temporary object"); + "Not allowed to update program variables in this case."); return; } @@ -1273,13 +1274,14 @@ void ValueObject::SetValueFromInteger(lldb::ValueObjectSP new_val_sp, if (new_val_type.IsInteger()) { auto value_or_err = new_val_sp->GetValueAsAPSInt(); if (value_or_err) - SetValueFromInteger(*value_or_err, error); + SetValueFromInteger(*value_or_err, error, can_update_var); else error = Status::FromErrorString("error getting APSInt from new_val_sp"); } else if (HasFloatingRepresentation(new_val_type)) { auto value_or_err = new_val_sp->GetValueAsAPFloat(); if (value_or_err) - SetValueFromInteger(value_or_err->bitcastToAPInt(), error); + SetValueFromInteger(value_or_err->bitcastToAPInt(), error, + can_update_var); else error = Status::FromErrorString("error getting APFloat from new_val_sp"); } else if (new_val_type.IsPointerType()) { @@ -1291,7 +1293,8 @@ void ValueObject::SetValueFromInteger(lldb::ValueObjectSP new_val_sp, if (auto temp = llvm::expectedToOptional( new_val_sp->GetCompilerType().GetBitSize(target.get()))) num_bits = temp.value(); - SetValueFromInteger(llvm::APInt(num_bits, int_val), error); + SetValueFromInteger(llvm::APInt(num_bits, int_val), error, + can_update_var); } else error = Status::FromErrorString("error converting new_val_sp to integer"); }