Allow specifying an exit code for the 'quit' command

[teemperor]

Summary

This patch adds the possibility to specify an exit code when calling quit.
We accept any int, even though it depends on the user what happens if the int is
out of the range of what the operating system supports as exit codes.

Fixes rdar://problem/38452312

[jingham]

This won't at all do what users expect in cases where the driver program doesn't cooperate. For instance, when run under Xcode, the lldb process can have many debuggers running concurrently, so the exit value of a single Debugger doesn't make much sense there.

I think we should require the client to say that it will obey the exit value, and have the command error out if the client has not told us it plans to respect our exit values.

[clayborg]

This change seems fine to me. It allows each debugger to have a different exit status and each debugger can grab one. Seems like this should belong in lldb_private::CommandInterpreter and lldb::SBCommandInterpreter instead of in lldb_private::Debugger and lldb::SBDebugger

include/lldb/API/SBDebugger.h

225–226

How do you know if the quit command has been called? I mean, we can return zero when "quit" hasn't been called yet, but it might be nice to know by changing the signature a bit:

int GetExitCode(bool &exited);

exited would be set to true if the quit command has been called, false otherwise if the command interpreter is still running?

Also "GetExitCode()" might be better and more clear as being named "GetQuitStatus()" or "GetQuitCommandStatus()" to be more clear this the result of the "quit" command?

This seems like this belongs more on the SBCommandInterpreter since it is the result of the "quit" command. Maybe something like:

class SBCommandInterpreter {
  bool QuitHasBeenCalled();
  int GetQuitStatus();
};

Then the user can call "SBCommandInterpreter::QuitHasBeenCalled()" first and follow it with SBCommandInterpreter::GetQuitStatus() to get the status? So I this should be moved to SBCommandInterpreter.h

include/lldb/Core/Debugger.h

331–334

move to command interpreter.

411–412

move to command interpreter

source/API/SBDebugger.cpp

1084–1087

move to SBCommandInterpreter

source/Commands/CommandObjectQuit.cpp

101

m_interpreter.SetExitCode(exit_code);

tools/driver/Driver.cpp

1162

Get exit from command interpreter

[teemperor]

• Client now has to explicitly allow the acceptance of exit codes.
• Moved logic to the CommandInterpreter.
• Added more tests.

Thanks for the feedback Jim and Greg, very much appreciated!

[jingham]

That seems reasonable to me.

[teemperor]

@clayborg was quite specific about the requested changes (which are all applied now), so I assume this is good to go.

include/lldb/API/SBDebugger.h

public:
   bool GetDescription(lldb::SBStream &description);
 
   uint32_t GetTerminalWidth() const;
 
   void SetTerminalWidth(uint32_t term_width);
 
   lldb::user_id_t GetID();
 
+  /// Returns the exit code that the user has set from the prompt.
+  int GetExitCode();
+
   const char *GetPrompt() const;
 
   void SetPrompt(const char *prompt);
 
   lldb::ScriptLanguage GetScriptLanguage() const;
 
   void SetScriptLanguage(lldb::ScriptLanguage script_lang);
 

include/lldb/Core/Debugger.h

public:
   // target with entities that will be copied over to new targets.
   Target *GetSelectedOrDummyTarget(bool prefer_dummy = false);
   Target *GetDummyTarget();
 
   lldb::BroadcasterManagerSP GetBroadcasterManager() {
     return m_broadcaster_manager_sp;
   }
 
+  void SetExitCode(int i) { m_exit_code = i; }
+
+  int GetExitCode() const { return m_exit_code; }
+
 protected:
   friend class CommandInterpreter;
   friend class REPL;
 
   bool StartEventHandlerThread();
 
   void StopEventHandlerThread();
 
protected:
   static LoadPluginCallbackType g_load_plugin_callback;
   typedef std::vector<llvm::sys::DynamicLibrary> LoadedPluginsList;
   LoadedPluginsList m_loaded_plugins;
   HostThread m_event_handler_thread;
   HostThread m_io_handler_thread;
   Broadcaster m_sync_broadcaster;
   lldb::ListenerSP m_forward_listener_sp;
   llvm::once_flag m_clear_once;
