Revert "Reimplement GetAppOutput and friends on Mac with posix_spawn."

This reverts commit dcb738f9.

Reason for revert: Broke mac-dbg compile https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/mac-dbg/1596

Original change's description:
> Reimplement GetAppOutput and friends on Mac with posix_spawn.
> 
> Mac no longer uses launch_posix.cc and all process creation goes through
> posix_spawn now.
> 
> Bug: 179923
> Change-Id: I5206cace69ea7863a72b58e1a59f2465492d4cbd
> Reviewed-on: https://chromium-review.googlesource.com/c/1313334
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Commit-Queue: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604982}

TBR=rsesek@chromium.org,mark@chromium.org

Change-Id: I9fd7e4c80f66de9acd35c7626cc65fdd5007d474
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 179923
Reviewed-on: https://chromium-review.googlesource.com/c/1315708Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604986}

base/BUILD.gn

@@ -1671,7 +1671,6 @@ jumbo_component("base") {
   # Desktop Mac.
   if (is_mac) {
     sources -= [
-      "process/launch_posix.cc",
       "profiler/native_stack_sampler_posix.cc",
       "sampling_heap_profiler/module_cache_posix.cc",
     ]

base/process/launch.h

@@ -297,12 +297,10 @@ BASE_EXPORT Process LaunchElevatedProcess(const CommandLine& cmdline,
 BASE_EXPORT Process LaunchProcess(const std::vector<std::string>& argv,
                                   const LaunchOptions& options);
 
-#if !defined(OS_MACOSX)
 // Close all file descriptors, except those which are a destination in the
 // given multimap. Only call this function in a child process where you know
 // that there aren't any other threads.
 BASE_EXPORT void CloseSuperfluousFds(const InjectiveMultimap& saved_map);
-#endif  // defined(OS_MACOSX)
 #endif  // defined(OS_WIN)
 
 #if defined(OS_WIN)
@@ -354,6 +352,23 @@ BASE_EXPORT bool GetAppOutputAndError(const std::vector<std::string>& argv,
 // the current process's scheduling priority to a high priority.
 BASE_EXPORT void RaiseProcessToHighPriority();
 
+#if defined(OS_MACOSX)
+// An implementation of LaunchProcess() that uses posix_spawn() instead of
+// fork()+exec(). This does not support the |pre_exec_delegate| and
+// |current_directory| options.
+Process LaunchProcessPosixSpawn(const std::vector<std::string>& argv,
+                                const LaunchOptions& options);
+
+// Restore the default exception handler, setting it to Apple Crash Reporter
+// (ReportCrash).  When forking and execing a new process, the child will
+// inherit the parent's exception ports, which may be set to the Breakpad
+// instance running inside the parent.  The parent's Breakpad instance should
+// not handle the child's exceptions.  Calling RestoreDefaultExceptionHandler
+// in the child after forking will restore the standard exception handler.
+// See http://crbug.com/20371/ for more details.
+void RestoreDefaultExceptionHandler();
+#endif  // defined(OS_MACOSX)
+
 // Creates a LaunchOptions object suitable for launching processes in a test
 // binary. This should not be called in production/released code.
 BASE_EXPORT LaunchOptions LaunchOptionsForTest();

base/process/launch_mac.cc

@@ -11,14 +11,10 @@
 #include <sys/syscall.h>
 #include <sys/wait.h>
 
-#include "base/command_line.h"
-#include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/mac/availability.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/threading/scoped_blocking_call.h"
-#include "base/threading/thread_restrictions.h"
-#include "base/trace_event/trace_event.h"
 
 // Changes the current thread's directory to a path or directory file
 // descriptor. libpthread only exposes a syscall wrapper starting in
@@ -31,13 +27,6 @@ int pthread_fchdir_np(int fd) API_AVAILABLE(macosx(10.12));
 
 namespace base {
 
-// Friend and derived class of ScopedAllowBaseSyncPrimitives which allows
-// GetAppOutputInternal() to join a process. GetAppOutputInternal() can't itself
-// be a friend of ScopedAllowBaseSyncPrimitives because it is in the anonymous
-// namespace.
-class GetAppOutputScopedAllowBaseSyncPrimitives
-    : public base::ScopedAllowBaseSyncPrimitives {};
-
 namespace {
 
 // DPSXCHECK is a Debug Posix Spawn Check macro. The posix_spawn* family of
@@ -111,78 +100,27 @@ int ResetCurrentThreadDirectory() {
   }
 }
 
-struct GetAppOutputOptions {
-  // Whether to pipe stderr to stdout in |output|.
-  bool include_stderr = false;
-  // Caller-supplied string poiter for the output.
-  std::string* output = nullptr;
-  // Result exit code of Process::Wait().
-  int exit_code = 0;
-};
-
-bool GetAppOutputInternal(const std::vector<std::string>& argv,
-                          GetAppOutputOptions* gao_options) {
-  ScopedFD read_fd, write_fd;
-  {
-    int pipefds[2];
-    if (pipe(pipefds) != 0) {
-      DPLOG(ERROR) << "pipe";
-      return false;
-    }
-    read_fd.reset(pipefds[0]);
-    write_fd.reset(pipefds[1]);
-  }
-
-  LaunchOptions launch_options;
-  launch_options.fds_to_remap.emplace_back(write_fd.get(), STDOUT_FILENO);
-  if (gao_options->include_stderr) {
-    launch_options.fds_to_remap.emplace_back(write_fd.get(), STDERR_FILENO);
-  }
-
-  Process process = LaunchProcess(argv, launch_options);
-
-  // Close the parent process' write descriptor, so that EOF is generated in
-  // read loop below.
-  write_fd.reset();
-
-  // Read the child's output before waiting for its exit, otherwise the pipe
-  // buffer may fill up if the process is producing a lot of output.
-  std::string* output = gao_options->output;
-  output->clear();
-
-  const size_t kBufferSize = 1024;
-  size_t total_bytes_read = 0;
-  ssize_t read_this_pass = 0;
-  do {
-    output->resize(output->size() + kBufferSize);
-    read_this_pass = HANDLE_EINTR(
-        read(read_fd.get(), &(*output)[total_bytes_read], kBufferSize));
-    if (read_this_pass >= 0) {
-      total_bytes_read += read_this_pass;
-      output->resize(total_bytes_read);
-    }
-  } while (read_this_pass > 0);
-
-  // Reap the child process.
-  GetAppOutputScopedAllowBaseSyncPrimitives allow_wait;
-  if (!process.WaitForExit(&gao_options->exit_code)) {
-    return false;
-  }
-
-  return read_this_pass == 0;
-}
-
 }  // namespace
 
-Process LaunchProcess(const CommandLine& cmdline,
-                      const LaunchOptions& options) {
-  return LaunchProcess(cmdline.argv(), options);
+void RestoreDefaultExceptionHandler() {
+  // This function is tailored to remove the Breakpad exception handler.
+  // exception_mask matches s_exception_mask in
+  // third_party/breakpad/breakpad/src/client/mac/handler/exception_handler.cc
+  const exception_mask_t exception_mask = EXC_MASK_BAD_ACCESS |
+                                          EXC_MASK_BAD_INSTRUCTION |
+                                          EXC_MASK_ARITHMETIC |
+                                          EXC_MASK_BREAKPOINT;
+
+  // Setting the exception port to MACH_PORT_NULL may not be entirely
+  // kosher to restore the default exception handler, but in practice,
+  // it results in the exception port being set to Apple Crash Reporter,
+  // the desired behavior.
+  task_set_exception_ports(mach_task_self(), exception_mask, MACH_PORT_NULL,
+                           EXCEPTION_DEFAULT, THREAD_STATE_NONE);
 }
 
-Process LaunchProcess(const std::vector<std::string>& argv,
-                      const LaunchOptions& options) {
-  TRACE_EVENT0("base", "LaunchProcess");
-
+Process LaunchProcessPosixSpawn(const std::vector<std::string>& argv,
+                                const LaunchOptions& options) {
   PosixSpawnAttr attr;
 
   short flags = POSIX_SPAWN_CLOEXEC_DEFAULT;
@@ -279,38 +217,4 @@ Process LaunchProcess(const std::vector<std::string>& argv,
   return Process(pid);
 }
 
-bool GetAppOutput(const CommandLine& cl, std::string* output) {
-  return GetAppOutput(cl.argv(), output);
-}
-
-bool GetAppOutputAndError(const CommandLine& cl, std::string* output) {
-  return GetAppOutputAndError(cl.argv(), output);
-}
-
-bool GetAppOutputWithExitCode(const CommandLine& cl,
-                              std::string* output,
-                              int* exit_code) {
-  GetAppOutputOptions options;
-  options.output = output;
-  bool rv = GetAppOutputInternal(cl.argv(), &options);
-  *exit_code = options.exit_code;
-  return rv;
-}
-
-bool GetAppOutput(const std::vector<std::string>& argv, std::string* output) {
-  GetAppOutputOptions options;
-  options.output = output;
-  return GetAppOutputInternal(argv, &options) &&
-         options.exit_code == EXIT_SUCCESS;
-}
-
-bool GetAppOutputAndError(const std::vector<std::string>& argv,
-                          std::string* output) {
-  GetAppOutputOptions options;
-  options.include_stderr = true;
-  options.output = output;
-  return GetAppOutputInternal(argv, &options) &&
-         options.exit_code == EXIT_SUCCESS;
-}
-
 }  // namespace base

base/process/launch_posix.cc

@@ -60,10 +60,13 @@
 #endif
 
 #if defined(OS_MACOSX)
-#error "macOS should use launch_mac.cc"
-#endif
+#include <crt_externs.h>
+#include <sys/event.h>
 
+#include "base/feature_list.h"
+#else
 extern char** environ;
+#endif
 
 namespace base {
 
@@ -78,16 +81,29 @@ class GetAppOutputScopedAllowBaseSyncPrimitives
 
 namespace {
 
+#if defined(OS_MACOSX)
+const Feature kMacLaunchProcessPosixSpawn{"MacLaunchProcessPosixSpawn",
+                                          FEATURE_ENABLED_BY_DEFAULT};
+#endif
+
 // Get the process's "environment" (i.e. the thing that setenv/getenv
 // work with).
 char** GetEnvironment() {
+#if defined(OS_MACOSX)
+  return *_NSGetEnviron();
+#else
   return environ;
+#endif
 }
 
 // Set the process's "environment" (i.e. the thing that setenv/getenv
 // work with).
 void SetEnvironment(char** env) {
+#if defined(OS_MACOSX)
+  *_NSGetEnviron() = env;
+#else
   environ = env;
+#endif
 }
 
 // Set the calling thread's signal mask to new_sigmask and return
@@ -206,6 +222,8 @@ typedef std::unique_ptr<DIR, ScopedDIRClose> ScopedDIR;
 
 #if defined(OS_LINUX) || defined(OS_AIX)
 static const char kFDDir[] = "/proc/self/fd";
+#elif defined(OS_MACOSX)
+static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_SOLARIS)
 static const char kFDDir[] = "/dev/fd";
 #elif defined(OS_FREEBSD)
@@ -284,6 +302,12 @@ Process LaunchProcess(const CommandLine& cmdline,
 Process LaunchProcess(const std::vector<std::string>& argv,
                       const LaunchOptions& options) {
   TRACE_EVENT0("base", "LaunchProcess");
+#if defined(OS_MACOSX)
+  // TODO(rsesek): Remove this feature. https://crbug.com/179923.
+  if (FeatureList::IsEnabled(kMacLaunchProcessPosixSpawn)) {
+    return LaunchProcessPosixSpawn(argv, options);
+  }
+#endif
 
   InjectiveMultimap fd_shuffle1;
   InjectiveMultimap fd_shuffle2;
@@ -403,6 +427,10 @@ Process LaunchProcess(const std::vector<std::string>& argv,
       }
     }
 
+#if defined(OS_MACOSX)
+    RestoreDefaultExceptionHandler();
+#endif  // defined(OS_MACOSX)
+
     ResetChildSignalHandlersToDefaults();
     SetSignalMask(orig_sigmask);
 
@@ -469,9 +497,11 @@ Process LaunchProcess(const std::vector<std::string>& argv,
       RAW_CHECK(chdir(current_directory) == 0);
     }
 
+#if !defined(OS_MACOSX)
     if (options.pre_exec_delegate != nullptr) {
       options.pre_exec_delegate->RunAsyncSafe();
     }
+#endif
 
     const char* executable_path = !options.real_path.empty() ?
         options.real_path.value().c_str() : argv_cstr[0];
@@ -553,6 +583,10 @@ static bool GetAppOutputInternal(
       // DANGER: no calls to malloc or locks are allowed from now on:
       // http://crbug.com/36678
 
+#if defined(OS_MACOSX)
+      RestoreDefaultExceptionHandler();
+#endif
+
       // Obscure fork() rule: in the child, if you don't end up doing exec*(),
       // you call _exit() instead of exit(). This is because _exit() does not
       // call any previously-registered (in the parent) exit handlers, which