[Support] Modernize process launching API

[aganea]

Summary

As discussed previously, and before adding /MP

The objective here is to stop doing user logic based on process return values, and instead provide a simple process lifetime API. Without this, it is a bit harder to compose programs launching, in order to handle /MP.

The other part of this patch is the explicit llvm::sys::ReturnCodes, which makes it easier to understand/find where LLVM programs return something.

For now I've left there the 'old' API (ie. llvm::sys::ExecuteAndWait, llvm::sys::Wait) which is now superseeded/temporarily.stubbed by the 'new' stateful API (ie. llvm::sys::Program::executeAndWait and llvm::sys::Program::waitCompletion). The plan is however to progressively remove usage of the 'old' API just after this patch is commited. Let me know if you want to see in advance how that looks.

I've tested this patch on several configurations - let me know if I should test more:

• Windows 10, compiled with MSVC 15.9.11, in Debug, with assertions on.
• Windows 10, compiled with Clang 8.0, Release, no assertions.
• WSL / Ubuntu 18.04, compiled with GCC 7.3, Release, no assertions.
• On a real Ubuntu 18.04 box, compiled with GCC 7.4, Release, no assertions.

[thakis]

$ pbpaste | diffstat
 10 files changed, 497 insertions(+), 276 deletions(-)

The "modernized" api is a good chunk wordier. Can you outline how composing launching would look like with and without this change?

include/llvm/Support/Program.h

68

Do we need so many different failure types? Isn't "Failure" enough?

lib/Support/Unix/Program.inc

180

clang-format

lib/Support/Windows/Program.inc

371

clang-format

405

clang-format

433

Both if and else now end in a return; can move that after the branches

[aganea]

In D62347#1516136, @thakis wrote:

The "modernized" api is a good chunk wordier. Can you outline how composing launching would look like with and without this change?

You're right, I'll reduce it. There are a few unrelated changes that could be done after. I'll have to get back to you regarding the "without this change".

include/llvm/Support/Program.h

68

The API needs at least that many states, that was the goal in the first place - to stop doing 'if's and 'switch'es based on the return value and/or the PID and/or whether there's an ErrorString or not. I'll see if I can simplify.

[aganea]

Dropping this, I'll just deal with the existing process launching API.

