[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:27:21 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/3] 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/3] 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 d004828664d6d6ccf8d63ffef55bb0ea42b4303f 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/3] Adding a constant for the output event buffer size.

---
 lldb/tools/lldb-dap/DAP.h                | 6 ++++++
 lldb/tools/lldb-dap/OutputRedirector.cpp | 6 ++++--
 lldb/tools/lldb-dap/lldb-dap.cpp         | 2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 7828272aa15a7d..fa6ec746b77f71 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -64,12 +64,18 @@
 #define VARREF_FIRST_VAR_IDX (int64_t)4
 #define NO_TYPENAME "<no-type>"
 
+#define LLDB_DAP_BUFFER_SIZE 4096
+
 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));



More information about the lldb-commits mailing list