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 ↗

(On Diff #153115)

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 ↗

(On Diff #153115)

move to command interpreter.

411–412 ↗

(On Diff #153115)

move to command interpreter

source/API/SBDebugger.cpp

1084–1087 ↗

(On Diff #153115)

move to SBCommandInterpreter

source/Commands/CommandObjectQuit.cpp

101 ↗

(On Diff #153115)

m_interpreter.SetExitCode(exit_code);

tools/driver/Driver.cpp

1162 ↗

(On Diff #153115)

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.

lldb/trunk/include/lldb/API/SBCommandInterpreter.h

public:
   //----------------------------------------------------------------------
   const char *GetIOHandlerControlSequence(char ch);
 
   bool GetPromptOnQuit();
 
   void SetPromptOnQuit(bool b);
 
   //----------------------------------------------------------------------
+  /// Sets whether the command interpreter should allow custom exit codes
+  /// for the 'quit' command.
+  //----------------------------------------------------------------------
+  void AllowExitCodeOnQuit(bool allow);
+
+  //----------------------------------------------------------------------
+  /// Returns true if the user has called the 'quit' command with a custom exit
+  /// code.
+  //----------------------------------------------------------------------
+  bool HasCustomQuitExitCode();
+
+  //----------------------------------------------------------------------
+  /// Returns the exit code that the user has specified when running the
+  /// 'quit' command. Returns 0 if the user hasn't called 'quit' at all or
+  /// without a custom exit code.
+  //----------------------------------------------------------------------
+  int GetQuitStatus();
+
+  //----------------------------------------------------------------------
   /// Resolve the command just as HandleCommand would, expanding abbreviations
   /// and aliases.  If successful, result->GetOutput has the full expansion.
   //----------------------------------------------------------------------
   void ResolveCommand(const char *command_line, SBCommandReturnObject &result);
 
 protected:
   lldb_private::CommandInterpreter &ref();
 

lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h

public:
   // Properties
   //------------------------------------------------------------------
   bool GetExpandRegexAliases() const;
 
   bool GetPromptOnQuit() const;
 
   void SetPromptOnQuit(bool b);
 
+  //------------------------------------------------------------------
+  /// Specify if the command interpreter should allow that the user can
+  /// specify a custom exit code when calling 'quit'.
+  //------------------------------------------------------------------
+  void AllowExitCodeOnQuit(bool allow);
+
+  //------------------------------------------------------------------
+  /// Sets the exit code for the quit command.
+  /// @param[in] exit_code
+  ///     The exit code that the driver should return on exit.
+  /// @return True if the exit code was successfully set; false if the
+  ///         interpreter doesn't allow custom exit codes.
+  /// @see AllowExitCodeOnQuit
+  //------------------------------------------------------------------
+  LLVM_NODISCARD bool SetQuitExitCode(int exit_code);
+
+  //------------------------------------------------------------------
+  /// Returns the exit code that the user has specified when running the
+  /// 'quit' command.
+  /// @param[out] exited
+  ///     Set to true if the user has called quit with a custom exit code.
+  //------------------------------------------------------------------
+  int GetQuitExitCode(bool &exited) const;
+
   void ResolveCommand(const char *command_line, CommandReturnObject &result);
 
   bool GetStopCmdSourceOnError() const;
 
   uint32_t GetNumErrors() const { return m_num_errors; }
 
   bool GetQuitRequested() const { return m_quit_requested; }
 
private:
   ChildrenTruncatedWarningStatus m_truncation_warning; // Whether we truncated
                                                        // children and whether
                                                        // the user has been told
   uint32_t m_command_source_depth;
   std::vector<uint32_t> m_command_source_flags;
   uint32_t m_num_errors;
   bool m_quit_requested;
   bool m_stopped_for_crash;
+
+  // The exit code the user has requested when calling the 'quit' command.
+  // No value means the user hasn't set a custom exit code so far.
+  llvm::Optional<int> m_quit_exit_code;
+  // If the driver is accepts custom exit codes for the 'quit' command.
+  bool m_allow_exit_code = false;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_CommandInterpreter_h_

lldb/trunk/lit/Quit/TestQuitExitCode-30.test

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

lldb/trunk/lit/Quit/TestQuitExitCode0.test

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

lldb/trunk/lit/Quit/TestQuitExitCode30.test

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

lldb/trunk/lit/Quit/TestQuitExitCodeHex0.test

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

lldb/trunk/lit/Quit/TestQuitExitCodeHexA.test

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

lldb/trunk/lit/Quit/TestQuitExitCodeImplicit0.test

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

lldb/trunk/lit/Quit/TestQuitExitCodeNonInt.test

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

lldb/trunk/lit/Quit/TestQuitExitCodeTooManyArgs.test

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

lldb/trunk/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)

lldb/trunk/lit/Quit/lit.local.cfg

+config.suffixes = ['.test']

lldb/trunk/packages/Python/lldbsuite/test/quit/TestQuit.py

+"""
+Test lldb's quit command.
+"""
+
+from __future__ import print_function
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class QuitCommandTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @no_debug_info_test
+    def test_quit_exit_code_disallow(self):
+        self.ci.AllowExitCodeOnQuit(False)
+        self.expect(
+            "quit 20",
+            substrs=[
+                "error: The current driver doesn't allow custom exit codes for the quit command"],
+            error=True)
+        self.assertFalse(self.ci.HasCustomQuitExitCode())
+
+    @no_debug_info_test
+    def test_quit_exit_code_allow(self):
+        self.ci.AllowExitCodeOnQuit(True)
+        self.runCmd("quit 10", check=False)
+        self.assertTrue(self.ci.HasCustomQuitExitCode())
+        self.assertEqual(self.ci.GetQuitStatus(), 10)

lldb/trunk/scripts/interface/SBCommandInterpreter.i

public:
 
     bool
     GetPromptOnQuit();
 
     void
     SetPromptOnQuit(bool b);
 
     void
+    AllowExitCodeOnQuit(bool b);
+
+    bool
+    HasCustomQuitExitCode();
+
+    int
+    GetQuitStatus();
+
+    void
     ResolveCommand(const char *command_line, SBCommandReturnObject &result);
 
     bool
     CommandExists (const char *cmd);
 
     bool
     AliasExists (const char *cmd);
 

lldb/trunk/source/API/SBCommandInterpreter.cpp

bool SBCommandInterpreter::GetPromptOnQuit() {
   return (IsValid() ? m_opaque_ptr->GetPromptOnQuit() : false);
 }
 
 void SBCommandInterpreter::SetPromptOnQuit(bool b) {
   if (IsValid())
     m_opaque_ptr->SetPromptOnQuit(b);
 }
 
+void SBCommandInterpreter::AllowExitCodeOnQuit(bool allow) {
+  if (m_opaque_ptr)
+    m_opaque_ptr->AllowExitCodeOnQuit(allow);
+}
+
+bool SBCommandInterpreter::HasCustomQuitExitCode() {
+  bool exited = false;
+  if (m_opaque_ptr)
+    m_opaque_ptr->GetQuitExitCode(exited);
+  return exited;
+}
+
+int SBCommandInterpreter::GetQuitStatus() {
+  bool exited = false;
+  return (m_opaque_ptr ? m_opaque_ptr->GetQuitExitCode(exited) : 0);
+}
+
 void SBCommandInterpreter::ResolveCommand(const char *command_line,
                                           SBCommandReturnObject &result) {
   result.Clear();
   if (command_line && IsValid()) {
     m_opaque_ptr->ResolveCommand(command_line, result.ref());
   } else {
     result->AppendError(
         "SBCommandInterpreter or the command line is not valid");

lldb/trunk/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;
+  }
+
+  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;
+    }
+    if (!m_interpreter.SetQuitExitCode(exit_code)) {
+      result.AppendError("The current driver doesn't allow custom exit codes"
+                         " for the quit command.");
+      result.SetStatus(eReturnStatusFailed);
+      return false;
+    }
+  }
+
   const uint32_t event_type =
       CommandInterpreter::eBroadcastBitQuitCommandReceived;
   m_interpreter.BroadcastEvent(event_type);
   result.SetStatus(eReturnStatusQuit);
   return true;
 }

lldb/trunk/source/Interpreter/CommandInterpreter.cpp

return m_collection_sp->GetPropertyAtIndexAsBoolean(
       nullptr, idx, g_properties[idx].default_uint_value != 0);
 }
 
 void CommandInterpreter::SetPromptOnQuit(bool b) {
   const uint32_t idx = ePropertyPromptOnQuit;
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
 }
 
+void CommandInterpreter::AllowExitCodeOnQuit(bool allow) {
+  m_allow_exit_code = allow;
+  if (!allow)
+    m_quit_exit_code.reset();
+}
+
+bool CommandInterpreter::SetQuitExitCode(int exit_code) {
+  if (!m_allow_exit_code)
+    return false;
+  m_quit_exit_code = exit_code;
+  return true;
+}
+
+int CommandInterpreter::GetQuitExitCode(bool &exited) const {
+  exited = m_quit_exit_code.hasValue();
+  if (exited)
+    return *m_quit_exit_code;
+  return 0;
+}
+
 void CommandInterpreter::ResolveCommand(const char *command_line,
                                         CommandReturnObject &result) {
   std::string command = command_line;
   if (ResolveCommandImpl(command, result) != nullptr) {
     result.AppendMessageWithFormat("%s", command.c_str());
     result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 }

lldb/trunk/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;
 

lldb/trunk/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
#endif
   // .lldbinit file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result);
   if (GetDebugMode()) {
     result.PutError(m_debugger.GetErrorFileHandle());
     result.PutOutput(m_debugger.GetOutputFileHandle());
   }
 
+  // We allow the user to specify an exit code when calling quit which we will
+  // return when exiting.
+  m_debugger.GetCommandInterpreter().AllowExitCodeOnQuit(true);
+
   // Now we handle options we got from the command line
   SBStream commands_stream;
 
   // First source in the commands specified to be run before the file arguments
   // are processed.
   WriteCommandsForSourcing(eCommandPlacementBeforeFile, commands_stream);
 
   const size_t num_args = m_option_data.m_args.size();
if (go_interactive) {
       m_debugger.SetInputFileHandle(stdin, true);
       m_debugger.RunCommandInterpreter(handle_events, spawn_thread);
     }
   }
 
   reset_stdin_termios();
   fclose(stdin);
 
+  int exit_code = sb_interpreter.GetQuitStatus();
   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;
 }