Skip to content

[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

Merged
merged 2 commits into from
Jun 20, 2025

Conversation

JDevlieghere
Copy link
Member

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)

@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

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)


Full diff: https://github.com/llvm/llvm-project/pull/144770.diff

5 Files Affected:

  • (modified) lldb/include/lldb/Host/Socket.h (+9)
  • (modified) lldb/source/Host/common/Socket.cpp (+25)
  • (modified) lldb/test/API/tools/lldb-dap/server/TestDAP_server.py (+3-3)
  • (modified) lldb/tools/lldb-dap/Options.td (+2-2)
  • (modified) lldb/tools/lldb-dap/tool/lldb-dap.cpp (+20-9)
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)
@JDevlieghere JDevlieghere force-pushed the lldb-dap-connect-scheme branch from ef08bad to adea12f Compare June 18, 2025 18:40
Copy link
Contributor

@ashgti ashgti left a 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"];
?

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@JDevlieghere JDevlieghere merged commit 4f991cc into llvm:main Jun 20, 2025
7 checks passed
@JDevlieghere JDevlieghere deleted the lldb-dap-connect-scheme branch June 20, 2025 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants