https://github.com/da-viper updated https://github.com/llvm/llvm-project/pull/179667
>From 98f7eacbbdd31940198b5fd59e7f123d6f667915 Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Wed, 4 Feb 2026 00:56:08 +0000 Subject: [PATCH 1/2] [lldb-dap] Allow evaluate requests without a frame context EvaluateRequests handler now uses the target's context if no valid frameId is provided, enabling evaluation of global variables without requiring a valid stack frame. In repl mode it now uses the last `sucessful` variable or command expression if the provided user's expression is empty. --- .../test/tools/lldb-dap/dap_server.py | 16 ++- .../lldb-dap/evaluate/TestDAP_evaluate.py | 37 +++++- lldb/tools/lldb-dap/DAP.h | 7 +- .../Handler/EvaluateRequestHandler.cpp | 109 ++++++++++++------ 4 files changed, 118 insertions(+), 51 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 14391db74302c..6feed2dff8843 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -1137,18 +1137,24 @@ def request_writeMemory(self, memoryReference, data, offset=0, allowPartial=Fals def request_evaluate( self, expression, - frameIndex=0, + frameIndex: Optional[int] = 0, threadId=None, context=None, is_hex: Optional[bool] = None, ) -> Response: - stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId) - if stackFrame is None: - raise ValueError("invalid frameIndex") args_dict = { "expression": expression, - "frameId": stackFrame["id"], } + + if frameIndex is not None: + if threadId is None: + threadId = self.get_thread_id() + stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId) + + if stackFrame is None: + raise ValueError("invalid frameIndex") + args_dict["frameId"] = stackFrame["id"] + if context: args_dict["context"] = context if is_hex is not None: diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index bc08462cfcba9..f09da191e4cda 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -27,13 +27,15 @@ def assertEvaluate( want_varref=False, want_memref=True, want_locref=False, + frame_index: Optional[int] = 0, is_hex=None, ): resp = self.dap_server.request_evaluate( - expression, context=self.context, is_hex=is_hex + expression, context=self.context, is_hex=is_hex, frameIndex=frame_index ) self.assertTrue( - resp["success"], f"Failed to evaluate expression {expression!r}" + resp["success"], + f"Failed to evaluate expression {expression!r} in frame {frame_index}", ) body: EvaluateResponseBody = resp["body"] self.assertRegex( @@ -73,9 +75,14 @@ def assertEvaluate( ) def assertEvaluateFailure(self, expression): + response = self.dap_server.request_evaluate(expression, context=self.context) + self.assertFalse( + response["success"], + f"Expression:'{expression}' should fail in {self.context} context \n got {response["body"]!r}", + ) self.assertNotIn( "result", - self.dap_server.request_evaluate(expression, context=self.context)["body"], + response["body"], ) def isResultExpandedDescription(self): @@ -203,10 +210,15 @@ def run_test_evaluate_expressions( "struct3", "0x.*0", want_varref=True, want_type="my_struct *" ) - if context == "repl": - # In the repl context expressions may be interpreted as lldb + if context == "repl" or context is None: + # In repl or unknown context expressions may be interpreted as lldb # commands since no variables have the same name as the command. self.assertEvaluate("list", r".*", want_memref=False) + # Changing the frame index should not make a difference + self.assertEvaluate( + "version", r".*lldb.+", want_memref=False, frame_index=1 + ) + else: self.assertEvaluateFailure("list") # local variable of a_function @@ -302,6 +314,21 @@ def run_test_evaluate_expressions( self.assertEvaluate("list", "42") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") + # variable from a different frame + self.assertEvaluate("var1", "20", frame_index=1) + + if self.isExpressionParsedExpected(): + # access global variable without a frame + # Run in variable mode to avoid interpreting it as a command + res = self.dap_server.request_evaluate( + "`lldb-dap repl-mode variable", context="repl" + ) + self.assertTrue(res["success"]) + self.assertEvaluate("static_int", "42", frame_index=None, want_memref=False) + res = self.dap_server.request_evaluate( + "`lldb-dap repl-mode auto", context="repl" + ) + self.assertTrue(res["success"]) self.assertEvaluateFailure("var1") self.assertEvaluateFailure("var2") diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 8d56a226dbb28..657105c8dd6b9 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -150,10 +150,9 @@ struct DAP final : public DAPTransport::MessageHandler { /// This is used to allow request_evaluate to handle empty expressions /// (ie the user pressed 'return' and expects the previous expression to - /// repeat). If the previous expression was a command, this string will be - /// empty; if the previous expression was a variable expression, this string - /// will contain that expression. - std::string last_nonempty_var_expression; + /// repeat). If the previous expression was a command, it will be empty. + /// Else it will contain the last valid variable expression. + std::string last_valid_variable_expression; /// The set of features supported by the connected client. llvm::DenseSet<ClientFeature> clientFeatures; diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp index ec26bb66e8aec..13c7db5957ddd 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "DAP.h" +#include "DAPError.h" #include "EventHelper.h" #include "JSONUtils.h" #include "LLDBUtils.h" @@ -23,24 +24,69 @@ using namespace lldb_dap::protocol; namespace lldb_dap { +static bool RunExpressionAsLLDBCommand(DAP &dap, lldb::SBFrame &frame, + std::string &expression, + EvaluateContext context) { + if (context != eEvaluateContextRepl && context != eEvaluateContextUnknown) + return false; + + // Since we don't know this context do not try to repeat the last command; + if (context == eEvaluateContextUnknown && expression.empty()) + return false; + + const bool repeat_last_command = + expression.empty() && dap.last_valid_variable_expression.empty(); + if (repeat_last_command) + return true; + + const ReplMode repl_mode = dap.DetectReplMode(frame, expression, false); + return repl_mode == ReplMode::Command; +} + +static lldb::SBValue EvaluateVariableExpression(lldb::SBTarget &target, + lldb::SBFrame &frame, + llvm::StringRef expression, + bool run_as_expression) { + const char *expression_cstr = expression.data(); + + lldb::SBValue value; + if (frame) { + // Check if it is a variable or an expression path for a variable. i.e. + // 'foo->bar' finds the 'bar' variable. It is more reliable than the + // expression parser in many cases and it is faster. + value = frame.GetValueForVariablePath(expression_cstr, + lldb::eDynamicDontRunTarget); + if (value || !run_as_expression) + return value; + + return frame.EvaluateExpression(expression_cstr); + } + + if (run_as_expression) + value = target.EvaluateExpression(expression_cstr); + + return value; +} + /// Evaluates the given expression in the context of a stack frame. /// /// The expression has access to any variables and arguments that are in scope. Expected<EvaluateResponseBody> EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const { + const lldb::StateType process_state = dap.target.GetProcess().GetState(); + if (!lldb::SBDebugger::StateIsStoppedState(process_state)) + return llvm::make_error<NotStoppedError>(); + EvaluateResponseBody body; lldb::SBFrame frame = dap.GetLLDBFrame(arguments.frameId); std::string expression = arguments.expression; - bool repeat_last_command = - expression.empty() && dap.last_nonempty_var_expression.empty(); + const EvaluateContext evaluate_context = arguments.context; + const bool is_repl_context = evaluate_context == eEvaluateContextRepl; - if (arguments.context == protocol::eEvaluateContextRepl && - (repeat_last_command || - (!expression.empty() && - dap.DetectReplMode(frame, expression, false) == ReplMode::Command))) { + if (RunExpressionAsLLDBCommand(dap, frame, expression, evaluate_context)) { // Since the current expression is not for a variable, clear the - // last_nonempty_var_expression field. - dap.last_nonempty_var_expression.clear(); + // last_valid_variable_expression field. + dap.last_valid_variable_expression.clear(); // If we're evaluating a command relative to the current frame, set the // focus_tid to the current frame for any thread related events. if (frame.IsValid()) { @@ -54,47 +100,36 @@ EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const { return body; } - if (arguments.context == eEvaluateContextRepl) { - // If the expression is empty and the last expression was for a - // variable, set the expression to the previous expression (repeat the - // evaluation); otherwise save the current non-empty expression for the - // next (possibly empty) variable expression. - if (expression.empty()) - expression = dap.last_nonempty_var_expression; - else - dap.last_nonempty_var_expression = expression; - } + // If the user expression is empty, evaluate the last valid variable + // expression. + if (expression.empty() && is_repl_context) + expression = dap.last_valid_variable_expression; - // Always try to get the answer from the local variables if possible. If - // this fails, then if the context is not "hover", actually evaluate an - // expression using the expression parser. - // - // "frame variable" is more reliable than the expression parser in - // many cases and it is faster. - lldb::SBValue value = frame.GetValueForVariablePath( - expression.data(), lldb::eDynamicDontRunTarget); - - // Freeze dry the value in case users expand it later in the debug console - if (value.GetError().Success() && arguments.context == eEvaluateContextRepl) - value = value.Persist(); - - if (value.GetError().Fail() && arguments.context != eEvaluateContextHover) - value = frame.EvaluateExpression(expression.data()); + const bool run_as_expression = evaluate_context != eEvaluateContextHover; + lldb::SBValue value = EvaluateVariableExpression( + dap.target, frame, expression, run_as_expression); if (value.GetError().Fail()) return ToError(value.GetError(), /*show_user=*/false); - const bool hex = arguments.format ? arguments.format->hex : false; + if (is_repl_context) { + // save the new variable expression + dap.last_valid_variable_expression = expression; + // Freeze dry the value in case users expand it later in the debug console + value = value.Persist(); + } + + const bool hex = arguments.format ? arguments.format->hex : false; VariableDescription desc(value, dap.configuration.enableAutoVariableSummaries, hex); - body.result = desc.GetResult(arguments.context); + body.result = desc.GetResult(evaluate_context); body.type = desc.display_type_name; if (value.MightHaveChildren() || ValuePointsToCode(value)) - body.variablesReference = dap.variables.InsertVariable( - value, /*is_permanent=*/arguments.context == eEvaluateContextRepl); + body.variablesReference = + dap.variables.InsertVariable(value, /*is_permanent=*/is_repl_context); if (lldb::addr_t addr = value.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS) body.memoryReference = EncodeMemoryReference(addr); >From 94a993a7d5765a3ae3f076fc76369e12ad2b7336 Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Wed, 4 Feb 2026 15:22:50 +0000 Subject: [PATCH 2/2] add review changes --- .../lldb-dap/Handler/EvaluateRequestHandler.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp index 13c7db5957ddd..d6a948feefd1f 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp @@ -73,9 +73,6 @@ static lldb::SBValue EvaluateVariableExpression(lldb::SBTarget &target, /// The expression has access to any variables and arguments that are in scope. Expected<EvaluateResponseBody> EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const { - const lldb::StateType process_state = dap.target.GetProcess().GetState(); - if (!lldb::SBDebugger::StateIsStoppedState(process_state)) - return llvm::make_error<NotStoppedError>(); EvaluateResponseBody body; lldb::SBFrame frame = dap.GetLLDBFrame(arguments.frameId); @@ -100,6 +97,14 @@ EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const { return body; } + const lldb::StateType process_state = dap.target.GetProcess().GetState(); + if (!lldb::SBDebugger::StateIsStoppedState(process_state)) + return llvm::make_error<DAPError>( + "Cannot evaluate expressions while the process is running. Pause " + "the process and try again.", + /**error_code=*/llvm::inconvertibleErrorCode(), + /**show_user=*/false); + // If the user expression is empty, evaluate the last valid variable // expression. if (expression.empty() && is_repl_context) @@ -114,7 +119,7 @@ EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const { if (is_repl_context) { // save the new variable expression - dap.last_valid_variable_expression = expression; + dap.last_valid_variable_expression = std::move(expression); // Freeze dry the value in case users expand it later in the debug console value = value.Persist(); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
