[Lldb-commits] [lldb] [lldb][NFC] Used shared_fd_t (PR #107553)

Dmitry Vasilyev via lldb-commits lldb-commits at lists.llvm.org
Fri Sep 6 03:53:48 PDT 2024


https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/107553

>From c5d5515a71690ef795912219b99408effb5031d8 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassiliev at accesssoftek.com>
Date: Fri, 6 Sep 2024 13:38:09 +0400
Subject: [PATCH] [lldb][NFC] Used shared_fd_t

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

This is prerequisite for #104238.
---
 .../gdb-remote/GDBRemoteCommunication.cpp     | 19 +++++++++++--------
 .../gdb-remote/GDBRemoteCommunication.h       |  5 +++--
 .../GDBRemoteCommunicationServerPlatform.cpp  |  5 +++--
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  2 +-
 lldb/tools/lldb-server/lldb-gdbserver.cpp     | 14 ++++++++------
 .../tools/lldb-server/tests/LLGSTest.cpp      |  4 ----
 .../tools/lldb-server/tests/TestClient.cpp    |  4 ----
 .../tools/lldb-server/tests/TestClient.h      |  4 ++++
 8 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index d3a782b22349a0..d49e7545827521 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -940,7 +940,7 @@ FileSpec GDBRemoteCommunication::GetDebugserverPath(Platform *platform) {
 
 Status GDBRemoteCommunication::StartDebugserverProcess(
     const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
-    uint16_t *port, const Args *inferior_args, int pass_comm_fd) {
+    uint16_t *port, const Args *inferior_args, shared_fd_t pass_comm_fd) {
   Log *log = GetLog(GDBRLog::Process);
   LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
             __FUNCTION__, url ? url : "<empty>", port ? *port : uint16_t(0));
@@ -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/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index 2f2750ec2b9209..c60a83d351ba09 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -208,8 +208,9 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
     port_ptr = nullptr;
   }
 
-  Status error = StartDebugserverProcess(
-      url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args, -1);
+  Status error = StartDebugserverProcess(url.str().c_str(), nullptr,
+                                         debugserver_launch_info, port_ptr,
+                                         &args, SharedSocket::kInvalidFD);
 
   pid = debugserver_launch_info.GetProcessID();
   if (pid != LLDB_INVALID_PROCESS_ID) {
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..d7503112697ef5 100644
--- a/lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -195,13 +195,13 @@ 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) {
     url = llvm::formatv("fd://{0}", connection_fd).str();
 
     // Create the connection.
@@ -338,7 +338,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 +404,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 +425,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 +434,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



More information about the lldb-commits mailing list