[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #122783)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Mon Jan 13 12:09:36 PST 2025
https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/122783
This moves the ownership of the threads that forward stdout/stderr to the DAP object itself to ensure that the threads are joined and that the forwarding is cleaned up when the DAP connection is disconnected.
This is part of a larger refactor to allow lldb-dap to run in a listening mode and accept multiple connections.
This reverts the previous revert and attempts to address the Windows failures by building on the previous iteration of the output redirection logic instead of using the `lldb_private::Pipe` object.
>From f35943b2acf1dec09e73181f69f5eb178873d743 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 8 Jan 2025 09:21:46 -0800
Subject: [PATCH 1/2] Reapply "[lldb-dap] Ensure the IO forwarding threads are
managed by the DAP object lifecycle. (#120457)"
This reverts commit 81898ac00e04ed3f352534a810829bdf4e6e14b7.
---
lldb/tools/lldb-dap/CMakeLists.txt | 9 +-
lldb/tools/lldb-dap/DAP.cpp | 105 ++++++++++----
lldb/tools/lldb-dap/DAP.h | 67 +++++----
lldb/tools/lldb-dap/IOStream.h | 6 +
lldb/tools/lldb-dap/OutputRedirector.cpp | 76 ++++++----
lldb/tools/lldb-dap/OutputRedirector.h | 34 ++++-
lldb/tools/lldb-dap/lldb-dap.cpp | 174 +++++++++++++++--------
7 files changed, 314 insertions(+), 157 deletions(-)
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index d68098bf7b3266..43fc18873feb33 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -1,7 +1,3 @@
-if ( CMAKE_SYSTEM_NAME MATCHES "Windows" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD" )
- list(APPEND extra_libs lldbHost)
-endif ()
-
if (HAVE_LIBPTHREAD)
list(APPEND extra_libs pthread)
endif ()
@@ -26,9 +22,11 @@ add_lldb_tool(lldb-dap
lldb-dap.cpp
Breakpoint.cpp
BreakpointBase.cpp
+ DAP.cpp
ExceptionBreakpoint.cpp
FifoFiles.cpp
FunctionBreakpoint.cpp
+ InstructionBreakpoint.cpp
IOStream.cpp
JSONUtils.cpp
LLDBUtils.cpp
@@ -36,12 +34,11 @@ add_lldb_tool(lldb-dap
ProgressEvent.cpp
RunInTerminal.cpp
SourceBreakpoint.cpp
- DAP.cpp
Watchpoint.cpp
- InstructionBreakpoint.cpp
LINK_LIBS
liblldb
+ lldbHost
${extra_libs}
LINK_COMPONENTS
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 35250d9eef608a..a67abe582abd40 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -6,34 +6,62 @@
//
//===----------------------------------------------------------------------===//
-#include <chrono>
-#include <cstdarg>
-#include <fstream>
-#include <mutex>
-
#include "DAP.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
+#include "OutputRedirector.h"
+#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
#include "lldb/API/SBLanguageRuntime.h"
#include "lldb/API/SBListener.h"
+#include "lldb/API/SBProcess.h"
#include "lldb/API/SBStream.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/raw_ostream.h"
+#include <algorithm>
+#include <cassert>
+#include <chrono>
+#include <cstdarg>
+#include <cstdio>
+#include <fstream>
+#include <mutex>
+#include <utility>
#if defined(_WIN32)
#define NOMINMAX
#include <fcntl.h>
#include <io.h>
#include <windows.h>
+#else
+#include <unistd.h>
#endif
using namespace lldb_dap;
+namespace {
+#ifdef _WIN32
+const char DEV_NULL[] = "nul";
+#else
+const char DEV_NULL[] = "/dev/null";
+#endif
+} // namespace
+
namespace lldb_dap {
-DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
- : debug_adaptor_path(path), broadcaster("lldb-dap"),
+DAP::DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
+ StreamDescriptor input, StreamDescriptor output)
+ : debug_adaptor_path(path), log(log), input(std::move(input)),
+ output(std::move(output)), broadcaster("lldb-dap"),
exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
stop_at_entry(false), is_attach(false),
enable_auto_variable_summaries(false),
@@ -43,21 +71,7 @@ DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
configuration_done_sent(false), waiting_for_run_in_terminal(false),
progress_event_reporter(
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
- reverse_request_seq(0), repl_mode(repl_mode) {
- const char *log_file_path = getenv("LLDBDAP_LOG");
-#if defined(_WIN32)
- // Windows opens stdout and stdin in text mode which converts \n to 13,10
- // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
- // fixes this.
- int result = _setmode(fileno(stdout), _O_BINARY);
- assert(result);
- result = _setmode(fileno(stdin), _O_BINARY);
- UNUSED_IF_ASSERT_DISABLED(result);
- assert(result);
-#endif
- if (log_file_path)
- log.reset(new std::ofstream(log_file_path));
-}
+ reverse_request_seq(0), repl_mode(repl_mode) {}
DAP::~DAP() = default;
@@ -173,6 +187,45 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) {
+ in = lldb::SBFile(std::fopen(DEV_NULL, "r"), /*transfer_ownership=*/true);
+
+ if (auto Error = out.RedirectTo([this](llvm::StringRef output) {
+ SendOutput(OutputType::Stdout, output);
+ }))
+ return Error;
+
+ if (overrideOut) {
+ auto fd = out.GetWriteFileDescriptor();
+ if (auto Error = fd.takeError())
+ return Error;
+
+ if (dup2(*fd, fileno(overrideOut)) == -1)
+ return llvm::errorCodeToError(llvm::errnoAsErrorCode());
+ }
+
+ if (auto Error = err.RedirectTo([this](llvm::StringRef output) {
+ SendOutput(OutputType::Stderr, output);
+ }))
+ return Error;
+
+ if (overrideErr) {
+ auto fd = err.GetWriteFileDescriptor();
+ if (auto Error = fd.takeError())
+ return Error;
+
+ if (dup2(*fd, fileno(overrideErr)) == -1)
+ return llvm::errorCodeToError(llvm::errnoAsErrorCode());
+ }
+
+ return llvm::Error::success();
+}
+
+void DAP::StopIO() {
+ out.Stop();
+ err.Stop();
+}
+
// Send the JSON in "json_str" to the "out" stream. Correctly send the
// "Content-Length:" field followed by the length, followed by the raw
// JSON bytes.
@@ -208,19 +261,19 @@ std::string DAP::ReadJSON() {
std::string json_str;
int length;
- if (!input.read_expected(log.get(), "Content-Length: "))
+ if (!input.read_expected(log, "Content-Length: "))
return json_str;
- if (!input.read_line(log.get(), length_str))
+ if (!input.read_line(log, length_str))
return json_str;
if (!llvm::to_integer(length_str, length))
return json_str;
- if (!input.read_expected(log.get(), "\r\n"))
+ if (!input.read_expected(log, "\r\n"))
return json_str;
- if (!input.read_full(log.get(), length, json_str))
+ if (!input.read_full(log, length, json_str))
return json_str;
if (log) {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index ae496236f13369..846300cb945b0d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -9,36 +9,38 @@
#ifndef LLDB_TOOLS_LLDB_DAP_DAP_H
#define LLDB_TOOLS_LLDB_DAP_DAP_H
-#include <cstdio>
-#include <iosfwd>
-#include <map>
-#include <optional>
-#include <thread>
-
-#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/JSON.h"
-#include "llvm/Support/Threading.h"
-#include "llvm/Support/raw_ostream.h"
-
-#include "lldb/API/SBAttachInfo.h"
-#include "lldb/API/SBCommandInterpreter.h"
-#include "lldb/API/SBCommandReturnObject.h"
-#include "lldb/API/SBDebugger.h"
-#include "lldb/API/SBEvent.h"
-#include "lldb/API/SBFormat.h"
-#include "lldb/API/SBLaunchInfo.h"
-#include "lldb/API/SBTarget.h"
-#include "lldb/API/SBThread.h"
-
+#include "DAPForward.h"
#include "ExceptionBreakpoint.h"
#include "FunctionBreakpoint.h"
#include "IOStream.h"
#include "InstructionBreakpoint.h"
+#include "OutputRedirector.h"
#include "ProgressEvent.h"
#include "SourceBreakpoint.h"
+#include "lldb/API/SBBroadcaster.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBFile.h"
+#include "lldb/API/SBFormat.h"
+#include "lldb/API/SBFrame.h"
+#include "lldb/API/SBTarget.h"
+#include "lldb/API/SBThread.h"
+#include "lldb/API/SBValue.h"
+#include "lldb/API/SBValueList.h"
+#include "lldb/lldb-types.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Threading.h"
+#include <map>
+#include <mutex>
+#include <optional>
+#include <thread>
+#include <vector>
#define VARREF_LOCALS (int64_t)1
#define VARREF_GLOBALS (int64_t)2
@@ -138,15 +140,18 @@ struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
struct DAP {
llvm::StringRef debug_adaptor_path;
+ std::ofstream *log;
InputStream input;
OutputStream output;
+ lldb::SBFile in;
+ OutputRedirector out;
+ OutputRedirector err;
lldb::SBDebugger debugger;
lldb::SBTarget target;
Variables variables;
lldb::SBBroadcaster broadcaster;
std::thread event_thread;
std::thread progress_event_thread;
- std::unique_ptr<std::ofstream> log;
llvm::StringMap<SourceBreakpointMap> source_breakpoints;
FunctionBreakpointMap function_breakpoints;
InstructionBreakpointMap instruction_breakpoints;
@@ -198,13 +203,23 @@ struct DAP {
// will contain that expression.
std::string last_nonempty_var_expression;
- DAP(llvm::StringRef path, ReplMode repl_mode);
+ DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
+ StreamDescriptor input, StreamDescriptor output);
~DAP();
DAP(const DAP &rhs) = delete;
void operator=(const DAP &rhs) = delete;
ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
+ /// Redirect stdout and stderr fo the IDE's console output.
+ ///
+ /// Errors in this operation will be printed to the log file and the IDE's
+ /// console output as well.
+ llvm::Error ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr);
+
+ /// Stop the redirected IO threads and associated pipes.
+ void StopIO();
+
// Serialize the JSON value into a string and send the JSON packet to
// the "out" stream.
void SendJSON(const llvm::json::Value &json);
diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h
index 57d5fd458b7165..74889eb2e5a866 100644
--- a/lldb/tools/lldb-dap/IOStream.h
+++ b/lldb/tools/lldb-dap/IOStream.h
@@ -52,6 +52,9 @@ struct StreamDescriptor {
struct InputStream {
StreamDescriptor descriptor;
+ explicit InputStream(StreamDescriptor descriptor)
+ : descriptor(std::move(descriptor)) {}
+
bool read_full(std::ofstream *log, size_t length, std::string &text);
bool read_line(std::ofstream *log, std::string &line);
@@ -62,6 +65,9 @@ struct InputStream {
struct OutputStream {
StreamDescriptor descriptor;
+ explicit OutputStream(StreamDescriptor descriptor)
+ : descriptor(std::move(descriptor)) {}
+
bool write_full(llvm::StringRef str);
};
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp b/lldb/tools/lldb-dap/OutputRedirector.cpp
index 2c2f49569869b4..8fcbcfec99c443 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===/
+#include "llvm/Support/Error.h"
+#include <system_error>
#if defined(_WIN32)
#include <fcntl.h>
#include <io.h>
@@ -17,47 +19,59 @@
#include "OutputRedirector.h"
#include "llvm/ADT/StringRef.h"
-using namespace llvm;
+using lldb_private::Pipe;
+using lldb_private::Status;
+using llvm::createStringError;
+using llvm::Error;
+using llvm::Expected;
+using llvm::StringRef;
namespace lldb_dap {
-Error RedirectFd(int fd, std::function<void(llvm::StringRef)> callback) {
- int new_fd[2];
-#if defined(_WIN32)
- if (_pipe(new_fd, 4096, O_TEXT) == -1) {
-#else
- if (pipe(new_fd) == -1) {
-#endif
- int error = errno;
- return createStringError(inconvertibleErrorCode(),
- "Couldn't create new pipe for fd %d. %s", fd,
- strerror(error));
- }
+Expected<int> OutputRedirector::GetWriteFileDescriptor() {
+ if (!m_pipe.CanWrite())
+ return createStringError(std::errc::bad_file_descriptor,
+ "write handle is not open for writing");
+ return m_pipe.GetWriteFileDescriptor();
+}
- if (dup2(new_fd[1], fd) == -1) {
- int error = errno;
- return createStringError(inconvertibleErrorCode(),
- "Couldn't override the fd %d. %s", fd,
- strerror(error));
- }
+Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
+ Status status = m_pipe.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+ return status.takeError();
- int read_fd = new_fd[0];
- std::thread t([read_fd, callback]() {
+ m_forwarder = std::thread([this, callback]() {
char buffer[OutputBufferSize];
- while (true) {
- ssize_t bytes_count = read(read_fd, &buffer, sizeof(buffer));
- if (bytes_count == 0)
- return;
- if (bytes_count == -1) {
- if (errno == EAGAIN || errno == EINTR)
- continue;
+ while (m_pipe.CanRead() && !m_stopped) {
+ size_t bytes_read;
+ Status status = m_pipe.Read(&buffer, sizeof(buffer), bytes_read);
+ if (status.Fail())
+ continue;
+
+ // EOF detected
+ if (bytes_read == 0 || m_stopped)
break;
- }
- callback(StringRef(buffer, bytes_count));
+
+ callback(StringRef(buffer, bytes_read));
}
});
- t.detach();
+
return Error::success();
}
+void OutputRedirector::Stop() {
+ m_stopped = true;
+
+ if (m_pipe.CanWrite()) {
+ // 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";
+ size_t bytes_written;
+ m_pipe.Write(buf, sizeof(buf), bytes_written);
+ m_pipe.CloseWriteFileDescriptor();
+ m_forwarder.join();
+ }
+}
+
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/OutputRedirector.h b/lldb/tools/lldb-dap/OutputRedirector.h
index e26d1648b104f9..41ea05c22c6919 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.h
+++ b/lldb/tools/lldb-dap/OutputRedirector.h
@@ -9,17 +9,39 @@
#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>
+#include <functional>
+#include <thread>
namespace lldb_dap {
-/// Redirects the output of a given file descriptor to a callback.
-///
-/// \return
-/// \a Error::success if the redirection was set up correctly, or an error
-/// otherwise.
-llvm::Error RedirectFd(int fd, std::function<void(llvm::StringRef)> callback);
+class OutputRedirector {
+public:
+ /// Creates writable file descriptor that will invoke the given callback on
+ /// each write in a background thread.
+ ///
+ /// \return
+ /// \a Error::success if the redirection was set up correctly, or an error
+ /// otherwise.
+ llvm::Error RedirectTo(std::function<void(llvm::StringRef)> callback);
+
+ llvm::Expected<int> GetWriteFileDescriptor();
+ void Stop();
+
+ ~OutputRedirector() { Stop(); }
+
+ OutputRedirector() = default;
+ OutputRedirector(const OutputRedirector &) = delete;
+ OutputRedirector &operator=(const OutputRedirector &) = delete;
+
+private:
+ std::atomic<bool> m_stopped = false;
+ lldb_private::Pipe m_pipe;
+ std::thread m_forwarder;
+};
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 7e8f7b5f6df679..6c524081c493eb 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -10,10 +10,10 @@
#include "FifoFiles.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
-#include "OutputRedirector.h"
#include "RunInTerminal.h"
#include "Watchpoint.h"
#include "lldb/API/SBDeclaration.h"
+#include "lldb/API/SBEvent.h"
#include "lldb/API/SBInstruction.h"
#include "lldb/API/SBListener.h"
#include "lldb/API/SBMemoryRegionInfo.h"
@@ -41,9 +41,11 @@
#include <cassert>
#include <climits>
#include <cstdarg>
+#include <cstdint>
#include <cstdio>
#include <cstdlib>
#include <cstring>
+#include <fcntl.h>
#include <map>
#include <memory>
#include <optional>
@@ -140,15 +142,14 @@ lldb::SBValueList *GetTopLevelScope(DAP &dap, int64_t variablesReference) {
}
}
-SOCKET AcceptConnection(DAP &dap, int portno) {
+SOCKET AcceptConnection(std::ofstream *log, int portno) {
// Accept a socket connection from any host on "portno".
SOCKET newsockfd = -1;
struct sockaddr_in serv_addr, cli_addr;
SOCKET sockfd = socket(AF_INET, SOCK_STREAM, 0);
if (sockfd < 0) {
- if (dap.log)
- *dap.log << "error: opening socket (" << strerror(errno) << ")"
- << std::endl;
+ if (log)
+ *log << "error: opening socket (" << strerror(errno) << ")" << std::endl;
} else {
memset((char *)&serv_addr, 0, sizeof(serv_addr));
serv_addr.sin_family = AF_INET;
@@ -156,9 +157,9 @@ SOCKET AcceptConnection(DAP &dap, int portno) {
serv_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
serv_addr.sin_port = htons(portno);
if (bind(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
- if (dap.log)
- *dap.log << "error: binding socket (" << strerror(errno) << ")"
- << std::endl;
+ if (log)
+ *log << "error: binding socket (" << strerror(errno) << ")"
+ << std::endl;
} else {
listen(sockfd, 5);
socklen_t clilen = sizeof(cli_addr);
@@ -166,8 +167,8 @@ SOCKET AcceptConnection(DAP &dap, int portno) {
llvm::sys::RetryAfterSignal(static_cast<SOCKET>(-1), accept, sockfd,
(struct sockaddr *)&cli_addr, &clilen);
if (newsockfd < 0)
- if (dap.log)
- *dap.log << "error: accept (" << strerror(errno) << ")" << std::endl;
+ if (log)
+ *log << "error: accept (" << strerror(errno) << ")" << std::endl;
}
#if defined(_WIN32)
closesocket(sockfd);
@@ -1102,6 +1103,7 @@ void request_disconnect(DAP &dap, const llvm::json::Object &request) {
dap.broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread);
dap.progress_event_thread.join();
}
+ dap.StopIO();
dap.disconnecting = true;
}
@@ -1871,7 +1873,36 @@ void request_initialize(DAP &dap, const llvm::json::Object &request) {
// which may affect the outcome of tests.
bool source_init_file = GetBoolean(arguments, "sourceInitFile", true);
- dap.debugger = lldb::SBDebugger::Create(source_init_file);
+ // Do not source init files until in/out/err are configured.
+ dap.debugger = lldb::SBDebugger::Create(false);
+ dap.debugger.SetInputFile(dap.in);
+ auto out_fd = dap.out.GetWriteFileDescriptor();
+ if (llvm::Error err = out_fd.takeError()) {
+ response["success"] = false;
+ EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
+ dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+ dap.debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false));
+ auto err_fd = dap.err.GetWriteFileDescriptor();
+ if (llvm::Error err = err_fd.takeError()) {
+ response["success"] = false;
+ EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
+ dap.SendJSON(llvm::json::Value(std::move(response)));
+ return;
+ }
+ dap.debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false));
+
+ auto interp = dap.debugger.GetCommandInterpreter();
+
+ if (source_init_file) {
+ dap.debugger.SkipLLDBInitFiles(false);
+ dap.debugger.SkipAppInitFiles(false);
+ lldb::SBCommandReturnObject init;
+ interp.SourceInitFileInGlobalDirectory(init);
+ interp.SourceInitFileInHomeDirectory(init);
+ }
+
if (llvm::Error err = dap.RunPreInitCommands()) {
response["success"] = false;
EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
@@ -4910,36 +4941,14 @@ static void redirection_test() {
fflush(stderr);
}
-/// Redirect stdout and stderr fo the IDE's console output.
-///
-/// Errors in this operation will be printed to the log file and the IDE's
-/// console output as well.
-///
-/// \return
-/// A fd pointing to the original stdout.
-static int SetupStdoutStderrRedirection(DAP &dap) {
- int stdoutfd = fileno(stdout);
- int new_stdout_fd = dup(stdoutfd);
- auto output_callback_stderr = [&dap](llvm::StringRef data) {
- dap.SendOutput(OutputType::Stderr, data);
- };
- auto output_callback_stdout = [&dap](llvm::StringRef data) {
- dap.SendOutput(OutputType::Stdout, data);
- };
- if (llvm::Error err = RedirectFd(stdoutfd, output_callback_stdout)) {
- std::string error_message = llvm::toString(std::move(err));
- if (dap.log)
- *dap.log << error_message << std::endl;
- output_callback_stderr(error_message);
- }
- if (llvm::Error err = RedirectFd(fileno(stderr), output_callback_stderr)) {
- std::string error_message = llvm::toString(std::move(err));
- if (dap.log)
- *dap.log << error_message << std::endl;
- output_callback_stderr(error_message);
- }
-
- return new_stdout_fd;
+/// Duplicates a file descriptor, setting FD_CLOEXEC if applicable.
+static int DuplicateFileDescriptor(int fd) {
+#if defined(F_DUPFD_CLOEXEC)
+ // Ensure FD_CLOEXEC is set.
+ return ::fcntl(fd, F_DUPFD_CLOEXEC, 0);
+#else
+ return ::dup(fd);
+#endif
}
int main(int argc, char *argv[]) {
@@ -5030,47 +5039,88 @@ int main(int argc, char *argv[]) {
}
#endif
+ std::unique_ptr<std::ofstream> log = nullptr;
+ const char *log_file_path = getenv("LLDBDAP_LOG");
+ if (log_file_path)
+ log = std::make_unique<std::ofstream>(log_file_path);
+
// Initialize LLDB first before we do anything.
- lldb::SBDebugger::Initialize();
+ lldb::SBError error = lldb::SBDebugger::InitializeWithErrorHandling();
+ if (error.Fail()) {
+ lldb::SBStream os;
+ error.GetDescription(os);
+ llvm::errs() << "lldb initialize failed: " << os.GetData() << "\n";
+ return EXIT_FAILURE;
+ }
// Terminate the debugger before the C++ destructor chain kicks in.
auto terminate_debugger =
llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
- DAP dap = DAP(program_path.str(), default_repl_mode);
-
- RegisterRequestCallbacks(dap);
-
- // stdout/stderr redirection to the IDE's console
- int new_stdout_fd = SetupStdoutStderrRedirection(dap);
-
+ StreamDescriptor input;
+ StreamDescriptor output;
+ std::FILE *redirectOut = nullptr;
+ std::FILE *redirectErr = nullptr;
if (portno != -1) {
printf("Listening on port %i...\n", portno);
- SOCKET socket_fd = AcceptConnection(dap, portno);
- if (socket_fd >= 0) {
- dap.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
- dap.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
- } else {
+ SOCKET socket_fd = AcceptConnection(log.get(), portno);
+ if (socket_fd < 0)
return EXIT_FAILURE;
- }
+
+ input = StreamDescriptor::from_socket(socket_fd, true);
+ output = StreamDescriptor::from_socket(socket_fd, false);
} else {
- dap.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
- dap.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
+#if defined(_WIN32)
+ // Windows opens stdout and stdin in text mode which converts \n to 13,10
+ // while the value is just 10 on Darwin/Linux. Setting the file mode to
+ // binary fixes this.
+ int result = _setmode(fileno(stdout), _O_BINARY);
+ assert(result);
+ result = _setmode(fileno(stdin), _O_BINARY);
+ UNUSED_IF_ASSERT_DISABLED(result);
+ assert(result);
+#endif
- /// used only by TestVSCode_redirection_to_console.py
- if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
- redirection_test();
+ int stdout_fd = DuplicateFileDescriptor(fileno(stdout));
+ if (stdout_fd == -1) {
+ llvm::logAllUnhandledErrors(
+ llvm::errorCodeToError(llvm::errnoAsErrorCode()), llvm::errs(),
+ "Failed to configure stdout redirect: ");
+ return EXIT_FAILURE;
+ }
+
+ redirectOut = stdout;
+ redirectErr = stderr;
+
+ input = StreamDescriptor::from_file(fileno(stdin), false);
+ output = StreamDescriptor::from_file(stdout_fd, false);
+ }
+
+ DAP dap = DAP(program_path.str(), log.get(), default_repl_mode,
+ std::move(input), std::move(output));
+
+ // stdout/stderr redirection to the IDE's console
+ if (auto Err = dap.ConfigureIO(redirectOut, redirectErr)) {
+ llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
+ "Failed to configure lldb-dap IO operations: ");
+ return EXIT_FAILURE;
}
+ RegisterRequestCallbacks(dap);
+
for (const std::string &arg :
input_args.getAllArgValues(OPT_pre_init_command)) {
dap.pre_init_commands.push_back(arg);
}
+ // used only by TestVSCode_redirection_to_console.py
+ if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
+ redirection_test();
+
bool CleanExit = true;
if (auto Err = dap.Loop()) {
- if (dap.log)
- *dap.log << "Transport Error: " << llvm::toString(std::move(Err)) << "\n";
+ if (log)
+ *log << "Transport Error: " << llvm::toString(std::move(Err)) << "\n";
CleanExit = false;
}
>From e4db919a8ef01bcd80bec9352a0cbbd7f09b0d63 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Mon, 13 Jan 2025 10:40:47 -0800
Subject: [PATCH 2/2] Changing to use the previous version of the IO forwarding
operation instead of lldb_private::Pipe and adding more logging to DAP tests.
---
.../test/tools/lldb-dap/lldbdap_testcase.py | 32 ++++++++++--
.../children/TestDAP_variables_children.py | 15 ++----
lldb/tools/lldb-dap/DAP.cpp | 1 -
lldb/tools/lldb-dap/OutputRedirector.cpp | 51 +++++++++++--------
lldb/tools/lldb-dap/OutputRedirector.h | 2 +-
5 files changed, 64 insertions(+), 37 deletions(-)
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 a25466f07fa557..163f5b49084a6d 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
@@ -10,7 +10,8 @@
class DAPTestCaseBase(TestBase):
# set timeout based on whether ASAN was enabled or not. Increase
# timeout by a factor of 10 if ASAN is enabled.
- timeoutval = 10 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
+ timeoutval = 10 * (10 if ("ASAN_OPTIONS" in os.environ) else 1)
+ lldbLogChannels = ["default"]
NO_DEBUG_INFO_TESTCASE = True
def create_debug_adaptor(self, lldbDAPEnv=None):
@@ -18,14 +19,37 @@ def create_debug_adaptor(self, lldbDAPEnv=None):
self.assertTrue(
is_exe(self.lldbDAPExec), "lldb-dap must exist and be executable"
)
- log_file_path = self.getBuildArtifact("dap.txt")
+
+ # DAP communication logs.
+ dap_log = self.getBuildArtifact("dap.txt")
+ # Enable lldb logs to aid in debugging failures.
+ lldb_log = self.getBuildArtifact("lldb.log")
+
+ init_commands = self.setUpCommands()
+ init_commands.append(
+ "log enable -Tpn -f {} lldb {}".format(
+ lldb_log, " ".join(self.lldbLogChannels)
+ )
+ )
+
self.dap_server = dap_server.DebugAdaptorServer(
executable=self.lldbDAPExec,
- init_commands=self.setUpCommands(),
- log_file=log_file_path,
+ init_commands=init_commands,
+ log_file=dap_log,
env=lldbDAPEnv,
)
+ def dump_log():
+ print("========= LLDB LOGS =========")
+ try:
+ with open(lldb_log, "r") as file:
+ print(file.read())
+ except FileNotFoundError:
+ print("no lldb log available")
+ print("========= END =========")
+
+ self.addTearDownHook(dump_log)
+
def build_and_create_debug_adaptor(self, lldbDAPEnv=None):
self.build()
self.create_debug_adaptor(lldbDAPEnv)
diff --git a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py
index 805e88ddf8f70b..345fcd02afd6dd 100644
--- a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py
+++ b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py
@@ -1,13 +1,11 @@
-import os
-
-import dap_server
import lldbdap_testcase
-from lldbsuite.test import lldbutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
class TestDAP_variables_children(lldbdap_testcase.DAPTestCaseBase):
+ lldbLogChannels = ["default", "formatters", "script"]
+
def test_get_num_children(self):
"""Test that GetNumChildren is not called for formatters not producing indexed children."""
program = self.getBuildArtifact("a.out")
@@ -18,16 +16,11 @@ def test_get_num_children(self):
],
)
source = "main.cpp"
- breakpoint1_line = line_number(source, "// break here")
- lines = [breakpoint1_line]
-
- breakpoint_ids = self.set_source_breakpoints(
- source, [line_number(source, "// break here")]
- )
+ line = line_number(source, "// break here")
+ breakpoint_ids = self.set_source_breakpoints(source, [line])
self.continue_to_breakpoints(breakpoint_ids)
local_vars = self.dap_server.get_local_variables()
- print(local_vars)
indexed = next(filter(lambda x: x["name"] == "indexed", local_vars))
not_indexed = next(filter(lambda x: x["name"] == "not_indexed", local_vars))
self.assertIn("indexedVariables", indexed)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index a67abe582abd40..2fe0214842700b 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -17,7 +17,6 @@
#include "lldb/API/SBListener.h"
#include "lldb/API/SBProcess.h"
#include "lldb/API/SBStream.h"
-#include "lldb/Host/FileSystem.h"
#include "lldb/Utility/Status.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp b/lldb/tools/lldb-dap/OutputRedirector.cpp
index 8fcbcfec99c443..2e2c2158381dc5 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -6,6 +6,9 @@
//
//===----------------------------------------------------------------------===/
+#include "OutputRedirector.h"
+#include "DAP.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include <system_error>
#if defined(_WIN32)
@@ -15,10 +18,6 @@
#include <unistd.h>
#endif
-#include "DAP.h"
-#include "OutputRedirector.h"
-#include "llvm/ADT/StringRef.h"
-
using lldb_private::Pipe;
using lldb_private::Status;
using llvm::createStringError;
@@ -29,28 +28,40 @@ using llvm::StringRef;
namespace lldb_dap {
Expected<int> OutputRedirector::GetWriteFileDescriptor() {
- if (!m_pipe.CanWrite())
+ if (m_fd == -1)
return createStringError(std::errc::bad_file_descriptor,
"write handle is not open for writing");
- return m_pipe.GetWriteFileDescriptor();
+
+ return m_fd;
}
Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
- Status status = m_pipe.CreateNew(/*child_process_inherit=*/false);
- if (status.Fail())
- return status.takeError();
+ int new_fd[2] = {0};
+#if defined(_WIN32)
+ if (::_pipe(new_fd, 4096, O_TEXT) == -1) {
+#else
+ if (::pipe(new_fd) == -1) {
+#endif
+ int error = errno;
+ return createStringError(llvm::inconvertibleErrorCode(),
+ "Couldn't create new pipe: %s", strerror(error));
+ }
- m_forwarder = std::thread([this, callback]() {
- char buffer[OutputBufferSize];
- while (m_pipe.CanRead() && !m_stopped) {
- size_t bytes_read;
- Status status = m_pipe.Read(&buffer, sizeof(buffer), bytes_read);
- if (status.Fail())
- continue;
+ int read_fd = new_fd[0];
+ m_fd = new_fd[1];
+ m_forwarder = std::thread([this, read_fd, callback]() {
+ char buffer[OutputBufferSize];
+ while (!m_stopped) {
+ ssize_t bytes_read = ::read(read_fd, &buffer, sizeof(buffer));
// EOF detected
if (bytes_read == 0 || m_stopped)
break;
+ if (bytes_read == -1) {
+ if (errno == EAGAIN || errno == EINTR)
+ continue;
+ break;
+ }
callback(StringRef(buffer, bytes_read));
}
@@ -62,14 +73,14 @@ Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
void OutputRedirector::Stop() {
m_stopped = true;
- if (m_pipe.CanWrite()) {
+ if (m_fd != -1) {
// 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";
- size_t bytes_written;
- m_pipe.Write(buf, sizeof(buf), bytes_written);
- m_pipe.CloseWriteFileDescriptor();
+ ::write(m_fd, buf, sizeof(buf));
+ ::close(m_fd);
+ m_fd = -1;
m_forwarder.join();
}
}
diff --git a/lldb/tools/lldb-dap/OutputRedirector.h b/lldb/tools/lldb-dap/OutputRedirector.h
index 41ea05c22c6919..c948b7ac854b55 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.h
+++ b/lldb/tools/lldb-dap/OutputRedirector.h
@@ -39,7 +39,7 @@ class OutputRedirector {
private:
std::atomic<bool> m_stopped = false;
- lldb_private::Pipe m_pipe;
+ std::atomic<int> m_fd = -1;
std::thread m_forwarder;
};
More information about the lldb-commits
mailing list