https://github.com/labath created
https://github.com/llvm/llvm-project/pull/115861
It's never set to true. Inheritable FDs are also dangerous as they can end up
processes which know nothing about them. It's better to explicitly pass a
specific FD to a specific subprocess, which we already mostly can do using the
ProcessLaunchInfo FileActions.
>From 4fc069b3517f4ff332020627285359b1b5989878 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Sat, 19 Oct 2024 16:44:49 +0200
Subject: [PATCH] [lldb] Remove ConnectionFileDescriptor::child_process_inherit
It's never set to true. Inheritable FDs are also dangerous as they can
end up processes which know nothing about them. It's better to
explicitly pass a specific FD to a specific subprocess, which we already
mostly can do using the ProcessLaunchInfo FileActions.
---
.../posix/ConnectionFileDescriptorPosix.h | 6 +---
.../posix/ConnectionFileDescriptorPosix.cpp | 29 +++++--------------
2 files changed, 9 insertions(+), 26 deletions(-)
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 35773d5907e913..b771f9c3f45a21 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -31,7 +31,7 @@ class ConnectionFileDescriptor : public Connection {
typedef llvm::function_ref<void(llvm::StringRef local_socket_id)>
socket_id_callback_type;
- ConnectionFileDescriptor(bool child_processes_inherit = false);
+ ConnectionFileDescriptor();
ConnectionFileDescriptor(int fd, bool owns_fd);
@@ -65,9 +65,6 @@ class ConnectionFileDescriptor : public Connection {
lldb::IOObjectSP GetReadObject() override { return m_io_sp; }
- bool GetChildProcessesInherit() const;
- void SetChildProcessesInherit(bool child_processes_inherit);
-
protected:
void OpenCommandPipe();
@@ -135,7 +132,6 @@ class ConnectionFileDescriptor : public Connection {
std::atomic<bool> m_shutting_down; // This marks that we are shutting down so
// if we get woken up from
// BytesAvailable to disconnect, we won't try to read again.
- bool m_child_processes_inherit;
std::string m_uri;
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index d0cc68826d4bb7..8a03e47ef3d900 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -52,18 +52,15 @@
using namespace lldb;
using namespace lldb_private;
-ConnectionFileDescriptor::ConnectionFileDescriptor(bool
child_processes_inherit)
- : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
-
- m_child_processes_inherit(child_processes_inherit) {
+ConnectionFileDescriptor::ConnectionFileDescriptor()
+ : Connection(), m_pipe(), m_mutex(), m_shutting_down(false) {
Log *log(GetLog(LLDBLog::Connection | LLDBLog::Object));
LLDB_LOGF(log, "%p ConnectionFileDescriptor::ConnectionFileDescriptor ()",
static_cast<void *>(this));
}
ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd)
- : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
- m_child_processes_inherit(false) {
+ : Connection(), m_pipe(), m_mutex(), m_shutting_down(false) {
m_io_sp =
std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, owns_fd);
@@ -76,8 +73,7 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(int fd,
bool owns_fd)
}
ConnectionFileDescriptor::ConnectionFileDescriptor(Socket *socket)
- : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
- m_child_processes_inherit(false) {
+ : Connection(), m_pipe(), m_mutex(), m_shutting_down(false) {
InitializeSocket(socket);
}
@@ -94,7 +90,7 @@ void ConnectionFileDescriptor::OpenCommandPipe() {
Log *log = GetLog(LLDBLog::Connection);
// Make the command file descriptor here:
- Status result = m_pipe.CreateNew(m_child_processes_inherit);
+ Status result = m_pipe.CreateNew(/*child_processes_inherit=*/false);
if (!result.Success()) {
LLDB_LOGF(log,
"%p ConnectionFileDescriptor::OpenCommandPipe () - could not "
@@ -539,7 +535,7 @@ lldb::ConnectionStatus
ConnectionFileDescriptor::AcceptSocket(
Status *error_ptr) {
Status error;
std::unique_ptr<Socket> listening_socket =
- Socket::Create(socket_protocol, m_child_processes_inherit, error);
+ Socket::Create(socket_protocol, /*child_processes_inherit=*/false,
error);
Socket *accepted_socket;
if (!error.Fail())
@@ -567,7 +563,7 @@
ConnectionFileDescriptor::ConnectSocket(Socket::SocketProtocol socket_protocol,
Status *error_ptr) {
Status error;
std::unique_ptr<Socket> socket =
- Socket::Create(socket_protocol, m_child_processes_inherit, error);
+ Socket::Create(socket_protocol, /*child_processes_inherit=*/false,
error);
if (!error.Fail())
error = socket->Connect(socket_name);
@@ -649,7 +645,7 @@ ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
if (error_ptr)
*error_ptr = Status();
llvm::Expected<std::unique_ptr<UDPSocket>> socket =
- Socket::UdpConnect(s, m_child_processes_inherit);
+ Socket::UdpConnect(s, /*child_processes_inherit=*/false);
if (!socket) {
if (error_ptr)
*error_ptr = Status::FromError(socket.takeError());
@@ -798,15 +794,6 @@ ConnectionStatus
ConnectionFileDescriptor::ConnectSerialPort(
llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
}
-bool ConnectionFileDescriptor::GetChildProcessesInherit() const {
- return m_child_processes_inherit;
-}
-
-void ConnectionFileDescriptor::SetChildProcessesInherit(
- bool child_processes_inherit) {
- m_child_processes_inherit = child_processes_inherit;
-}
-
void ConnectionFileDescriptor::InitializeSocket(Socket *socket) {
m_io_sp.reset(socket);
m_uri = socket->GetRemoteConnectionURI();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
