Add pretty-printer for wait(2) statuses and modernize the code handli…

…ng them

Summary:
A number of places were trying to decode the result of wait(). Add a simple
utility function that does that and a struct that encapsulates the
decoded result. Then also provide a pretty-printer for that class.

Reviewers: zturner, krytarowski, eugene

Subscribers: lldb-commits, mgorny

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

llvm-svn: 305689

lldb/include/lldb/Host/Host.h

@@ -28,6 +28,27 @@ namespace lldb_private {
 class FileAction;
 class ProcessLaunchInfo;
 
+//----------------------------------------------------------------------
+// Exit Type for inferior processes
+//----------------------------------------------------------------------
+struct WaitStatus {
+  enum Type : uint8_t {
+    Exit,   // The status represents the return code from normal
+            // program exit (i.e. WIFEXITED() was true)
+    Signal, // The status represents the signal number that caused
+            // the program to exit (i.e. WIFSIGNALED() was true)
+    Stop,   // The status represents the signal number that caused the
+            // program to stop (i.e. WIFSTOPPED() was true)
+  };
+
+  Type type;
+  uint8_t status;
+
+  WaitStatus(Type type, uint8_t status) : type(type), status(status) {}
+
+  static WaitStatus Decode(int wstatus);
+};
+
 //----------------------------------------------------------------------
 /// @class Host Host.h "lldb/Host/Host.h"
 /// @brief A class that provides host computer information.
@@ -230,5 +251,14 @@ class Host {
 
 } // namespace lldb_private
 
+namespace llvm {
+template <> struct format_provider<lldb_private::WaitStatus> {
+  /// Options = "" gives a human readable description of the status
+  /// Options = "g" gives a gdb-remote protocol status (e.g., X09)
+  static void format(const lldb_private::WaitStatus &WS, raw_ostream &OS,
+                     llvm::StringRef Options);
+};
+} // namespace llvm
+
 #endif // #if defined(__cplusplus)
 #endif // liblldb_Host_h_

lldb/include/lldb/Host/common/NativeProcessProtocol.h

@@ -11,6 +11,7 @@
 #define liblldb_NativeProcessProtocol_h_
 
 #include "lldb/Core/TraceOptions.h"
+#include "lldb/Host/Host.h"
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private-forward.h"
@@ -158,12 +159,9 @@ class NativeProcessProtocol
   //----------------------------------------------------------------------
   // Exit Status
   //----------------------------------------------------------------------
-  virtual bool GetExitStatus(lldb_private::ExitType *exit_type, int *status,
-                             std::string &exit_description);
+  virtual llvm::Optional<WaitStatus> GetExitStatus();
 
-  virtual bool SetExitStatus(lldb_private::ExitType exit_type, int status,
-                             const char *exit_description,
-                             bool bNotifyStateChange);
+  virtual bool SetExitStatus(WaitStatus status, bool bNotifyStateChange);
 
   //----------------------------------------------------------------------
   // Access to threads
@@ -421,9 +419,8 @@ class NativeProcessProtocol
   lldb::StateType m_state;
   mutable std::recursive_mutex m_state_mutex;
 
-  lldb_private::ExitType m_exit_type;
-  int m_exit_status;
-  std::string m_exit_description;
+  llvm::Optional<WaitStatus> m_exit_status;
+
   std::recursive_mutex m_delegates_mutex;
   std::vector<NativeDelegate *> m_delegates;
   NativeBreakpointList m_breakpoint_list;

lldb/include/lldb/lldb-private-enumerations.h

@@ -211,19 +211,6 @@ enum class LineStatus {
   Done     // Lines are complete
 };
 
-//----------------------------------------------------------------------
-// Exit Type for inferior processes
-//----------------------------------------------------------------------
-typedef enum ExitType {
-  eExitTypeInvalid,
-  eExitTypeExit,   // The exit status represents the return code from normal
-                   // program exit (i.e. WIFEXITED() was true)
-  eExitTypeSignal, // The exit status represents the signal number that caused
-                   // the program to exit (i.e. WIFSIGNALED() was true)
-  eExitTypeStop,   // The exit status represents the stop signal that caused the
-                   // program to exit (i.e. WIFSTOPPED() was true)
-} ExitType;
-
 //----------------------------------------------------------------------
 // Boolean result of running a Type Validator
 //----------------------------------------------------------------------

lldb/source/Host/common/Host.cpp

@@ -68,6 +68,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private-forward.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 
@@ -641,3 +642,51 @@ const UnixSignalsSP &Host::GetUnixSignals() {
       UnixSignals::Create(HostInfo::GetArchitecture());
   return s_unix_signals_sp;
 }
+
+#if defined(LLVM_ON_UNIX)
+WaitStatus WaitStatus::Decode(int wstatus) {
+  if (WIFEXITED(wstatus))
+    return {Exit, uint8_t(WEXITSTATUS(wstatus))};
+  else if (WIFSIGNALED(wstatus))
+    return {Signal, uint8_t(WTERMSIG(wstatus))};
+  else if (WIFSTOPPED(wstatus))
+    return {Stop, uint8_t(WSTOPSIG(wstatus))};
+  llvm_unreachable("Unknown wait status");
+}
+#endif
+
+void llvm::format_provider<WaitStatus>::format(const WaitStatus &WS,
+                                               raw_ostream &OS,
+                                               StringRef Options) {
+  if (Options == "g") {
+    char type;
+    switch (WS.type) {
+    case WaitStatus::Exit:
+      type = 'W';
+      break;
+    case WaitStatus::Signal:
+      type = 'X';
+      break;
+    case WaitStatus::Stop:
+      type = 'S';
+      break;
+    }
+    OS << formatv("{0}{1:x-2}", type, WS.status);
+    return;
+  }
+
+  assert(Options.empty());
+  const char *desc;
+  switch(WS.type) {
+  case WaitStatus::Exit:
+    desc = "Exited with status";
+    break;
+  case WaitStatus::Signal:
+    desc = "Killed by signal";
+    break;
+  case WaitStatus::Stop:
+    desc = "Stopped by signal";
+    break;
+  }
+  OS << desc << " " << int(WS.status);
+}

lldb/source/Host/common/NativeProcessProtocol.cpp

@@ -32,7 +32,6 @@ using namespace lldb_private;
 NativeProcessProtocol::NativeProcessProtocol(lldb::pid_t pid)
     : m_pid(pid), m_threads(), m_current_thread_id(LLDB_INVALID_THREAD_ID),
       m_threads_mutex(), m_state(lldb::eStateInvalid), m_state_mutex(),
-      m_exit_type(eExitTypeInvalid), m_exit_status(0), m_exit_description(),
       m_delegates_mutex(), m_delegates(), m_breakpoint_list(),
       m_watchpoint_list(), m_terminal_fd(-1), m_stop_id(0) {}
 
@@ -59,46 +58,29 @@ NativeProcessProtocol::GetMemoryRegionInfo(lldb::addr_t load_addr,
   return Status("not implemented");
 }
 
-bool NativeProcessProtocol::GetExitStatus(ExitType *exit_type, int *status,
-                                          std::string &exit_description) {
-  if (m_state == lldb::eStateExited) {
-    *exit_type = m_exit_type;
-    *status = m_exit_status;
-    exit_description = m_exit_description;
-    return true;
-  }
+llvm::Optional<WaitStatus> NativeProcessProtocol::GetExitStatus() {
+  if (m_state == lldb::eStateExited)
+    return m_exit_status;
 
-  *status = 0;
-  return false;
+  return llvm::None;
 }
 
-bool NativeProcessProtocol::SetExitStatus(ExitType exit_type, int status,
-                                          const char *exit_description,
+bool NativeProcessProtocol::SetExitStatus(WaitStatus status,
                                           bool bNotifyStateChange) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-  if (log)
-    log->Printf("NativeProcessProtocol::%s(%d, %d, %s, %s) called",
-                __FUNCTION__, exit_type, status,
-                exit_description ? exit_description : "nullptr",
-                bNotifyStateChange ? "true" : "false");
+  LLDB_LOG(log, "status = {0}, notify = {1}", status, bNotifyStateChange);
 
   // Exit status already set
   if (m_state == lldb::eStateExited) {
-    if (log)
-      log->Printf("NativeProcessProtocol::%s exit status already set to %d, "
-                  "ignoring new set to %d",
-                  __FUNCTION__, m_exit_status, status);
+    if (m_exit_status)
+      LLDB_LOG(log, "exit status already set to {0}", *m_exit_status);
+    else
+      LLDB_LOG(log, "state is exited, but status not set");
     return false;
   }
 
   m_state = lldb::eStateExited;
-
-  m_exit_type = exit_type;
   m_exit_status = status;
-  if (exit_description && exit_description[0])
-    m_exit_description = exit_description;
-  else
-    m_exit_description.clear();
 
   if (bNotifyStateChange)
     SynchronouslyNotifyProcessStateChanged(lldb::eStateExited);

lldb/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp

@@ -1085,8 +1085,7 @@ Status NativeProcessDarwin::HandleWaitpidResult() {
                   "waitpid exiting pid from the pipe.  Will notify "
                   "as if parent process died with exit status -1.",
                   __FUNCTION__);
-    SetExitStatus(eExitTypeInvalid, -1, "failed to receive waitpid result",
-                  notify_status);
+    SetExitStatus(WaitStatus(WaitStatus::Exit, -1), notify_status);
     return error;
   }
 
@@ -1099,8 +1098,7 @@ Status NativeProcessDarwin::HandleWaitpidResult() {
                   "waitpid exit status from the pipe.  Will notify "
                   "as if parent process died with exit status -1.",
                   __FUNCTION__);
-    SetExitStatus(eExitTypeInvalid, -1, "failed to receive waitpid result",
-                  notify_status);
+    SetExitStatus(WaitStatus(WaitStatus::Exit, -1), notify_status);
     return error;
   }
 
@@ -1111,18 +1109,7 @@ Status NativeProcessDarwin::HandleWaitpidResult() {
                 __FUNCTION__, pid,
                 (pid == m_pid) ? "the inferior" : "not the inferior", status);
 
-  ExitType exit_type = eExitTypeInvalid;
-  int exit_status = -1;
-
-  if (WIFEXITED(status)) {
-    exit_type = eExitTypeExit;
-    exit_status = WEXITSTATUS(status);
-  } else if (WIFSIGNALED(status)) {
-    exit_type = eExitTypeSignal;
-    exit_status = WTERMSIG(status);
-  }
-
-  SetExitStatus(exit_type, exit_status, nullptr, notify_status);
+  SetExitStatus(WaitStatus::Decode(status), notify_status);
   return error;
 }
 

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

