[lldb-dap] Make connection URLs match lldb

[JDevlieghere]

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]

@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

[ashgti]

Can we also update

llvm-project/lldb/tools/lldb-dap/src-ts/lldb-dap-server.ts

Line 29 in 156a64c

const dapArgs = [...args, "--connection", "connect://localhost:0"];

?

[labath]

Thanks.