lemo created this revision.
Herald added a subscriber: ki.stfu.

The core of this change is the new CommandInterpreter::m_command_state, which 
models the state transitions for interactive commands, including an 
"interrupted" state transition.

In general, command interruption requires cooperation from the code executing 
the command, which needs to poll for interruption requests through 
CommandInterpreter::WasInterrupted().

CommandInterpreter::PrintCommandOutput() implements an optionally interruptible 
printing of the command output, which for large outputs was likely the longest 
blocking part

  (ex. target modules dump symtab on a complex binary could take 10+ minutes)

Also included are a few fixes in the handlers for SIGINT (thread safety and 
using the signal-safe _exit() instead of exit())


https://reviews.llvm.org/D37923

Files:
  include/lldb/Core/IOHandler.h
  include/lldb/Interpreter/CommandInterpreter.h
  source/Commands/CommandObjectTarget.cpp
  source/Interpreter/CommandInterpreter.cpp
  tools/driver/Driver.cpp
  tools/lldb-mi/MIDriverMain.cpp

Index: tools/lldb-mi/MIDriverMain.cpp
===================================================================
--- tools/lldb-mi/MIDriverMain.cpp
+++ tools/lldb-mi/MIDriverMain.cpp
@@ -72,14 +72,13 @@
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows
   signal(SIGINT, sigint_handler);
 #endif
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   CMIDriverMgr &rDriverMgr = CMIDriverMgr::Instance();
   lldb::SBDebugger *pDebugger = rDriverMgr.DriverGetTheDebugger();
   if (pDebugger != nullptr) {
-    if (!g_interrupt_sent) {
-      g_interrupt_sent = true;
+    if (!g_interrupt_sent.test_and_set()) {
       pDebugger->DispatchInputInterrupt();
-      g_interrupt_sent = false;
+      g_interrupt_sent.clear();
     }
   }
 
Index: tools/driver/Driver.cpp
===================================================================
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -1177,17 +1177,16 @@
 }
 
 void sigint_handler(int signo) {
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   if (g_driver) {
-    if (!g_interrupt_sent) {
-      g_interrupt_sent = true;
+    if (!g_interrupt_sent.test_and_set()) {
       g_driver->GetDebugger().DispatchInputInterrupt();
-      g_interrupt_sent = false;
+      g_interrupt_sent.clear();
       return;
     }
   }
 
-  exit(signo);
+  _exit(signo);
 }
 
 void sigtstp_handler(int signo) {
Index: source/Interpreter/CommandInterpreter.cpp
===================================================================
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -2677,6 +2677,58 @@
   return total_bytes;
 }
 
+void CommandInterpreter::StartHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eInProgress);
+  assert(prev_state == CommandHandlingState::eIdle);
+}
+
+void CommandInterpreter::FinishHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle);
+  assert(prev_state != CommandHandlingState::eIdle);
+}
+
+bool CommandInterpreter::InterruptCommand() {
+  auto in_progress = CommandHandlingState::eInProgress;
+  return m_command_state.compare_exchange_strong(
+      in_progress, CommandHandlingState::eInterrupted);
+}
+
+bool CommandInterpreter::WasInterrupted() const {
+  return m_command_state == CommandHandlingState::eInterrupted;
+}
+
+void CommandInterpreter::PrintCommandOutput(Stream &stream, llvm::StringRef str,
+                                            bool interruptible) {
+  if (str.empty()) {
+    return;
+  }
+
+  if (interruptible) {
+    // Split the output into lines and poll for interrupt requests
+    const char *data = str.data();
+    size_t size = str.size();
+    while (size > 0 && !WasInterrupted()) {
+      size_t chunk_size = 0;
+      for (; chunk_size < size; ++chunk_size) {
+        assert(data[chunk_size] != '\0');
+        if (data[chunk_size] == '\n') {
+          ++chunk_size;
+          break;
+        }
+      }
+      chunk_size = stream.Write(data, chunk_size);
+      assert(size >= chunk_size);
+      data += chunk_size;
+      size -= chunk_size;
+    }
+    if (size > 0) {
+      stream.Printf("\n... Interrupted.\n");
+    }
+  } else {
+    stream.PutCString(str);
+  }
+}
+
 void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
                                                 std::string &line) {
   const bool is_interactive = io_handler.GetIsInteractive();
@@ -2700,6 +2752,8 @@
                                                line.c_str());
   }
 
+  StartHandlingCommand();
+
   lldb_private::CommandReturnObject result;
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
@@ -2710,18 +2764,20 @@
 
     if (!result.GetImmediateOutputStream()) {
       llvm::StringRef output = result.GetOutputData();
-      if (!output.empty())
-        io_handler.GetOutputStreamFile()->PutCString(output);
+      PrintCommandOutput(*io_handler.GetOutputStreamFile(), output,
+                         is_interactive);
     }
 
     // Now emit the command error text from the command we just executed
     if (!result.GetImmediateErrorStream()) {
       llvm::StringRef error = result.GetErrorData();
-      if (!error.empty())
-        io_handler.GetErrorStreamFile()->PutCString(error);
+      PrintCommandOutput(*io_handler.GetErrorStreamFile(), error,
+                         is_interactive);
     }
   }
 