@@ -503,35 +503,9 @@ Status NativeProcessLinux::SetDefaultPtraceOpts(lldb::pid_t pid) {
   return PtraceWrapper(PTRACE_SETOPTIONS, pid, nullptr, (void *)ptrace_opts);
 }
 
-static ExitType convert_pid_status_to_exit_type(int status) {
-  if (WIFEXITED(status))
-    return ExitType::eExitTypeExit;
-  else if (WIFSIGNALED(status))
-    return ExitType::eExitTypeSignal;
-  else if (WIFSTOPPED(status))
-    return ExitType::eExitTypeStop;
-  else {
-    // We don't know what this is.
-    return ExitType::eExitTypeInvalid;
-  }
-}
-
-static int convert_pid_status_to_return_code(int status) {
-  if (WIFEXITED(status))
-    return WEXITSTATUS(status);
-  else if (WIFSIGNALED(status))
-    return WTERMSIG(status);
-  else if (WIFSTOPPED(status))
-    return WSTOPSIG(status);
-  else {
-    // We don't know what this is.
-    return ExitType::eExitTypeInvalid;
-  }
-}
-
 // Handles all waitpid events from the inferior process.
 void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, bool exited,
-                                         int signal, int status) {
+                                         WaitStatus status) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
 
   // Certain activities differ based on whether the pid is the tid of the main
