[Lldb-commits] [lldb] ae7b5af - [lldb] Remove ConnectionFileDescriptor::child_process_inherit (#115861)

[via lldb-commits]

Author: Pavel Labath
Date: 2024-11-13T08:59:42+01:00
New Revision: ae7b5af904850db71308915836f32a8d79553dd8

URL: https://github.com/llvm/llvm-project/commit/ae7b5af904850db71308915836f32a8d79553dd8
DIFF: https://github.com/llvm/llvm-project/commit/ae7b5af904850db71308915836f32a8d79553dd8.diff

LOG: [lldb] Remove ConnectionFileDescriptor::child_process_inherit (#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.

Added: 
    

Modified: 
    lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
    lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

Removed: 
    


################################################################################
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();