Author: John Harrison Date: 2025-02-13T10:35:50-08:00 New Revision: c2e96778e04197dd266f7c540bf174b6ec28a434
URL: https://github.com/llvm/llvm-project/commit/c2e96778e04197dd266f7c540bf174b6ec28a434 DIFF: https://github.com/llvm/llvm-project/commit/c2e96778e04197dd266f7c540bf174b6ec28a434.diff LOG: [lldb-dap] Ensure we do not print the close sentinel when closing stdout. (#126833) If you have an lldb-dap log file you'll almost always see a final message like: ``` <-- Content-Length: 94 { "body": { "category": "stdout", "output": "\u0000\u0000" }, "event": "output", "seq": 0, "type": "event" } <-- Content-Length: 94 { "body": { "category": "stderr", "output": "\u0000\u0000" }, "event": "output", "seq": 0, "type": "event" } ``` 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. --------- Co-authored-by: Pavel Labath <pa...@labath.sk> Added: Modified: lldb/test/API/tools/lldb-dap/output/TestDAP_output.py lldb/test/API/tools/lldb-dap/output/main.c lldb/tools/lldb-dap/OutputRedirector.cpp lldb/tools/lldb-dap/OutputRedirector.h Removed: ################################################################################ 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 a23572ab7ae04..df21fb850dc55 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> @@ -18,12 +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; + +static constexpr auto kCloseSentinel = StringLiteral::withInnerNUL("\0"); namespace lldb_dap { @@ -58,9 +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) - break; if (bytes_count == -1) { // Skip non-fatal errors. if (errno == EAGAIN || errno == EINTR || errno == EWOULDBLOCK) @@ -68,7 +63,13 @@ Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) { break; } - callback(StringRef(buffer, bytes_count)); + StringRef data(buffer, bytes_count); + if (m_stopped) + data.consume_back(kCloseSentinel); + if (data.empty()) + break; + + callback(data); } ::close(read_fd); }); @@ -85,8 +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. - char buf[] = "\0"; - (void)::write(fd, buf, sizeof(buf)); + (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