@@ -564,8 +538,7 @@ void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, bool exited,
                          : "thread metadata not found",
             GetState());
         // The main thread exited.  We're done monitoring.  Report to delegate.
-        SetExitStatus(convert_pid_status_to_exit_type(status),
-                      convert_pid_status_to_return_code(status), nullptr, true);
+        SetExitStatus(status, true);
 
         // Notify delegate that our process has exited.
         SetState(StateType::eStateExited, true);
@@ -658,8 +631,7 @@ void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, bool exited,
         // Notify the delegate - our process is not available but appears to
         // have been killed outside
         // our control.  Is eStateExited the right exit state in this case?
-        SetExitStatus(convert_pid_status_to_exit_type(status),
-                      convert_pid_status_to_return_code(status), nullptr, true);
+        SetExitStatus(status, true);
         SetState(StateType::eStateExited, true);
       } else {
         // This thread was pulled out from underneath us.  Anything to do here?
@@ -830,10 +802,8 @@ void NativeProcessLinux::MonitorSIGTRAP(const siginfo_t &info,
              data, WIFEXITED(data), WIFSIGNALED(data), thread.GetID(),
              is_main_thread);
 
-    if (is_main_thread) {
-      SetExitStatus(convert_pid_status_to_exit_type(data),
-                    convert_pid_status_to_return_code(data), nullptr, true);
-    }
+    if (is_main_thread)
+      SetExitStatus(WaitStatus::Decode(data), true);
 
     StateType state = thread.GetState();
     if (!StateIsRunningState(state)) {
@@ -2384,33 +2354,17 @@ void NativeProcessLinux::SigchldHandler() {
       break;
     }
 
-    bool exited = false;
-    int signal = 0;
-    int exit_status = 0;
-    const char *status_cstr = nullptr;
-    if (WIFSTOPPED(status)) {
-      signal = WSTOPSIG(status);
-      status_cstr = "STOPPED";
-    } else if (WIFEXITED(status)) {
-      exit_status = WEXITSTATUS(status);
-      status_cstr = "EXITED";
-      exited = true;
-    } else if (WIFSIGNALED(status)) {
-      signal = WTERMSIG(status);
-      status_cstr = "SIGNALED";
-      if (wait_pid == static_cast<::pid_t>(GetID())) {
-        exited = true;
-        exit_status = -1;
-      }
-    } else
-      status_cstr = "(\?\?\?)";
+    WaitStatus wait_status = WaitStatus::Decode(status);
+    bool exited = wait_status.type == WaitStatus::Exit ||
+                  (wait_status.type == WaitStatus::Signal &&
+                   wait_pid == static_cast<::pid_t>(GetID()));
 
-    LLDB_LOG(log,
-             "waitpid (-1, &status, _) => pid = {0}, status = {1:x} "
-             "({2}), signal = {3}, exit_state = {4}",
-             wait_pid, status, status_cstr, signal, exit_status);
+    LLDB_LOG(
+        log,
+        "waitpid (-1, &status, _) => pid = {0}, status = {1}, exited = {2}",
+        wait_pid, wait_status, exited);
 
-    MonitorCallback(wait_pid, exited, signal, exit_status);
+    MonitorCallback(wait_pid, exited, wait_status);
   }
 }
 

lldb/source/Plugins/Process/Linux/NativeProcessLinux.h

@@ -153,7 +153,7 @@ class NativeProcessLinux : public NativeProcessProtocol {
 
   static void *MonitorThread(void *baton);
 
-  void MonitorCallback(lldb::pid_t pid, bool exited, int signal, int status);
+  void MonitorCallback(lldb::pid_t pid, bool exited, WaitStatus status);
 
   void WaitForNewThread(::pid_t tid);