-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[lldb-dap] Make connection URLs match lldb #144770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesUse the same scheme as ConnectionFileDescriptor::Connect and use "listen" and "accept". Addresses feedback from a Pavel in a different PR [1]. Full diff: https://github.com/llvm/llvm-project/pull/144770.diff 5 Files Affected:
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 4585eac12efb9..9af0e6f3fef08 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -74,6 +74,11 @@ class Socket : public IOObject {
ProtocolUnixAbstract
};
+ enum SocketMode {
+ ModeAccept,
+ ModeConnect,
+ };
+
struct HostAndPort {
std::string hostname;
uint16_t port;
@@ -83,6 +88,10 @@ class Socket : public IOObject {
}
};
+ using ProtocolModePair = std::pair<SocketProtocol, SocketMode>;
+ static llvm::Expected<ProtocolModePair>
+ GetProtocolAndMode(llvm::StringRef scheme);
+
static const NativeSocket kInvalidSocketValue;
~Socket() override;
diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp
index 76f74401ac4d0..373e700c490ab 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -476,3 +476,28 @@ llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &OS,
const Socket::HostAndPort &HP) {
return OS << '[' << HP.hostname << ']' << ':' << HP.port;
}
+
+llvm::Expected<Socket::ProtocolModePair>
+Socket::GetProtocolAndMode(llvm::StringRef scheme) {
+ // Keep in sync with ConnectionFileDescriptor::Connect.
+ return llvm::StringSwitch<llvm::Expected<ProtocolModePair>>(scheme)
+ .Case("listen", ProtocolModePair{SocketProtocol::ProtocolTcp,
+ SocketMode::ModeAccept})
+ .Cases("accept", "unix-accept",
+ ProtocolModePair{SocketProtocol::ProtocolUnixDomain,
+ SocketMode::ModeAccept})
+ .Case("unix-abstract-accept",
+ ProtocolModePair{SocketProtocol::ProtocolUnixAbstract,
+ SocketMode::ModeAccept})
+ .Cases("connect", "tcp-connect",
+ ProtocolModePair{SocketProtocol::ProtocolTcp,
+ SocketMode::ModeConnect})
+ .Case("udp", ProtocolModePair{SocketProtocol::ProtocolTcp,
+ SocketMode::ModeConnect})
+ .Case("unix-connect", ProtocolModePair{SocketProtocol::ProtocolUnixDomain,
+ SocketMode::ModeConnect})
+ .Case("unix-abstract-connect",
+ ProtocolModePair{SocketProtocol::ProtocolUnixAbstract,
+ SocketMode::ModeConnect})
+ .Default(llvm::createStringError("unsupported scheme"));
+}
diff --git a/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py b/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py
index ed17044a220d4..592a4cfb0a88b 100644
--- a/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py
+++ b/lldb/test/API/tools/lldb-dap/server/TestDAP_server.py
@@ -54,7 +54,7 @@ def test_server_port(self):
Test launching a binary with a lldb-dap in server mode on a specific port.
"""
self.build()
- (_, connection) = self.start_server(connection="tcp://localhost:0")
+ (_, connection) = self.start_server(connection="listen://localhost:0")
self.run_debug_session(connection, "Alice")
self.run_debug_session(connection, "Bob")
@@ -72,7 +72,7 @@ def cleanup():
self.addTearDownHook(cleanup)
self.build()
- (_, connection) = self.start_server(connection="unix://" + name)
+ (_, connection) = self.start_server(connection="accept://" + name)
self.run_debug_session(connection, "Alice")
self.run_debug_session(connection, "Bob")
@@ -82,7 +82,7 @@ def test_server_interrupt(self):
Test launching a binary with lldb-dap in server mode and shutting down the server while the debug session is still active.
"""
self.build()
- (process, connection) = self.start_server(connection="tcp://localhost:0")
+ (process, connection) = self.start_server(connection="listen://localhost:0")
self.dap_server = dap_server.DebugAdapterServer(
connection=connection,
)
diff --git a/lldb/tools/lldb-dap/Options.td b/lldb/tools/lldb-dap/Options.td
index aecf91797ac70..867753e9294a6 100644
--- a/lldb/tools/lldb-dap/Options.td
+++ b/lldb/tools/lldb-dap/Options.td
@@ -28,8 +28,8 @@ def connection
MetaVarName<"<connection>">,
HelpText<
"Communicate with the lldb-dap tool over the specified connection. "
- "Connections are specified like 'tcp://[host]:port' or "
- "'unix:///path'.">;
+ "Connections are specified like 'listen://[host]:port' or "
+ "'accept:///path'.">;
def launch_target: S<"launch-target">,
MetaVarName<"<target>">,
diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
index 9b9de5e21a742..026eaf946ddad 100644
--- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
@@ -226,23 +226,34 @@ static llvm::Expected<std::pair<Socket::SocketProtocol, std::string>>
validateConnection(llvm::StringRef conn) {
auto uri = lldb_private::URI::Parse(conn);
- if (uri && (uri->scheme == "tcp" || uri->scheme == "connect" ||
- !uri->hostname.empty() || uri->port)) {
+ auto make_error = [conn]() -> llvm::Error {
+ return llvm::createStringError(
+ "Unsupported connection specifier, expected 'accept:///path' or "
+ "'listen://[host]:port', got '%s'.",
+ conn.str().c_str());
+ };
+
+ if (!uri)
+ return make_error();
+
+ llvm::Expected<Socket::ProtocolModePair> protocol_and_mode =
+ Socket::GetProtocolAndMode(uri->scheme);
+ if (!protocol_and_mode)
+ return protocol_and_mode.takeError();
+ if (protocol_and_mode->second != Socket::ModeAccept)
+ return make_error();
+
+ if (protocol_and_mode->first == Socket::ProtocolTcp) {
return std::make_pair(
Socket::ProtocolTcp,
formatv("[{0}]:{1}", uri->hostname.empty() ? "0.0.0.0" : uri->hostname,
uri->port.value_or(0)));
}
- if (uri && (uri->scheme == "unix" || uri->scheme == "unix-connect" ||
- uri->path != "/")) {
+ if (protocol_and_mode->first == Socket::ProtocolUnixDomain)
return std::make_pair(Socket::ProtocolUnixDomain, uri->path.str());
- }
- return llvm::createStringError(
- "Unsupported connection specifier, expected 'unix-connect:///path' or "
- "'connect://[host]:port', got '%s'.",
- conn.str().c_str());
+ return make_error();
}
static llvm::Error
|
Use the same scheme as ConnectionFileDescriptor::Connect and use "listen" and "accept". Addresses feedback from a Pavel in a different PR [1]. [1] llvm#143628 (comment)
ef08bad
to
adea12f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also update
const dapArgs = [...args, "--connection", "connect://localhost:0"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Use the same scheme as ConnectionFileDescriptor::Connect and use "listen" and "accept". Addresses feedback from a Pavel in a different PR [1].
[1] #143628 (comment)