https://github.com/ashgti updated
https://github.com/llvm/llvm-project/pull/126833
>From af07cbad6524cfb0509f93565eb86584e4c3576c Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Tue, 11 Feb 2025 16:58:20 -0800
Subject: [PATCH 1/2] [lldb-dap] Ensure we do not print the close sentinel when
closing stdout.
The OutputRedirect is always writing the `"\0"` byte as a final stdout message
during shutdown. Instead, I adjusted this to detect the sentinel value and
break out of the read loop as if we detected EOF.
---
lldb/tools/lldb-dap/OutputRedirector.cpp | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp
b/lldb/tools/lldb-dap/OutputRedirector.cpp
index a23572ab7ae04..c5090338cfd1c 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -10,6 +10,7 @@
#include "DAP.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include <cstring>
#include <system_error>
#if defined(_WIN32)
#include <fcntl.h>
@@ -25,6 +26,10 @@ using llvm::Expected;
using llvm::inconvertibleErrorCode;
using llvm::StringRef;
+namespace {
+char kCloseSentinel[] = "\0";
+} // namespace
+
namespace lldb_dap {
int OutputRedirector::kInvalidDescriptor = -1;
@@ -59,7 +64,10 @@ Error
OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
while (!m_stopped) {
ssize_t bytes_count = ::read(read_fd, &buffer, sizeof(buffer));
// EOF detected.
- if (bytes_count == 0)
+ if (bytes_count == 0 ||
+ (bytes_count == sizeof(kCloseSentinel) &&
+ std::memcmp(buffer, kCloseSentinel, sizeof(kCloseSentinel)) == 0 &&
+ m_fd == kInvalidDescriptor))
break;
if (bytes_count == -1) {
// Skip non-fatal errors.
@@ -85,8 +93,7 @@ void OutputRedirector::Stop() {
// Closing the pipe may not be sufficient to wake up the thread in case the
// write descriptor is duplicated (to stdout/err or to another process).
// Write a null byte to ensure the read call returns.
- char buf[] = "\0";
- (void)::write(fd, buf, sizeof(buf));
+ (void)::write(fd, kCloseSentinel, sizeof(kCloseSentinel));
::close(fd);
m_forwarder.join();
}
>From 941df4009f36f15060452a54b152bcc535a1e5e1 Mon Sep 17 00:00:00 2001
From: John Harrison <harj...@google.com>
Date: Wed, 12 Feb 2025 10:43:45 -0800
Subject: [PATCH 2/2] Applying reviewer suggestions and updating the existing
tests to ensure output produced by the debuggee is not affected by the output
redirection.
---
.../tools/lldb-dap/output/TestDAP_output.py | 24 ++++++++++++------
lldb/test/API/tools/lldb-dap/output/main.c | 3 +++
lldb/tools/lldb-dap/OutputRedirector.cpp | 25 +++++++------------
lldb/tools/lldb-dap/OutputRedirector.h | 1 -
4 files changed, 29 insertions(+), 24 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 02c34ba10321b..eae7629409844 100644
--- a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
+++ b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
@@ -10,23 +10,33 @@
class TestDAP_output(lldbdap_testcase.DAPTestCaseBase):
@skipIfWindows
def test_output(self):
+ """
+ Test output handling for the running process.
+ """
program = self.getBuildArtifact("a.out")
- self.build_and_launch(program)
+ self.build_and_launch(
+ program,
+ exitCommands=[
+ # Ensure that output produced by lldb itself is not consumed
by the OutputRedirector.
+ "?script print('out\\0\\0', end='\\r\\n', file=sys.stdout)",
+ "?script print('err\\0\\0', end='\\r\\n', file=sys.stderr)",
+ ],
+ )
source = "main.c"
lines = [line_number(source, "// breakpoint 1")]
breakpoint_ids = self.set_source_breakpoints(source, lines)
self.continue_to_breakpoints(breakpoint_ids)
-
+
# 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.assertTrue(output and len(output) > 0, "expect program stdout")
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.assertTrue(output and len(output) > 0, "expect program stdout")
self.assertIn(
- "abcdefghi\r\nhello world\r\n",
+ "abcdefghi\r\nhello world\r\nfinally\0\0out\0\0\r\nerr\0\0",
output,
- 'full output not found in: ' + output,
+ "full stdout not found in: " + repr(output),
)
diff --git a/lldb/test/API/tools/lldb-dap/output/main.c
b/lldb/test/API/tools/lldb-dap/output/main.c
index 0cfcf604aa68f..226cb607a1234 100644
--- a/lldb/test/API/tools/lldb-dap/output/main.c
+++ b/lldb/test/API/tools/lldb-dap/output/main.c
@@ -8,5 +8,8 @@ int main() {
printf("def");
printf("ghi\n");
printf("hello world\n"); // breakpoint 1
+ // Ensure the OutputRedirector does not consume the programs \0\0 output.
+ char buf[] = "finally\0";
+ write(STDOUT_FILENO, buf, sizeof(buf));
return 0;
}
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp
b/lldb/tools/lldb-dap/OutputRedirector.cpp
index c5090338cfd1c..dddef8f6f2b43 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -19,16 +19,9 @@
#include <unistd.h>
#endif
-using lldb_private::Pipe;
-using llvm::createStringError;
-using llvm::Error;
-using llvm::Expected;
-using llvm::inconvertibleErrorCode;
-using llvm::StringRef;
+using namespace llvm;
-namespace {
-char kCloseSentinel[] = "\0";
-} // namespace
+static constexpr auto kCloseSentinel = StringLiteral::withInnerNUL("\0");
namespace lldb_dap {
@@ -63,12 +56,6 @@ Error
OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
char buffer[OutputBufferSize];
while (!m_stopped) {
ssize_t bytes_count = ::read(read_fd, &buffer, sizeof(buffer));
- // EOF detected.
- if (bytes_count == 0 ||
- (bytes_count == sizeof(kCloseSentinel) &&
- std::memcmp(buffer, kCloseSentinel, sizeof(kCloseSentinel)) == 0 &&
- m_fd == kInvalidDescriptor))
- break;
if (bytes_count == -1) {
// Skip non-fatal errors.
if (errno == EAGAIN || errno == EINTR || errno == EWOULDBLOCK)
@@ -76,6 +63,12 @@ Error
OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
break;
}
+ StringRef data(buffer, bytes_count);
+ if (m_stopped)
+ data.consume_back(kCloseSentinel);
+ if (data.empty())
+ break;
+
callback(StringRef(buffer, bytes_count));
}
::close(read_fd);
@@ -93,7 +86,7 @@ void OutputRedirector::Stop() {
// Closing the pipe may not be sufficient to wake up the thread in case the
// write descriptor is duplicated (to stdout/err or to another process).
// Write a null byte to ensure the read call returns.
- (void)::write(fd, kCloseSentinel, sizeof(kCloseSentinel));
+ (void)::write(fd, kCloseSentinel.data(), kCloseSentinel.size());
::close(fd);
m_forwarder.join();
}
diff --git a/lldb/tools/lldb-dap/OutputRedirector.h
b/lldb/tools/lldb-dap/OutputRedirector.h
index d2bd39797f3d7..a47ea96f71f14 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.h
+++ b/lldb/tools/lldb-dap/OutputRedirector.h
@@ -9,7 +9,6 @@
#ifndef LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
#define LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
-#include "lldb/Host/Pipe.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include <atomic>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
