[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 25 10:00:41 PST 2025
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 <harjohn at google.com>
Date: Mon, 24 Feb 2025 15:07:13 -0800
Subject: [PATCH 1/2] [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 <harjohn at google.com>
Date: Tue, 25 Feb 2025 10:00:21 -0800
Subject: [PATCH 2/2] 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;
};
More information about the lldb-commits
mailing list