Skip to content

Conversation

@slydiman
Copy link
Contributor

@slydiman slydiman commented Sep 6, 2024

Replaced int connection_fd = -1 with shared_fd_t connection_fd = SharedSocket::kInvalidFD.

This is prerequisite for #104238.

@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2024

@llvm/pr-subscribers-lldb

Author: Dmitry Vasilyev (slydiman)

Changes

Replaced int connection_fd = -1 with shared_fd_t connection_fd = SharedSocket::kInvalidFD.

This is prerequisite for #104238.


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

7 Files Affected:

  • (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (+10-7)
  • (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (+3-2)
  • (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+1-1)
  • (modified) lldb/tools/lldb-server/lldb-gdbserver.cpp (+23-8)
  • (modified) lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp (-4)
  • (modified) lldb/unittests/tools/lldb-server/tests/TestClient.cpp (-4)
  • (modified) lldb/unittests/tools/lldb-server/tests/TestClient.h (+4)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 04f0f525e3ebba..ca517c2f3025d4 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -962,16 +962,19 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
 #endif
 
     // If a url is supplied then use it
-    if (url)
+    if (url && url[0])
       debugserver_args.AppendArgument(llvm::StringRef(url));
 
-    if (pass_comm_fd >= 0) {
+    if (pass_comm_fd != SharedSocket::kInvalidFD) {
       StreamString fd_arg;
-      fd_arg.Printf("--fd=%i", pass_comm_fd);
+      fd_arg.Printf("--fd=%" PRIi64, (int64_t)pass_comm_fd);
       debugserver_args.AppendArgument(fd_arg.GetString());
       // Send "pass_comm_fd" down to the inferior so it can use it to
-      // communicate back with this process
-      launch_info.AppendDuplicateFileAction(pass_comm_fd, pass_comm_fd);
+      // communicate back with this process. Ignored on Windows.
+#ifndef _WIN32
+      launch_info.AppendDuplicateFileAction((int)pass_comm_fd,
+                                            (int)pass_comm_fd);
+#endif
     }
 
     // use native registers, not the GDB registers
@@ -991,7 +994,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
     // port is null when debug server should listen on domain socket - we're
     // not interested in port value but rather waiting for debug server to
     // become available.
-    if (pass_comm_fd == -1) {
+    if (pass_comm_fd == SharedSocket::kInvalidFD) {
       if (url) {
 // Create a temporary file to get the stdout/stderr and redirect the output of
 // the command into this file. We will later read this file if all goes well
@@ -1137,7 +1140,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
 
     if (error.Success() &&
         (launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) &&
-        pass_comm_fd == -1) {
+        pass_comm_fd == SharedSocket::kInvalidFD) {
       if (named_pipe_path.size() > 0) {
         error = socket_pipe.OpenAsReader(named_pipe_path, false);
         if (error.Fail())
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index a182291f6c8594..754797ba7f5045 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -21,6 +21,7 @@
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/HostThread.h"
+#include "lldb/Host/Socket.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Predicate.h"
@@ -156,8 +157,8 @@ class GDBRemoteCommunication : public Communication {
       Platform *platform, // If non nullptr, then check with the platform for
                           // the GDB server binary if it can't be located
       ProcessLaunchInfo &launch_info, uint16_t *port, const Args *inferior_args,
-      int pass_comm_fd); // Communication file descriptor to pass during
-                         // fork/exec to avoid having to connect/accept
+      shared_fd_t pass_comm_fd); // Communication file descriptor to pass during
+                                 // fork/exec to avoid having to connect/accept
 
   void DumpHistory(Stream &strm);
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 65b1b3a0f26863..5eaf9ce2a302aa 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3447,7 +3447,7 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
     }
 #endif
 
-    int communication_fd = -1;
+    shared_fd_t communication_fd = SharedSocket::kInvalidFD;
 #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
     // Use a socketpair on non-Windows systems for security and performance
     // reasons.
diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp
index 563284730bc705..3b80e922be73a9 100644
--- a/lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -24,8 +24,8 @@
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Pipe.h"
-#include "lldb/Host/Socket.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
+#include "lldb/Host/common/TCPSocket.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Status.h"
@@ -195,18 +195,31 @@ void ConnectToRemote(MainLoop &mainloop,
                      bool reverse_connect, llvm::StringRef host_and_port,
                      const char *const progname, const char *const subcommand,
                      const char *const named_pipe_path, pipe_t unnamed_pipe,
-                     int connection_fd) {
+                     shared_fd_t connection_fd) {
   Status error;
 
   std::unique_ptr<Connection> connection_up;
   std::string url;
 
-  if (connection_fd != -1) {
+  if (connection_fd != SharedSocket::kInvalidFD) {
+#ifdef _WIN32
+    NativeSocket sockfd;
+    error = SharedSocket::GetNativeSocket(connection_fd, sockfd);
+    if (error.Fail()) {
+      llvm::errs() << llvm::formatv("error: GetNativeSocket failed: {0}\n",
+                                    error.AsCString());
+      exit(-1);
+    }
+    connection_up =
+        std::unique_ptr<Connection>(new ConnectionFileDescriptor(new TCPSocket(
+            sockfd, /*should_close=*/true, /*child_processes_inherit=*/false)));
+#else
     url = llvm::formatv("fd://{0}", connection_fd).str();
 
     // Create the connection.
-#if LLDB_ENABLE_POSIX && !defined _WIN32
+#if LLDB_ENABLE_POSIX
     ::fcntl(connection_fd, F_SETFD, FD_CLOEXEC);
+#endif
 #endif
   } else if (!host_and_port.empty()) {
     llvm::Expected<std::string> url_exp =
@@ -338,7 +351,7 @@ int main_gdbserver(int argc, char *argv[]) {
       log_channels; // e.g. "lldb process threads:gdb-remote default:linux all"
   lldb::pipe_t unnamed_pipe = LLDB_INVALID_PIPE;
   bool reverse_connect = false;
-  int connection_fd = -1;
+  shared_fd_t connection_fd = SharedSocket::kInvalidFD;
 
   // ProcessLaunchInfo launch_info;
   ProcessAttachInfo attach_info;
@@ -404,10 +417,12 @@ int main_gdbserver(int argc, char *argv[]) {
     unnamed_pipe = (pipe_t)Arg;
   }
   if (Args.hasArg(OPT_fd)) {
-    if (!llvm::to_integer(Args.getLastArgValue(OPT_fd), connection_fd)) {
+    int64_t fd;
+    if (!llvm::to_integer(Args.getLastArgValue(OPT_fd), fd)) {
       WithColor::error() << "invalid '--fd' argument\n" << HelpText;
       return 1;
     }
+    connection_fd = (shared_fd_t)fd;
   }
 
   if (!LLDBServerUtilities::SetupLogging(
@@ -423,7 +438,7 @@ int main_gdbserver(int argc, char *argv[]) {
     for (const char *Val : Arg->getValues())
       Inputs.push_back(Val);
   }
-  if (Inputs.empty() && connection_fd == -1) {
+  if (Inputs.empty() && connection_fd == SharedSocket::kInvalidFD) {
     WithColor::error() << "no connection arguments\n" << HelpText;
     return 1;
   }
@@ -432,7 +447,7 @@ int main_gdbserver(int argc, char *argv[]) {
   GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager);
 
   llvm::StringRef host_and_port;
-  if (!Inputs.empty()) {
+  if (!Inputs.empty() && connection_fd == SharedSocket::kInvalidFD) {
     host_and_port = Inputs.front();
     Inputs.erase(Inputs.begin());
   }
diff --git a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
index 66e92415911afb..34f8d23e94e4c9 100644
--- a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -14,10 +14,6 @@ using namespace llgs_tests;
 using namespace lldb_private;
 using namespace llvm;
 
-#ifdef SendMessage
-#undef SendMessage
-#endif
-
 // Disable this test on Windows as it appears to have a race condition
 // that causes lldb-server not to exit after the inferior hangs up.
 #if !defined(_WIN32)
diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
index 2d7ce36bacfaa3..a6f2dc32c6d0c0 100644
--- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -26,10 +26,6 @@ using namespace lldb_private;
 using namespace llvm;
 using namespace llgs_tests;
 
-#ifdef SendMessage
-#undef SendMessage
-#endif
-
 TestClient::TestClient(std::unique_ptr<Connection> Conn) {
   SetConnection(std::move(Conn));
   SetPacketTimeout(std::chrono::seconds(10));
diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.h b/lldb/unittests/tools/lldb-server/tests/TestClient.h
index deb6802d0da707..5d1eacaf6198e8 100644
--- a/lldb/unittests/tools/lldb-server/tests/TestClient.h
+++ b/lldb/unittests/tools/lldb-server/tests/TestClient.h
@@ -20,6 +20,10 @@
 #include <optional>
 #include <string>
 
+#ifdef SendMessage
+#undef SendMessage
+#endif
+
 #if LLDB_SERVER_IS_DEBUGSERVER
 #define LLGS_TEST(x) DISABLED_ ## x
 #define DS_TEST(x) x

@slydiman slydiman force-pushed the lldb-server-shared_fd_t branch from 77be482 to 23df946 Compare September 6, 2024 10:22
Replaced `int connection_fd = -1` with `shared_fd_t connection_fd = SharedSocket::kInvalidFD`.

This is prerequisite for llvm#104238.
@slydiman slydiman merged commit 5d2b337 into llvm:main Sep 6, 2024
@slydiman slydiman deleted the lldb-server-shared_fd_t branch April 18, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants