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