From 17ffd39ed8b5e224684bb77fd913fb1f1b5d77a0 Mon Sep 17 00:00:00 2001 From: Leonard Mosescu Date: Thu, 5 Oct 2017 23:41:28 +0000 Subject: [PATCH] Implement interactive command interruption 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) Differential Revision: https://reviews.llvm.org/D37923 llvm-svn: 315037 --- lldb/include/lldb/API/SBCommandInterpreter.h | 2 + lldb/include/lldb/Core/IOHandler.h | 3 +- .../lldb/Interpreter/CommandInterpreter.h | 21 ++++ .../test/functionalities/command_source/.lldb | 3 +- .../command_source/commands.txt | 2 + lldb/scripts/interface/SBCommandInterpreter.i | 14 +-- lldb/source/API/SBCommandInterpreter.cpp | 4 + lldb/source/Commands/CommandObjectTarget.cpp | 14 +++ lldb/source/Core/Debugger.cpp | 2 +- .../source/Interpreter/CommandInterpreter.cpp | 98 ++++++++++++++++--- 10 files changed, 139 insertions(+), 24 deletions(-) create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/command_source/commands.txt diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h b/lldb/include/lldb/API/SBCommandInterpreter.h index f684071740e39..80f24ceca7b41 100644 --- a/lldb/include/lldb/API/SBCommandInterpreter.h +++ b/lldb/include/lldb/API/SBCommandInterpreter.h @@ -165,6 +165,8 @@ class SBCommandInterpreter { int match_start_point, int max_return_elements, lldb::SBStringList &matches); + bool WasInterrupted() const; + // Catch commands before they execute by registering a callback that will // get called when the command gets executed. This allows GUI or command // line interfaces to intercept a command and stop it from happening diff --git a/lldb/include/lldb/Core/IOHandler.h b/lldb/include/lldb/Core/IOHandler.h index ebf56d7908b47..e8cfbade5c61e 100644 --- a/lldb/include/lldb/Core/IOHandler.h +++ b/lldb/include/lldb/Core/IOHandler.h @@ -195,7 +195,7 @@ class IOHandlerDelegate { 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 @@ class IOHandlerDelegate { protected: Completion m_completion; // Support for common builtin completions - bool m_io_handler_done; }; //---------------------------------------------------------------------- diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 73bd7d6e6220d..5e561f47946d7 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -242,6 +242,8 @@ class CommandInterpreter : public Broadcaster, bool repeat_on_empty_command = true, bool no_context_switching = false); + bool WasInterrupted() const; + //------------------------------------------------------------------ /// Execute a list of commands in sequence. /// @@ -522,6 +524,25 @@ class CommandInterpreter : public Broadcaster, StringList &commands_help, CommandObject::CommandMap &command_map); + // An interruptible wrapper around the stream output + void PrintCommandOutput(Stream &stream, llvm::StringRef str); + + // A very simple state machine which models the command handling transitions + enum class CommandHandlingState { + eIdle, + eInProgress, + eInterrupted, + }; + + std::atomic m_command_state{ + CommandHandlingState::eIdle}; + + int m_iohandler_nesting_level = 0; + + 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 diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/command_source/.lldb b/lldb/packages/Python/lldbsuite/test/functionalities/command_source/.lldb index c544523832e76..ecbdcff44626d 100644 --- a/lldb/packages/Python/lldbsuite/test/functionalities/command_source/.lldb +++ b/lldb/packages/Python/lldbsuite/test/functionalities/command_source/.lldb @@ -1 +1,2 @@ -script import my +# one more level of indirection to stress the command interpreter reentrancy +command source commands.txt diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/command_source/commands.txt b/lldb/packages/Python/lldbsuite/test/functionalities/command_source/commands.txt new file mode 100644 index 0000000000000..8e4de66d46901 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/command_source/commands.txt @@ -0,0 +1,2 @@ +script import my +p 1 + 1 diff --git a/lldb/scripts/interface/SBCommandInterpreter.i b/lldb/scripts/interface/SBCommandInterpreter.i index c427d38b14d82..255e8ba4496ac 100644 --- a/lldb/scripts/interface/SBCommandInterpreter.i +++ b/lldb/scripts/interface/SBCommandInterpreter.i @@ -125,18 +125,18 @@ public: { eBroadcastBitThreadShouldExit = (1 << 0), eBroadcastBitResetPrompt = (1 << 1), - eBroadcastBitQuitCommandReceived = (1 << 2), // User entered quit + eBroadcastBitQuitCommandReceived = (1 << 2), // User entered quit eBroadcastBitAsynchronousOutputData = (1 << 3), eBroadcastBitAsynchronousErrorData = (1 << 4) }; SBCommandInterpreter (const lldb::SBCommandInterpreter &rhs); - + ~SBCommandInterpreter (); - static const char * + static const char * GetArgumentTypeAsCString (const lldb::CommandArgumentType arg_type); - + static const char * GetArgumentDescriptionAsCString (const lldb::CommandArgumentType arg_type); @@ -181,7 +181,7 @@ public: lldb::SBProcess GetProcess (); - + lldb::SBDebugger GetDebugger (); @@ -209,10 +209,12 @@ public: int match_start_point, int max_return_elements, lldb::SBStringList &matches); - + bool IsActive (); + bool + WasInterrupted () const; }; } // namespace lldb diff --git a/lldb/source/API/SBCommandInterpreter.cpp b/lldb/source/API/SBCommandInterpreter.cpp index aa4953999b84e..c9bb8f65fa2e9 100644 --- a/lldb/source/API/SBCommandInterpreter.cpp +++ b/lldb/source/API/SBCommandInterpreter.cpp @@ -161,6 +161,10 @@ bool SBCommandInterpreter::IsActive() { return (IsValid() ? m_opaque_ptr->IsActive() : false); } +bool SBCommandInterpreter::WasInterrupted() const { + return (IsValid() ? m_opaque_ptr->WasInterrupted() : false); +} + const char *SBCommandInterpreter::GetIOHandlerControlSequence(char ch) { return (IsValid() ? m_opaque_ptr->GetDebugger() diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index bb1b3f8ae7da8..4eaec5b4cea91 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -2053,6 +2053,8 @@ class CommandObjectTargetModulesDumpSymtab result.GetOutputStream().EOL(); result.GetOutputStream().EOL(); } + if (m_interpreter.WasInterrupted()) + break; num_dumped++; DumpModuleSymtab( m_interpreter, result.GetOutputStream(), @@ -2081,6 +2083,8 @@ class CommandObjectTargetModulesDumpSymtab 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 +2150,8 @@ class CommandObjectTargetModulesDumpSections " 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 +2173,8 @@ class CommandObjectTargetModulesDumpSections 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 +2247,8 @@ class CommandObjectTargetModulesDumpSymfile " 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 +2270,8 @@ class CommandObjectTargetModulesDumpSymfile 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 +2339,8 @@ class CommandObjectTargetModulesDumpLineTable 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), diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index d42e4df56d8bc..a4d78151c75fd 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1170,7 +1170,7 @@ TestPromptFormats (StackFrame *frame) return; StreamString s; - const char *prompt_format = + const char *prompt_format = "{addr = '${addr}'\n}" "{addr-file-or-load = '${addr-file-or-load}'\n}" "{current-pc-arrow = '${current-pc-arrow}'\n}" diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 5efc6d1db27c0..8dc5637b6e4eb 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -546,7 +546,7 @@ void CommandInterpreter::LoadCommandDictionary() { char buffer[1024]; int num_printed = snprintf(buffer, 1024, "%s %s", break_regexes[i][1], "-o"); - assert(num_printed < 1024); + lldbassert(num_printed < 1024); UNUSED_IF_ASSERT_DISABLED(num_printed); success = tbreak_regex_cmd_ap->AddRegexCommand(break_regexes[i][0], buffer); @@ -891,8 +891,8 @@ bool CommandInterpreter::AddCommand(llvm::StringRef name, const lldb::CommandObjectSP &cmd_sp, bool can_replace) { if (cmd_sp.get()) - assert((this == &cmd_sp->GetCommandInterpreter()) && - "tried to add a CommandObject from a different interpreter"); + lldbassert((this == &cmd_sp->GetCommandInterpreter()) && + "tried to add a CommandObject from a different interpreter"); if (name.empty()) return false; @@ -913,8 +913,8 @@ bool CommandInterpreter::AddUserCommand(llvm::StringRef name, const lldb::CommandObjectSP &cmd_sp, bool can_replace) { if (cmd_sp.get()) - assert((this == &cmd_sp->GetCommandInterpreter()) && - "tried to add a CommandObject from a different interpreter"); + lldbassert((this == &cmd_sp->GetCommandInterpreter()) && + "tried to add a CommandObject from a different interpreter"); if (!name.empty()) { // do not allow replacement of internal commands @@ -1062,8 +1062,8 @@ CommandInterpreter::AddAlias(llvm::StringRef alias_name, lldb::CommandObjectSP &command_obj_sp, llvm::StringRef args_string) { if (command_obj_sp.get()) - assert((this == &command_obj_sp->GetCommandInterpreter()) && - "tried to add a CommandObject from a different interpreter"); + lldbassert((this == &command_obj_sp->GetCommandInterpreter()) && + "tried to add a CommandObject from a different interpreter"); std::unique_ptr command_alias_up( new CommandAlias(*this, command_obj_sp, args_string, alias_name)); @@ -1541,6 +1541,12 @@ bool CommandInterpreter::HandleCommand(const char *command_line, if (!no_context_switching) UpdateExecutionContext(override_context); + if (WasInterrupted()) { + result.AppendError("interrupted"); + result.SetStatus(eReturnStatusFailed); + return false; + } + bool add_to_history; if (lazy_add_to_history == eLazyBoolCalculate) add_to_history = (m_command_source_depth == 0); @@ -1839,7 +1845,7 @@ int CommandInterpreter::HandleCompletion( matches.Clear(); // Only max_return_elements == -1 is supported at present: - assert(max_return_elements == -1); + lldbassert(max_return_elements == -1); bool word_complete; num_command_matches = HandleCompletionMatches( parsed_line, cursor_index, cursor_char_position, match_start_point, @@ -2210,7 +2216,7 @@ void CommandInterpreter::HandleCommands(const StringList &commands, m_debugger.SetAsyncExecution(false); } - for (size_t idx = 0; idx < num_lines; idx++) { + for (size_t idx = 0; idx < num_lines && !WasInterrupted(); idx++) { const char *cmd = commands.GetStringAtIndex(idx); if (cmd[0] == '\0') continue; @@ -2677,8 +2683,67 @@ size_t CommandInterpreter::GetProcessOutput() { return total_bytes; } +void CommandInterpreter::StartHandlingCommand() { + auto idle_state = CommandHandlingState::eIdle; + if (m_command_state.compare_exchange_strong( + idle_state, CommandHandlingState::eInProgress)) + lldbassert(m_iohandler_nesting_level == 0); + else + lldbassert(m_iohandler_nesting_level > 0); + ++m_iohandler_nesting_level; +} + +void CommandInterpreter::FinishHandlingCommand() { + lldbassert(m_iohandler_nesting_level > 0); + if (--m_iohandler_nesting_level == 0) { + auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle); + lldbassert(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 { + bool was_interrupted = + (m_command_state == CommandHandlingState::eInterrupted); + lldbassert(!was_interrupted || m_iohandler_nesting_level > 0); + return was_interrupted; +} + +void CommandInterpreter::PrintCommandOutput(Stream &stream, + llvm::StringRef str) { + // 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) { + lldbassert(data[chunk_size] != '\0'); + if (data[chunk_size] == '\n') { + ++chunk_size; + break; + } + } + chunk_size = stream.Write(data, chunk_size); + lldbassert(size >= chunk_size); + data += chunk_size; + size -= chunk_size; + } + if (size > 0) { + stream.Printf("\n... Interrupted.\n"); + } +} + void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, std::string &line) { + // If we were interrupted, bail out... + if (WasInterrupted()) + return; + const bool is_interactive = io_handler.GetIsInteractive(); if (is_interactive == false) { // When we are not interactive, don't execute blank lines. This will happen @@ -2700,6 +2765,8 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, line.c_str()); } + StartHandlingCommand(); + lldb_private::CommandReturnObject result; HandleCommand(line.c_str(), eLazyBoolCalculate, result); @@ -2710,18 +2777,18 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, if (!result.GetImmediateOutputStream()) { llvm::StringRef output = result.GetOutputData(); - if (!output.empty()) - io_handler.GetOutputStreamFile()->PutCString(output); + PrintCommandOutput(*io_handler.GetOutputStreamFile(), output); } // 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); } } + FinishHandlingCommand(); + switch (result.GetStatus()) { case eReturnStatusInvalid: case eReturnStatusSuccessFinishNoResult: @@ -2777,6 +2844,9 @@ bool CommandInterpreter::IOHandlerInterrupt(IOHandler &io_handler) { ExecutionContext exe_ctx(GetExecutionContext()); Process *process = exe_ctx.GetProcessPtr(); + if (InterruptCommand()) + return true; + if (process) { StateType state = process->GetState(); if (StateIsRunningState(state)) { @@ -2998,7 +3068,7 @@ CommandInterpreter::ResolveCommandImpl(std::string &command_line, result.AppendRawError(error_msg.GetString()); } else { // We didn't have only one match, otherwise we wouldn't get here. - assert(num_matches == 0); + lldbassert(num_matches == 0); result.AppendErrorWithFormat("'%s' is not a valid command.\n", next_word.c_str()); }