[lldb] Make CommandInterpreter's execution context the same as debugger's one.

Currently, the interpreter's context is not updated until a command is executed.
This has resulted in the behavior of SB-interface functions and some commands
depends on previous user actions. The interpreter's context can stay uninitialized,
point to a currently selected target, or point to one of previously selected targets.

This patch removes any usages of CommandInterpreter::UpdateExecutionContext.
CommandInterpreter::HandleCommand* functions still may override context temporarily,
but now they always restore it before exiting. CommandInterpreter saves overriden
contexts to the stack, that makes nesting commands possible.

Added test reproduces one of the issues. Without this fix, the last assertion fails
because interpreter's execution context is empty until running "target list", so,
the value of the global property was updated instead of process's local instance.

Differential Revision: https://reviews.llvm.org/D92164
This commit is contained in:
Tatyana Krasnukha
2020-12-10 19:30:04 +03:00
committed by Tatyana Krasnukha
parent 7832d7e95a
commit a01b26fb51
15 changed files with 189 additions and 142 deletions

View File

@@ -24,7 +24,9 @@
#include "lldb/Utility/StringList.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-private.h"
#include <mutex>
#include <stack>
namespace lldb_private {
class CommandInterpreter;
@@ -245,7 +247,7 @@ public:
CommandInterpreter(Debugger &debugger, bool synchronous_execution);
~CommandInterpreter() override;
~CommandInterpreter() override = default;
// These two functions fill out the Broadcaster interface:
@@ -300,10 +302,11 @@ public:
CommandReturnObject &result);
bool HandleCommand(const char *command_line, LazyBool add_to_history,
CommandReturnObject &result,
ExecutionContext *override_context = nullptr,
bool repeat_on_empty_command = true,
bool no_context_switching = false);
const ExecutionContext &override_context,
CommandReturnObject &result);
bool HandleCommand(const char *command_line, LazyBool add_to_history,
CommandReturnObject &result);
bool WasInterrupted() const;
@@ -312,9 +315,7 @@ public:
/// \param[in] commands
/// The list of commands to execute.
/// \param[in,out] context
/// The execution context in which to run the commands. Can be nullptr in
/// which case the default
/// context will be used.
/// The execution context in which to run the commands.
/// \param[in] options
/// This object holds the options used to control when to stop, whether to
/// execute commands,
@@ -324,8 +325,13 @@ public:
/// safely,
/// and failed with some explanation if we aborted executing the commands
/// at some point.
void HandleCommands(const StringList &commands, ExecutionContext *context,
CommandInterpreterRunOptions &options,
void HandleCommands(const StringList &commands,
const ExecutionContext &context,
const CommandInterpreterRunOptions &options,
CommandReturnObject &result);
void HandleCommands(const StringList &commands,
const CommandInterpreterRunOptions &options,
CommandReturnObject &result);
/// Execute a list of commands from a file.
@@ -333,9 +339,7 @@ public:
/// \param[in] file
/// The file from which to read in commands.
/// \param[in,out] context
/// The execution context in which to run the commands. Can be nullptr in
/// which case the default
/// context will be used.
/// The execution context in which to run the commands.
/// \param[in] options
/// This object holds the options used to control when to stop, whether to
/// execute commands,
@@ -345,8 +349,12 @@ public:
/// safely,
/// and failed with some explanation if we aborted executing the commands
/// at some point.
void HandleCommandsFromFile(FileSpec &file, ExecutionContext *context,
CommandInterpreterRunOptions &options,
void HandleCommandsFromFile(FileSpec &file, const ExecutionContext &context,
const CommandInterpreterRunOptions &options,
CommandReturnObject &result);
void HandleCommandsFromFile(FileSpec &file,
const CommandInterpreterRunOptions &options,
CommandReturnObject &result);
CommandObject *GetCommandObjectForCommand(llvm::StringRef &command_line);
@@ -391,12 +399,7 @@ public:
Debugger &GetDebugger() { return m_debugger; }
ExecutionContext GetExecutionContext() {
const bool thread_and_frame_only_if_stopped = true;
return m_exe_ctx_ref.Lock(thread_and_frame_only_if_stopped);
}
void UpdateExecutionContext(ExecutionContext *override_context);
ExecutionContext GetExecutionContext() const;
lldb::PlatformSP GetPlatform(bool prefer_target_platform);
@@ -581,6 +584,10 @@ protected:
StringList *descriptions = nullptr) const;
private:
void OverrideExecutionContext(const ExecutionContext &override_context);
void RestoreExecutionContext();
Status PreprocessCommand(std::string &command);
void SourceInitFile(FileSpec file, CommandReturnObject &result);
@@ -619,8 +626,9 @@ private:
Debugger &m_debugger; // The debugger session that this interpreter is
// associated with
ExecutionContextRef m_exe_ctx_ref; // The current execution context to use
// when handling commands
// Execution contexts that were temporarily set by some of HandleCommand*
// overloads.
std::stack<ExecutionContext> m_overriden_exe_contexts;
bool m_synchronous_execution;
bool m_skip_lldbinit_files;
bool m_skip_app_init_files;

View File

@@ -171,27 +171,23 @@ lldb::ReturnStatus SBCommandInterpreter::HandleCommand(
lldb::SBCommandReturnObject &, bool),
command_line, override_context, result, add_to_history);
ExecutionContext ctx, *ctx_ptr;
if (override_context.get()) {
ctx = override_context.get()->Lock(true);
ctx_ptr = &ctx;
} else
ctx_ptr = nullptr;
result.Clear();
if (command_line && IsValid()) {
result.ref().SetInteractive(false);
m_opaque_ptr->HandleCommand(command_line,
add_to_history ? eLazyBoolYes : eLazyBoolNo,
result.ref(), ctx_ptr);
auto do_add_to_history = add_to_history ? eLazyBoolYes : eLazyBoolNo;
if (override_context.get())
m_opaque_ptr->HandleCommand(command_line, do_add_to_history,
override_context.get()->Lock(true),
result.ref());
else
m_opaque_ptr->HandleCommand(command_line, do_add_to_history,
result.ref());
} else {
result->AppendError(
"SBCommandInterpreter or the command line is not valid");
result->SetStatus(eReturnStatusFailed);
}
return result.GetStatus();
}
@@ -219,15 +215,14 @@ void SBCommandInterpreter::HandleCommandsFromFile(
}
FileSpec tmp_spec = file.ref();
ExecutionContext ctx, *ctx_ptr;
if (override_context.get()) {
ctx = override_context.get()->Lock(true);
ctx_ptr = &ctx;
} else
ctx_ptr = nullptr;
if (override_context.get())
m_opaque_ptr->HandleCommandsFromFile(tmp_spec,
override_context.get()->Lock(true),
options.ref(),
result.ref());
m_opaque_ptr->HandleCommandsFromFile(tmp_spec, ctx_ptr, options.ref(),
result.ref());
else
m_opaque_ptr->HandleCommandsFromFile(tmp_spec, options.ref(), result.ref());
}
int SBCommandInterpreter::HandleCompletion(

View File

@@ -649,7 +649,7 @@ bool BreakpointOptions::BreakpointOptionsCallbackFunction(
options.SetPrintErrors(true);
options.SetAddToHistory(false);
debugger.GetCommandInterpreter().HandleCommands(commands, &exe_ctx,
debugger.GetCommandInterpreter().HandleCommands(commands, exe_ctx,
options, result);
result.GetImmediateOutputStream()->Flush();
result.GetImmediateErrorStream()->Flush();

View File

@@ -134,15 +134,12 @@ protected:
FileSpec cmd_file(command[0].ref());
FileSystem::Instance().Resolve(cmd_file);
ExecutionContext *exe_ctx = nullptr; // Just use the default context.
CommandInterpreterRunOptions options;
// If any options were set, then use them
if (m_options.m_stop_on_error.OptionWasSet() ||
m_options.m_silent_run.OptionWasSet() ||
m_options.m_stop_on_continue.OptionWasSet()) {
// Use user set settings
CommandInterpreterRunOptions options;
if (m_options.m_stop_on_continue.OptionWasSet())
options.SetStopOnContinue(
m_options.m_stop_on_continue.GetCurrentValue());
@@ -159,14 +156,9 @@ protected:
options.SetEchoCommands(m_interpreter.GetEchoCommands());
options.SetEchoCommentCommands(m_interpreter.GetEchoCommentCommands());
}
m_interpreter.HandleCommandsFromFile(cmd_file, exe_ctx, options, result);
} else {
// No options were set, inherit any settings from nested "command source"
// commands, or set to sane default settings...
CommandInterpreterRunOptions options;
m_interpreter.HandleCommandsFromFile(cmd_file, exe_ctx, options, result);
}
m_interpreter.HandleCommandsFromFile(cmd_file, options, result);
return result.Succeeded();
}

View File

@@ -292,18 +292,12 @@ void CommandObjectExpression::HandleCompletion(CompletionRequest &request) {
options.SetAutoApplyFixIts(false);
options.SetGenerateDebugInfo(false);
// We need a valid execution context with a frame pointer for this
// completion, so if we don't have one we should try to make a valid
// execution context.
if (m_interpreter.GetExecutionContext().GetFramePtr() == nullptr)
m_interpreter.UpdateExecutionContext(nullptr);
ExecutionContext exe_ctx(m_interpreter.GetExecutionContext());
// This didn't work, so let's get out before we start doing things that
// expect a valid frame pointer.
if (m_interpreter.GetExecutionContext().GetFramePtr() == nullptr)
// Get out before we start doing things that expect a valid frame pointer.
if (exe_ctx.GetFramePtr() == nullptr)
return;
ExecutionContext exe_ctx(m_interpreter.GetExecutionContext());
Target *exe_target = exe_ctx.GetTargetPtr();
Target &target = exe_target ? *exe_target : GetDummyTarget();

View File

@@ -380,7 +380,6 @@ protected:
return false;
}
m_interpreter.UpdateExecutionContext(nullptr);
StreamString stream;
const auto error = target->Attach(m_options.attach_info, &stream);
if (error.Success()) {

View File

@@ -53,8 +53,8 @@ bool CommandObjectRegexCommand::DoExecute(llvm::StringRef command,
// Pass in true for "no context switching". The command that called us
// should have set up the context appropriately, we shouldn't have to
// redo that.
return m_interpreter.HandleCommand(
new_command.c_str(), eLazyBoolCalculate, result, nullptr, true, true);
return m_interpreter.HandleCommand(new_command.c_str(),
eLazyBoolCalculate, result);
}
}
result.SetStatus(eReturnStatusFailed);

View File

@@ -469,14 +469,13 @@ protected:
bool DoExecute(Args &command, CommandReturnObject &result) override {
FileSpec file(m_options.m_filename);
FileSystem::Instance().Resolve(file);
ExecutionContext clean_ctx;
CommandInterpreterRunOptions options;
options.SetAddToHistory(false);
options.SetEchoCommands(false);
options.SetPrintResults(true);
options.SetPrintErrors(true);
options.SetStopOnError(false);
m_interpreter.HandleCommandsFromFile(file, &clean_ctx, options, result);
m_interpreter.HandleCommandsFromFile(file, options, result);
return result.Succeeded();
}

View File

@@ -301,7 +301,7 @@ are no syntax errors may indicate that a function was declared but never called.
options.SetPrintErrors(true);
options.SetAddToHistory(false);
debugger.GetCommandInterpreter().HandleCommands(commands, &exe_ctx,
debugger.GetCommandInterpreter().HandleCommands(commands, exe_ctx,
options, result);
result.GetImmediateOutputStream()->Flush();
result.GetImmediateErrorStream()->Flush();

View File

@@ -1390,8 +1390,6 @@ public:
ConstString broadcaster_class(
broadcaster->GetBroadcasterClass());
if (broadcaster_class == broadcaster_class_process) {
debugger.GetCommandInterpreter().UpdateExecutionContext(
nullptr);
m_update_screen = true;
continue; // Don't get any key, just update our view
}
@@ -1403,7 +1401,6 @@ public:
HandleCharResult key_result = m_window_sp->HandleChar(ch);
switch (key_result) {
case eKeyHandled:
debugger.GetCommandInterpreter().UpdateExecutionContext(nullptr);
m_update_screen = true;
break;
case eKeyNotHandled:

View File

@@ -1634,12 +1634,18 @@ Status CommandInterpreter::PreprocessCommand(std::string &command) {
bool CommandInterpreter::HandleCommand(const char *command_line,
LazyBool lazy_add_to_history,
CommandReturnObject &result,
ExecutionContext *override_context,
bool repeat_on_empty_command,
bool no_context_switching)
const ExecutionContext &override_context,
CommandReturnObject &result) {
{
OverrideExecutionContext(override_context);
bool status = HandleCommand(command_line, lazy_add_to_history, result);
RestoreExecutionContext();
return status;
}
bool CommandInterpreter::HandleCommand(const char *command_line,
LazyBool lazy_add_to_history,
CommandReturnObject &result) {
std::string command_string(command_line);
std::string original_command_string(command_line);
@@ -1653,9 +1659,6 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
Timer scoped_timer(func_cat, "Handling command: %s.", command_line);
if (!no_context_switching)
UpdateExecutionContext(override_context);
if (WasInterrupted()) {
result.AppendError("interrupted");
result.SetStatus(eReturnStatusFailed);
@@ -1701,26 +1704,22 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
}
if (empty_command) {
if (repeat_on_empty_command) {
if (m_command_history.IsEmpty()) {
result.AppendError("empty command");
result.SetStatus(eReturnStatusFailed);
return false;
} else {
command_line = m_repeat_command.c_str();
command_string = command_line;
original_command_string = command_line;
if (m_repeat_command.empty()) {
result.AppendError("No auto repeat.");
result.SetStatus(eReturnStatusFailed);
return false;
}
}
add_to_history = false;
} else {
result.SetStatus(eReturnStatusSuccessFinishNoResult);
return true;
if (m_command_history.IsEmpty()) {
result.AppendError("empty command");
result.SetStatus(eReturnStatusFailed);
return false;
}
command_line = m_repeat_command.c_str();
command_string = command_line;
original_command_string = command_line;
if (m_repeat_command.empty()) {
result.AppendError("No auto repeat.");
result.SetStatus(eReturnStatusFailed);
return false;
}
add_to_history = false;
} else if (comment_command) {
result.SetStatus(eReturnStatusSuccessFinishNoResult);
return true;
@@ -1857,8 +1856,6 @@ void CommandInterpreter::HandleCompletionMatches(CompletionRequest &request) {
void CommandInterpreter::HandleCompletion(CompletionRequest &request) {
UpdateExecutionContext(nullptr);
// Don't complete comments, and if the line we are completing is just the
// history repeat character, substitute the appropriate history line.
llvm::StringRef first_arg = request.GetParsedLine().GetArgumentAtIndex(0);
@@ -1890,8 +1887,6 @@ CommandInterpreter::GetAutoSuggestionForCommand(llvm::StringRef line) {
return llvm::None;
}
CommandInterpreter::~CommandInterpreter() {}
void CommandInterpreter::UpdatePrompt(llvm::StringRef new_prompt) {
EventSP prompt_change_event_sp(
new Event(eBroadcastBitResetPrompt, new EventDataBytes(new_prompt)));
@@ -2133,13 +2128,12 @@ void CommandInterpreter::SourceInitFile(FileSpec file,
// broadcasting of the commands back to any appropriate listener (see
// CommandObjectSource::Execute for more details).
const bool saved_batch = SetBatchCommandMode(true);
ExecutionContext *ctx = nullptr;
CommandInterpreterRunOptions options;
options.SetSilent(true);
options.SetPrintErrors(true);
options.SetStopOnError(false);
options.SetStopOnContinue(true);
HandleCommandsFromFile(file, ctx, options, result);
HandleCommandsFromFile(file, options, result);
SetBatchCommandMode(saved_batch);
}
@@ -2230,7 +2224,8 @@ PlatformSP CommandInterpreter::GetPlatform(bool prefer_target_platform) {
}
bool CommandInterpreter::DidProcessStopAbnormally() const {
TargetSP target_sp = m_debugger.GetTargetList().GetSelectedTarget();
auto exe_ctx = GetExecutionContext();
TargetSP target_sp = exe_ctx.GetTargetSP();
if (!target_sp)
return false;
@@ -2268,9 +2263,19 @@ bool CommandInterpreter::DidProcessStopAbnormally() const {
return false;
}
void
CommandInterpreter::HandleCommands(const StringList &commands,
const ExecutionContext &override_context,
const CommandInterpreterRunOptions &options,
CommandReturnObject &result) {
OverrideExecutionContext(override_context);
HandleCommands(commands, options, result);
RestoreExecutionContext();
}
void CommandInterpreter::HandleCommands(const StringList &commands,
ExecutionContext *override_context,
CommandInterpreterRunOptions &options,
const CommandInterpreterRunOptions &options,
CommandReturnObject &result) {
size_t num_lines = commands.GetSize();
@@ -2280,13 +2285,6 @@ void CommandInterpreter::HandleCommands(const StringList &commands,
bool old_async_execution = m_debugger.GetAsyncExecution();
// If we've been given an execution context, set it at the start, but don't
// keep resetting it or we will cause series of commands that change the
// context, then do an operation that relies on that context to fail.
if (override_context != nullptr)
UpdateExecutionContext(override_context);
if (!options.GetStopOnContinue()) {
m_debugger.SetAsyncExecution(false);
}
@@ -2305,19 +2303,12 @@ void CommandInterpreter::HandleCommands(const StringList &commands,
CommandReturnObject tmp_result(m_debugger.GetUseColor());
tmp_result.SetInteractive(result.GetInteractive());
// If override_context is not NULL, pass no_context_switching = true for
// HandleCommand() since we updated our context already.
// We might call into a regex or alias command, in which case the
// add_to_history will get lost. This m_command_source_depth dingus is the
// way we turn off adding to the history in that case, so set it up here.
if (!options.GetAddToHistory())
m_command_source_depth++;
bool success =
HandleCommand(cmd, options.m_add_to_history, tmp_result,
nullptr, /* override_context */
true, /* repeat_on_empty_command */
override_context != nullptr /* no_context_switching */);
bool success = HandleCommand(cmd, options.m_add_to_history, tmp_result);
if (!options.GetAddToHistory())
m_command_source_depth--;
@@ -2418,8 +2409,15 @@ enum {
};
void CommandInterpreter::HandleCommandsFromFile(
FileSpec &cmd_file, ExecutionContext *context,
CommandInterpreterRunOptions &options, CommandReturnObject &result) {
FileSpec &cmd_file, const ExecutionContext &context,
const CommandInterpreterRunOptions &options, CommandReturnObject &result) {
OverrideExecutionContext(context);
HandleCommandsFromFile(cmd_file, options, result);
RestoreExecutionContext();
}
void CommandInterpreter::HandleCommandsFromFile(FileSpec &cmd_file,
const CommandInterpreterRunOptions &options, CommandReturnObject &result) {
if (!FileSystem::Instance().Exists(cmd_file)) {
result.AppendErrorWithFormat(
"Error reading commands from file %s - file not found.\n",
@@ -2720,24 +2718,26 @@ void CommandInterpreter::FindCommandsForApropos(llvm::StringRef search_word,
m_alias_dict);
}
void CommandInterpreter::UpdateExecutionContext(
ExecutionContext *override_context) {
if (override_context != nullptr) {
m_exe_ctx_ref = *override_context;
} else {
const bool adopt_selected = true;
m_exe_ctx_ref.SetTargetPtr(m_debugger.GetSelectedTarget().get(),
adopt_selected);
}
ExecutionContext CommandInterpreter::GetExecutionContext() const {
return !m_overriden_exe_contexts.empty()
? m_overriden_exe_contexts.top()
: m_debugger.GetSelectedExecutionContext();
}
void CommandInterpreter::OverrideExecutionContext(
const ExecutionContext &override_context) {
m_overriden_exe_contexts.push(override_context);
}
void CommandInterpreter::RestoreExecutionContext() {
if (!m_overriden_exe_contexts.empty())
m_overriden_exe_contexts.pop();
}
void CommandInterpreter::GetProcessOutput() {
TargetSP target_sp(m_debugger.GetTargetList().GetSelectedTarget());
if (!target_sp)
return;
if (ProcessSP process_sp = target_sp->GetProcessSP())
m_debugger.FlushProcessOutput(*process_sp, /*flush_stdout*/ true,
auto *process_ptr = GetExecutionContext().GetProcessPtr();
if (process_ptr != nullptr)
m_debugger.FlushProcessOutput(*process_ptr, /*flush_stdout*/ true,
/*flush_stderr*/ true);
}

View File

@@ -3351,7 +3351,7 @@ Target::StopHookCommandLine::HandleStop(ExecutionContext &exc_ctx,
// Force Async:
bool old_async = debugger.GetAsyncExecution();
debugger.SetAsyncExecution(true);
debugger.GetCommandInterpreter().HandleCommands(GetCommands(), &exc_ctx,
debugger.GetCommandInterpreter().HandleCommands(GetCommands(), exc_ctx,
options, result);
debugger.SetAsyncExecution(old_async);
lldb::ReturnStatus status = result.GetStatus();

View File

@@ -0,0 +1,3 @@
CXX_SOURCES := main.cpp
include Makefile.rules

View File

@@ -43,3 +43,54 @@ class DebuggerAPITestCase(TestBase):
target = lldb.SBTarget()
self.assertFalse(target.IsValid())
self.dbg.DeleteTarget(target)
def test_debugger_internal_variable(self):
"""Ensure that SBDebugger reachs the same instance of properties
regardless CommandInterpreter's context initialization"""
self.build()
exe = self.getBuildArtifact("a.out")
# Create a target by the debugger.
target = self.dbg.CreateTarget(exe)
self.assertTrue(target, VALID_TARGET)
property_name = "target.process.memory-cache-line-size"
def get_cache_line_size():
value_list = lldb.SBStringList()
value_list = self.dbg.GetInternalVariableValue(property_name,
self.dbg.GetInstanceName())
self.assertEqual(value_list.GetSize(), 1)
try:
return int(value_list.GetStringAtIndex(0))
except ValueError as error:
self.fail("Value is not a number: " + error)
# Get global property value while there are no processes.
global_cache_line_size = get_cache_line_size()
# Run a process via SB interface. CommandInterpreter's execution context
# remains empty.
error = lldb.SBError()
launch_info = lldb.SBLaunchInfo(None)
launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
process = target.Launch(launch_info, error)
self.assertTrue(process, PROCESS_IS_VALID)
# This should change the value of a process's local property.
new_cache_line_size = global_cache_line_size + 512
error = self.dbg.SetInternalVariable(property_name,
str(new_cache_line_size),
self.dbg.GetInstanceName())
self.assertTrue(error.Success(),
property_name + " value was changed successfully")
# Check that it was set actually.
self.assertEqual(get_cache_line_size(), new_cache_line_size)
# Run any command to initialize CommandInterpreter's execution context.
self.runCmd("target list")
# Test the local property again, is it set to new_cache_line_size?
self.assertEqual(get_cache_line_size(), new_cache_line_size)

View File

@@ -0,0 +1,9 @@
// This simple program is to test the lldb Python API SBDebugger.
int func(int val) {
return val - 1;
}
int main (int argc, char const *argv[]) {
return func(argc);
}