-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb-dap] Send an Invalidated event on thread stack change. #163976
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When the user send `thread return <expr>` command this changes the stack length. but the UI does not update. Send stack invalidated event to update the stack
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesWhen the user send Full diff: https://github.com/llvm/llvm-project/pull/163976.diff 10 Files Affected:
diff --git a/lldb/test/API/tools/lldb-dap/invalidated-event/Makefile b/lldb/test/API/tools/lldb-dap/invalidated-event/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/invalidated-event/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/invalidated-event/TestDAP_invalidatedEvent.py b/lldb/test/API/tools/lldb-dap/invalidated-event/TestDAP_invalidatedEvent.py
new file mode 100644
index 0000000000000..aef63ffe81060
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/invalidated-event/TestDAP_invalidatedEvent.py
@@ -0,0 +1,55 @@
+"""
+Test lldb-dap recieves invalidated-events when the area such as
+stack, variables, threads has changes but the client does not
+know about it.
+"""
+
+import lldbdap_testcase
+from lldbsuite.test.lldbtest import line_number
+from dap_server import Event
+
+
+class TestDAP_invalidatedEvent(lldbdap_testcase.DAPTestCaseBase):
+ def verify_top_frame_name(self, frame_name: str):
+ all_frames = self.get_stackFrames()
+ self.assertGreaterEqual(len(all_frames), 1, "Expected at least one frame.")
+ top_frame_name = all_frames[0]["name"]
+ self.assertRegex(top_frame_name, f"{frame_name}.*")
+
+ def test_invalidated_stack_area_event(self):
+ """
+ Test an invalidated event for the stack area.
+ The event is sent when the command `thread return <expr>` is sent by the user.
+ """
+ other_source = "other.h"
+ return_bp_line = line_number(other_source, "// thread return breakpoint")
+
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ self.set_source_breakpoints(other_source, [return_bp_line])
+ self.continue_to_next_stop()
+
+ self.verify_top_frame_name("add")
+ thread_id = self.dap_server.get_thread_id()
+ self.assertIsNotNone(thread_id, "Exepected a thread id.")
+
+ # run thread return
+ thread_command = "thread return 20"
+ eval_resp = self.dap_server.request_evaluate(thread_command, context="repl")
+ self.assertTrue(eval_resp["success"], f"Failed to evaluate `{thread_command}`.")
+
+ # wait for the invalidated stack event.
+ stack_event = self.dap_server.wait_for_event(["invalidated"])
+ self.assertIsNotNone(stack_event, "Expected an invalidated event.")
+ event_body: Event = stack_event["body"]
+ self.assertIn("stacks", event_body["areas"])
+ self.assertIn("threadId", event_body.keys())
+ self.assertEqual(
+ thread_id,
+ event_body["threadId"],
+ f"Expected the event from thread {thread_id}.",
+ )
+
+ # confirm we are back at the main frame.
+ self.verify_top_frame_name("main")
+ self.continue_to_exit()
diff --git a/lldb/test/API/tools/lldb-dap/invalidated-event/main.cpp b/lldb/test/API/tools/lldb-dap/invalidated-event/main.cpp
new file mode 100644
index 0000000000000..9fed5cf90f3f1
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/invalidated-event/main.cpp
@@ -0,0 +1,9 @@
+#include "other.h"
+
+int main() {
+ int first = 5;
+ int second = 10;
+ const int result = add(first, second);
+
+ return 0;
+}
\ No newline at end of file
diff --git a/lldb/test/API/tools/lldb-dap/invalidated-event/other.h b/lldb/test/API/tools/lldb-dap/invalidated-event/other.h
new file mode 100644
index 0000000000000..c3e242676612b
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/invalidated-event/other.h
@@ -0,0 +1,10 @@
+#ifndef OTHER_H
+#define OTHER_H
+
+int add(int a, int b) {
+ int first = a;
+ int second = b; // thread return breakpoint
+ int result = first + second;
+ return result;
+}
+#endif // OTHER_H
\ No newline at end of file
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index f76656e98ca01..61226cceb6db0 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -1368,6 +1368,12 @@ void DAP::EventThread() {
broadcaster.AddListener(listener, eBroadcastBitStopEventThread);
debugger.GetBroadcaster().AddListener(
listener, lldb::eBroadcastBitError | lldb::eBroadcastBitWarning);
+
+ // listen for thread events.
+ listener.StartListeningForEventClass(
+ debugger, lldb::SBThread::GetBroadcasterClassName(),
+ lldb::SBThread::eBroadcastBitStackChanged);
+
bool done = false;
while (!done) {
if (listener.WaitForEvent(1, event)) {
@@ -1503,6 +1509,9 @@ void DAP::EventThread() {
SendJSON(llvm::json::Value(std::move(bp_event)));
}
}
+
+ } else if (lldb::SBThread::EventIsThreadEvent(event)) {
+ HandleThreadEvent(event);
} else if (event_mask & lldb::eBroadcastBitError ||
event_mask & lldb::eBroadcastBitWarning) {
lldb::SBStructuredData data =
@@ -1522,6 +1531,16 @@ void DAP::EventThread() {
}
}
+void DAP::HandleThreadEvent(const lldb::SBEvent &event) {
+ uint32_t event_type = event.GetType();
+
+ if (event_type & lldb::SBThread::eBroadcastBitStackChanged) {
+ const lldb::SBThread evt_thread = lldb::SBThread::GetThreadFromEvent(event);
+ SendInvalidatedEvent(*this, {InvalidatedEventBody::eAreaStacks},
+ evt_thread.GetThreadID());
+ }
+}
+
std::vector<protocol::Breakpoint> DAP::SetSourceBreakpoints(
const protocol::Source &source,
const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints) {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index a90ddf59671ee..bf2c3f146a396 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -460,6 +460,7 @@ struct DAP final : public DAPTransport::MessageHandler {
/// Event threads.
/// @{
void EventThread();
+ void HandleThreadEvent(const lldb::SBEvent &event);
void ProgressEventThread();
std::thread event_thread;
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 2b9ed229405a8..04ed8a052bd76 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -15,6 +15,7 @@
#include "Protocol/ProtocolRequests.h"
#include "Protocol/ProtocolTypes.h"
#include "lldb/API/SBFileSpec.h"
+#include "lldb/lldb-defines.h"
#include "llvm/Support/Error.h"
#include <utility>
@@ -276,11 +277,16 @@ void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process) {
}
void SendInvalidatedEvent(
- DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas) {
+ DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas,
+ lldb::tid_t tid) {
if (!dap.clientFeatures.contains(protocol::eClientFeatureInvalidatedEvent))
return;
protocol::InvalidatedEventBody body;
body.areas = areas;
+
+ if (tid != LLDB_INVALID_THREAD_ID)
+ body.threadId = tid;
+
dap.Send(protocol::Event{"invalidated", std::move(body)});
}
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 48eb5af6bd0b9..be783d032a5ae 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -11,6 +11,8 @@
#include "DAPForward.h"
#include "Protocol/ProtocolEvents.h"
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-types.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/Error.h"
@@ -35,7 +37,8 @@ void SendContinuedEvent(DAP &dap);
void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
void SendInvalidatedEvent(
- DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas);
+ DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas,
+ lldb::tid_t tid = LLDB_INVALID_THREAD_ID);
void SendMemoryEvent(DAP &dap, lldb::SBValue variable);
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
index b896eca817be6..df6be06637a13 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
@@ -51,7 +51,7 @@ llvm::json::Value toJSON(const InvalidatedEventBody::Area &IEBA) {
llvm::json::Value toJSON(const InvalidatedEventBody &IEB) {
json::Object Result{{"areas", IEB.areas}};
if (IEB.threadId)
- Result.insert({"threadID", IEB.threadId});
+ Result.insert({"threadId", IEB.threadId});
if (IEB.stackFrameId)
Result.insert({"stackFrameId", IEB.stackFrameId});
return Result;
diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp
index a5ae856a185b7..8170abdd25bc6 100644
--- a/lldb/unittests/DAP/ProtocolTypesTest.cpp
+++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp
@@ -1079,14 +1079,17 @@ TEST(ProtocolTypesTest, InvalidatedEventBody) {
body.areas = {InvalidatedEventBody::eAreaStacks,
InvalidatedEventBody::eAreaThreads};
body.stackFrameId = 1;
- StringRef json = R"({
- "areas": [
- "stacks",
- "threads"
- ],
- "stackFrameId": 1
-})";
- EXPECT_EQ(json, pp(body));
+ body.threadId = 20;
+ Expected<json::Value> expected = json::parse(R"({
+ "areas": [
+ "stacks",
+ "threads"
+ ],
+ "stackFrameId": 1,
+ "threadId": 20
+ })");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ EXPECT_EQ(pp(*expected), pp(body));
}
TEST(ProtocolTypesTest, MemoryEventBody) {
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although we currently can't handle eBroadcastBitSelectedFrameChanged
and eBroadcastBitThreadSelected
events in terms of DAP (maybe send new stopped event as a hack). I'm very excited about this patch that itmproves synchronization between debug console and VS Code UI. LGTM
When the user send
thread return <expr>
command this changes the stack length but the UI does not update.Send stack invalidated event to the client to update the stack.