+  // The exit code the user has requested for the current debugger process.
+  int m_exit_code = 0;
 
   //----------------------------------------------------------------------
   // Events for m_sync_broadcaster
   //----------------------------------------------------------------------
   enum {
     eBroadcastBitEventThreadIsListening = (1 << 0),
   };
 

lit/Quit/TestQuitExitCode-30.test

+# UNSUPPORTED: windows
+# RUN: python %S/expect_exit_code.py 226 %lldb -b -s %s
+q -30

lit/Quit/TestQuitExitCode0.test

+# UNSUPPORTED: windows
+# RUN: %lldb -b -s %s
+q 0

lit/Quit/TestQuitExitCode30.test

+# UNSUPPORTED: windows
+# RUN: python %S/expect_exit_code.py 30  %lldb -b -s %s
+q 30

lit/Quit/TestQuitExitCodeHex0.test

+# UNSUPPORTED: windows
+# RUN: %lldb -b -s %s
+q 0x0

lit/Quit/TestQuitExitCodeHexA.test

+# UNSUPPORTED: windows
+# RUN: python %S/expect_exit_code.py 10 %lldb -b -s %s
+q 0xA

lit/Quit/TestQuitExitCodeImplicit0.test

+# UNSUPPORTED: windows
+# RUN: %lldb -b -s %s
+q

lit/Quit/TestQuitExitCodeNonInt.test

+# UNSUPPORTED: windows
+# RUN: %lldb -b -s %s 2>&1 | FileCheck %s
+q str
+// CHECK: Couldn't parse 'str'

lit/Quit/TestQuitExitCodeTooManyArgs.test

+# UNSUPPORTED: windows
+# RUN: %lldb -b -s %s 2>&1 | FileCheck %s
+q 1 2
+// CHECK: Too many arguments for 'quit'

lit/Quit/expect_exit_code.py

+#!/usr/bin/env python2
+
+import subprocess
+import sys
+
+args = sys.argv
+
+expected_exit_code = args[1]
+
+args = args[2:]
+print("Running " + (" ".join(args)))
+real_exit_code = subprocess.call(args)
+
+if str(real_exit_code) != expected_exit_code:
+  print("Got exit code %d but expected %s" % (real_exit_code, expected_exit_code))
+  exit(1)

lit/Quit/lit.local.cfg

+config.suffixes = ['.test']

source/API/SBDebugger.cpp

bool SBDebugger::GetDescription(SBStream &description) {
 
   return true;
 }
 
 user_id_t SBDebugger::GetID() {
   return (m_opaque_sp ? m_opaque_sp->GetID() : LLDB_INVALID_UID);
 }
 
