From e4cc6e9bcdff5fe979ab72025cb803d723cd9c31 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 14 Oct 2020 16:39:25 +0200 Subject: [PATCH] [lldb] Modernize PseudoTerminal::Fork --- lldb/include/lldb/Host/PseudoTerminal.h | 8 +- lldb/source/Host/common/PseudoTerminal.cpp | 100 +++++++-------------- .../Plugins/Process/FreeBSD/ProcessMonitor.cpp | 21 ++--- 3 files changed, 42 insertions(+), 87 deletions(-) diff --git a/lldb/include/lldb/Host/PseudoTerminal.h b/lldb/include/lldb/Host/PseudoTerminal.h index f7f8e63..350f926 100644 --- a/lldb/include/lldb/Host/PseudoTerminal.h +++ b/lldb/include/lldb/Host/PseudoTerminal.h @@ -62,15 +62,11 @@ public: /// @li PseudoTerminal::ReleasePrimaryFileDescriptor() @li /// PseudoTerminal::ReleaseSaveFileDescriptor() /// - /// \param[out] error_str - /// An pointer to an error that can describe any errors that - /// occur. This can be NULL if no error status is desired. - /// /// \return /// \b Parent process: a child process ID that is greater - /// than zero, or -1 if the fork fails. + /// than zero, or an error if the fork fails. /// \b Child process: zero. - lldb::pid_t Fork(char *error_str, size_t error_len); + llvm::Expected Fork(); /// The primary file descriptor accessor. /// diff --git a/lldb/source/Host/common/PseudoTerminal.cpp b/lldb/source/Host/common/PseudoTerminal.cpp index dc0298c..c5f101c 100644 --- a/lldb/source/Host/common/PseudoTerminal.cpp +++ b/lldb/source/Host/common/PseudoTerminal.cpp @@ -28,12 +28,6 @@ int posix_openpt(int flags); using namespace lldb_private; -// Write string describing error number -static void ErrnoToStr(char *error_str, size_t error_len) { - std::string strerror = llvm::sys::StrError(); - ::snprintf(error_str, error_len, "%s", strerror.c_str()); -} - // PseudoTerminal constructor PseudoTerminal::PseudoTerminal() : m_primary_fd(invalid_fd), m_secondary_fd(invalid_fd) {} @@ -123,79 +117,47 @@ std::string PseudoTerminal::GetSecondaryName() const { #endif } -// Fork a child process and have its stdio routed to a pseudo terminal. -// -// In the parent process when a valid pid is returned, the primary file -// descriptor can be used as a read/write access to stdio of the child process. -// -// In the child process the stdin/stdout/stderr will already be routed to the -// secondary pseudo terminal and the primary file descriptor will be closed as -// it is no longer needed by the child process. -// -// This class will close the file descriptors for the primary/secondary when the -// destructor is called, so be sure to call ReleasePrimaryFileDescriptor() or -// ReleaseSecondaryFileDescriptor() if any file descriptors are going to be used -// past the lifespan of this object. -// -// RETURNS: -// in the parent process: the pid of the child, or -1 if fork fails -// in the child process: zero -lldb::pid_t PseudoTerminal::Fork(char *error_str, size_t error_len) { - if (error_str) - error_str[0] = '\0'; - pid_t pid = LLDB_INVALID_PROCESS_ID; +llvm::Expected PseudoTerminal::Fork() { #if LLDB_ENABLE_POSIX - if (llvm::Error Err = OpenFirstAvailablePrimary(O_RDWR | O_CLOEXEC)) { - snprintf(error_str, error_len, "%s", toString(std::move(Err)).c_str()); - return LLDB_INVALID_PROCESS_ID; - } + if (llvm::Error Err = OpenFirstAvailablePrimary(O_RDWR | O_CLOEXEC)) + return std::move(Err); - pid = ::fork(); + pid_t pid = ::fork(); if (pid < 0) { - // Fork failed - if (error_str) - ErrnoToStr(error_str, error_len); - } else if (pid == 0) { - // Child Process - ::setsid(); - - if (llvm::Error Err = OpenSecondary(O_RDWR)) { - snprintf(error_str, error_len, "%s", toString(std::move(Err)).c_str()); - return LLDB_INVALID_PROCESS_ID; - } + return llvm::errorCodeToError( + std::error_code(errno, std::generic_category())); + } + if (pid > 0) { + // Parent process. + return pid; + } - // Primary FD should have O_CLOEXEC set, but let's close it just in - // case... - ClosePrimaryFileDescriptor(); + // Child Process + ::setsid(); -#if defined(TIOCSCTTY) - // Acquire the controlling terminal - if (::ioctl(m_secondary_fd, TIOCSCTTY, (char *)0) < 0) { - if (error_str) - ErrnoToStr(error_str, error_len); - } -#endif - // Duplicate all stdio file descriptors to the secondary pseudo terminal - if (::dup2(m_secondary_fd, STDIN_FILENO) != STDIN_FILENO) { - if (error_str && !error_str[0]) - ErrnoToStr(error_str, error_len); - } + if (llvm::Error Err = OpenSecondary(O_RDWR)) + return std::move(Err); - if (::dup2(m_secondary_fd, STDOUT_FILENO) != STDOUT_FILENO) { - if (error_str && !error_str[0]) - ErrnoToStr(error_str, error_len); - } + // Primary FD should have O_CLOEXEC set, but let's close it just in + // case... + ClosePrimaryFileDescriptor(); - if (::dup2(m_secondary_fd, STDERR_FILENO) != STDERR_FILENO) { - if (error_str && !error_str[0]) - ErrnoToStr(error_str, error_len); +#if defined(TIOCSCTTY) + // Acquire the controlling terminal + if (::ioctl(m_secondary_fd, TIOCSCTTY, (char *)0) < 0) { + return llvm::errorCodeToError( + std::error_code(errno, std::generic_category())); + } +#endif + // Duplicate all stdio file descriptors to the secondary pseudo terminal + for (int fd : {STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO}) { + if (::dup2(m_secondary_fd, fd) != fd) { + return llvm::errorCodeToError( + std::error_code(errno, std::generic_category())); } - } else { - // Parent Process - // Do nothing and let the pid get returned! } #endif - return pid; + return 0; } // The primary file descriptor accessor. This object retains ownership of the diff --git a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp index 6a9209d..e85470d 100644 --- a/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp +++ b/lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp @@ -821,17 +821,14 @@ bool ProcessMonitor::Launch(LaunchArgs *args) { const FileSpec &working_dir = args->m_working_dir; PseudoTerminal terminal; - const size_t err_len = 1024; - char err_str[err_len]; - ::pid_t pid; // Propagate the environment if one is not supplied. Environment::Envp envp = (args->m_env.empty() ? Host::GetEnvironment() : args->m_env).getEnvp(); - if ((pid = terminal.Fork(err_str, err_len)) == -1) { - args->m_error.SetErrorToGenericError(); - args->m_error.SetErrorString("Process fork failed."); + Expected pid = terminal.Fork(); + if (!pid) { + args->m_error = pid.takeError(); goto FINISH; } @@ -847,7 +844,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) { }; // Child process. - if (pid == 0) { + if (*pid == 0) { // Trace this process. if (PTRACE(PT_TRACE_ME, 0, NULL, 0) < 0) exit(ePtraceFailed); @@ -892,7 +889,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) { // Wait for the child process to to trap on its call to execve. ::pid_t wpid; int status; - if ((wpid = waitpid(pid, &status, 0)) < 0) { + if ((wpid = waitpid(*pid, &status, 0)) < 0) { args->m_error.SetErrorToErrno(); goto FINISH; } else if (WIFEXITED(status)) { @@ -926,13 +923,13 @@ bool ProcessMonitor::Launch(LaunchArgs *args) { } goto FINISH; } - assert(WIFSTOPPED(status) && wpid == (::pid_t)pid && + assert(WIFSTOPPED(status) && wpid == (::pid_t)*pid && "Could not sync with inferior process."); #ifdef notyet // Have the child raise an event on exit. This is used to keep the child in // limbo until it is destroyed. - if (PTRACE(PTRACE_SETOPTIONS, pid, NULL, PTRACE_O_TRACEEXIT) < 0) { + if (PTRACE(PTRACE_SETOPTIONS, *pid, NULL, PTRACE_O_TRACEEXIT) < 0) { args->m_error.SetErrorToErrno(); goto FINISH; } @@ -940,7 +937,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) { // Release the master terminal descriptor and pass it off to the // ProcessMonitor instance. Similarly stash the inferior pid. monitor->m_terminal_fd = terminal.ReleasePrimaryFileDescriptor(); - monitor->m_pid = pid; + monitor->m_pid = *pid; // Set the terminal fd to be in non blocking mode (it simplifies the // implementation of ProcessFreeBSD::GetSTDOUT to have a non-blocking @@ -948,7 +945,7 @@ bool ProcessMonitor::Launch(LaunchArgs *args) { if (!EnsureFDFlags(monitor->m_terminal_fd, O_NONBLOCK, args->m_error)) goto FINISH; - process.SendMessage(ProcessMessage::Attach(pid)); + process.SendMessage(ProcessMessage::Attach(*pid)); FINISH: return args->m_error.Success(); -- 2.7.4