[lldb] Check for and use ptsname_r if available

ptsname is not thread-safe. ptsname_r is available on most (but not all)
systems -- use it preferentially.

In the patch I also improve the thread-safety of the ptsname fallback
path by wrapping it in a mutex. This should guarantee the safety of a
typical ptsname implementation using a single static buffer, as long as
all callers go through this function.

I also remove the error arguments, as the only way this function can
fail is if the "primary" fd is not valid. This is a programmer error as
this requirement is documented, and all callers ensure that is the case.

Differential Revision: https://reviews.llvm.org/D88728

lldb/cmake/modules/LLDBGenerateConfig.cmake

@@ -7,6 +7,7 @@ include(CheckLibraryExists)
 
 set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
 check_symbol_exists(ppoll poll.h HAVE_PPOLL)
+check_symbol_exists(ptsname_r stdlib.h HAVE_PTSNAME_R)
 set(CMAKE_REQUIRED_DEFINITIONS)
 check_symbol_exists(sigaction signal.h HAVE_SIGACTION)
 check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)

lldb/include/lldb/Host/Config.h.cmake

@@ -20,6 +20,8 @@
 
 #cmakedefine01 HAVE_PPOLL
 
+#cmakedefine01 HAVE_PTSNAME_R
+
 #cmakedefine01 HAVE_SIGACTION
 
 #cmakedefine01 HAVE_PROCESS_VM_READV

lldb/include/lldb/Host/PseudoTerminal.h

@@ -105,20 +105,11 @@ public:
   /// A primary pseudo terminal should already be valid prior to
   /// calling this function.
   ///
-  /// \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
-  ///     The name of the secondary pseudo terminal as a NULL terminated
-  ///     C. This string that comes from static memory, so a copy of
-  ///     the string should be made as subsequent calls can change
-  ///     this value. NULL is returned if this object doesn't have
-  ///     a valid primary pseudo terminal opened or if the call to
-  ///     \c ptsname() fails.
+  ///     The name of the secondary pseudo terminal.
   ///
   /// \see PseudoTerminal::OpenFirstAvailablePrimary()
-  const char *GetSecondaryName(char *error_str, size_t error_len) const;
+  std::string GetSecondaryName() const;
 
   /// Open the first available pseudo terminal.
   ///

lldb/source/Host/common/ProcessLaunchInfo.cpp

@@ -222,7 +222,7 @@ llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "PTY::OpenFirstAvailablePrimary failed");
   }
-  const FileSpec secondary_file_spec(m_pty->GetSecondaryName(nullptr, 0));
+  const FileSpec secondary_file_spec(m_pty->GetSecondaryName());
 
   // Only use the secondary tty if we don't have anything specified for
   // input and don't have an action for stdin

lldb/source/Host/common/PseudoTerminal.cpp

@@ -8,9 +8,11 @@
 
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Config.h"
-
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
-
+#include <cassert>
+#include <limits.h>
+#include <mutex>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -128,15 +130,8 @@ bool PseudoTerminal::OpenSecondary(int oflag, char *error_str,
 
   CloseSecondaryFileDescriptor();
 
-  // Open the primary side of a pseudo terminal
-  const char *secondary_name = GetSecondaryName(error_str, error_len);
-
-  if (secondary_name == nullptr)
-    return false;
-
-  m_secondary_fd =
-      llvm::sys::RetryAfterSignal(-1, ::open, secondary_name, oflag);
-
+  std::string name = GetSecondaryName();
+  m_secondary_fd = llvm::sys::RetryAfterSignal(-1, ::open, name.c_str(), oflag);
   if (m_secondary_fd < 0) {
     if (error_str)
       ErrnoToStr(error_str, error_len);
@@ -146,32 +141,21 @@ bool PseudoTerminal::OpenSecondary(int oflag, char *error_str,
   return true;
 }
 
-// Get the name of the secondary pseudo terminal. A primary pseudo terminal
-// should already be valid prior to calling this function (see
-// OpenFirstAvailablePrimary()).
-//
-// RETURNS:
-//  NULL if no valid primary pseudo terminal or if ptsname() fails.
-//  The name of the secondary pseudo terminal as a NULL terminated C string
-//  that comes from static memory, so a copy of the string should be
-//  made as subsequent calls can change this value.
-const char *PseudoTerminal::GetSecondaryName(char *error_str,
-                                             size_t error_len) const {
-  if (error_str)
-    error_str[0] = '\0';
-
-  if (m_primary_fd < 0) {
-    if (error_str)
-      ::snprintf(error_str, error_len, "%s",
-                 "primary file descriptor is invalid");
-    return nullptr;
-  }
-  const char *secondary_name = ::ptsname(m_primary_fd);
-
-  if (error_str && secondary_name == nullptr)
-    ErrnoToStr(error_str, error_len);
-
-  return secondary_name;
+std::string PseudoTerminal::GetSecondaryName() const {
+  assert(m_primary_fd >= 0);
+#if HAVE_PTSNAME_R
+  char buf[PATH_MAX];
+  buf[0] = '\0';
+  int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
+  assert(r == 0);
+  return buf;
+#else
+  static std::mutex mutex;
+  std::lock_guard<std::mutex> guard(mutex);
+  const char *r = ptsname(m_primary_fd);
+  assert(r != nullptr);
+  return r;
+#endif
 }
 
 // Fork a child process and have its stdio routed to a pseudo terminal.

lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm

@@ -407,10 +407,7 @@ static Status HandleFileAction(ProcessLaunchInfo &launch_info,
         const int master_fd = launch_info.GetPTY().GetPrimaryFileDescriptor();
         if (master_fd != PseudoTerminal::invalid_fd) {
           // Check in case our file action open wants to open the secondary
-          const char *secondary_path =
-              launch_info.GetPTY().GetSecondaryName(NULL, 0);
-          if (secondary_path) {
-            FileSpec secondary_spec(secondary_path);
+          FileSpec secondary_spec(launch_info.GetPTY().GetSecondaryName());
           if (file_spec == secondary_spec) {
             int secondary_fd =
                 launch_info.GetPTY().GetSecondaryFileDescriptor();
@@ -427,7 +424,6 @@ static Status HandleFileAction(ProcessLaunchInfo &launch_info,
             return error; // Success
           }
         }
-        }
         Status posix_error;
         int oflag = file_action->GetActionArgument();
         int created_fd =

lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp

@@ -380,8 +380,7 @@ Status ProcessFreeBSD::DoLaunch(Module *module,
   FileSpec stdout_file_spec{};
   FileSpec stderr_file_spec{};
 
-  const FileSpec dbg_pts_file_spec{
-      launch_info.GetPTY().GetSecondaryName(NULL, 0)};
+  const FileSpec dbg_pts_file_spec{launch_info.GetPTY().GetSecondaryName()};
 
   file_action = launch_info.GetFileActionForFD(STDIN_FILENO);
   stdin_file_spec =

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -818,7 +818,7 @@ Status ProcessGDBRemote::DoLaunch(lldb_private::Module *exe_module,
         // does a lot of output.
         if ((!stdin_file_spec || !stdout_file_spec || !stderr_file_spec) &&
             pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY, nullptr, 0)) {
-          FileSpec secondary_name{pty.GetSecondaryName(nullptr, 0)};
+          FileSpec secondary_name(pty.GetSecondaryName());
 
           if (!stdin_file_spec)
             stdin_file_spec = secondary_name;