[Lldb-commits] [lldb] [lldb-dap] When sending a DAP Output Event break each message into separate lines. (PR #105456)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Wed Aug 21 13:30:56 PDT 2024
https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/105456
>From 9246649c24991127b8f54ae1f21121386cef7254 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Tue, 20 Aug 2024 16:33:14 -0700
Subject: [PATCH 1/4] When sending a DAP Output Event break each message into
separate lines.
Previously, when output like `"hello\nworld\n"` was produced by lldb (or the process) the message would be sent as a single Output event.
By being a single event this causes VS Code to treat this as a single message in the console when handling displaying and filtering in the Debug Console.
Instead, with these changes we send each line as its own event. This results in VS Code representing each line of output from lldb-dap as an individual output message.
---
.../test/tools/lldb-dap/lldbdap_testcase.py | 5 +++
lldb/test/API/tools/lldb-dap/output/Makefile | 3 ++
.../tools/lldb-dap/output/TestDAP_output.py | 37 +++++++++++++++++++
lldb/test/API/tools/lldb-dap/output/main.c | 11 ++++++
lldb/tools/lldb-dap/DAP.cpp | 22 ++++++++---
5 files changed, 72 insertions(+), 6 deletions(-)
create mode 100644 lldb/test/API/tools/lldb-dap/output/Makefile
create mode 100644 lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
create mode 100644 lldb/test/API/tools/lldb-dap/output/main.c
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 a312a88ebd7e58..8341bfda748206 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
@@ -202,6 +202,11 @@ def collect_console(self, timeout_secs, pattern=None):
"console", timeout_secs=timeout_secs, pattern=pattern
)
+ def collect_stdout(self, timeout_secs, pattern=None):
+ return self.dap_server.collect_output(
+ "stdout", timeout_secs=timeout_secs, pattern=pattern
+ )
+
def get_local_as_int(self, name, threadId=None):
value = self.dap_server.get_local_variable_value(name, threadId=threadId)
# 'value' may have the variable value and summary.
diff --git a/lldb/test/API/tools/lldb-dap/output/Makefile b/lldb/test/API/tools/lldb-dap/output/Makefile
new file mode 100644
index 00000000000000..10495940055b63
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/output/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
new file mode 100644
index 00000000000000..08d5f07f224e4c
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
@@ -0,0 +1,37 @@
+"""
+Test lldb-dap output events
+"""
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import re
+
+
+class TestDAP_output(lldbdap_testcase.DAPTestCaseBase):
+ def test_output(self):
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.c"
+ main_source_path = self.getSourcePath(source)
+ lines = [line_number(source, "// breakpoint 1")]
+ breakpoint_ids = self.set_source_breakpoints(source, lines)
+ self.continue_to_breakpoints(breakpoint_ids)
+
+ output = self.collect_stdout(
+ timeout_secs=1.0,
+ pattern="abcdef"
+ )
+ self.assertTrue(output and len(output) > 0, "expect no program output")
+
+ self.continue_to_exit()
+
+ output += self.get_stdout(timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval)
+ self.assertTrue(output and len(output) > 0, "expect no program output")
+ self.assertIn(
+ "abcdefghi\r\nhello world\r\n",
+ output,
+ 'full output not found in: ' + output
+ )
diff --git a/lldb/test/API/tools/lldb-dap/output/main.c b/lldb/test/API/tools/lldb-dap/output/main.c
new file mode 100644
index 00000000000000..62a3337d865db2
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/output/main.c
@@ -0,0 +1,11 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int main() {
+ printf("abc");
+ printf("def");
+ printf("ghi\n");
+ printf("hello world\n"); // breakpoint 1
+ return 0;
+}
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index c3c70e9d739846..1fd560f21904ab 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -294,8 +294,6 @@ void DAP::SendOutput(OutputType o, const llvm::StringRef output) {
if (output.empty())
return;
- llvm::json::Object event(CreateEventObject("output"));
- llvm::json::Object body;
const char *category = nullptr;
switch (o) {
case OutputType::Console:
@@ -311,10 +309,22 @@ void DAP::SendOutput(OutputType o, const llvm::StringRef output) {
category = "telemetry";
break;
}
- body.try_emplace("category", category);
- EmplaceSafeString(body, "output", output.str());
- event.try_emplace("body", std::move(body));
- SendJSON(llvm::json::Value(std::move(event)));
+
+ // Send each line of output as an individual event, including the newline if
+ // present.
+ ::size_t idx = 0;
+ do {
+ ::size_t end = output.find('\n', idx);
+ if (end == llvm::StringRef::npos)
+ end = output.size() - 1;
+ llvm::json::Object event(CreateEventObject("output"));
+ llvm::json::Object body;
+ body.try_emplace("category", category);
+ EmplaceSafeString(body, "output", output.slice(idx, end + 1).str());
+ event.try_emplace("body", std::move(body));
+ SendJSON(llvm::json::Value(std::move(event)));
+ idx = end + 1;
+ } while (idx < output.size());
}
// interface ProgressStartEvent extends Event {
>From e5f7d85bc2e4d1a15db3afef630ebd41296cb953 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 21 Aug 2024 10:31:47 -0700
Subject: [PATCH 2/4] Tweaking the buffer size and fixing the python format.
---
.../API/tools/lldb-dap/output/TestDAP_output.py | 14 ++++----------
lldb/tools/lldb-dap/lldb-dap.cpp | 2 +-
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
index 08d5f07f224e4c..0d40ce993dc31c 100644
--- a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
+++ b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
@@ -2,12 +2,9 @@
Test lldb-dap output events
"""
-import dap_server
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
import lldbdap_testcase
-import re
class TestDAP_output(lldbdap_testcase.DAPTestCaseBase):
@@ -15,15 +12,12 @@ def test_output(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
source = "main.c"
- main_source_path = self.getSourcePath(source)
lines = [line_number(source, "// breakpoint 1")]
breakpoint_ids = self.set_source_breakpoints(source, lines)
self.continue_to_breakpoints(breakpoint_ids)
- output = self.collect_stdout(
- timeout_secs=1.0,
- pattern="abcdef"
- )
+ # Ensure partial messages are still sent.
+ output = self.collect_stdout(timeout_secs=1.0, pattern="abcdef")
self.assertTrue(output and len(output) > 0, "expect no program output")
self.continue_to_exit()
@@ -33,5 +27,5 @@ def test_output(self):
self.assertIn(
"abcdefghi\r\nhello world\r\n",
output,
- 'full output not found in: ' + output
- )
+ 'full output not found in: ' + output,
+ )
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index f50a6c17310739..78b33428f8c7e2 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -399,7 +399,7 @@ void SendProcessEvent(LaunchMethod launch_method) {
// Grab any STDOUT and STDERR from the process and send it up to VS Code
// via an "output" event to the "stdout" and "stderr" categories.
void SendStdOutStdErr(lldb::SBProcess &process) {
- char buffer[1024];
+ char buffer[4096];
size_t count;
while ((count = process.GetSTDOUT(buffer, sizeof(buffer))) > 0)
g_dap.SendOutput(OutputType::Stdout, llvm::StringRef(buffer, count));
>From 106a6bab4972913d989b97108c4f5f25bef2eca3 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 21 Aug 2024 13:26:46 -0700
Subject: [PATCH 3/4] Adding a constant for the output event buffer size.
---
lldb/tools/lldb-dap/DAP.h | 4 ++++
lldb/tools/lldb-dap/OutputRedirector.cpp | 6 ++++--
lldb/tools/lldb-dap/lldb-dap.cpp | 2 +-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 7828272aa15a7d..27ea6c7ff8423f 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -68,8 +68,12 @@ namespace lldb_dap {
typedef llvm::DenseMap<uint32_t, SourceBreakpoint> SourceBreakpointMap;
typedef llvm::StringMap<FunctionBreakpoint> FunctionBreakpointMap;
+
enum class OutputType { Console, Stdout, Stderr, Telemetry };
+/// Buffer size for handling output events.
+constexpr uint64_t OutputBufferSize = (1u << 12);
+
enum DAPBroadcasterBits {
eBroadcastBitStopEventThread = 1u << 0,
eBroadcastBitStopProgressThread = 1u << 1
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp b/lldb/tools/lldb-dap/OutputRedirector.cpp
index 4e6907ce6c7806..bba6a9bf5f80c3 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -13,8 +13,10 @@
#include <unistd.h>
#endif
-#include "OutputRedirector.h"
#include "llvm/ADT/StringRef.h"
+#include "DAP.h"
+#include "OutputRedirector.h"
+
using namespace llvm;
@@ -42,7 +44,7 @@ Error RedirectFd(int fd, std::function<void(llvm::StringRef)> callback) {
int read_fd = new_fd[0];
std::thread t([read_fd, callback]() {
- char buffer[4096];
+ char buffer[OutputBufferSize];
while (true) {
ssize_t bytes_count = read(read_fd, &buffer, sizeof(buffer));
if (bytes_count == 0)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 78b33428f8c7e2..52aa35cd06e00d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -399,7 +399,7 @@ void SendProcessEvent(LaunchMethod launch_method) {
// Grab any STDOUT and STDERR from the process and send it up to VS Code
// via an "output" event to the "stdout" and "stderr" categories.
void SendStdOutStdErr(lldb::SBProcess &process) {
- char buffer[4096];
+ char buffer[OutputBufferSize];
size_t count;
while ((count = process.GetSTDOUT(buffer, sizeof(buffer))) > 0)
g_dap.SendOutput(OutputType::Stdout, llvm::StringRef(buffer, count));
>From 77bd53ed3577c1feddf0a07ddc352826dad411a8 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 21 Aug 2024 13:30:38 -0700
Subject: [PATCH 4/4] Added a comment to clarify a test scenario.
---
lldb/test/API/tools/lldb-dap/output/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/test/API/tools/lldb-dap/output/main.c b/lldb/test/API/tools/lldb-dap/output/main.c
index 62a3337d865db2..0cfcf604aa68f7 100644
--- a/lldb/test/API/tools/lldb-dap/output/main.c
+++ b/lldb/test/API/tools/lldb-dap/output/main.c
@@ -3,6 +3,7 @@
#include <unistd.h>
int main() {
+ // Ensure multiple partial lines are detected and sent.
printf("abc");
printf("def");
printf("ghi\n");
More information about the lldb-commits
mailing list