[lldb] Remove child_process_inherit argument from Pipe (#145516)

labath · web-flow · commit b77114b723eb · 2025-06-26T13:51:14.000+02:00
It's not necessary on posix platforms as of #126935 and it's ignored on windows as of #138896. For both platforms, we have a better way of inheriting FDs/HANDLEs.
diff --git a/lldb/include/lldb/Host/PipeBase.h b/lldb/include/lldb/Host/PipeBase.h
@@ -21,18 +21,14 @@ class PipeBase {
 public:
   virtual ~PipeBase();
 
-  virtual Status CreateNew(bool child_process_inherit) = 0;
-  virtual Status CreateNew(llvm::StringRef name,
-                           bool child_process_inherit) = 0;
+  virtual Status CreateNew() = 0;
+  virtual Status CreateNew(llvm::StringRef name) = 0;
   virtual Status CreateWithUniqueName(llvm::StringRef prefix,
-                                      bool child_process_inherit,
                                       llvm::SmallVectorImpl<char> &name) = 0;
 
-  virtual Status OpenAsReader(llvm::StringRef name,
-                              bool child_process_inherit) = 0;
+  virtual Status OpenAsReader(llvm::StringRef name) = 0;
 
   virtual llvm::Error OpenAsWriter(llvm::StringRef name,
-                                   bool child_process_inherit,
                                    const Timeout<std::micro> &timeout) = 0;
 
   virtual bool CanRead() const = 0;
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -32,14 +32,12 @@ class PipePosix : public PipeBase {
 
   ~PipePosix() override;
 
-  Status CreateNew(bool child_process_inherit) override;
-  Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
+  Status CreateNew() override;
+  Status CreateNew(llvm::StringRef name) override;
   Status CreateWithUniqueName(llvm::StringRef prefix,
-                              bool child_process_inherit,
                               llvm::SmallVectorImpl<char> &name) override;
-  Status OpenAsReader(llvm::StringRef name,
-                      bool child_process_inherit) override;
-  llvm::Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit,
+  Status OpenAsReader(llvm::StringRef name) override;
+  llvm::Error OpenAsWriter(llvm::StringRef name,
                            const Timeout<std::micro> &timeout) override;
 
   bool CanRead() const override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -29,16 +29,14 @@ class PipeWindows : public PipeBase {
   ~PipeWindows() override;
 
   // Create an unnamed pipe.
-  Status CreateNew(bool child_process_inherit) override;
+  Status CreateNew() override;
 
   // Create a named pipe.
-  Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
+  Status CreateNew(llvm::StringRef name) override;
   Status CreateWithUniqueName(llvm::StringRef prefix,
-                              bool child_process_inherit,
                               llvm::SmallVectorImpl<char> &name) override;
-  Status OpenAsReader(llvm::StringRef name,
-                      bool child_process_inherit) override;
-  llvm::Error OpenAsWriter(llvm::StringRef name, bool child_process_inherit,
+  Status OpenAsReader(llvm::StringRef name) override;
+  llvm::Error OpenAsWriter(llvm::StringRef name,
                            const Timeout<std::micro> &timeout) override;
 
   bool CanRead() const override;
@@ -72,8 +70,7 @@ class PipeWindows : public PipeBase {
   HANDLE GetWriteNativeHandle();
 
 private:
-  Status OpenNamedPipe(llvm::StringRef name, bool child_process_inherit,
-                       bool is_read);
+  Status OpenNamedPipe(llvm::StringRef name, bool is_read);
 
   HANDLE m_read;
   HANDLE m_write;
diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp
@@ -69,7 +69,7 @@ SharedSocket::SharedSocket(const Socket *socket, Status &error) {
   m_fd = kInvalidFD;
 
   // Create a pipe to transfer WSAPROTOCOL_INFO to the child process.
-  error = m_socket_pipe.CreateNew(true);
+  error = m_socket_pipe.CreateNew();
   if (error.Fail())
     return;
 
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -92,7 +92,7 @@ void ConnectionFileDescriptor::OpenCommandPipe() {
 
   Log *log = GetLog(LLDBLog::Connection);
   // Make the command file descriptor here:
-  Status result = m_pipe.CreateNew(/*child_processes_inherit=*/false);
+  Status result = m_pipe.CreateNew();
   if (!result.Success()) {
     LLDB_LOGF(log,
               "%p ConnectionFileDescriptor::OpenCommandPipe () - could not "
diff --git a/lldb/source/Host/posix/MainLoopPosix.cpp b/lldb/source/Host/posix/MainLoopPosix.cpp
@@ -208,7 +208,7 @@ void MainLoopPosix::RunImpl::ProcessReadEvents() {
 #endif
 
 MainLoopPosix::MainLoopPosix() {
-  Status error = m_interrupt_pipe.CreateNew(/*child_process_inherit=*/false);
+  Status error = m_interrupt_pipe.CreateNew();
   assert(error.Success());
 
   // Make the write end of the pipe non-blocking.
diff --git a/lldb/source/Host/posix/PipePosix.cpp b/lldb/source/Host/posix/PipePosix.cpp
@@ -79,24 +79,22 @@ PipePosix &PipePosix::operator=(PipePosix &&pipe_posix) {
 
 PipePosix::~PipePosix() { Close(); }
 
-Status PipePosix::CreateNew(bool child_processes_inherit) {
+Status PipePosix::CreateNew() {
   std::scoped_lock<std::mutex, std::mutex> guard(m_read_mutex, m_write_mutex);
   if (CanReadUnlocked() || CanWriteUnlocked())
     return Status(EINVAL, eErrorTypePOSIX);
 
   Status error;
 #if PIPE2_SUPPORTED
-  if (::pipe2(m_fds, (child_processes_inherit) ? 0 : O_CLOEXEC) == 0)
+  if (::pipe2(m_fds, O_CLOEXEC) == 0)
     return error;
 #else
   if (::pipe(m_fds) == 0) {
 #ifdef FD_CLOEXEC
-    if (!child_processes_inherit) {
-      if (!SetCloexecFlag(m_fds[0]) || !SetCloexecFlag(m_fds[1])) {
-        error = Status::FromErrno();
-        CloseUnlocked();
-        return error;
-      }
+    if (!SetCloexecFlag(m_fds[0]) || !SetCloexecFlag(m_fds[1])) {
+      error = Status::FromErrno();
+      CloseUnlocked();
+      return error;
     }
 #endif
     return error;
@@ -109,7 +107,7 @@ Status PipePosix::CreateNew(bool child_processes_inherit) {
   return error;
 }
 
-Status PipePosix::CreateNew(llvm::StringRef name, bool child_process_inherit) {
+Status PipePosix::CreateNew(llvm::StringRef name) {
   std::scoped_lock<std::mutex, std::mutex> guard(m_read_mutex, m_write_mutex);
   if (CanReadUnlocked() || CanWriteUnlocked())
     return Status::FromErrorString("Pipe is already opened");
@@ -121,7 +119,6 @@ Status PipePosix::CreateNew(llvm::StringRef name, bool child_process_inherit) {
 }
 
 Status PipePosix::CreateWithUniqueName(llvm::StringRef prefix,
-                                       bool child_process_inherit,
                                        llvm::SmallVectorImpl<char> &name) {
   llvm::SmallString<128> named_pipe_path;
   llvm::SmallString<128> pipe_spec((prefix + ".%%%%%%").str());
@@ -137,24 +134,21 @@ Status PipePosix::CreateWithUniqueName(llvm::StringRef prefix,
   do {
     llvm::sys::fs::createUniquePath(tmpdir_file_spec.GetPath(), named_pipe_path,
                                     /*MakeAbsolute=*/false);
-    error = CreateNew(named_pipe_path, child_process_inherit);
+    error = CreateNew(named_pipe_path);
   } while (error.GetError() == EEXIST);
 
   if (error.Success())
     name = named_pipe_path;
   return error;
 }
 
-Status PipePosix::OpenAsReader(llvm::StringRef name,
-                               bool child_process_inherit) {
+Status PipePosix::OpenAsReader(llvm::StringRef name) {
   std::scoped_lock<std::mutex, std::mutex> guard(m_read_mutex, m_write_mutex);
 
   if (CanReadUnlocked() || CanWriteUnlocked())
     return Status::FromErrorString("Pipe is already opened");
 
-  int flags = O_RDONLY | O_NONBLOCK;
-  if (!child_process_inherit)
-    flags |= O_CLOEXEC;
+  int flags = O_RDONLY | O_NONBLOCK | O_CLOEXEC;
 
   Status error;
   int fd = FileSystem::Instance().Open(name.str().c_str(), flags);
@@ -167,15 +161,12 @@ Status PipePosix::OpenAsReader(llvm::StringRef name,
 }
 
 llvm::Error PipePosix::OpenAsWriter(llvm::StringRef name,
-                                    bool child_process_inherit,
                                     const Timeout<std::micro> &timeout) {
   std::lock_guard<std::mutex> guard(m_write_mutex);
   if (CanReadUnlocked() || CanWriteUnlocked())
     return llvm::createStringError("Pipe is already opened");
 
-  int flags = O_WRONLY | O_NONBLOCK;
-  if (!child_process_inherit)
-    flags |= O_CLOEXEC;
+  int flags = O_WRONLY | O_NONBLOCK | O_CLOEXEC;
 
   using namespace std::chrono;
   std::optional<time_point<steady_clock>> finish_time;
diff --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -255,8 +255,7 @@ ProcessLauncherPosixFork::LaunchProcess(const ProcessLaunchInfo &launch_info,
                                         Status &error) {
   // A pipe used by the child process to report errors.
   PipePosix pipe;
-  const bool child_processes_inherit = false;
-  error = pipe.CreateNew(child_processes_inherit);
+  error = pipe.CreateNew();
   if (error.Fail())
     return HostProcess();
 
diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp
@@ -66,19 +66,18 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write)
 
 PipeWindows::~PipeWindows() { Close(); }
 
-Status PipeWindows::CreateNew(bool child_process_inherit) {
+Status PipeWindows::CreateNew() {
   // Even for anonymous pipes, we open a named pipe.  This is because you
   // cannot get overlapped i/o on Windows without using a named pipe.  So we
   // synthesize a unique name.
   uint32_t serial = g_pipe_serial.fetch_add(1);
   std::string pipe_name = llvm::formatv(
       "lldb.pipe.{0}.{1}.{2}", GetCurrentProcessId(), &g_pipe_serial, serial);
 
-  return CreateNew(pipe_name.c_str(), child_process_inherit);
+  return CreateNew(pipe_name.c_str());
 }
 
-Status PipeWindows::CreateNew(llvm::StringRef name,
-                              bool child_process_inherit) {
+Status PipeWindows::CreateNew(llvm::StringRef name) {
   if (name.empty())
     return Status(ERROR_INVALID_PARAMETER, eErrorTypeWin32);
 
@@ -109,7 +108,7 @@ Status PipeWindows::CreateNew(llvm::StringRef name,
 
   // Open the write end of the pipe. Note that closing either the read or 
   // write end of the pipe could directly close the pipe itself.
-  Status result = OpenNamedPipe(name, child_process_inherit, false);
+  Status result = OpenNamedPipe(name, false);
   if (!result.Success()) {
     CloseReadFileDescriptor();
     return result;
@@ -119,7 +118,6 @@ Status PipeWindows::CreateNew(llvm::StringRef name,
 }
 
 Status PipeWindows::CreateWithUniqueName(llvm::StringRef prefix,
-                                         bool child_process_inherit,
                                          llvm::SmallVectorImpl<char> &name) {
   llvm::SmallString<128> pipe_name;
   Status error;
@@ -133,7 +131,7 @@ Status PipeWindows::CreateWithUniqueName(llvm::StringRef prefix,
     pipe_name += "-";
     pipe_name += reinterpret_cast<char *>(unique_string);
     ::RpcStringFreeA(&unique_string);
-    error = CreateNew(pipe_name, child_process_inherit);
+    error = CreateNew(pipe_name);
   } else {
     error = Status(status, eErrorTypeWin32);
   }
@@ -142,25 +140,22 @@ Status PipeWindows::CreateWithUniqueName(llvm::StringRef prefix,
   return error;
 }
 
-Status PipeWindows::OpenAsReader(llvm::StringRef name,
-                                 bool child_process_inherit) {
+Status PipeWindows::OpenAsReader(llvm::StringRef name) {
   if (CanRead())
     return Status(); // Note the name is ignored.
 
-  return OpenNamedPipe(name, child_process_inherit, true);
+  return OpenNamedPipe(name, true);
 }
 
 llvm::Error PipeWindows::OpenAsWriter(llvm::StringRef name,
-                                      bool child_process_inherit,
                                       const Timeout<std::micro> &timeout) {
   if (CanWrite())
     return llvm::Error::success(); // Note the name is ignored.
 
-  return OpenNamedPipe(name, child_process_inherit, false).takeError();
+  return OpenNamedPipe(name, false).takeError();
 }
 
-Status PipeWindows::OpenNamedPipe(llvm::StringRef name,
-                                  bool child_process_inherit, bool is_read) {
+Status PipeWindows::OpenNamedPipe(llvm::StringRef name, bool is_read) {
   if (name.empty())
     return Status(ERROR_INVALID_PARAMETER, eErrorTypeWin32);
 
diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -220,7 +220,7 @@ ScriptInterpreterIORedirect::ScriptInterpreterIORedirect(
     m_input_file_sp = debugger.GetInputFileSP();
 
     Pipe pipe;
-    Status pipe_result = pipe.CreateNew(false);
+    Status pipe_result = pipe.CreateNew();
 #if defined(_WIN32)
     lldb::file_t read_file = pipe.GetReadNativeHandle();
     pipe.ReleaseReadFileDescriptor();
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -944,7 +944,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
 #if defined(__APPLE__)
     // Using a named pipe as debugserver does not support --pipe.
     Status error = socket_pipe.CreateWithUniqueName("debugserver-named-pipe",
-                                                    false, named_pipe_path);
+                                                    named_pipe_path);
     if (error.Fail()) {
       LLDB_LOG(log, "named pipe creation failed: {0}", error);
       return error;
@@ -953,15 +953,15 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
     debugserver_args.AppendArgument(named_pipe_path);
 #else
     // Using an unnamed pipe as it's simpler.
-    Status error = socket_pipe.CreateNew(true);
+    Status error = socket_pipe.CreateNew();
     if (error.Fail()) {
       LLDB_LOG(log, "unnamed pipe creation failed: {0}", error);
       return error;
     }
     pipe_t write = socket_pipe.GetWritePipe();
     debugserver_args.AppendArgument(llvm::StringRef("--pipe"));
     debugserver_args.AppendArgument(llvm::to_string(write));
-    launch_info.AppendCloseFileAction(socket_pipe.GetReadFileDescriptor());
+    launch_info.AppendDuplicateFileAction((int64_t)write, (int64_t)write);
 #endif
   }
 
@@ -1044,7 +1044,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
 
   Status error;
   if (named_pipe_path.size() > 0) {
-    error = socket_pipe.OpenAsReader(named_pipe_path, false);
+    error = socket_pipe.OpenAsReader(named_pipe_path);
     if (error.Fail()) {
       LLDB_LOG(log, "failed to open named pipe {0} for reading: {1}",
                named_pipe_path, error);
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
@@ -4614,7 +4614,7 @@ class IOHandlerProcessSTDIO : public IOHandler {
         m_process(process),
         m_read_file(GetInputFD(), File::eOpenOptionReadOnly, false),
         m_write_file(write_fd, File::eOpenOptionWriteOnly, false) {
-    m_pipe.CreateNew(false);
+    m_pipe.CreateNew();
   }
 
   ~IOHandlerProcessSTDIO() override = default;
diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -185,7 +185,7 @@ Status writeSocketIdToPipe(const char *const named_pipe_path,
                            llvm::StringRef socket_id) {
   Pipe port_name_pipe;
   // Wait for 10 seconds for pipe to be opened.
-  if (llvm::Error err = port_name_pipe.OpenAsWriter(named_pipe_path, false,
+  if (llvm::Error err = port_name_pipe.OpenAsWriter(named_pipe_path,
                                                     std::chrono::seconds{10}))
     return Status::FromError(std::move(err));
 
diff --git a/lldb/unittests/Core/CommunicationTest.cpp b/lldb/unittests/Core/CommunicationTest.cpp
@@ -141,8 +141,7 @@ TEST_F(CommunicationTest, SynchronizeWhileClosing) {
 #if LLDB_ENABLE_POSIX
 TEST_F(CommunicationTest, WriteAll) {
   Pipe pipe;
-  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
-                    llvm::Succeeded());
+  ASSERT_THAT_ERROR(pipe.CreateNew().ToError(), llvm::Succeeded());
 
   // Make the write end non-blocking in order to easily reproduce a partial
   // write.
diff --git a/lldb/unittests/Host/HostTest.cpp b/lldb/unittests/Host/HostTest.cpp
@@ -145,8 +145,7 @@ TEST(Host, LaunchProcessDuplicatesHandle) {
     exit(1);
   }
   Pipe pipe;
-  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).takeError(),
-                    llvm::Succeeded());
+  ASSERT_THAT_ERROR(pipe.CreateNew().takeError(), llvm::Succeeded());
   SCOPED_TRACE(llvm::formatv("Pipe handles are: {0}/{1}",
                              (uint64_t)pipe.GetReadPipe(),
                              (uint64_t)pipe.GetWritePipe())
diff --git a/lldb/unittests/Host/PipeTest.cpp b/lldb/unittests/Host/PipeTest.cpp
diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp
diff --git a/lldb/unittests/TestingSupport/Host/PipeTestUtilities.h b/lldb/unittests/TestingSupport/Host/PipeTestUtilities.h