+  FinishHandlingCommand();
+
   switch (result.GetStatus()) {
   case eReturnStatusInvalid:
   case eReturnStatusSuccessFinishNoResult:
@@ -2777,6 +2833,10 @@
   ExecutionContext exe_ctx(GetExecutionContext());
   Process *process = exe_ctx.GetProcessPtr();
 
+  if (InterruptCommand()) {
+    return true;
+  }
+
   if (process) {
     StateType state = process->GetState();
     if (StateIsRunningState(state)) {
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -2053,6 +2053,9 @@
               result.GetOutputStream().EOL();
               result.GetOutputStream().EOL();
             }
+            if (m_interpreter.WasInterrupted()) {
+              break;
+            }
             num_dumped++;
             DumpModuleSymtab(
                 m_interpreter, result.GetOutputStream(),
@@ -2081,6 +2084,9 @@
                   result.GetOutputStream().EOL();
                   result.GetOutputStream().EOL();
                 }
+                if (m_interpreter.WasInterrupted()) {
+                  break;
+                }
                 num_dumped++;
                 DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
                                  module, m_options.m_sort_order);
@@ -2146,6 +2152,9 @@
                                           " modules.\n",
                                           (uint64_t)num_modules);
           for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
+            if (m_interpreter.WasInterrupted()) {
+              break;
+            }
             num_dumped++;
             DumpModuleSections(
                 m_interpreter, result.GetOutputStream(),
@@ -2167,6 +2176,9 @@
               FindModulesByName(target, arg_cstr, module_list, true);
           if (num_matches > 0) {
             for (size_t i = 0; i < num_matches; ++i) {
+              if (m_interpreter.WasInterrupted()) {
+                break;
+              }
               Module *module = module_list.GetModulePointerAtIndex(i);
               if (module) {
                 num_dumped++;
@@ -2239,6 +2251,9 @@
                                           " modules.\n",
                                           (uint64_t)num_modules);
           for (uint32_t image_idx = 0; image_idx < num_modules; ++image_idx) {
+            if (m_interpreter.WasInterrupted()) {
+              break;
+            }
             if (DumpModuleSymbolVendor(
                     result.GetOutputStream(),
                     target_modules.GetModulePointerAtIndexUnlocked(image_idx)))
@@ -2260,6 +2275,9 @@
               FindModulesByName(target, arg_cstr, module_list, true);
           if (num_matches > 0) {
             for (size_t i = 0; i < num_matches; ++i) {
+              if (m_interpreter.WasInterrupted()) {
+                break;
+              }
               Module *module = module_list.GetModulePointerAtIndex(i);
               if (module) {
                 if (DumpModuleSymbolVendor(result.GetOutputStream(), module))
@@ -2327,6 +2345,9 @@
         if (num_modules > 0) {
           uint32_t num_dumped = 0;
           for (uint32_t i = 0; i < num_modules; ++i) {
+            if (m_interpreter.WasInterrupted()) {
+              break;
+            }
             if (DumpCompileUnitLineTable(
                     m_interpreter, result.GetOutputStream(),
                     target_modules.GetModulePointerAtIndexUnlocked(i),
Index: include/lldb/Interpreter/CommandInterpreter.h
===================================================================
--- include/lldb/Interpreter/CommandInterpreter.h
+++ include/lldb/Interpreter/CommandInterpreter.h
@@ -242,6 +242,8 @@
                      bool repeat_on_empty_command = true,
                      bool no_context_switching = false);
 
+  bool WasInterrupted() const;
+
   //------------------------------------------------------------------
   /// Execute a list of commands in sequence.
   ///
@@ -522,6 +524,23 @@
                               StringList &commands_help,
                               CommandObject::CommandMap &command_map);
 
+  // An interruptible wrapper around the stream output
+  void PrintCommandOutput(Stream &stream, llvm::StringRef str,
+                          bool interruptible);
+
+  // A very simple state machine which models the command handling transitions
+  enum class CommandHandlingState {
+    eIdle,
+    eInProgress,
+    eInterrupted,
+  };
+
+  std::atomic<CommandHandlingState> m_command_state {CommandHandlingState::eIdle};
+
+  void StartHandlingCommand();
+  void FinishHandlingCommand();
+  bool InterruptCommand();
+
   Debugger &m_debugger; // The debugger session that this interpreter is
                         // associated with
   ExecutionContextRef m_exe_ctx_ref; // The current execution context to use
Index: include/lldb/Core/IOHandler.h
===================================================================
--- include/lldb/Core/IOHandler.h
+++ include/lldb/Core/IOHandler.h
@@ -195,7 +195,7 @@
   enum class Completion { None, LLDBCommand, Expression };
 
   IOHandlerDelegate(Completion completion = Completion::None)
-      : m_completion(completion), m_io_handler_done(false) {}
+      : m_completion(completion) {}
 
   virtual ~IOHandlerDelegate() = default;
 
@@ -296,7 +296,6 @@
 
 protected:
   Completion m_completion; // Support for common builtin completions
-  bool m_io_handler_done;
 };
 
 //----------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to