include/llvm/Support/Program.h

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/ErrorOr.h"
 #include <system_error>
 
 namespace llvm {
 namespace sys {
 
-  /// This is the OS-specific separator for PATH like environment variables:
-  // a colon on Unix or a semicolon on Windows.
+/// Some return codes Clang or other LLVM tools can return
+enum class ReturnCode {
+  Success = 0,
+  Failure = 1, // General program failure.
+               // Also used by Windows' TaskManager when a program is killed
+               // manually by the user. Clang also uses this as time-out
+               // termination code for its children processes.
+#ifdef _WIN32
+  Win32Abort = 3, // Returned by abort() on Windows.
+#endif
+  GenerateCrashDiagnostic =
+      70, // EX_SOFTWARE on BSD, used by Clang to forcibly generate a crash dump
+#if LLVM_ON_UNIX
+  BrokenPipe = 74, // EX_IOERR
+  UnixNotAnExecutable = 126,
+  UnixExecutableNotFound = 127,
+#endif
+  BugpointClientFail = 255
+};
+
 #if defined(LLVM_ON_UNIX)
-  const char EnvPathSeparator = ':';
-#elif defined (_WIN32)
-  const char EnvPathSeparator = ';';
+#elif defined(_WIN32)
 #endif
 
+/// The program execution status, as a finite state machine:
+///
+///            /--> ExecutionFailure
+/// NotStarted ---> StartedAndExecuting ---> CompletedSuccess
+///                                     \--> CompletedWithError
+///                                     \--> Crashed
+///                                     \--> Killed
+enum class ExecutionStatus : uint8_t {
+  NotStarted = 0,
+  StartedAndExecuting, // process is up and running
+  CompletedSuccess,    // process completed with no error
+  CompletedWithError,  // process completed with error, see ReturnCode
+  ExecutionFailure,    // process can't be started
+  Crashed,             // process crashed
+  TimedOut             // process was forcibly terminated
+};
+
+/// This struct encapsulates information about a process.
+class Program {
+public:
 #if defined(_WIN32)
-  typedef unsigned long procid_t; // Must match the type of DWORD on Windows.
-  typedef void *process_t;        // Must match the type of HANDLE on Windows.
+  typedef void *HandleType; // Must match the type of HANDLE on Windows.
 #else
-  typedef pid_t procid_t;
-  typedef procid_t process_t;
+  typedef pid_t HandleType;
 #endif
 
-  /// This struct encapsulates information about a process.
-  struct ProcessInfo {
-    enum : procid_t { InvalidPid = 0 };
+  HandleType Handle{}; /// Platform-dependent process object.
+
+  /// The program execution status
+  ExecutionStatus Status = ExecutionStatus::NotStarted;
+
+private:
+  /// The return code after execution, only set if the program terminates with
+  /// an error
+  int ReturnCode = (int)ReturnCode::Success;
+
+  /// A possible error message after execution
+  std::string ErrMsg;
+
+public:
+  static Program makeExecuting(HandleType H);
+  static Program makeSuccessResult();
+  static Program makeErrorResult(int RC);
+  static Program makeFailureResult(const std::string &S);
+  static Program makeCrashedResult(int RC, const std::string &S);
+
+  static Program executeAndWait(StringRef Program, ArrayRef<StringRef> Args,
+                                Optional<ArrayRef<StringRef>> Env = None,
+                                ArrayRef<Optional<StringRef>> Redirects = {},
+                                unsigned SecondsToWait = 0,
+                                unsigned MemoryLimit = 0);
 
-    procid_t Pid;      /// The process identifier.
-    process_t Process; /// Platform-dependent process object.
+  static Program executeNoWait(StringRef Program, ArrayRef<StringRef> Args,
+                               Optional<ArrayRef<StringRef>> Env = None,
+                               ArrayRef<Optional<StringRef>> Redirects = {},
+                               unsigned MemoryLimit = 0);
+
+  void waitCompletion(unsigned SecondsToWait, bool WaitUntilTerminates);
+
+  /// true if the program is running or has completed successfully
+  explicit operator bool() const;
+
+  /// Program was started
+  bool started() const;
+  
+  /// Program was started and still running
+  bool running() const;
+
+  /// Program has completed with errors
+  bool completedError() const;
+  
+  /// Program has completed with successfully
+  bool completedSuccess() const;
   
-    /// The return code, set after execution.
-    int ReturnCode;
+  /// Program can't start, has crashed, or has been killed
+  bool notCompletedOrFailed() const;
 
-    ProcessInfo();
-  };
+  /// Program can't start
+  bool executionFailure() const;
+
+  /// Program has crashed
+  bool crashed(bool ExpectCrash = false) const;
+  
+  /// Program timed out a specified delay and was killed
+  bool timedOut() const;
+
+  void clear();
+  
+  int ret() const;
+
+  const std::string &errMsg() const;
 
   /// Find the first executable file \p Name in \p Paths.
   ///
   /// This does not perform hashing as a shell would but instead stats each PATH
   /// entry individually so should generally be avoided. Core LLVM library
   /// functions and options should instead require fully specified paths.
   ///
   /// \param Name name of the executable to find. If it contains any system
int ExecuteAndWait(
       ///< - no memory limit.
       std::string *ErrMsg = nullptr, ///< If non-zero, provides a pointer to a
       ///< string instance in which error messages will be returned. If the
       ///< string is non-empty upon return an error occurred while invoking the
       ///< program.
       bool *ExecutionFailed = nullptr);
 
   /// Similar to ExecuteAndWait, but returns immediately.
-  /// @returns The \see ProcessInfo of the newly launced process.
+  /// @returns The \see ProcessInfo of the newly launched process.
   /// \note On Microsoft Windows systems, users will need to either call
   /// \see Wait until the process finished execution or win32 CloseHandle() API
   /// on ProcessInfo.ProcessHandle to avoid memory leaks.
-  ProcessInfo ExecuteNoWait(StringRef Program, ArrayRef<StringRef> Args,
-                            Optional<ArrayRef<StringRef>> Env,
-                            ArrayRef<Optional<StringRef>> Redirects = {},
-                            unsigned MemoryLimit = 0,
-                            std::string *ErrMsg = nullptr,
-                            bool *ExecutionFailed = nullptr);
+  Program ExecuteNoWait(StringRef Program, ArrayRef<StringRef> Args,
+                        Optional<ArrayRef<StringRef>> Env = None,
 
   /// Return true if the given arguments fit within system-specific
   /// argument length limits.
   bool commandLineFitsWithinSystemLimits(StringRef Program,
                                          ArrayRef<StringRef> Args);
 
   /// Return true if the given arguments fit within system-specific
   /// argument length limits.
public:
 
   /// This function waits for the process specified by \p PI to finish.
   /// \returns A \see ProcessInfo struct with Pid set to:
   /// \li The process id of the child process if the child process has changed
   /// state.
   /// \li 0 if the child process has not changed state.
   /// \note Users of this function should always check the ReturnCode member of
   /// the \see ProcessInfo returned from this function.
-  ProcessInfo Wait(
-      const ProcessInfo &PI, ///< The child process that should be waited on.
+  Program Wait(
+      const Program &P, ///< The child process that should be waited on.
       unsigned SecondsToWait, ///< If non-zero, this specifies the amount of
       ///< time to wait for the child process to exit. If the time expires, the
       ///< child is killed and this function returns. If zero, this function
       ///< will perform a non-blocking wait on the child process.
       bool WaitUntilTerminates, ///< If true, ignores \p SecondsToWait and waits
       ///< until child has terminated.
       std::string *ErrMsg = nullptr ///< If non-zero, provides a pointer to a
       ///< string instance in which error messages will be returned. If the
       ///< string is non-empty upon return an error occurred while invoking the
       ///< program.
       );
 
 #if defined(_WIN32)
   /// Given a list of command line arguments, quote and escape them as necessary
   /// to build a single flat command line appropriate for calling CreateProcess
   /// on
   /// Windows.
   std::string flattenWindowsCommandLine(ArrayRef<StringRef> Args);
 #endif
   }
+
+  std::string toString(const sys::Program &P);
 }
 
 #endif

lib/Support/Program.cpp

 //===-- Program.cpp - Implement OS Program Concept --------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements the operating system Program concept.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Program.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/ErrorHandling.h"
 #include <system_error>
+
 using namespace llvm;
 using namespace sys;
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only TRULY operating system
 //===          independent code.
 //===----------------------------------------------------------------------===//
 
-static bool Execute(ProcessInfo &PI, StringRef Program,
-                    ArrayRef<StringRef> Args, Optional<ArrayRef<StringRef>> Env,
+static sys::Program Execute(StringRef Program, ArrayRef<StringRef> Args,
+                            Optional<ArrayRef<StringRef>> Env,
+                            ArrayRef<Optional<StringRef>> Redirects,
+                            unsigned MemoryLimit);
+
+sys::Program
+sys::Program::executeAndWait(StringRef Program, ArrayRef<StringRef> Args,
+                             Optional<ArrayRef<StringRef>> Env,
+                             ArrayRef<Optional<StringRef>> Redirects,
+                             unsigned SecondsToWait, unsigned MemoryLimit) {
+  assert(Redirects.empty() || Redirects.size() == 3);
+  sys::Program P = Execute(Program, Args, Env, Redirects, MemoryLimit);
+  if (!P)
+    return P;
+  P.waitCompletion(SecondsToWait, SecondsToWait == 0);
+  return P;
+}
+
+sys::Program
+sys::Program::executeNoWait(StringRef Program, ArrayRef<StringRef> Args,
+                            Optional<ArrayRef<StringRef>> Env,
                             ArrayRef<Optional<StringRef>> Redirects,
-                    unsigned MemoryLimit, std::string *ErrMsg);
+                            unsigned MemoryLimit) {
+  assert(Redirects.empty() || Redirects.size() == 3);
+  return Execute(Program, Args, Env, Redirects, MemoryLimit);
+}
 
 int sys::ExecuteAndWait(StringRef Program, ArrayRef<StringRef> Args,
                         Optional<ArrayRef<StringRef>> Env,
                         ArrayRef<Optional<StringRef>> Redirects,
                         unsigned SecondsToWait, unsigned MemoryLimit,
                         std::string *ErrMsg, bool *ExecutionFailed) {
   assert(Redirects.empty() || Redirects.size() == 3);
-  ProcessInfo PI;
-  if (Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg)) {
+  sys::Program P = Execute(Program, Args, Env, Redirects, MemoryLimit);
+  if (!P) {
+    if (ErrMsg && (P.completedError() || P.notCompletedOrFailed()))
+      *ErrMsg = P.errMsg();
     if (ExecutionFailed)
-      *ExecutionFailed = false;
-    ProcessInfo Result = Wait(
-        PI, SecondsToWait, /*WaitUntilTerminates=*/SecondsToWait == 0, ErrMsg);
-    return Result.ReturnCode;
+      *ExecutionFailed = !P;
+    if (P.completedError() || P.crashed())
+      return P.ret();
+    if (P.timedOut())
+      return -2;
+    if (P.notCompletedOrFailed())
+      return -1;
+    return 0;
   }
-
+  P.waitCompletion(SecondsToWait, SecondsToWait == 0);
+  if (ErrMsg && (P.completedError() || P.notCompletedOrFailed()))
+    *ErrMsg = P.errMsg();
   if (ExecutionFailed)
-    *ExecutionFailed = true;
-
-  return -1;
+    *ExecutionFailed = !P;
+  if (P.completedError() || P.crashed())
+    return P.ret();
+  return 0;
 }
 
-ProcessInfo sys::ExecuteNoWait(StringRef Program, ArrayRef<StringRef> Args,
+sys::Program sys::ExecuteNoWait(StringRef Program, ArrayRef<StringRef> Args,
                                 Optional<ArrayRef<StringRef>> Env,
                                 ArrayRef<Optional<StringRef>> Redirects,
                                 unsigned MemoryLimit, std::string *ErrMsg,
                                 bool *ExecutionFailed) {
   assert(Redirects.empty() || Redirects.size() == 3);
-  ProcessInfo PI;
-  if (ExecutionFailed)
-    *ExecutionFailed = false;
-  if (!Execute(PI, Program, Args, Env, Redirects, MemoryLimit, ErrMsg))
-    if (ExecutionFailed)
-      *ExecutionFailed = true;
+  return Execute(Program, Args, Env, Redirects, MemoryLimit);
+}
 
-  return PI;
+Program sys::Wait(const Program &P, unsigned SecondsToWait,
+                  bool WaitUntilTerminates, std::string *ErrMsg) {
+  Program WaitResult = P;
+  WaitResult.waitCompletion(SecondsToWait, WaitUntilTerminates);
+  if (!WaitResult && ErrMsg && (P.completedError() || P.notCompletedOrFailed()))
+    *ErrMsg = WaitResult.errMsg();
+  return WaitResult;
 }
 
 bool sys::commandLineFitsWithinSystemLimits(StringRef Program,
                                             ArrayRef<const char *> Args) {
   SmallVector<StringRef, 8> StringRefArgs;
   StringRefArgs.reserve(Args.size());
   for (const char *A : Args)
     StringRefArgs.emplace_back(A);
   return commandLineFitsWithinSystemLimits(Program, StringRefArgs);
 }
 
+Program Program::makeFailureResult(const std::string &S) {
+  Program P;
+  P.Status = ExecutionStatus::ExecutionFailure;
+  P.ReturnCode = (int)ReturnCode::Failure;
+  P.ErrMsg = S;
+  return P;
+}
+
+Program Program::makeCrashedResult(int RC, const std::string &S) {
+  Program P;
+  P.Status = ExecutionStatus::Crashed;
+  P.ErrMsg = S;
+  P.ReturnCode = RC;
+  return P;
+}
+
+Program Program::makeErrorResult(int RC) {
+  Program P;
+  P.Status = ExecutionStatus::CompletedWithError;
+  P.ReturnCode = RC;
+  return P;
+}
+
+Program Program::makeSuccessResult() {
+  Program P;
+  P.Status = ExecutionStatus::CompletedSuccess;
+  return P;
+}
+
+Program Program::makeExecuting(HandleType H) {
+  sys::Program P;
+  P.Status = sys::ExecutionStatus::StartedAndExecuting;
+  P.Handle = H;
+  return P;
+}
+
+Program::operator bool() const {
+  return Status == ExecutionStatus::StartedAndExecuting ||
+         Status == ExecutionStatus::CompletedSuccess;
+}
+
+bool Program::started() const { return Status > ExecutionStatus::NotStarted; }
+
+bool Program::running() const {
+  return Status == ExecutionStatus::StartedAndExecuting;
+}
+
+bool Program::completedError() const {
+  return Status == ExecutionStatus::CompletedWithError;
+}
+
+bool Program::completedSuccess() const {
+  return Status == ExecutionStatus::CompletedSuccess;
+}
+
+bool Program::notCompletedOrFailed() const {
+  return Status >= ExecutionStatus::ExecutionFailure;
+}
+
+bool Program::executionFailure() const {
+  return Status == ExecutionStatus::ExecutionFailure;
+}
+
+bool Program::crashed(bool ExpectCrash) const {
+  if (Status == ExecutionStatus::Crashed)
+    return true;
+  if (!ReturnCode)
+    return false;
+  if (ReturnCode < 0 || ReturnCode == (int)ReturnCode::GenerateCrashDiagnostic)
+    return true;
+#ifdef _WIN32
+  // Handle abort() in msvcrt -- It has exit code as 3.  abort(), aka
+  // unreachable, should be recognized as a crash.  However, some binaries use
+  // exit code 3 on non-crash failure paths, so only do this if we expect a
+  // crash.
+  if (ExpectCrash && ReturnCode == (int)ReturnCode::Win32Abort)
+    return true;
+#endif
+  return false;
+}
+
+bool Program::timedOut() const { return Status == ExecutionStatus::TimedOut; }
+
+void Program::clear() { *this = Program(); }
+
+int Program::ret() const {
+  assert(Status == ExecutionStatus::CompletedWithError ||
+         Status == ExecutionStatus::Crashed);
+  return ReturnCode;
+}
+
+const std::string &Program::errMsg() const {
+  assert(Status >= ExecutionStatus::CompletedWithError);
+  return ErrMsg;
+}
+
+std::string llvm::toString(const sys::Program &P) {
+  switch (P.Status) {
+  case ExecutionStatus::NotStarted:
+    return "not started";
+  case ExecutionStatus::StartedAndExecuting:
+    return "running";
+  case ExecutionStatus::CompletedSuccess:
+    return "success";
+  case ExecutionStatus::CompletedWithError:
+    return ("exited with code (" + Twine(P.ret()) + ")").str();
+  case ExecutionStatus::ExecutionFailure:
+    return "execute failed: " + P.errMsg();
+  case ExecutionStatus::Crashed:
+    return ("crashed (" + Twine(P.ret()) + ") " + P.errMsg()).str();
+  case ExecutionStatus::TimedOut:
+    return "timed out";
+  }
+  llvm_unreachable("Unhandled execution status!");
+}
+
 // Include the platform-specific parts of this class.
 #ifdef LLVM_ON_UNIX
 #include "Unix/Program.inc"
 #endif
 #ifdef _WIN32
 #include "Windows/Program.inc"
 #endif

lib/Support/Unix/Program.inc

 #include <crt_externs.h> // _NSGetEnviron
 #endif
 #endif
 
 namespace llvm {
 
 using namespace sys;
 
-ProcessInfo::ProcessInfo() : Pid(0), ReturnCode(0) {}
-
 ErrorOr<std::string> sys::findProgramByName(StringRef Name,
                                             ArrayRef<StringRef> Paths) {
   assert(!Name.empty() && "Must have a name!");
   // Use the given path verbatim if it contains any slashes; this matches
   // the behavior of sh(1) and friends.
   if (Name.find('/') != StringRef::npos)
     return std::string(Name);
 
for (auto Path : Paths) {
     SmallString<128> FilePath(Path);
     sys::path::append(FilePath, Name);
     if (sys::fs::can_execute(FilePath.c_str()))
       return std::string(FilePath.str()); // Found the executable!
   }
   return errc::no_such_file_or_directory;
 }
 
-static bool RedirectIO(Optional<StringRef> Path, int FD, std::string* ErrMsg) {
+static Optional<Program> RedirectIO(Optional<StringRef> Path, int FD) {
   if (!Path) // Noop
-    return false;
+    return None;
   std::string File;
   if (Path->empty())
     // Redirect empty paths to /dev/null
     File = "/dev/null";
   else
     File = *Path;
 
   // Open the file
   int InFD = open(File.c_str(), FD == 0 ? O_RDONLY : O_WRONLY|O_CREAT, 0666);
-  if (InFD == -1) {
-    MakeErrMsg(ErrMsg, "Cannot open file '" + File + "' for "
-              + (FD == 0 ? "input" : "output"));
-    return true;
-  }
+  if (InFD == -1)
+    return Program::makeFailureResult(
+        MakeErrMsg("Cannot open file '" + File + "' for " +
+                   (FD == 0 ? "input" : "output")));
 
   // Install it as the requested FD
   if (dup2(InFD, FD) == -1) {
-    MakeErrMsg(ErrMsg, "Cannot dup2");
+    int errnum = errno;
     close(InFD);
-    return true;
+    return Program::makeFailureResult(MakeErrMsg("Cannot dup2", errnum));
   }
   close(InFD);      // Close the original FD
-  return false;
+  return None;
 }
 
 #ifdef HAVE_POSIX_SPAWN
-static bool RedirectIO_PS(const std::string *Path, int FD, std::string *ErrMsg,
-                          posix_spawn_file_actions_t *FileActions) {
+static Optional<Program>
+RedirectIO_PS(const std::string *Path, int FD,
   if (!Path) // Noop
-    return false;
+    return None;
   const char *File;
   if (Path->empty())
     // Redirect empty paths to /dev/null
     File = "/dev/null";
   else
     File = Path->c_str();
 
   if (int Err = posix_spawn_file_actions_addopen(
           FileActions, FD, File,
           FD == 0 ? O_RDONLY : O_WRONLY | O_CREAT, 0666))
-    return MakeErrMsg(ErrMsg, "Cannot dup2", Err);
-  return false;
+    return Program::makeFailureResult(MakeErrMsg("Cannot dup2", Err));
+
+  return None;
 }
 #endif
 
 static void TimeOutHandler(int Sig) {
 }
 
 static void SetMemoryLimits(unsigned size) {
 #if HAVE_SYS_RESOURCE_H && HAVE_GETRLIMIT && HAVE_SETRLIMIT

 toNullTerminatedCStringArray(ArrayRef<StringRef> Strings, StringSaver &Saver) {
   std::vector<const char *> Result;
   for (StringRef S : Strings)
     Result.push_back(Saver.save(S).data());
   Result.push_back(nullptr);
   return Result;
 }
 
-static bool Execute(ProcessInfo &PI, StringRef Program,
-                    ArrayRef<StringRef> Args, Optional<ArrayRef<StringRef>> Env,
+static Program Execute(StringRef Program, ArrayRef<StringRef> Args,
+                       Optional<ArrayRef<StringRef>> Env,
                        ArrayRef<Optional<StringRef>> Redirects,
-                    unsigned MemoryLimit, std::string *ErrMsg) {
-  if (!llvm::sys::fs::exists(Program)) {
-    if (ErrMsg)
-      *ErrMsg = std::string("Executable \"") + Program.str() +
-                std::string("\" doesn't exist!");
-    return false;
-  }
+                       unsigned MemoryLimit) {
+  if (!llvm::sys::fs::exists(Program))
+    return Program::makeFailureResult("Executable \"" + Program.str() + "\" doesn't exist!");
 
   BumpPtrAllocator Allocator;
   StringSaver Saver(Allocator);
   std::vector<const char *> ArgVector, EnvVector;
   const char **Argv = nullptr;
   const char **Envp = nullptr;
   ArgVector = toNullTerminatedCStringArray(Args, Saver);
   Argv = ArgVector.data();
if (!Redirects.empty()) {
           RedirectsStr[I] = &RedirectsStorage[I];
         }
       }
 
       FileActions = &FileActionsStore;
       posix_spawn_file_actions_init(FileActions);
 
       // Redirect stdin/stdout.
-      if (RedirectIO_PS(RedirectsStr[0], 0, ErrMsg, FileActions) ||
-          RedirectIO_PS(RedirectsStr[1], 1, ErrMsg, FileActions))
-        return false;
+      if (auto E = RedirectIO_PS(RedirectsStr[0], 0, FileActions))
+        return *E;
+        
+      if (auto E = RedirectIO_PS(RedirectsStr[1], 1, FileActions))
+        return *E;
+
       if (!Redirects[1] || !Redirects[2] || *Redirects[1] != *Redirects[2]) {
         // Just redirect stderr
-        if (RedirectIO_PS(RedirectsStr[2], 2, ErrMsg, FileActions))
-          return false;
+        if (auto E = RedirectIO_PS(RedirectsStr[2], 2, FileActions))
+          return *E;
       } else {
         // If stdout and stderr should go to the same place, redirect stderr
         // to the FD already open for stdout.
         if (int Err = posix_spawn_file_actions_adddup2(FileActions, 1, 2))
-          return !MakeErrMsg(ErrMsg, "Can't redirect stderr to stdout", Err);
+          return Program::makeFailureResult(
+              MakeErrMsg("Can't redirect stderr to stdout", Err));
       }
     }
 
     if (!Envp)
 #if !USE_NSGETENVIRON
       Envp = const_cast<const char **>(environ);
 #else
       // environ is missing in dylibs.
do {
                         /*attrp*/ nullptr, const_cast<char **>(Argv),
                         const_cast<char **>(Envp));
     } while (Err == EINTR && ++retries < maxRetries);
 
     if (FileActions)
       posix_spawn_file_actions_destroy(FileActions);
 
     if (Err)
-     return !MakeErrMsg(ErrMsg, "posix_spawn failed", Err);
+     return Program::makeFailureResult(MakeErrMsg("posix_spawn failed", Err));
 
-    PI.Pid = PID;
-    PI.Process = PID;
-
-    return true;
+    return Program::makeExecuting(PID);
   }
 #endif
 
   // Create a child process.
   int child = fork();
   switch (child) {
     // An error occurred:  Return to the caller.
     case -1:
-      MakeErrMsg(ErrMsg, "Couldn't fork");
-      return false;
+      return Program::makeFailureResult("Couldn't fork");
 
     // Child process: Execute the program.
     case 0: {
       // Redirect file descriptors...
       if (!Redirects.empty()) {
         // Redirect stdin
-        if (RedirectIO(Redirects[0], 0, ErrMsg)) { return false; }
+        if (auto E = RedirectIO(Redirects[0], 0))
+          return *E;
         // Redirect stdout
-        if (RedirectIO(Redirects[1], 1, ErrMsg)) { return false; }
+        if (auto E = RedirectIO(Redirects[1], 1))
+          return *E;
         if (Redirects[1] && Redirects[2] && *Redirects[1] == *Redirects[2]) {
           // If stdout and stderr should go to the same place, redirect stderr
           // to the FD already open for stdout.
           if (-1 == dup2(1,2)) {
-            MakeErrMsg(ErrMsg, "Can't redirect stderr to stdout");
-            return false;
+            return Program::makeFailureResult("Can't redirect stderr to stdout");
           }
         } else {
           // Just redirect stderr
-          if (RedirectIO(Redirects[2], 2, ErrMsg)) { return false; }
+          if (auto E = RedirectIO(Redirects[2], 2))
+            return *E;
         }
       }
 
       // Set memory limits
       if (MemoryLimit!=0) {
         SetMemoryLimits(MemoryLimit);
       }
 
       // Execute!
       std::string PathStr = Program;
       if (Envp != nullptr)
         execve(PathStr.c_str(), const_cast<char **>(Argv),
                const_cast<char **>(Envp));
       else
         execv(PathStr.c_str(), const_cast<char **>(Argv));
       // If the execve() failed, we should exit. Follow Unix protocol and
       // return 127 if the executable was not found, and 126 otherwise.
       // Use _exit rather than exit so that atexit functions and static
       // object destructors cloned from the parent process aren't
       // redundantly run, and so that any data buffered in stdio buffers
       // cloned from the parent aren't redundantly written out.
-      _exit(errno == ENOENT ? 127 : 126);
+      _exit(errno == ENOENT ? (int)ReturnCode::UnixExecutableNotFound
+                            : (int)ReturnCode::UnixNotAnExecutable);
     }
 
     // Parent process: Break out of the switch to do our processing.
     default:
       break;
   }
-
-  PI.Pid = child;
-  PI.Process = child;
-
-  return true;
+  return Program::makeExecuting(child);
 }
 
 namespace llvm {
 
-ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
-                      bool WaitUntilTerminates, std::string *ErrMsg) {
+void sys::Program::waitCompletion(unsigned SecondsToWait, bool WaitUntilTerminates) {
   struct sigaction Act, Old;
-  assert(PI.Pid && "invalid pid to wait on, process not started?");
+  assert(Handle && "invalid pid to wait on, process not started?");
+  assert(Status == ExecutionStatus::StartedAndExecuting);
 
   int WaitPidOptions = 0;
-  pid_t ChildPid = PI.Pid;
+  pid_t ChildPid = Handle;
   if (WaitUntilTerminates) {
     SecondsToWait = 0;
   } else if (SecondsToWait) {
     // Install a timeout handler.  The handler itself does nothing, but the
     // simple fact of having a handler at all causes the wait below to return
     // with EINTR, unlike if we used SIG_IGN.
     memset(&Act, 0, sizeof(Act));
     Act.sa_handler = TimeOutHandler;
     sigemptyset(&Act.sa_mask);
     sigaction(SIGALRM, &Act, &Old);
     alarm(SecondsToWait);
-  } else if (SecondsToWait == 0)
+  } else if (SecondsToWait == 0) {
     WaitPidOptions = WNOHANG;
+  }
 
   // Parent process: Wait for the child process to terminate.
   int status;
-  ProcessInfo WaitResult;
+  pid_t WaitResult;
 
   do {
-    WaitResult.Pid = waitpid(ChildPid, &status, WaitPidOptions);
-  } while (WaitUntilTerminates && WaitResult.Pid == -1 && errno == EINTR);
+    WaitResult = waitpid(ChildPid, &status, WaitPidOptions);
+  } while (WaitUntilTerminates && WaitResult == -1 && errno == EINTR);
 
-  if (WaitResult.Pid != PI.Pid) {
-    if (WaitResult.Pid == 0) {
+  if (WaitResult != Handle) {
+    if (WaitResult == 0) {
       // Non-blocking wait.
-      return WaitResult;
+      return;
     } else {
       if (SecondsToWait && errno == EINTR) {
-        // Kill the child.
-        kill(PI.Pid, SIGKILL);
+        // Timeout, kill the child process.
+        kill(Handle, SIGKILL);
 
         // Turn off the alarm and restore the signal handler
         alarm(0);
         sigaction(SIGALRM, &Old, nullptr);
 
         // Wait for child to die
         if (wait(&status) != ChildPid)
-          MakeErrMsg(ErrMsg, "Child timed out but wouldn't die");
+          ErrMsg = MakeErrMsg("Child timed out but wouldn't die");
         else
-          MakeErrMsg(ErrMsg, "Child timed out", 0);
+          ErrMsg = MakeErrMsg("Child timed out", 0);
 
-        WaitResult.ReturnCode = -2; // Timeout detected
-        return WaitResult;
+        Status = ExecutionStatus::TimedOut;
+        return;
       } else if (errno != EINTR) {
-        MakeErrMsg(ErrMsg, "Error waiting for child process");
-        WaitResult.ReturnCode = -1;
-        return WaitResult;
+        ErrMsg = MakeErrMsg("Error waiting for child process");
+        Status = ExecutionStatus::Crashed;
+        return;
       }
     }
   }
 
   // We exited normally without timeout, so turn off the timer.
   if (SecondsToWait && !WaitUntilTerminates) {
     alarm(0);
     sigaction(SIGALRM, &Old, nullptr);
   }
 
   // Return the proper exit status. Detect error conditions
   // so we can return -1 for them and set ErrMsg informatively.
   int result = 0;
   if (WIFEXITED(status)) {
     result = WEXITSTATUS(status);
-    WaitResult.ReturnCode = result;
+    ReturnCode = result;
+
+    if (ReturnCode == 0)
+      Status = ExecutionStatus::CompletedSuccess;
+    else
+      Status = ExecutionStatus::CompletedWithError;
 
-    if (result == 127) {
-      if (ErrMsg)
-        *ErrMsg = llvm::sys::StrError(ENOENT);
-      WaitResult.ReturnCode = -1;
-      return WaitResult;
-    }
-    if (result == 126) {
-      if (ErrMsg)
-        *ErrMsg = "Program could not be executed";
-      WaitResult.ReturnCode = -1;
-      return WaitResult;
+    if (result == (int)sys::ReturnCode::UnixExecutableNotFound) {
+      ErrMsg = llvm::sys::StrError(ENOENT);
+    }
+    else if (result == (int)sys::ReturnCode::UnixNotAnExecutable) {
+      ErrMsg = "Program could not be executed";
     }
   } else if (WIFSIGNALED(status)) {
-    if (ErrMsg) {
-      *ErrMsg = strsignal(WTERMSIG(status));
+    ErrMsg = strsignal(WTERMSIG(status));
 #ifdef WCOREDUMP
-      if (WCOREDUMP(status))
-        *ErrMsg += " (core dumped)";
+      ErrMsg += " (core dumped)";
 #endif
-    }
     // Return a special value to indicate that the process received an unhandled
     // signal during execution as opposed to failing to execute.
-    WaitResult.ReturnCode = -2;
+    Status = ExecutionStatus::Crashed;
   }
-  return WaitResult;
+  return;
 }
 
 std::error_code sys::ChangeStdinToBinary() {
   // Do nothing, as Unix doesn't differentiate between text and binary.
   return std::error_code();
 }
 
 std::error_code sys::ChangeStdoutToBinary() {
for (StringRef Arg : Args) {
     ArgLength += Arg.size() + 1;
     if (ArgLength > size_t(HalfArgMax)) {
       return false;
     }
   }
 
   return true;
 }
+
+std::string MakeErrMsg(Twine prefix, int errnum) {
+  if (errnum == -1)
+    errnum = errno;
+  return (prefix + ": " + llvm::sys::StrError(errnum)).str();
+}
 }

lib/Support/Unix/Signals.inc

sigprocmask(SIG_UNBLOCK, &SigMask, nullptr);
 
     if (std::find(std::begin(IntSigs), std::end(IntSigs), Sig)
         != std::end(IntSigs)) {
       if (auto OldInterruptFunction = InterruptFunction.exchange(nullptr))
         return OldInterruptFunction();
 
       // Send a special return code that drivers can check for, from sysexits.h.
       if (Sig == SIGPIPE)
-        exit(EX_IOERR);
+        exit((int)llvm::sys::ReturnCode::BrokenPipe);
 
       raise(Sig);   // Execute the default handler.
       return;
    }
   }
 
   // Otherwise if it is a fault (like SEGV) run any handler.
   llvm::sys::RunSignalHandlers();

lib/Support/Unix/Unix.h

 #ifdef HAVE_DLFCN_H
 # include <dlfcn.h>
 #endif
 
 #ifdef HAVE_FCNTL_H
 # include <fcntl.h>
 #endif
 
-/// This function builds an error message into \p ErrMsg using the \p prefix
+namespace llvm {
+
+/// This function returns an error message using the \p prefix
 /// string and the Unix error number given by \p errnum. If errnum is -1, the
 /// default then the value of errno is used.
 /// Make an error message
 ///
 /// If the error number can be converted to a string, it will be
 /// separated from prefix by ": ".
-static inline bool MakeErrMsg(
-  std::string* ErrMsg, const std::string& prefix, int errnum = -1) {
-  if (!ErrMsg)
-    return true;
-  if (errnum == -1)
-    errnum = errno;
-  *ErrMsg = prefix + ": " + llvm::sys::StrError(errnum);
-  return true;
-}
+std::string MakeErrMsg(Twine prefix, int errnum = -1);
 
-namespace llvm {
 namespace sys {
 
 /// Convert a struct timeval to a duration. Note that timeval can be used both
 /// as a time point and a duration. Be sure to check what the input represents.
 inline std::chrono::microseconds toDuration(const struct timeval &TV) {
   return std::chrono::seconds(TV.tv_sec) +
          std::chrono::microseconds(TV.tv_usec);
 }

lib/Support/Windows/DynamicLibrary.inc

void *DynamicLibrary::HandleSet::DLOpen(const char *File, std::string *Err) {
   // Create the instance and return it to be the *Process* handle
   // simillar to dlopen(NULL, RTLD_LAZY|RTLD_GLOBAL)
   if (!File)
     return &(*OpenedHandles);
 
   SmallVector<wchar_t, MAX_PATH> FileUnicode;
   if (std::error_code ec = windows::UTF8ToUTF16(File, FileUnicode)) {
     SetLastError(ec.value());
-    MakeErrMsg(Err, std::string(File) + ": Can't convert to UTF-16");
+    if (Err)
+      *Err = MakeErrMsg(Twine(File) + ": Can't convert to UTF-16");
     return &DynamicLibrary::Invalid;
   }
 
   HMODULE Handle = LoadLibraryW(FileUnicode.data());
   if (Handle == NULL) {
-    MakeErrMsg(Err, std::string(File) + ": Can't open");
+    if (Err)
+      *Err = MakeErrMsg(Twine(File) + ": Can't open");
     return &DynamicLibrary::Invalid;
   }
 
   return reinterpret_cast<void*>(Handle);
 }
 
 static DynamicLibrary::HandleSet *IsOpenedHandlesInstance(void *Handle) {
   if (!OpenedHandles.isConstructed())
static bool GetProcessModules(HANDLE H, DWORD &Bytes, HMODULE *Data = nullptr) {
   // MingW-32 don't have EnumProcessModulesEx.
   if (
 #ifdef _WIN64
       !EnumProcessModulesEx(H, Data, Bytes, &Bytes, LIST_MODULES_64BIT)
 #else
       !EnumProcessModules(H, Data, Bytes, &Bytes)
 #endif
      ) {
-    std::string Err;
-    if (MakeErrMsg(&Err, "EnumProcessModules failure"))
-      llvm::errs() << Err << "\n";
+    llvm::errs() << MakeErrMsg("EnumProcessModules failure") << "\n";
     return false;
   }
   return true;
 }
 
 void *DynamicLibrary::HandleSet::DLSym(void *Handle, const char *Symbol) {
   HandleSet* HS = IsOpenedHandlesInstance(Handle);
   if (!HS)

lib/Support/Windows/Process.inc

 const char *Process::ResetColor() {
   if (UseANSI) return "\033[0m";
   SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), defaultColors());
   return 0;
 }
 
 // Include GetLastError() in a fatal error message.
 static void ReportLastErrorFatal(const char *Msg) {
-  std::string ErrMsg;
-  MakeErrMsg(&ErrMsg, Msg);
+  std::string ErrMsg = MakeErrMsg(Msg);
   report_fatal_error(ErrMsg);
 }
 
 unsigned Process::GetRandomNumber() {
   HCRYPTPROV HCPC;
   if (!::CryptAcquireContextW(&HCPC, NULL, NULL, PROV_RSA_FULL,
                               CRYPT_VERIFYCONTEXT))
     ReportLastErrorFatal("Could not acquire a cryptographic context");

lib/Support/Windows/Program.inc

 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only Win32 specific code
 //===          and must not be UNIX code
 //===----------------------------------------------------------------------===//
 
 namespace llvm {
 
-ProcessInfo::ProcessInfo() : Pid(0), Process(0), ReturnCode(0) {}
-
 ErrorOr<std::string> sys::findProgramByName(StringRef Name,
                                             ArrayRef<StringRef> Paths) {
   assert(!Name.empty() && "Must have a name!");
 
   if (Name.find_first_of("/\\") != StringRef::npos)
     return std::string(Name);
 
   const wchar_t *Path = nullptr;
ErrorOr<std::string> sys::findProgramByName(StringRef Name,
   SmallVector<char, MAX_PATH> U8Result;
   if (std::error_code EC =
           windows::UTF16ToUTF8(U16Result.data(), U16Result.size(), U8Result))
     return EC;
 
   return std::string(U8Result.begin(), U8Result.end());
 }
 
-bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix) {
-  if (!ErrMsg)
-    return true;
+std::string MakeErrMsg(Twine prefix) {
   char *buffer = NULL;
   DWORD LastError = GetLastError();
   DWORD R = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER |
                                FORMAT_MESSAGE_FROM_SYSTEM |
                                FORMAT_MESSAGE_MAX_WIDTH_MASK,
                            NULL, LastError, 0, (LPSTR)&buffer, 1, NULL);
+  Twine ErrMsg;
   if (R)
-    *ErrMsg = prefix + ": " + buffer;
+    ErrMsg.concat(prefix + ": " + buffer);
   else
-    *ErrMsg = prefix + ": Unknown error";
-  *ErrMsg += " (0x" + llvm::utohexstr(LastError) + ")";
+    ErrMsg.concat(prefix + ": Unknown error");
+  ErrMsg.concat(" (0x" + llvm::utohexstr(LastError) + ")");
 
   LocalFree(buffer);
-  return R != 0;
+  return ErrMsg.str();
 }
 
-static HANDLE RedirectIO(Optional<StringRef> Path, int fd,
-                         std::string *ErrMsg) {
+static HANDLE RedirectIO(Optional<StringRef> Path, int fd) {
   HANDLE h;
   if (!Path) {
     if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)_get_osfhandle(fd),
                          GetCurrentProcess(), &h,
                          0, TRUE, DUPLICATE_SAME_ACCESS))
       return INVALID_HANDLE_VALUE;
     return h;
   }
if (windows::UTF8ToUTF16(fname, fnameUnicode))
       return INVALID_HANDLE_VALUE;
   } else {
     if (path::widenPath(fname, fnameUnicode))
       return INVALID_HANDLE_VALUE;
   }
   h = CreateFileW(fnameUnicode.data(), fd ? GENERIC_WRITE : GENERIC_READ,
                   FILE_SHARE_READ, &sa, fd == 0 ? OPEN_EXISTING : CREATE_ALWAYS,
                   FILE_ATTRIBUTE_NORMAL, NULL);
-  if (h == INVALID_HANDLE_VALUE) {
-    MakeErrMsg(ErrMsg, fname + ": Can't open file for " +
-        (fd ? "input" : "output"));
-  }
-
   return h;
 }
 
 }
 
-static bool Execute(ProcessInfo &PI, StringRef Program,
-                    ArrayRef<StringRef> Args, Optional<ArrayRef<StringRef>> Env,
+static Program Execute(StringRef Program, ArrayRef<StringRef> Args,
+                       Optional<ArrayRef<StringRef>> Env,
                        ArrayRef<Optional<StringRef>> Redirects,
-                    unsigned MemoryLimit, std::string *ErrMsg) {
-  if (!sys::fs::can_execute(Program)) {
-    if (ErrMsg)
-      *ErrMsg = "program not executable";
-    return false;
-  }
+                       unsigned MemoryLimit) {
+  if (!sys::fs::can_execute(Program))
+    return Program::makeFailureResult("program not executable");
 
   // can_execute may succeed by looking at Program + ".exe". CreateProcessW
   // will implicitly add the .exe if we provide a command line without an
   // executable path, but since we use an explicit executable, we have to add
   // ".exe" ourselves.
   SmallString<64> ProgramStorage;
   if (!sys::fs::exists(Program))
     Program = Twine(Program + ".exe").toStringRef(ProgramStorage);
static Program Execute(StringRef Program, ArrayRef<StringRef> Args,
   if (Env) {
     // An environment block consists of a null-terminated block of
     // null-terminated strings. Convert the array of environment variables to
     // an environment block by concatenating them.
     for (const auto E : *Env) {
       SmallVector<wchar_t, MAX_PATH> EnvString;
       if (std::error_code ec = windows::UTF8ToUTF16(E, EnvString)) {
         SetLastError(ec.value());
-        MakeErrMsg(ErrMsg, "Unable to convert environment variable to UTF-16");
-        return false;
+        return Program::makeFailureResult(
+            MakeErrMsg("Unable to convert environment variable to UTF-16"));
       }
 
       EnvBlock.insert(EnvBlock.end(), EnvString.begin(), EnvString.end());
       EnvBlock.push_back(0);
     }
     EnvBlock.push_back(0);
   }
 
   // Create a child process.
   STARTUPINFOW si;
   memset(&si, 0, sizeof(si));
   si.cb = sizeof(si);
   si.hStdInput = INVALID_HANDLE_VALUE;
   si.hStdOutput = INVALID_HANDLE_VALUE;
   si.hStdError = INVALID_HANDLE_VALUE;
 
   if (!Redirects.empty()) {
     si.dwFlags = STARTF_USESTDHANDLES;
 
-    si.hStdInput = RedirectIO(Redirects[0], 0, ErrMsg);
+    si.hStdInput = RedirectIO(Redirects[0], 0);
     if (si.hStdInput == INVALID_HANDLE_VALUE) {
-      MakeErrMsg(ErrMsg, "can't redirect stdin");
-      return false;
+      return Program::makeFailureResult(MakeErrMsg("can't redirect stdin"));
     }
-    si.hStdOutput = RedirectIO(Redirects[1], 1, ErrMsg);
+    si.hStdOutput = RedirectIO(Redirects[1], 1);
     if (si.hStdOutput == INVALID_HANDLE_VALUE) {
       CloseHandle(si.hStdInput);
-      MakeErrMsg(ErrMsg, "can't redirect stdout");
-      return false;
+      return Program::makeFailureResult(MakeErrMsg("can't redirect stdout"));
     }
     if (Redirects[1] && Redirects[2] && *Redirects[1] == *Redirects[2]) {
       // If stdout and stderr should go to the same place, redirect stderr
       // to the handle already open for stdout.
       if (!DuplicateHandle(GetCurrentProcess(), si.hStdOutput,
                            GetCurrentProcess(), &si.hStdError,
                            0, TRUE, DUPLICATE_SAME_ACCESS)) {
         CloseHandle(si.hStdInput);
         CloseHandle(si.hStdOutput);
-        MakeErrMsg(ErrMsg, "can't dup stderr to stdout");
-        return false;
+        return Program::makeFailureResult(
+            MakeErrMsg("can't dup stderr to stdout"));
       }
     } else {
       // Just redirect stderr
-      si.hStdError = RedirectIO(Redirects[2], 2, ErrMsg);
+      si.hStdError = RedirectIO(Redirects[2], 2);
       if (si.hStdError == INVALID_HANDLE_VALUE) {
         CloseHandle(si.hStdInput);
         CloseHandle(si.hStdOutput);
-        MakeErrMsg(ErrMsg, "can't redirect stderr");
-        return false;
+        return Program::makeFailureResult(MakeErrMsg("can't redirect stderr"));
       }
     }
   }
 
   PROCESS_INFORMATION pi;
   memset(&pi, 0, sizeof(pi));
 
   fflush(stdout);
   fflush(stderr);
 
   SmallVector<wchar_t, MAX_PATH> ProgramUtf16;
   if (std::error_code ec = path::widenPath(Program, ProgramUtf16)) {
     SetLastError(ec.value());
-    MakeErrMsg(ErrMsg,
-               std::string("Unable to convert application name to UTF-16"));
-    return false;
+    return Program::makeFailureResult(
+        MakeErrMsg("Unable to convert application name to UTF-16"));
   }
 
   SmallVector<wchar_t, MAX_PATH> CommandUtf16;
   if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16)) {
     SetLastError(ec.value());
-    MakeErrMsg(ErrMsg,
-               std::string("Unable to convert command-line to UTF-16"));
-    return false;
+    return Program::makeFailureResult(
+        MakeErrMsg("Unable to convert command-line to UTF-16"));
   }
 
   BOOL rc = CreateProcessW(ProgramUtf16.data(), CommandUtf16.data(), 0, 0,
                            TRUE, CREATE_UNICODE_ENVIRONMENT,
                            EnvBlock.empty() ? 0 : EnvBlock.data(), 0, &si,
                            &pi);
   DWORD err = GetLastError();
 
   // Regardless of whether the process got created or not, we are done with
   // the handles we created for it to inherit.
   CloseHandle(si.hStdInput);
   CloseHandle(si.hStdOutput);
   CloseHandle(si.hStdError);
 
   // Now return an error if the process didn't get created.
   if (!rc) {
     SetLastError(err);
-    MakeErrMsg(ErrMsg, std::string("Couldn't execute program '") +
-               Program.str() + "'");
-    return false;
+    return Program::makeFailureResult(
+        MakeErrMsg("Couldn't execute program '" + Program + "'"));
   }
 
-  PI.Pid = pi.dwProcessId;
-  PI.Process = pi.hProcess;
-
   // Make sure these get closed no matter what.
   ScopedCommonHandle hThread(pi.hThread);
 
   // Assign the process to a job if a memory limit is defined.
   ScopedJobHandle hJob;
   if (MemoryLimit != 0) {
     hJob = CreateJobObjectW(0, 0);
     bool success = false;
     if (hJob) {
       JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli;
       memset(&jeli, 0, sizeof(jeli));
       jeli.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_PROCESS_MEMORY;
       jeli.ProcessMemoryLimit = uintptr_t(MemoryLimit) * 1048576;
       if (SetInformationJobObject(hJob, JobObjectExtendedLimitInformation,
                                   &jeli, sizeof(jeli))) {
         if (AssignProcessToJobObject(hJob, pi.hProcess))
           success = true;
       }
     }
     if (!success) {
       SetLastError(GetLastError());
-      MakeErrMsg(ErrMsg, std::string("Unable to set memory limit"));
+      std::string ErrMsg = MakeErrMsg("Unable to set memory limit");
       TerminateProcess(pi.hProcess, 1);
       WaitForSingleObject(pi.hProcess, INFINITE);
-      return false;
+      return Program::makeFailureResult(ErrMsg);
     }
   }
-
-  return true;
+  return Program::makeExecuting(pi.hProcess);
 }
 
 static bool argNeedsQuotes(StringRef Arg) {
   if (Arg.empty())
     return true;
   return StringRef::npos != Arg.find_first_of("\t \"&\'()*<>\\`^|\n");
 }
 
while (!Arg.empty()) {
     // Drop all the backslashes, plus the following character.
     Arg = Arg.drop_front(FirstNonBackslash + 1);
   }
 
   Result.push_back('"');
   return Result;
 }
 
+static bool CompleteProcess(sys::Program *P) {
+  DWORD status;
+  BOOL rc = GetExitCodeProcess(P->Handle, &status);
+  if (rc && status == STILL_ACTIVE) {
+    assert(P->Status == ExecutionStatus::StartedAndExecuting);
+    return true;
+  }
+
+  DWORD err = GetLastError();
+  if (err != ERROR_INVALID_HANDLE) {
+    CloseHandle(P->Handle);
+  }
+  P->Handle = Program::HandleType();
+
+  if (!rc) {
+    SetLastError(err);
+    *P = Program::makeCrashedResult(-1, MakeErrMsg("Failed getting return status for program"));
+    return false;
+  }
+
+  if (!status) {
+    *P = Program::makeSuccessResult();
+    return true;
+  }
+
+  if ((status & 0xBFFF0000U) == 0x80000000U)
+    *P = Program::makeCrashedResult(static_cast<int>(status), "");
+  else if (status & 0xFF)
+    *P = Program::makeErrorResult(status & 0x7FFFFFFF);
+  else
+    *P = Program::makeErrorResult((int)ReturnCode::Failure);
+
+  return true;
+}
+
 namespace llvm {
 std::string sys::flattenWindowsCommandLine(ArrayRef<StringRef> Args) {
   std::string Command;
   for (StringRef Arg : Args) {
     if (argNeedsQuotes(Arg))
       Command += quoteSingleArg(Arg);
     else
       Command += Arg;
 
     Command.push_back(' ');
   }
 
   return Command;
 }
 
-ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
-                      bool WaitUntilChildTerminates, std::string *ErrMsg) {
-  assert(PI.Pid && "invalid pid to wait on, process not started?");
-  assert((PI.Process && PI.Process != INVALID_HANDLE_VALUE) &&
+void sys::Program::waitCompletion(unsigned SecondsToWait, bool WaitUntilChildTerminates) {
+  assert((Handle && Handle != INVALID_HANDLE_VALUE) &&
          "invalid process handle to wait on, process not started?");
+  assert(Status == ExecutionStatus::StartedAndExecuting);
+
   DWORD milliSecondsToWait = 0;
   if (WaitUntilChildTerminates)
     milliSecondsToWait = INFINITE;
   else if (SecondsToWait > 0)
     milliSecondsToWait = SecondsToWait * 1000;
 
-  ProcessInfo WaitResult = PI;
-  DWORD WaitStatus = WaitForSingleObject(PI.Process, milliSecondsToWait);
+  DWORD WaitStatus = WaitForSingleObject(Handle , milliSecondsToWait);
   if (WaitStatus == WAIT_TIMEOUT) {
     if (SecondsToWait) {
-      if (!TerminateProcess(PI.Process, 1)) {
-        if (ErrMsg)
-          MakeErrMsg(ErrMsg, "Failed to terminate timed-out program");
-
-        // -2 indicates a crash or timeout as opposed to failure to execute.
-        WaitResult.ReturnCode = -2;
-        CloseHandle(PI.Process);
-        return WaitResult;
-      }
-      WaitForSingleObject(PI.Process, INFINITE);
-      CloseHandle(PI.Process);
+      if (!TerminateProcess(Handle, 1)) {
+        ErrMsg = MakeErrMsg("Failed to terminate timed-out program");
+        Status = ExecutionStatus::TimedOut;
+        CloseHandle(Handle);
+        Handle = HandleType();
+        return;
+      }
+      WaitForSingleObject(Handle, INFINITE); // wait for process termination
+      Status = ExecutionStatus::TimedOut;
+      CloseHandle(Handle);
+      Handle = HandleType();
+      return;
     } else {
       // Non-blocking wait.
-      return ProcessInfo();
+      return;
     }
   }
 
-  // Get its exit status.
-  DWORD status;
-  BOOL rc = GetExitCodeProcess(PI.Process, &status);
-  DWORD err = GetLastError();
-  if (err != ERROR_INVALID_HANDLE)
-    CloseHandle(PI.Process);
-
-  if (!rc) {
-    SetLastError(err);
-    if (ErrMsg)
-      MakeErrMsg(ErrMsg, "Failed getting status for program");
-
-    // -2 indicates a crash or timeout as opposed to failure to execute.
-    WaitResult.ReturnCode = -2;
-    return WaitResult;
-  }
-
-  if (!status)
-    return WaitResult;
-
-  // Pass 10(Warning) and 11(Error) to the callee as negative value.
-  if ((status & 0xBFFF0000U) == 0x80000000U)
-    WaitResult.ReturnCode = static_cast<int>(status);
-  else if (status & 0xFF)
-    WaitResult.ReturnCode = status & 0x7FFFFFFF;
-  else
-    WaitResult.ReturnCode = 1;
-
-  return WaitResult;
+  CompleteProcess(this);
 }
 
 std::error_code sys::ChangeStdinToBinary() {
   int result = _setmode(_fileno(stdin), _O_BINARY);
   if (result == -1)
     return std::error_code(errno, std::generic_category());
   return std::error_code();
 }

lib/Support/Windows/WindowsSupport.h

 bool RunningWindows8OrGreater();
 
 /// Returns the Windows version as Major.Minor.0.BuildNumber. Uses
 /// RtlGetVersion or GetVersionEx under the hood depending on what is available.
 /// GetVersionEx is deprecated, but this API exposes the build number which can
 /// be useful for working around certain kernel bugs.
 llvm::VersionTuple GetWindowsOSVersion();
 
-bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix);
+std::string MakeErrMsg(Twine prefix);
 
 template <typename HandleTraits>
 class ScopedHandle {
   typedef typename HandleTraits::handle_type handle_type;
   handle_type Handle;
 
   ScopedHandle(const ScopedHandle &other) = delete;
   void operator=(const ScopedHandle &other) = delete;

unittests/Support/ProgramTest.cpp

TEST_F(ProgramEnvTest, TestExecuteNoWait) {
   StringRef argv[] = {Executable,
                       "--gtest_filter=ProgramEnvTest.TestExecuteNoWait"};
 
   // Add LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT to the environment of the child.
   addEnvVar("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT=1");
 
   std::string Error;
   bool ExecutionFailed;
-  ProcessInfo PI1 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error,
-                                  &ExecutionFailed);
+  Program PI1 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error,
   ASSERT_FALSE(ExecutionFailed) << Error;
-  ASSERT_NE(PI1.Pid, ProcessInfo::InvalidPid) << "Invalid process id";
+  ASSERT_NE(PI1.Handle, Program::HandleType{}) << "Invalid process id";
 
   unsigned LoopCount = 0;
 
   // Test that Wait() with WaitUntilTerminates=true works. In this case,
   // LoopCount should only be incremented once.
   while (true) {
     ++LoopCount;
-    ProcessInfo WaitResult = llvm::sys::Wait(PI1, 0, true, &Error);
+    Program WaitResult = llvm::sys::Wait(PI1, 0, true, &Error);
     ASSERT_TRUE(Error.empty());
-    if (WaitResult.Pid == PI1.Pid)
+    if (WaitResult.Handle == PI1.Handle)
       break;
   }
 
   EXPECT_EQ(LoopCount, 1u) << "LoopCount should be 1";
 
-  ProcessInfo PI2 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error,
-                                  &ExecutionFailed);
+  Program PI2 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error,
   ASSERT_FALSE(ExecutionFailed) << Error;
-  ASSERT_NE(PI2.Pid, ProcessInfo::InvalidPid) << "Invalid process id";
+  ASSERT_NE(PI2.Handle, Program::HandleType{}) << "Invalid process id";
 
   // Test that Wait() with SecondsToWait=0 performs a non-blocking wait. In this
   // cse, LoopCount should be greater than 1 (more than one increment occurs).
   while (true) {
     ++LoopCount;
-    ProcessInfo WaitResult = llvm::sys::Wait(PI2, 0, false, &Error);
+    Program WaitResult = llvm::sys::Wait(PI2, 0, false, &Error);
     ASSERT_TRUE(Error.empty());
-    if (WaitResult.Pid == PI2.Pid)
+    if (WaitResult.Handle == PI2.Handle)
       break;
   }
 
   ASSERT_GT(LoopCount, 1u) << "LoopCount should be >1";
 }
 
 TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) {
   using namespace llvm::sys;
ASSERT_TRUE(RetCode < 0) << "On error ExecuteAndWait should return 0 or "
                                 "positive value indicating the result code";
     ASSERT_TRUE(ExecutionFailed);
     ASSERT_FALSE(Error.empty());
   }
 
   {
     std::string Error;
     bool ExecutionFailed;
-    ProcessInfo PI = ExecuteNoWait(Executable, argv, llvm::None, {}, 0, &Error,
-                                   &ExecutionFailed);
-    ASSERT_EQ(PI.Pid, ProcessInfo::InvalidPid)
-        << "On error ExecuteNoWait should return an invalid ProcessInfo";
+    Program PI = ExecuteNoWait(Executable, argv, llvm::None, {}, 0, &Error,
+    ASSERT_EQ(PI.Handle, Program::HandleType{})
+        << "On error ExecuteNoWait should return an invalid Program";
     ASSERT_TRUE(ExecutionFailed);
     ASSERT_FALSE(Error.empty());
   }
 
 }
 
 #ifdef _WIN32
 const char utf16le_text[] =