lemo updated this revision to Diff 117900.
lemo edited the summary of this revision.
lemo added a comment.
Updating the original changes to handle reentrant calls to
CommandInterpreter::IOHandlerInputComplete().
- This fixes bug #34758
- Also enhanced the existing test case for "command source" to cover this
regression
- I considered changing SBCommandReturnObject::PutOutput() to handle output
interruptions too, but CommandReturnObjects are decoupled from
CommandInterpreter instances, so there's no easy way to query for interruption
in the current scheme.
https://reviews.llvm.org/D37923
Files:
include/lldb/API/SBCommandInterpreter.h
include/lldb/Core/IOHandler.h
include/lldb/Interpreter/CommandInterpreter.h
packages/Python/lldbsuite/test/functionalities/command_source/.lldb
packages/Python/lldbsuite/test/functionalities/command_source/commands.txt
scripts/interface/SBCommandInterpreter.i
source/API/SBCommandInterpreter.cpp
source/Commands/CommandObjectTarget.cpp
source/Core/Debugger.cpp
source/Interpreter/CommandInterpreter.cpp
Index: source/Interpreter/CommandInterpreter.cpp
===================================================================
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -546,7 +546,7 @@
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 @@
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 @@
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 @@
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<CommandAlias> command_alias_up(
new CommandAlias(*this, command_obj_sp, args_string, alias_name));
@@ -1541,6 +1541,12 @@
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 @@
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 @@
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 @@
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 @@
line.c_str());
}
+ StartHandlingCommand();
+
lldb_private::CommandReturnObject result;
HandleCommand(line.c_str(), eLazyBoolCalculate, result);
@@ -2710,18 +2777,18 @@
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 @@
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 @@
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());
}
Index: source/Core/Debugger.cpp
===================================================================
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -1170,7 +1170,7 @@
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}"
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -2053,6 +2053,8 @@
result.GetOutputStream().EOL();
result.GetOutputStream().EOL();
}
+ if (m_interpreter.WasInterrupted())
+ break;
num_dumped++;
DumpModuleSymtab(
m_interpreter, result.GetOutputStream(),
@@ -2081,6 +2083,8 @@
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 @@
" 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 @@
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 @@
" 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 @@
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 @@
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: source/API/SBCommandInterpreter.cpp
===================================================================
--- source/API/SBCommandInterpreter.cpp
+++ source/API/SBCommandInterpreter.cpp
@@ -161,6 +161,10 @@
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()
Index: scripts/interface/SBCommandInterpreter.i
===================================================================
--- scripts/interface/SBCommandInterpreter.i
+++ scripts/interface/SBCommandInterpreter.i
@@ -125,18 +125,18 @@
{
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 @@
lldb::SBProcess
GetProcess ();
-
+
lldb::SBDebugger
GetDebugger ();
@@ -209,10 +209,12 @@
int match_start_point,
int max_return_elements,
lldb::SBStringList &matches);
-
+
bool
IsActive ();
+ bool
+ WasInterrupted () const;
};
} // namespace lldb
Index: packages/Python/lldbsuite/test/functionalities/command_source/commands.txt
===================================================================
--- packages/Python/lldbsuite/test/functionalities/command_source/commands.txt
+++ packages/Python/lldbsuite/test/functionalities/command_source/commands.txt
@@ -0,0 +1,2 @@
+script import my
+p 1 + 1
Index: packages/Python/lldbsuite/test/functionalities/command_source/.lldb
===================================================================
--- packages/Python/lldbsuite/test/functionalities/command_source/.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
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,25 @@
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<CommandHandlingState> 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
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;
};
//----------------------------------------------------------------------
Index: include/lldb/API/SBCommandInterpreter.h
===================================================================
--- include/lldb/API/SBCommandInterpreter.h
+++ include/lldb/API/SBCommandInterpreter.h
@@ -165,6 +165,8 @@
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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