+int SBDebugger::GetExitCode() {
+  return (m_opaque_sp ? m_opaque_sp->GetExitCode() : 0);
+}
+
 SBError SBDebugger::SetCurrentPlatform(const char *platform_name_cstr) {
   SBError sb_error;
   if (m_opaque_sp) {
     if (platform_name_cstr && platform_name_cstr[0]) {
       ConstString platform_name(platform_name_cstr);
       PlatformSP platform_sp(Platform::Find(platform_name));
 
       if (platform_sp) {

source/Commands/CommandObjectQuit.cpp

 
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 //-------------------------------------------------------------------------
 // CommandObjectQuit
 //-------------------------------------------------------------------------
 
 CommandObjectQuit::CommandObjectQuit(CommandInterpreter &interpreter)
     : CommandObjectParsed(interpreter, "quit", "Quit the LLDB debugger.",
-                          "quit") {}
+                          "quit [exit-code]") {}
 
 CommandObjectQuit::~CommandObjectQuit() {}
 
 // returns true if there is at least one alive process is_a_detach will be true
 // if all alive processes will be detached when you quit and false if at least
 // one process will be killed instead
 bool CommandObjectQuit::ShouldAskForConfirmation(bool &is_a_detach) {
   if (m_interpreter.GetPromptOnQuit() == false)
if (ShouldAskForConfirmation(is_a_detach)) {
     message.Printf("Quitting LLDB will %s one or more processes. Do you really "
                    "want to proceed",
                    (is_a_detach ? "detach from" : "kill"));
     if (!m_interpreter.Confirm(message.GetString(), true)) {
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
   }
+
+  if (command.GetArgumentCount() > 1) {
+    result.AppendError("Too many arguments for 'quit'. Only an optional exit "
+                       "code is allowed");
+    result.SetStatus(eReturnStatusFailed);
+    return false;
+  }
+
+  // We parse the exit code argument if there is one.
+  if (command.GetArgumentCount() == 1) {
+    llvm::StringRef arg = command.GetArgumentAtIndex(0);
+    int exit_code;
+    if (arg.getAsInteger(/*autodetect radix*/ 0, exit_code)) {
+      lldb_private::StreamString s;
+      std::string arg_str = arg.str();
+      s.Printf("Couldn't parse '%s' as integer for exit code.", arg_str.data());
+      result.AppendError(s.GetString());
+      result.SetStatus(eReturnStatusFailed);
+      return false;
+    }
+    m_interpreter.GetDebugger().SetExitCode(exit_code);
+  }
+
   const uint32_t event_type =
       CommandInterpreter::eBroadcastBitQuitCommandReceived;
   m_interpreter.BroadcastEvent(event_type);
   result.SetStatus(eReturnStatusQuit);
   return true;
 }

tools/driver/Driver.h

typedef enum CommandPlacement {
     eCommandPlacementAfterFile,
     eCommandPlacementAfterCrash,
   } CommandPlacement;
 
   Driver();
 
   virtual ~Driver();
 
-  void MainLoop();
+  /// Runs the main loop.
+  ///
+  /// @return The exit code that the process should return.
+  int MainLoop();
 
   lldb::SBError ParseArgs(int argc, const char *argv[], FILE *out_fh,
                           bool &do_exit);
 
   const char *GetFilename() const;
 
   const char *GetCrashLogFilename() const;
 

tools/driver/Driver.cpp

std::string EscapeString(std::string arg) {
   std::string::size_type pos = 0;
   while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
     arg.insert(pos, 1, '\\');
     pos += 2;
   }
   return '"' + arg + '"';
 }
 
-void Driver::MainLoop() {
+int Driver::MainLoop() {
   if (::tcgetattr(STDIN_FILENO, &g_old_stdin_termios) == 0) {
     g_old_stdin_termios_is_valid = true;
     atexit(reset_stdin_termios);
   }
 
 #ifndef _MSC_VER
   // Disabling stdin buffering with MSVC's 2015 CRT exposes a bug in fgets
   // which causes it to miss newlines depending on whether there have been an
if (go_interactive) {
       m_debugger.SetInputFileHandle(stdin, true);
       m_debugger.RunCommandInterpreter(handle_events, spawn_thread);
     }
   }
 
   reset_stdin_termios();
   fclose(stdin);
 
+  int exit_code = m_debugger.GetExitCode();
   SBDebugger::Destroy(m_debugger);
+  return exit_code;
 }
 
 void Driver::ResizeWindow(unsigned short col) {
   GetDebugger().SetTerminalWidth(col);
 }
 
 void sigwinch_handler(int signo) {
   struct winsize window_size;
#endif
   signal(SIGINT, sigint_handler);
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
   signal(SIGTSTP, sigtstp_handler);
   signal(SIGCONT, sigcont_handler);
 #endif
 
+  int exit_code = 0;
   // Create a scope for driver so that the driver object will destroy itself
   // before SBDebugger::Terminate() is called.
   {
     Driver driver;
 
     bool exiting = false;
     SBError error(driver.ParseArgs(argc, argv, stdout, exiting));
     if (error.Fail()) {
+      exit_code = 1;
       const char *error_cstr = error.GetCString();
       if (error_cstr)
         ::fprintf(stderr, "error: %s\n", error_cstr);
     } else if (!exiting) {
-      driver.MainLoop();
+      exit_code = driver.MainLoop();
     }
   }
 
   SBDebugger::Terminate();
-  return 0;
+  return exit_code;
 }