[Lldb-commits] [lldb] 58d28b9 - [lldb] [lldb-gdbserver] Unify listen/connect code to use ConnectionFileDescriptor
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 26 04:06:30 PDT 2021
Author: Michał Górny
Date: 2021-10-26T13:06:19+02:00
New Revision: 58d28b931f92f5ea2a6a01e088b794ee6ebd05e7
URL: https://github.com/llvm/llvm-project/commit/58d28b931f92f5ea2a6a01e088b794ee6ebd05e7
DIFF: https://github.com/llvm/llvm-project/commit/58d28b931f92f5ea2a6a01e088b794ee6ebd05e7.diff
LOG: [lldb] [lldb-gdbserver] Unify listen/connect code to use ConnectionFileDescriptor
Unify the listen and connect code inside lldb-server to use
ConnectionFileDescriptor uniformly rather than a mix of it and Acceptor.
This involves:
- adding a function to map legacy values of host:port parameter
(including legacy server URLs) into CFD-style URLs
- adding a callback to return "local socket id" (i.e. UNIX socket path
or TCP port number) between listen() and accept() calls in CFD
- adding a "unix-abstract-accept" scheme to CFD
As an additional advantage, this permits lldb-server to accept any URL
known to CFD including the new serial:// scheme. Effectively,
lldb-server can now listen on the serial port. Tests for connecting
over a pty are added to test that.
Differential Revision: https://reviews.llvm.org/D111964
Added:
lldb/test/API/tools/lldb-server/TestPtyServer.py
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp
Modified:
lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/tools/lldb-server/lldb-gdbserver.cpp
lldb/unittests/Process/gdb-remote/CMakeLists.txt
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 7500ec63707b..6d89fd710ab4 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -28,6 +28,9 @@ class SocketAddress;
class ConnectionFileDescriptor : public Connection {
public:
+ typedef llvm::function_ref<void(llvm::StringRef local_socket_id)>
+ socket_id_callback_type;
+
ConnectionFileDescriptor(bool child_processes_inherit = false);
ConnectionFileDescriptor(int fd, bool owns_fd);
@@ -38,7 +41,12 @@ class ConnectionFileDescriptor : public Connection {
bool IsConnected() const override;
- lldb::ConnectionStatus Connect(llvm::StringRef s, Status *error_ptr) override;
+ lldb::ConnectionStatus Connect(llvm::StringRef url,
+ Status *error_ptr) override;
+
+ lldb::ConnectionStatus Connect(llvm::StringRef url,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
lldb::ConnectionStatus Disconnect(Status *error_ptr) override;
@@ -67,29 +75,51 @@ class ConnectionFileDescriptor : public Connection {
void CloseCommandPipe();
- lldb::ConnectionStatus SocketListenAndAccept(llvm::StringRef host_and_port,
- Status *error_ptr);
+ lldb::ConnectionStatus
+ SocketListenAndAccept(llvm::StringRef host_and_port,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
lldb::ConnectionStatus ConnectTCP(llvm::StringRef host_and_port,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
+
+ lldb::ConnectionStatus ConnectUDP(llvm::StringRef args,
+ socket_id_callback_type socket_id_callback,
Status *error_ptr);
- lldb::ConnectionStatus ConnectUDP(llvm::StringRef args, Status *error_ptr);
+ lldb::ConnectionStatus
+ NamedSocketConnect(llvm::StringRef socket_name,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
- lldb::ConnectionStatus NamedSocketConnect(llvm::StringRef socket_name,
- Status *error_ptr);
+ lldb::ConnectionStatus
+ NamedSocketAccept(llvm::StringRef socket_name,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
- lldb::ConnectionStatus NamedSocketAccept(llvm::StringRef socket_name,
- Status *error_ptr);
+ lldb::ConnectionStatus
+ UnixAbstractSocketAccept(llvm::StringRef socket_name,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
- lldb::ConnectionStatus UnixAbstractSocketConnect(llvm::StringRef socket_name,
- Status *error_ptr);
+ lldb::ConnectionStatus
+ UnixAbstractSocketConnect(llvm::StringRef socket_name,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
- lldb::ConnectionStatus ConnectFD(llvm::StringRef args, Status *error_ptr);
+ lldb::ConnectionStatus ConnectFD(llvm::StringRef args,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
- lldb::ConnectionStatus ConnectFile(llvm::StringRef args, Status *error_ptr);
+ lldb::ConnectionStatus ConnectFile(llvm::StringRef args,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
- lldb::ConnectionStatus ConnectSerialPort(llvm::StringRef args,
- Status *error_ptr);
+ lldb::ConnectionStatus
+ ConnectSerialPort(llvm::StringRef args,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr);
lldb::IOObjectSP m_io_sp;
@@ -103,7 +133,6 @@ class ConnectionFileDescriptor : public Connection {
std::atomic<bool> m_shutting_down; // This marks that we are shutting down so
// if we get woken up from
// BytesAvailable to disconnect, we won't try to read again.
- bool m_waiting_for_accept = false;
bool m_child_processes_inherit;
std::string m_uri;
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 8e8ff8a4e3a0..fdbf660a1527 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -13,10 +13,10 @@
#define _DARWIN_UNLIMITED_SELECT
#endif
-#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
#include "lldb/Host/Config.h"
#include "lldb/Host/Socket.h"
#include "lldb/Host/SocketAddress.h"
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
#include "lldb/Utility/SelectHelper.h"
#include "lldb/Utility/Timeout.h"
@@ -62,7 +62,7 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(bool child_processes_inherit)
ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd)
: Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
- m_waiting_for_accept(false), m_child_processes_inherit(false) {
+ m_child_processes_inherit(false) {
m_io_sp =
std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, owns_fd);
@@ -77,7 +77,7 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd)
ConnectionFileDescriptor::ConnectionFileDescriptor(Socket *socket)
: Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
- m_waiting_for_accept(false), m_child_processes_inherit(false) {
+ m_child_processes_inherit(false) {
InitializeSocket(socket);
}
@@ -124,6 +124,13 @@ bool ConnectionFileDescriptor::IsConnected() const {
ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
Status *error_ptr) {
+ return Connect(path, nullptr, error_ptr);
+}
+
+ConnectionStatus
+ConnectionFileDescriptor::Connect(llvm::StringRef path,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
LLDB_LOGF(log, "%p ConnectionFileDescriptor::Connect (url = '%s')",
@@ -143,10 +150,12 @@ ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
if (!path.empty()) {
auto method =
llvm::StringSwitch<ConnectionStatus (ConnectionFileDescriptor::*)(
- llvm::StringRef, Status *)>(scheme)
+ llvm::StringRef, socket_id_callback_type, Status *)>(scheme)
.Case("listen", &ConnectionFileDescriptor::SocketListenAndAccept)
.Cases("accept", "unix-accept",
&ConnectionFileDescriptor::NamedSocketAccept)
+ .Case("unix-abstract-accept",
+ &ConnectionFileDescriptor::UnixAbstractSocketAccept)
.Cases("connect", "tcp-connect",
&ConnectionFileDescriptor::ConnectTCP)
.Case("udp", &ConnectionFileDescriptor::ConnectUDP)
@@ -161,7 +170,7 @@ ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
.Default(nullptr);
if (method)
- return (this->*method)(path, error_ptr);
+ return (this->*method)(path, socket_id_callback, error_ptr);
}
if (error_ptr)
@@ -498,7 +507,8 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
// data from that pipe:
char c;
- ssize_t bytes_read = llvm::sys::RetryAfterSignal(-1, ::read, pipe_fd, &c, 1);
+ ssize_t bytes_read =
+ llvm::sys::RetryAfterSignal(-1, ::read, pipe_fd, &c, 1);
assert(bytes_read == 1);
(void)bytes_read;
switch (c) {
@@ -522,24 +532,36 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
return eConnectionStatusLostConnection;
}
-ConnectionStatus
-ConnectionFileDescriptor::NamedSocketAccept(llvm::StringRef socket_name,
- Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::NamedSocketAccept(
+ llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
+ Status error;
+ std::unique_ptr<Socket> listen_socket = Socket::Create(
+ Socket::ProtocolUnixDomain, m_child_processes_inherit, error);
Socket *socket = nullptr;
- Status error =
- Socket::UnixDomainAccept(socket_name, m_child_processes_inherit, socket);
+
+ if (!error.Fail())
+ error = listen_socket->Listen(socket_name, 5);
+
+ if (!error.Fail()) {
+ socket_id_callback(socket_name);
+ error = listen_socket->Accept(socket);
+ }
+
+ if (!error.Fail()) {
+ m_io_sp.reset(socket);
+ m_uri.assign(socket_name.str());
+ return eConnectionStatusSuccess;
+ }
+
if (error_ptr)
*error_ptr = error;
- m_io_sp.reset(socket);
- if (error.Fail())
- return eConnectionStatusError;
- m_uri.assign(std::string(socket_name));
- return eConnectionStatusSuccess;
+ return eConnectionStatusError;
}
-ConnectionStatus
-ConnectionFileDescriptor::NamedSocketConnect(llvm::StringRef socket_name,
- Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::NamedSocketConnect(
+ llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
Socket *socket = nullptr;
Status error =
Socket::UnixDomainConnect(socket_name, m_child_processes_inherit, socket);
@@ -552,9 +574,37 @@ ConnectionFileDescriptor::NamedSocketConnect(llvm::StringRef socket_name,
return eConnectionStatusSuccess;
}
-lldb::ConnectionStatus
-ConnectionFileDescriptor::UnixAbstractSocketConnect(llvm::StringRef socket_name,
- Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketAccept(
+ llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
+ Status error;
+ std::unique_ptr<Socket> listen_socket = Socket::Create(
+ Socket::ProtocolUnixAbstract, m_child_processes_inherit, error);
+ Socket *socket = nullptr;
+
+ if (!error.Fail())
+ error = listen_socket->Listen(socket_name, 5);
+
+ if (!error.Fail())
+ socket_id_callback(socket_name);
+
+ if (!error.Fail())
+ error = listen_socket->Accept(socket);
+
+ if (!error.Fail()) {
+ m_io_sp.reset(socket);
+ m_uri.assign(socket_name.str());
+ return eConnectionStatusSuccess;
+ }
+
+ if (error_ptr)
+ *error_ptr = error;
+ return eConnectionStatusError;
+}
+
+lldb::ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketConnect(
+ llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
Socket *socket = nullptr;
Status error = Socket::UnixAbstractConnect(socket_name,
m_child_processes_inherit, socket);
@@ -567,14 +617,13 @@ ConnectionFileDescriptor::UnixAbstractSocketConnect(llvm::StringRef socket_name,
return eConnectionStatusSuccess;
}
-ConnectionStatus
-ConnectionFileDescriptor::SocketListenAndAccept(llvm::StringRef s,
- Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::SocketListenAndAccept(
+ llvm::StringRef s, socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
if (error_ptr)
*error_ptr = Status();
m_port_predicate.SetValue(0, eBroadcastNever);
- m_waiting_for_accept = true;
llvm::Expected<std::unique_ptr<TCPSocket>> listening_socket =
Socket::TcpListen(s, m_child_processes_inherit, &m_port_predicate);
if (!listening_socket) {
@@ -586,6 +635,8 @@ ConnectionFileDescriptor::SocketListenAndAccept(llvm::StringRef s,
return eConnectionStatusError;
}
+ uint16_t port = listening_socket.get()->GetLocalPortNumber();
+ socket_id_callback(std::to_string(port));
Socket *accepted_socket;
Status error = listening_socket.get()->Accept(accepted_socket);
@@ -598,8 +649,10 @@ ConnectionFileDescriptor::SocketListenAndAccept(llvm::StringRef s,
return eConnectionStatusSuccess;
}
-ConnectionStatus ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
- Status *error_ptr) {
+ConnectionStatus
+ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
if (error_ptr)
*error_ptr = Status();
@@ -618,8 +671,10 @@ ConnectionStatus ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
return eConnectionStatusSuccess;
}
-ConnectionStatus ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
- Status *error_ptr) {
+ConnectionStatus
+ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
if (error_ptr)
*error_ptr = Status();
llvm::Expected<std::unique_ptr<UDPSocket>> socket =
@@ -637,8 +692,10 @@ ConnectionStatus ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
return eConnectionStatusSuccess;
}
-ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
- Status *error_ptr) {
+ConnectionStatus
+ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
+ socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
#if LLDB_ENABLE_POSIX
// Just passing a native file descriptor within this current process that
// is already opened (possibly from a service or other source).
@@ -691,8 +748,9 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
}
-ConnectionStatus ConnectionFileDescriptor::ConnectFile(llvm::StringRef s,
- Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::ConnectFile(
+ llvm::StringRef s, socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
#if LLDB_ENABLE_POSIX
std::string addr_str = s.str();
// file:///PATH
@@ -722,16 +780,15 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFile(llvm::StringRef s,
llvm::sys::RetryAfterSignal(-1, ::tcsetattr, fd, TCSANOW, &options);
}
- m_io_sp =
- std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, true);
+ m_io_sp = std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, true);
return eConnectionStatusSuccess;
#endif // LLDB_ENABLE_POSIX
llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
}
-ConnectionStatus
-ConnectionFileDescriptor::ConnectSerialPort(llvm::StringRef s,
- Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::ConnectSerialPort(
+ llvm::StringRef s, socket_id_callback_type socket_id_callback,
+ Status *error_ptr) {
#if LLDB_ENABLE_POSIX
llvm::StringRef path, qs;
// serial:///PATH?k1=v1&k2=v2...
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index b1e9e3cb4415..27acf7e204fc 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -25,6 +25,7 @@
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Host/PosixApi.h"
+#include "lldb/Host/Socket.h"
#include "lldb/Host/common/NativeProcessProtocol.h"
#include "lldb/Host/common/NativeRegisterContext.h"
#include "lldb/Host/common/NativeThreadProtocol.h"
@@ -39,6 +40,7 @@
#include "lldb/Utility/State.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/UnimplementedError.h"
+#include "lldb/Utility/UriParser.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -3888,3 +3890,42 @@ void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
assert(!bool(flags & ~m_process_factory.GetSupportedExtensions()));
process.SetEnabledExtensions(flags);
}
+
+std::string
+lldb_private::process_gdb_remote::LLGSArgToURL(llvm::StringRef url_arg,
+ bool reverse_connect) {
+ // Try parsing the argument as URL.
+ if (llvm::Optional<URI> url = URI::Parse(url_arg)) {
+ if (reverse_connect)
+ return url_arg.str();
+
+ // Translate the scheme from LLGS notation to ConnectionFileDescriptor.
+ // If the scheme doesn't match any, pass it through to support using CFD
+ // schemes directly.
+ std::string new_url = llvm::StringSwitch<std::string>(url->scheme)
+ .Case("tcp", "listen")
+ .Case("unix", "unix-accept")
+ .Case("unix-abstract", "unix-abstract-accept")
+ .Default(url->scheme.str());
+ llvm::append_range(new_url, url_arg.substr(url->scheme.size()));
+ return new_url;
+ }
+
+ std::string host_port = url_arg.str();
+ // If host_and_port starts with ':', default the host to be "localhost" and
+ // expect the remainder to be the port.
+ if (url_arg.startswith(":"))
+ host_port.insert(0, "localhost");
+
+ std::string host_str;
+ std::string port_str;
+ uint16_t port;
+ // Try parsing the (preprocessed) argument as host:port pair.
+ if (!llvm::errorToBool(
+ Socket::DecodeHostAndPort(host_port, host_str, port_str, port)))
+ return (reverse_connect ? "connect://" : "listen://") + host_port;
+
+ // If none of the above applied, interpret the argument as UNIX socket path.
+ return (reverse_connect ? "unix-connect://" : "unix-accept://") +
+ url_arg.str();
+}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index 89c0fd8d7b7f..6c75771f6427 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -289,6 +289,8 @@ class GDBRemoteCommunicationServerLLGS
operator=(const GDBRemoteCommunicationServerLLGS &) = delete;
};
+std::string LLGSArgToURL(llvm::StringRef url_arg, bool reverse_connect);
+
} // namespace process_gdb_remote
} // namespace lldb_private
diff --git a/lldb/test/API/tools/lldb-server/TestPtyServer.py b/lldb/test/API/tools/lldb-server/TestPtyServer.py
new file mode 100644
index 000000000000..5a12f565d9e4
--- /dev/null
+++ b/lldb/test/API/tools/lldb-server/TestPtyServer.py
@@ -0,0 +1,73 @@
+import gdbremote_testcase
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbgdbserverutils import *
+
+import xml.etree.ElementTree as ET
+
+
+ at skipIfWindows
+class PtyServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
+ mydir = TestBase.compute_mydir(__file__)
+
+ def setUp(self):
+ super().setUp()
+ import pty
+ import tty
+ master, slave = pty.openpty()
+ tty.setraw(master)
+ self._master = io.FileIO(master, 'r+b')
+ self._slave = io.FileIO(slave, 'r+b')
+
+ def get_debug_monitor_command_line_args(self, attach_pid=None):
+ commandline_args = self.debug_monitor_extra_args
+ if attach_pid:
+ commandline_args += ["--attach=%d" % attach_pid]
+
+ libc = ctypes.CDLL(None)
+ libc.ptsname.argtypes = (ctypes.c_int,)
+ libc.ptsname.restype = ctypes.c_char_p
+ pty_path = libc.ptsname(self._master.fileno()).decode()
+ commandline_args += ["serial://%s" % (pty_path,)]
+ return commandline_args
+
+ def connect_to_debug_monitor(self, attach_pid=None):
+ self.reverse_connect = False
+ server = self.launch_debug_monitor(attach_pid=attach_pid)
+ self.assertIsNotNone(server)
+
+ # TODO: make it into proper abstraction
+ class FakeSocket:
+ def __init__(self, fd):
+ self.fd = fd
+
+ def sendall(self, frame):
+ self.fd.write(frame)
+
+ def recv(self, count):
+ return self.fd.read(count)
+
+ self.sock = FakeSocket(self._master)
+ self._server = Server(self.sock, server)
+ return server
+
+ @add_test_categories(["llgs"])
+ def test_pty_server(self):
+ self.build()
+ self.set_inferior_startup_launch()
+ self.prep_debug_monitor_and_inferior()
+
+ # target.xml transfer should trigger a large enough packet to check
+ # for partial write regression
+ self.test_sequence.add_log_lines([
+ "read packet: $qXfer:features:read:target.xml:0,200000#00",
+ {
+ "direction": "send",
+ "regex": re.compile("^\$l(.+)#[0-9a-fA-F]{2}$"),
+ "capture": {1: "target_xml"},
+ }],
+ True)
+ context = self.expect_gdbremote_sequence()
+ # verify that we have received a complete, non-malformed XML
+ self.assertIsNotNone(ET.fromstring(context.get("target_xml")))
diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp
index dce7905e55c7..906ae4c378b6 100644
--- a/lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -17,7 +17,6 @@
#include <unistd.h>
#endif
-#include "Acceptor.h"
#include "LLDBServerUtilities.h"
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h"
#include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
@@ -164,15 +163,14 @@ void handle_launch(GDBRemoteCommunicationServerLLGS &gdb_server,
}
}
-Status writeSocketIdToPipe(Pipe &port_pipe, const std::string &socket_id) {
+Status writeSocketIdToPipe(Pipe &port_pipe, llvm::StringRef socket_id) {
size_t bytes_written = 0;
// Write the port number as a C string with the NULL terminator.
- return port_pipe.Write(socket_id.c_str(), socket_id.size() + 1,
- bytes_written);
+ return port_pipe.Write(socket_id.data(), socket_id.size() + 1, bytes_written);
}
Status writeSocketIdToPipe(const char *const named_pipe_path,
- const std::string &socket_id) {
+ llvm::StringRef socket_id) {
Pipe port_name_pipe;
// Wait for 10 seconds for pipe to be opened.
auto error = port_name_pipe.OpenAsWriterWithTimeout(named_pipe_path, false,
@@ -183,7 +181,7 @@ Status writeSocketIdToPipe(const char *const named_pipe_path,
}
Status writeSocketIdToPipe(lldb::pipe_t unnamed_pipe,
- const std::string &socket_id) {
+ llvm::StringRef socket_id) {
Pipe port_pipe{LLDB_INVALID_PIPE, unnamed_pipe};
return writeSocketIdToPipe(port_pipe, socket_id);
}
@@ -197,120 +195,69 @@ void ConnectToRemote(MainLoop &mainloop,
Status error;
std::unique_ptr<Connection> connection_up;
+ std::string url;
+
if (connection_fd != -1) {
- // Build the connection string.
- std::string connection_url = llvm::formatv("fd://{0}", connection_fd).str();
+ url = llvm::formatv("fd://{0}", connection_fd).str();
// Create the connection.
#if LLDB_ENABLE_POSIX && !defined _WIN32
::fcntl(connection_fd, F_SETFD, FD_CLOEXEC);
#endif
- connection_up.reset(new ConnectionFileDescriptor);
- auto connection_result = connection_up->Connect(connection_url, &error);
+ } else if (!host_and_port.empty()) {
+ llvm::Expected<std::string> url_exp =
+ LLGSArgToURL(host_and_port, reverse_connect);
+ if (!url_exp) {
+ llvm::errs() << llvm::formatv("error: invalid host:port or URL '{0}': "
+ "{1}\n",
+ host_and_port,
+ llvm::toString(url_exp.takeError()));
+ exit(-1);
+ }
+
+ url = std::move(url_exp.get());
+ }
+
+ if (!url.empty()) {
+ // Create the connection or server.
+ std::unique_ptr<ConnectionFileDescriptor> conn_fd_up{
+ new ConnectionFileDescriptor};
+ auto connection_result = conn_fd_up->Connect(
+ url,
+ [named_pipe_path, unnamed_pipe](llvm::StringRef socket_id) {
+ // If we have a named pipe to write the socket id back to, do that
+ // now.
+ if (named_pipe_path && named_pipe_path[0]) {
+ Status error = writeSocketIdToPipe(named_pipe_path, socket_id);
+ if (error.Fail())
+ llvm::errs() << llvm::formatv(
+ "failed to write to the named peipe '{0}': {1}\n",
+ named_pipe_path, error.AsCString());
+ }
+ // If we have an unnamed pipe to write the socket id back to, do
+ // that now.
+ else if (unnamed_pipe != LLDB_INVALID_PIPE) {
+ Status error = writeSocketIdToPipe(unnamed_pipe, socket_id);
+ if (error.Fail())
+ llvm::errs() << llvm::formatv(
+ "failed to write to the unnamed pipe: {0}\n", error);
+ }
+ },
+ &error);
+
if (error.Fail()) {
llvm::errs() << llvm::formatv(
- "error: failed to connect to client at '{0}': {1}\n", connection_url,
- error);
+ "error: failed to connect to client at '{0}': {1}\n", url, error);
exit(-1);
}
if (connection_result != eConnectionStatusSuccess) {
llvm::errs() << llvm::formatv(
"error: failed to connect to client at '{0}' "
"(connection status: {1})\n",
- connection_url, static_cast<int>(connection_result));
+ url, static_cast<int>(connection_result));
exit(-1);
}
- } else if (!host_and_port.empty()) {
- // Parse out host and port.
- std::string final_host_and_port;
-
- // If host_and_port starts with ':', default the host to be "localhost" and
- // expect the remainder to be the port.
- if (host_and_port[0] == ':')
- final_host_and_port.append("localhost");
- final_host_and_port.append(host_and_port.str());
-
- if (reverse_connect) {
- // llgs will connect to the gdb-remote client.
-
- // Ensure we have a port number for the connection.
- // Note: use rfind, because the host/port may look like "[::1]:12345".
- uint32_t connection_portno = 0;
- const std::string::size_type colon_pos = final_host_and_port.rfind(':');
- if (colon_pos != std::string::npos)
- llvm::to_integer(final_host_and_port.substr(colon_pos + 1),
- connection_portno);
- if (connection_portno == 0) {
- llvm::errs() << "error: port number must be specified on when using "
- "reverse connect\n";
- exit(1);
- }
-
- // Build the connection string.
- final_host_and_port.insert(0, "connect://");
-
- // Create the connection.
- connection_up.reset(new ConnectionFileDescriptor);
- auto connection_result =
- connection_up->Connect(final_host_and_port, &error);
- if (error.Fail()) {
- llvm::errs() << llvm::formatv(
- "error: failed to connect to client at '{0}': {1}\n",
- final_host_and_port, error);
- exit(-1);
- }
- if (connection_result != eConnectionStatusSuccess) {
- llvm::errs() << llvm::formatv(
- "error: failed to connect to client at '{0}' "
- "(connection status: {1})\n",
- final_host_and_port, static_cast<int>(connection_result));
- exit(-1);
- }
- } else {
- std::unique_ptr<Acceptor> acceptor_up(
- Acceptor::Create(final_host_and_port, false, error));
- if (error.Fail()) {
- llvm::errs() << llvm::formatv("failed to create acceptor: {0}\n",
- error);
- exit(1);
- }
- error = acceptor_up->Listen(1);
- if (error.Fail()) {
- llvm::errs() << llvm::formatv("failed to listen: {0}\n", error);
- exit(1);
- }
- const std::string socket_id = acceptor_up->GetLocalSocketId();
- if (!socket_id.empty()) {
- // If we have a named pipe to write the socket id back to, do that now.
- if (named_pipe_path && named_pipe_path[0]) {
- error = writeSocketIdToPipe(named_pipe_path, socket_id);
- if (error.Fail())
- llvm::errs() << llvm::formatv(
- "failed to write to the named peipe '{0}': {1}\n",
- named_pipe_path, error.AsCString());
- }
- // If we have an unnamed pipe to write the socket id back to, do that
- // now.
- else if (unnamed_pipe != LLDB_INVALID_PIPE) {
- error = writeSocketIdToPipe(unnamed_pipe, socket_id);
- if (error.Fail())
- llvm::errs() << llvm::formatv(
- "failed to write to the unnamed pipe: {0}\n", error);
- }
- } else {
- llvm::errs()
- << "unable to get the socket id for the listening connection\n";
- }
-
- Connection *conn = nullptr;
- error = acceptor_up->Accept(false, conn);
- if (error.Fail()) {
- llvm::errs() << llvm::formatv("failed to accept new connection: {0}\n",
- error);
- exit(1);
- }
- connection_up.reset(conn);
- }
+ connection_up = std::move(conn_fd_up);
}
error = gdb_server.InitializeConnection(std::move(connection_up));
if (error.Fail()) {
diff --git a/lldb/unittests/Process/gdb-remote/CMakeLists.txt b/lldb/unittests/Process/gdb-remote/CMakeLists.txt
index 7988fff3e70c..de14dc0169c1 100644
--- a/lldb/unittests/Process/gdb-remote/CMakeLists.txt
+++ b/lldb/unittests/Process/gdb-remote/CMakeLists.txt
@@ -1,6 +1,7 @@
add_lldb_unittest(ProcessGdbRemoteTests
GDBRemoteClientBaseTest.cpp
GDBRemoteCommunicationClientTest.cpp
+ GDBRemoteCommunicationServerLLGSTest.cpp
GDBRemoteCommunicationServerTest.cpp
GDBRemoteCommunicationTest.cpp
GDBRemoteTestUtils.cpp
diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp
new file mode 100644
index 000000000000..23ab9a1b09dc
--- /dev/null
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationServerLLGSTest.cpp
@@ -0,0 +1,63 @@
+//===-- GDBRemoteCommunicationServerLLGSTest.cpp --------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h"
+
+using namespace lldb_private::process_gdb_remote;
+
+TEST(GDBRemoteCommunicationServerLLGSTest, LLGSArgToURL) {
+ // LLGS new-style URLs should be passed through (indepenently of
+ // --reverse-connect)
+ EXPECT_EQ(LLGSArgToURL("listen://127.0.0.1:1234", false),
+ "listen://127.0.0.1:1234");
+ EXPECT_EQ(LLGSArgToURL("listen://127.0.0.1:1234", true),
+ "listen://127.0.0.1:1234");
+ EXPECT_EQ(LLGSArgToURL("connect://127.0.0.1:1234", false),
+ "connect://127.0.0.1:1234");
+ EXPECT_EQ(LLGSArgToURL("connect://127.0.0.1:1234", true),
+ "connect://127.0.0.1:1234");
+
+ // LLGS legacy listen URLs should be converted if !reverse_connect
+ EXPECT_EQ(LLGSArgToURL("tcp://127.0.0.1:1234", false),
+ "listen://127.0.0.1:1234");
+ EXPECT_EQ(LLGSArgToURL("unix:///tmp/foo", false), "unix-accept:///tmp/foo");
+ EXPECT_EQ(LLGSArgToURL("unix-abstract://foo", false),
+ "unix-abstract-accept://foo");
+
+ // LLGS listen host:port pairs should be converted to listen://
+ EXPECT_EQ(LLGSArgToURL("127.0.0.1:1234", false), "listen://127.0.0.1:1234");
+ EXPECT_EQ(LLGSArgToURL("[::1]:1234", false), "listen://[::1]:1234");
+ EXPECT_EQ(LLGSArgToURL("[[::1]:1234]", false), "listen://[[::1]:1234]");
+ EXPECT_EQ(LLGSArgToURL("localhost:1234", false), "listen://localhost:1234");
+ EXPECT_EQ(LLGSArgToURL("*:1234", false), "listen://*:1234");
+
+ // LLGS listen :port special-case should be converted to listen://
+ EXPECT_EQ(LLGSArgToURL(":1234", false), "listen://localhost:1234");
+
+ // LLGS listen UNIX sockets should be converted to unix-accept://
+ EXPECT_EQ(LLGSArgToURL("/tmp/foo", false), "unix-accept:///tmp/foo");
+ EXPECT_EQ(LLGSArgToURL("127.0.0.1", false), "unix-accept://127.0.0.1");
+ EXPECT_EQ(LLGSArgToURL("[::1]", false), "unix-accept://[::1]");
+ EXPECT_EQ(LLGSArgToURL("localhost", false), "unix-accept://localhost");
+ EXPECT_EQ(LLGSArgToURL(":frobnicate", false), "unix-accept://:frobnicate");
+
+ // LLGS reverse connect host:port pairs should be converted to connect://
+ EXPECT_EQ(LLGSArgToURL("127.0.0.1:1234", true), "connect://127.0.0.1:1234");
+ EXPECT_EQ(LLGSArgToURL("[::1]:1234", true), "connect://[::1]:1234");
+ EXPECT_EQ(LLGSArgToURL("[[::1]:1234]", true), "connect://[[::1]:1234]");
+ EXPECT_EQ(LLGSArgToURL("localhost:1234", true), "connect://localhost:1234");
+
+ // with LLGS reverse connect, anything else goes as unix-connect://
+ EXPECT_EQ(LLGSArgToURL("/tmp/foo", true), "unix-connect:///tmp/foo");
+ EXPECT_EQ(LLGSArgToURL("127.0.0.1", true), "unix-connect://127.0.0.1");
+ EXPECT_EQ(LLGSArgToURL("[::1]", true), "unix-connect://[::1]");
+ EXPECT_EQ(LLGSArgToURL("localhost", true), "unix-connect://localhost");
+ EXPECT_EQ(LLGSArgToURL(":frobnicate", true), "unix-connect://:frobnicate");
+}
More information about the lldb-commits
mailing list