https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/128750
>From b3f60a464deedae59d902f7417c74a64c5cbf5a1 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Mon, 24 Feb 2025 15:07:13 -0800 Subject: [PATCH 1/3] [lldb-dap] Use existing lldb::IOObjectSP for IO (NFC). This simplifies the IOStream.cpp implementation by building on top of the existing lldb::IOObjectSP. Additionally, this should help ensure clients connected of a `--connection` specifier properly detect shutdown requests when the Socket is closed. Previously, the StreamDescriptor was just accessing the underyling native handle and was not aware of the `Close()` call to the Socket itself. --- lldb/tools/lldb-dap/DAP.cpp | 3 +- lldb/tools/lldb-dap/DAP.h | 9 +-- lldb/tools/lldb-dap/IOStream.cpp | 117 ++++--------------------------- lldb/tools/lldb-dap/IOStream.h | 38 ++-------- lldb/tools/lldb-dap/lldb-dap.cpp | 36 +++++----- 5 files changed, 45 insertions(+), 158 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 01f294e14de6a..95f2e5c6521c2 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -17,6 +17,7 @@ #include "lldb/API/SBListener.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" +#include "lldb/Utility/IOObject.h" // IWYU pragma: keep #include "lldb/Utility/Status.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-enumerations.h" @@ -59,7 +60,7 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log, - StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode, + lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector<std::string> pre_init_commands) : name(std::move(name)), debug_adaptor_path(path), log(log), input(std::move(input)), output(std::move(output)), diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index f87841a56f4d3..b4e77b78c5273 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -29,6 +29,7 @@ #include "lldb/API/SBThread.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" +#include "lldb/lldb-forward.h" #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -125,21 +126,21 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; + explicit ReplModeRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d) {}; + explicit SendEventRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; @@ -211,7 +212,7 @@ struct DAP { std::string last_nonempty_var_expression; DAP(std::string name, llvm::StringRef path, std::ofstream *log, - StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode, + lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector<std::string> pre_init_commands); ~DAP(); DAP(const DAP &rhs) = delete; diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp index 7d0f363440f53..d8543b560c050 100644 --- a/lldb/tools/lldb-dap/IOStream.cpp +++ b/lldb/tools/lldb-dap/IOStream.cpp @@ -7,125 +7,34 @@ //===----------------------------------------------------------------------===// #include "IOStream.h" +#include "lldb/Utility/IOObject.h" // IWYU pragma: keep +#include "lldb/Utility/Status.h" #include <fstream> #include <string> -#if defined(_WIN32) -#include <io.h> -#else -#include <netinet/in.h> -#include <sys/socket.h> -#include <unistd.h> -#endif - using namespace lldb_dap; -StreamDescriptor::StreamDescriptor() = default; - -StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) { - *this = std::move(other); -} - -StreamDescriptor::~StreamDescriptor() { - if (!m_close) - return; - - if (m_is_socket) -#if defined(_WIN32) - ::closesocket(m_socket); -#else - ::close(m_socket); -#endif - else - ::close(m_fd); -} - -StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) { - m_close = other.m_close; - other.m_close = false; - m_is_socket = other.m_is_socket; - if (m_is_socket) - m_socket = other.m_socket; - else - m_fd = other.m_fd; - return *this; -} - -StreamDescriptor StreamDescriptor::from_socket(SOCKET s, bool close) { - StreamDescriptor sd; - sd.m_is_socket = true; - sd.m_socket = s; - sd.m_close = close; - return sd; -} - -StreamDescriptor StreamDescriptor::from_file(int fd, bool close) { - StreamDescriptor sd; - sd.m_is_socket = false; - sd.m_fd = fd; - sd.m_close = close; - return sd; -} - bool OutputStream::write_full(llvm::StringRef str) { - while (!str.empty()) { - int bytes_written = 0; - if (descriptor.m_is_socket) - bytes_written = ::send(descriptor.m_socket, str.data(), str.size(), 0); - else - bytes_written = ::write(descriptor.m_fd, str.data(), str.size()); - - if (bytes_written < 0) { - if (errno == EINTR || errno == EAGAIN) - continue; - return false; - } - str = str.drop_front(bytes_written); - } + if (!descriptor) + return false; - return true; + size_t num_bytes = str.size(); + auto status = descriptor->Write(str.data(), num_bytes); + return status.Success(); } bool InputStream::read_full(std::ofstream *log, size_t length, std::string &text) { + if (!descriptor) + return false; + std::string data; data.resize(length); - char *ptr = &data[0]; - while (length != 0) { - int bytes_read = 0; - if (descriptor.m_is_socket) - bytes_read = ::recv(descriptor.m_socket, ptr, length, 0); - else - bytes_read = ::read(descriptor.m_fd, ptr, length); - - if (bytes_read == 0) { - if (log) - *log << "End of file (EOF) reading from input file.\n"; - return false; - } - if (bytes_read < 0) { - int reason = 0; -#if defined(_WIN32) - if (descriptor.m_is_socket) - reason = WSAGetLastError(); - else - reason = errno; -#else - reason = errno; - if (reason == EINTR || reason == EAGAIN) - continue; -#endif - - if (log) - *log << "Error " << reason << " reading from input file.\n"; - return false; - } + auto status = descriptor->Read(data.data(), length); + if (status.Fail()) + return false; - assert(bytes_read >= 0 && (size_t)bytes_read <= length); - ptr += bytes_read; - length -= bytes_read; - } text += data; return true; } diff --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h index c91b2f717893c..e9fb8e11c92da 100644 --- a/lldb/tools/lldb-dap/IOStream.h +++ b/lldb/tools/lldb-dap/IOStream.h @@ -9,45 +9,17 @@ #ifndef LLDB_TOOLS_LLDB_DAP_IOSTREAM_H #define LLDB_TOOLS_LLDB_DAP_IOSTREAM_H -#if defined(_WIN32) -#include "lldb/Host/windows/windows.h" -#include <winsock2.h> -#else -typedef int SOCKET; -#endif - +#include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" - #include <fstream> #include <string> -// Windows requires different system calls for dealing with sockets and other -// types of files, so we can't simply have one code path that just uses read -// and write everywhere. So we need an abstraction in order to allow us to -// treat them identically. namespace lldb_dap { -struct StreamDescriptor { - StreamDescriptor(); - ~StreamDescriptor(); - StreamDescriptor(StreamDescriptor &&other); - - StreamDescriptor &operator=(StreamDescriptor &&other); - - static StreamDescriptor from_socket(SOCKET s, bool close); - static StreamDescriptor from_file(int fd, bool close); - - bool m_is_socket = false; - bool m_close = false; - union { - int m_fd; - SOCKET m_socket; - }; -}; struct InputStream { - StreamDescriptor descriptor; + lldb::IOObjectSP descriptor; - explicit InputStream(StreamDescriptor descriptor) + explicit InputStream(lldb::IOObjectSP descriptor) : descriptor(std::move(descriptor)) {} bool read_full(std::ofstream *log, size_t length, std::string &text); @@ -58,9 +30,9 @@ struct InputStream { }; struct OutputStream { - StreamDescriptor descriptor; + lldb::IOObjectSP descriptor; - explicit OutputStream(StreamDescriptor descriptor) + explicit OutputStream(lldb::IOObjectSP descriptor) : descriptor(std::move(descriptor)) {} bool write_full(llvm::StringRef str); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 3b029b2ef047a..c94faa4958039 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -15,6 +15,7 @@ #include "RunInTerminal.h" #include "lldb/API/SBStream.h" #include "lldb/Host/Config.h" +#include "lldb/Host/File.h" #include "lldb/Host/MainLoop.h" #include "lldb/Host/MainLoopBase.h" #include "lldb/Host/Socket.h" @@ -80,7 +81,11 @@ typedef int socklen_t; #endif using namespace lldb_dap; -using lldb_private::NativeSocket; +using lldb_private::File; +using lldb_private::IOObject; +using lldb_private::MainLoop; +using lldb_private::MainLoopBase; +using lldb_private::NativeFile; using lldb_private::Socket; using lldb_private::Status; @@ -309,14 +314,14 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, // address. llvm::outs().flush(); - static lldb_private::MainLoop g_loop; + static MainLoop g_loop; llvm::sys::SetInterruptFunction([]() { g_loop.AddPendingCallback( - [](lldb_private::MainLoopBase &loop) { loop.RequestTermination(); }); + [](MainLoopBase &loop) { loop.RequestTermination(); }); }); std::condition_variable dap_sessions_condition; std::mutex dap_sessions_mutex; - std::map<Socket *, DAP *> dap_sessions; + std::map<IOObject *, DAP *> dap_sessions; unsigned int clientCount = 0; auto handle = listener->Accept(g_loop, [=, &dap_sessions_condition, &dap_sessions_mutex, &dap_sessions, @@ -330,18 +335,15 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, << " client connected: " << name << "\n"; } + lldb::IOObjectSP io(std::move(sock)); + // Move the client into a background thread to unblock accepting the next // client. std::thread client([=, &dap_sessions_condition, &dap_sessions_mutex, - &dap_sessions, sock = std::move(sock)]() { + &dap_sessions]() { llvm::set_thread_name(name + ".runloop"); - StreamDescriptor input = - StreamDescriptor::from_socket(sock->GetNativeSocket(), false); - // Close the output last for the best chance at error reporting. - StreamDescriptor output = - StreamDescriptor::from_socket(sock->GetNativeSocket(), false); - DAP dap = DAP(name, program_path, log, std::move(input), - std::move(output), default_repl_mode, pre_init_commands); + DAP dap = DAP(name, program_path, log, io, io, default_repl_mode, + pre_init_commands); if (auto Err = dap.ConfigureIO()) { llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), @@ -353,7 +355,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, { std::scoped_lock<std::mutex> lock(dap_sessions_mutex); - dap_sessions[sock.get()] = &dap; + dap_sessions[io.get()] = &dap; } if (auto Err = dap.Loop()) { @@ -369,7 +371,7 @@ serveConnection(const Socket::SocketProtocol &protocol, const std::string &name, } std::unique_lock<std::mutex> lock(dap_sessions_mutex); - dap_sessions.erase(sock.get()); + dap_sessions.erase(io.get()); std::notify_all_at_thread_exit(dap_sessions_condition, std::move(lock)); }); client.detach(); @@ -561,8 +563,10 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - StreamDescriptor input = StreamDescriptor::from_file(fileno(stdin), false); - StreamDescriptor output = StreamDescriptor::from_file(stdout_fd, false); + lldb::IOObjectSP input = std::make_shared<NativeFile>( + fileno(stdin), File::eOpenOptionReadOnly, true); + lldb::IOObjectSP output = std::make_shared<NativeFile>( + stdout_fd, File::eOpenOptionWriteOnly, false); DAP dap = DAP("stdin/stdout", program_path, log.get(), std::move(input), std::move(output), default_repl_mode, pre_init_commands); >From bf4db5440881ddd66916afcb22be6b6788658123 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Tue, 25 Feb 2025 10:00:21 -0800 Subject: [PATCH 2/3] Applying formatting. --- lldb/tools/lldb-dap/DAP.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index b4e77b78c5273..9831308f9e364 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -126,21 +126,21 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d){}; + explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d){}; + explicit SendEventRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; >From 11acbd02cd33331584f5c5448fdffa18390605c5 Mon Sep 17 00:00:00 2001 From: John Harrison <a...@greaterthaninfinity.com> Date: Tue, 25 Feb 2025 13:44:09 -0800 Subject: [PATCH 3/3] Update lldb/tools/lldb-dap/DAP.cpp Co-authored-by: Jonas Devlieghere <jo...@devlieghere.com> --- lldb/tools/lldb-dap/DAP.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 95f2e5c6521c2..c146dcab3e6bb 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -17,7 +17,7 @@ #include "lldb/API/SBListener.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" -#include "lldb/Utility/IOObject.h" // IWYU pragma: keep +#include "lldb/Utility/IOObject.h" #include "lldb/Utility/Status.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-enumerations.h" _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits