[Lldb-commits] [lldb] [lldb-dap] Check the process state before thread or frame validation. (PR #179202)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)

<details>
<summary>Changes</summary>

We are sending an invalid thread error on a valid thread because the process is not in a stopped state.
Send a `notStoppedError` so the client can retry at a later state after we have sent a stopped event.

This can happen when debugging a large binary and the user spams the step over button.

---
Full diff: https://github.com/llvm/llvm-project/pull/179202.diff


4 Files Affected:

- (modified) lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp (+3-3) 
- (modified) lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp (+5) 
- (modified) lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp (+3-3) 
- (modified) lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp (+4-4) 


``````````diff
diff --git a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
index 2b48350dfba1b..ecb086b36c817 100644
--- a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
@@ -27,13 +27,13 @@ namespace lldb_dap {
 /// adapter first sends the response and then a `stopped` event (with reason
 /// `step`) after the step has completed.
 Error NextRequestHandler::Run(const NextArguments &args) const {
+  if (!SBDebugger::StateIsStoppedState(dap.target.GetProcess().GetState()))
+    return make_error<NotStoppedError>();
+
   lldb::SBThread thread = dap.GetLLDBThread(args.threadId);
   if (!thread.IsValid())
     return make_error<DAPError>("invalid thread");
 
-  if (!SBDebugger::StateIsStoppedState(dap.target.GetProcess().GetState()))
-    return make_error<NotStoppedError>();
-
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   dap.focus_tid = thread.GetThreadID();
diff --git a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
index 7064d356a6479..9dc9b3735734d 100644
--- a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
@@ -14,6 +14,7 @@
 #include "ProtocolUtils.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/lldb-enumerations.h"
 
 using namespace lldb_dap;
 using namespace lldb_dap::protocol;
@@ -189,6 +190,10 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
 
 llvm::Expected<protocol::StackTraceResponseBody>
 StackTraceRequestHandler::Run(const protocol::StackTraceArguments &args) const {
+  const lldb::StateType process_state = dap.target.GetProcess().GetState();
+  if (!lldb::SBDebugger::StateIsStoppedState(process_state))
+    return llvm::make_error<NotStoppedError>();
+
   lldb::SBThread thread = dap.GetLLDBThread(args.threadId);
   if (!thread.IsValid())
     return llvm::make_error<DAPError>("invalid thread");
diff --git a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
index 6742c791a5486..8f11d10389b1c 100644
--- a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
@@ -32,6 +32,9 @@ namespace lldb_dap {
 // possible targets for a given source line can be retrieved via the
 // `stepInTargets` request.
 Error StepInRequestHandler::Run(const StepInArguments &args) const {
+  if (!SBDebugger::StateIsStoppedState(dap.target.GetProcess().GetState()))
+    return make_error<NotStoppedError>();
+
   SBThread thread = dap.GetLLDBThread(args.threadId);
   if (!thread.IsValid())
     return make_error<DAPError>("invalid thread");
@@ -40,9 +43,6 @@ Error StepInRequestHandler::Run(const StepInArguments &args) const {
   // "threadCausedFocus" boolean value in the "stopped" events.
   dap.focus_tid = thread.GetThreadID();
 
-  if (!SBDebugger::StateIsStoppedState(dap.target.GetProcess().GetState()))
-    return make_error<NotStoppedError>();
-
   lldb::SBError error;
   if (args.granularity == eSteppingGranularityInstruction) {
     thread.StepInstruction(/*step_over=*/false, error);
diff --git a/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
index e896e03720b6b..baf790a5f27f0 100644
--- a/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Support/Error.h"
 
 using namespace llvm;
+using namespace lldb;
 using namespace lldb_dap::protocol;
 
 namespace lldb_dap {
@@ -29,14 +30,13 @@ namespace lldb_dap {
 /// The debug adapter first sends the response and then a `stopped` event (with
 /// reason `step`) after the step has completed."
 Error StepOutRequestHandler::Run(const StepOutArguments &arguments) const {
+  if (!SBDebugger::StateIsStoppedState(dap.target.GetProcess().GetState()))
+    return make_error<NotStoppedError>();
+
   lldb::SBThread thread = dap.GetLLDBThread(arguments.threadId);
   if (!thread.IsValid())
     return make_error<DAPError>("invalid thread");
 
-  if (!lldb::SBDebugger::StateIsStoppedState(
-          dap.target.GetProcess().GetState()))
-    return make_error<NotStoppedError>();
-
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   dap.focus_tid = thread.GetThreadID();

``````````

</details>


https://github.com/llvm/llvm-project/pull/179202