[Lldb-commits] [lldb] [lldb][NFC] Used shared_fd_t (PR #107553)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 6 02:45:49 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Dmitry Vasilyev (slydiman)
<details>
<summary>Changes</summary>
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
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/107553
More information about the lldb-commits
mailing list