[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 11 17:31:14 PDT 2024
https://github.com/jeffreytan81 created https://github.com/llvm/llvm-project/pull/98568
Some targets have non-trivial amount of threads, when stopping, multiple stopped events can be sent causing VSCode to show multiple yellow lines in code editor which can be confusing to end users.
This PR introduces a new single stopped event mode in lldb-dap which ensures only one stopped event is sent to VSCode UI. The implementation is preferring the stop reason from original thread with `focus_tid`.
Note: if `singleStoppedEvent` option is not specified, it functions the same as before.
New tests are added.
>From 13af0ff31688ca0a23f1fec65ca2d5797b65e31f Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Thu, 11 Jul 2024 17:24:41 -0700
Subject: [PATCH] Support single stopped event in lldb-dap
---
.../test/tools/lldb-dap/dap_server.py | 21 ++++-
.../test/tools/lldb-dap/lldbdap_testcase.py | 5 +-
.../API/tools/lldb-dap/stop-events/Makefile | 4 +
.../stop-events/TestDAP_stopEvents.py | 92 +++++++++++++++++++
.../API/tools/lldb-dap/stop-events/main.cpp | 74 +++++++++++++++
lldb/tools/lldb-dap/DAP.cpp | 2 +-
lldb/tools/lldb-dap/DAP.h | 3 +
lldb/tools/lldb-dap/lldb-dap.cpp | 47 +++++++++-
8 files changed, 240 insertions(+), 8 deletions(-)
create mode 100644 lldb/test/API/tools/lldb-dap/stop-events/Makefile
create mode 100644 lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py
create mode 100644 lldb/test/API/tools/lldb-dap/stop-events/main.cpp
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 a324af57b61df..84e6fe82a06dc 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
@@ -125,6 +125,8 @@ def __init__(self, recv, send, init_commands, log_file=None):
self.initialize_body = None
self.thread_stop_reasons = {}
self.breakpoint_events = []
+ self.thread_events_body = []
+ self.stopped_events = []
self.progress_events = []
self.reverse_requests = []
self.sequence = 1
@@ -227,6 +229,8 @@ def handle_recv_packet(self, packet):
# When a new process is attached or launched, remember the
# details that are available in the body of the event
self.process_event_body = body
+ elif event == "continued":
+ self.stopped_events = []
elif event == "stopped":
# Each thread that stops with a reason will send a
# 'stopped' event. We need to remember the thread stop
@@ -234,6 +238,7 @@ def handle_recv_packet(self, packet):
# that information.
self._process_stopped()
tid = body["threadId"]
+ self.stopped_events.append(packet)
self.thread_stop_reasons[tid] = body
elif event == "breakpoint":
# Breakpoint events come in when a breakpoint has locations
@@ -242,6 +247,10 @@ def handle_recv_packet(self, packet):
self.breakpoint_events.append(packet)
# no need to add 'breakpoint' event packets to our packets list
return keepGoing
+ elif event == "thread":
+ self.thread_events_body.append(body)
+ # no need to add 'thread' event packets to our packets list
+ return keepGoing
elif event.startswith("progress"):
# Progress events come in as 'progressStart', 'progressUpdate',
# and 'progressEnd' events. Keep these around in case test
@@ -418,6 +427,15 @@ def get_threads(self):
self.request_threads()
return self.threads
+ def get_thread_events(self, reason=None):
+ if reason == None:
+ return self.thread_events_body
+ else:
+ return [body for body in self.thread_events_body if body["reason"] == reason]
+
+ def get_stopped_events(self):
+ return self.stopped_events
+
def get_thread_id(self, threadIndex=0):
"""Utility function to get the first thread ID in the thread list.
If the thread list is empty, then fetch the threads.
@@ -707,7 +725,7 @@ def request_evaluate(self, expression, frameIndex=0, threadId=None, context=None
}
return self.send_recv(command_dict)
- def request_initialize(self, sourceInitFile):
+ def request_initialize(self, sourceInitFile, singleStoppedEvent=False):
command_dict = {
"command": "initialize",
"type": "request",
@@ -723,6 +741,7 @@ def request_initialize(self, sourceInitFile):
"supportsVariableType": True,
"supportsStartDebuggingRequest": True,
"sourceInitFile": sourceInitFile,
+ "singleStoppedEvent": singleStoppedEvent,
},
}
response = self.send_recv(command_dict)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index a312a88ebd7e5..90ab2a8d4898a 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -350,6 +350,7 @@ def launch(
cwd=None,
env=None,
stopOnEntry=False,
+ singleStoppedEvent=False,
disableASLR=True,
disableSTDIO=False,
shellExpandArguments=False,
@@ -387,7 +388,7 @@ def cleanup():
self.addTearDownHook(cleanup)
# Initialize and launch the program
- self.dap_server.request_initialize(sourceInitFile)
+ self.dap_server.request_initialize(sourceInitFile, singleStoppedEvent)
response = self.dap_server.request_launch(
program,
args=args,
@@ -432,6 +433,7 @@ def build_and_launch(
cwd=None,
env=None,
stopOnEntry=False,
+ singleStoppedEvent=False,
disableASLR=True,
disableSTDIO=False,
shellExpandArguments=False,
@@ -468,6 +470,7 @@ def build_and_launch(
cwd,
env,
stopOnEntry,
+ singleStoppedEvent,
disableASLR,
disableSTDIO,
shellExpandArguments,
diff --git a/lldb/test/API/tools/lldb-dap/stop-events/Makefile b/lldb/test/API/tools/lldb-dap/stop-events/Makefile
new file mode 100644
index 0000000000000..de4ec12b13cbc
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stop-events/Makefile
@@ -0,0 +1,4 @@
+ENABLE_THREADS := YES
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py b/lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py
new file mode 100644
index 0000000000000..ce4ef7f2fabd3
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py
@@ -0,0 +1,92 @@
+"""
+Test lldb-dap setBreakpoints request
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+from lldbsuite.test import lldbutil
+
+
+class TestDAP_stopEvents(lldbdap_testcase.DAPTestCaseBase):
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_single_stop_event(self):
+ """
+ Ensure single stopped event is sent during stop when singleStoppedEvent
+ is set to True.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program, singleStoppedEvent=True)
+ source = "main.cpp"
+ breakpoint_line = line_number(source, "// Set breakpoint1 here")
+ lines = [breakpoint_line]
+ # Set breakpoint in the thread function
+ breakpoint_ids = self.set_source_breakpoints(source, lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+ self.assertEqual(
+ len(self.dap_server.get_stopped_events()), 1, "expect one thread stopped"
+ )
+
+ loop_count = 10
+ while loop_count > 0:
+ self.dap_server.request_continue()
+ stopped_event = self.dap_server.wait_for_stopped()
+ self.assertEqual(
+ len(self.dap_server.get_stopped_events()),
+ 1,
+ "expect one thread stopped",
+ )
+ loop_count -= 1
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_correct_thread_count(self):
+ """
+ Test that the correct number of threads are reported in the stop event.
+ No thread exited events are sent.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program, singleStoppedEvent=True)
+ source = "main.cpp"
+ breakpoint_line = line_number(source, "// break worker thread here")
+ lines = [breakpoint_line]
+ # Set breakpoint in the thread function
+ breakpoint_ids = self.set_source_breakpoints(source, lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+
+ threads = self.dap_server.get_threads()
+ self.assertEqual(len(threads), 2, "expect two threads in first worker thread")
+
+ self.dap_server.request_continue()
+ stopped_event = self.dap_server.wait_for_stopped()
+ threads = self.dap_server.get_threads()
+ self.assertEqual(
+ len(threads), 3, "expect three threads in second worker thread"
+ )
+
+ main_thread_breakpoint_line = line_number(source, "// break main thread here")
+ # Set breakpoint in the thread function
+ main_breakpoint_ids = self.set_source_breakpoints(
+ source, [main_thread_breakpoint_line]
+ )
+ self.continue_to_breakpoints(main_breakpoint_ids)
+
+ threads = self.dap_server.get_threads()
+ self.assertEqual(
+ len(threads), 3, "expect three threads in second worker thread"
+ )
+
+ exited_threads = self.dap_server.get_thread_events("exited")
+ self.assertEqual(
+ len(exited_threads),
+ 0,
+ "expect no threads exited after hitting main thread breakpoint during context switch",
+ )
diff --git a/lldb/test/API/tools/lldb-dap/stop-events/main.cpp b/lldb/test/API/tools/lldb-dap/stop-events/main.cpp
new file mode 100644
index 0000000000000..de870dbc51062
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stop-events/main.cpp
@@ -0,0 +1,74 @@
+#include <condition_variable>
+#include <iostream>
+#include <mutex>
+#include <thread>
+
+std::mutex mtx;
+std::condition_variable cv;
+int ready_id = 0;
+
+void worker(int id) {
+ std::cout << "Worker " << id << " executing..." << std::endl;
+ // Signal the main thread to continue main thread
+ {
+ std::lock_guard<std::mutex> lock(mtx);
+ ready_id = id; // break worker thread here
+ }
+ cv.notify_one();
+
+ // Simulate some work
+ std::this_thread::sleep_for(std::chrono::seconds(10));
+ std::cout << "Worker " << id << " finished." << std::endl;
+}
+
+void thread_proc(int threadId) {
+ std::mutex repro_mtx;
+ for (;;) {
+ int i = 0;
+ ++i; // Set breakpoint1 here
+ repro_mtx.lock();
+ std::cout << "Thread " << threadId << " is running, " << i << std::endl;
+ repro_mtx.unlock();
+
+ // Simulate some work
+ std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+ i += 10; // Set breakpoint2 here
+ repro_mtx.lock();
+ std::cout << "Thread " << threadId << " finished" << std::endl;
+ repro_mtx.unlock(); // Unlock the mutex after printing
+ }
+}
+
+void run_threads_in_loop(int numThreads) {
+ std::thread threads[numThreads];
+ // Create threads
+ for (int i = 0; i < numThreads; ++i) {
+ threads[i] = std::thread(thread_proc, i);
+ }
+ for (int i = 0; i < numThreads; ++i) {
+ threads[i].join();
+ }
+}
+
+int main() {
+ // Create the first worker thread
+ std::thread t1(worker, 1);
+
+ // Wait until signaled by the first thread
+ std::unique_lock<std::mutex> lock(mtx);
+ cv.wait(lock, [] { return ready_id == 1; });
+
+ // Create the second worker thread
+ std::thread t2(worker, 2);
+
+ // Wait until signaled by the second thread
+ cv.wait(lock, [] { return ready_id == 2; });
+
+ // Join the first thread to ensure main waits for it to finish
+ t1.join(); // break main thread here
+ t2.join();
+
+ run_threads_in_loop(100);
+ return 0;
+}
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 0196aed819f2b..ba74d9fda2d3f 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -34,7 +34,7 @@ DAP g_dap;
DAP::DAP()
: broadcaster("lldb-dap"), exception_breakpoints(),
focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false),
- enable_auto_variable_summaries(false),
+ single_stopped_event(false), enable_auto_variable_summaries(false),
enable_synthetic_child_debugging(false),
restarting_process_id(LLDB_INVALID_PROCESS_ID),
configuration_done_sent(false), waiting_for_run_in_terminal(false),
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 37e57d58968d9..38126005d753c 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -174,6 +174,9 @@ struct DAP {
llvm::once_flag terminated_event_flag;
bool stop_at_entry;
bool is_attach;
+ // Whether to send single stopped event when multiple stopped events
+ // occured at the same time (eg. breakpoints by threads simultanously)
+ bool single_stopped_event;
bool enable_auto_variable_summaries;
bool enable_synthetic_child_debugging;
// The process event thread normally responds to process exited events by
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index b74474b9d383c..36eb543fbe1dc 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -243,8 +243,6 @@ void SendThreadStoppedEvent() {
if (process.IsValid()) {
auto state = process.GetState();
if (state == lldb::eStateStopped) {
- llvm::DenseSet<lldb::tid_t> old_thread_ids;
- old_thread_ids.swap(g_dap.thread_ids);
uint32_t stop_id = process.GetStopID();
const uint32_t num_threads = process.GetNumThreads();
@@ -255,28 +253,34 @@ void SendThreadStoppedEvent() {
lldb::tid_t first_tid_with_reason = LLDB_INVALID_THREAD_ID;
uint32_t num_threads_with_reason = 0;
bool focus_thread_exists = false;
+ lldb::SBThread focus_thread, first_thread_with_reason;
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
const lldb::tid_t tid = thread.GetThreadID();
const bool has_reason = ThreadHasStopReason(thread);
// If the focus thread doesn't have a stop reason, clear the thread ID
if (tid == g_dap.focus_tid) {
+ focus_thread = thread;
focus_thread_exists = true;
if (!has_reason)
g_dap.focus_tid = LLDB_INVALID_THREAD_ID;
}
if (has_reason) {
++num_threads_with_reason;
- if (first_tid_with_reason == LLDB_INVALID_THREAD_ID)
+ if (first_tid_with_reason == LLDB_INVALID_THREAD_ID) {
first_tid_with_reason = tid;
+ first_thread_with_reason = thread;
+ }
}
}
// We will have cleared g_dap.focus_tid if the focus thread doesn't have
// a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
// then set the focus thread to the first thread with a stop reason.
- if (!focus_thread_exists || g_dap.focus_tid == LLDB_INVALID_THREAD_ID)
+ if (!focus_thread_exists || g_dap.focus_tid == LLDB_INVALID_THREAD_ID) {
g_dap.focus_tid = first_tid_with_reason;
+ focus_thread = first_thread_with_reason;
+ }
// If no threads stopped with a reason, then report the first one so
// we at least let the UI know we stopped.
@@ -284,16 +288,40 @@ void SendThreadStoppedEvent() {
lldb::SBThread thread = process.GetThreadAtIndex(0);
g_dap.focus_tid = thread.GetThreadID();
g_dap.SendJSON(CreateThreadStopped(thread, stop_id));
+ } else if (g_dap.single_stopped_event) {
+ // If single_stopped_event option is true only one stopped event will
+ // be sent during debugger stop. The focused thread's stopped event is
+ // preferred if it is stopped with a reason; otherwise, we simply use
+ // the first stopped thread.
+ //
+ // This option would be preferred for VSCode IDE because multiple
+ // stopped events would cause confusing UX.
+ //
+ // TODO: do we need to give priority to exception/signal stopped event
+ // over normal stepping complete/breakpoint?
+ //
+
+ assert(focus_thread.IsValid());
+ assert(g_dap.focus_tid == focus_thread.GetThreadID());
+ g_dap.SendJSON(CreateThreadStopped(focus_thread, stop_id));
} else {
for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
- g_dap.thread_ids.insert(thread.GetThreadID());
if (ThreadHasStopReason(thread)) {
g_dap.SendJSON(CreateThreadStopped(thread, stop_id));
}
}
}
+ // Update existing thread ids and send thread exit event
+ // for non-exist ones.
+ llvm::DenseSet<lldb::tid_t> old_thread_ids;
+ old_thread_ids.swap(g_dap.thread_ids);
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
+ g_dap.thread_ids.insert(thread.GetThreadID());
+ }
+
for (auto tid : old_thread_ids) {
auto end = g_dap.thread_ids.end();
auto pos = g_dap.thread_ids.find(tid);
@@ -880,6 +908,10 @@ void request_attach(const llvm::json::Object &request) {
void request_continue(const llvm::json::Object &request) {
llvm::json::Object response;
FillResponse(request, response);
+ auto arguments = request.getObject("arguments");
+ lldb::SBThread thread = g_dap.GetLLDBThread(*arguments);
+ g_dap.focus_tid = thread.GetThreadID();
+
lldb::SBProcess process = g_dap.target.GetProcess();
lldb::SBError error = process.Continue();
llvm::json::Object body;
@@ -1625,6 +1657,11 @@ void request_initialize(const llvm::json::Object &request) {
"Get or set the repl behavior of lldb-dap evaluation requests.");
g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction);
+ // singleStoppedEvent option is not from formal DAP specification. It is an
+ // lldb specific option to experiment stopped events behaivor against
+ // application with multiple threads.
+ g_dap.single_stopped_event =
+ GetBoolean(arguments, "singleStoppedEvent", false);
// Start our event thread so we can receive events from the debugger, target,
// process and more.
More information about the lldb-commits
mailing list