From 6c961ae1b5073699285fcdec242cdb4e84602c05 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani Date: Fri, 14 Apr 2023 14:54:01 -0700 Subject: [PATCH] [lldb] Move ScriptedProcess private state update to implementation While debugging a Scripted Process, in order to update its state and work nicely with lldb's execution model, it needs to toggle its private state from running to stopped, which will result in broadcasting a process state changed event to the debugger listener. Originally, this state update was done systematically in the Scripted Process C++ plugin, however in order to make scripted process interactive, we need to be able to update their state dynamically. This patch makes use of the recent addition of the SBProcess::ForceScriptedState to programatically, and moves the process private state update to the python implementation of the resume method instead of doing it in ScriptedProcess::DoResume. This patch also removes the unused ShouldStop & Stop scripted process APIs, and adds new ScriptedInterface transform methods for boolean arguments. This allow the user to programmatically decide if after running the process, we should stop it (which is the default setting). Differential Revision: https://reviews.llvm.org/D145295 Signed-off-by: Med Ismail Bennani --- .../python/scripted_process/scripted_process.py | 31 ++++++------- .../lldb/Interpreter/ScriptedProcessInterface.h | 4 -- .../Plugins/Process/scripted/ScriptedProcess.cpp | 54 ++++------------------ .../Plugins/Process/scripted/ScriptedProcess.h | 2 - .../ScriptInterpreter/Python/PythonDataObjects.h | 1 + .../Python/ScriptedProcessPythonInterface.cpp | 17 +------ .../Python/ScriptedProcessPythonInterface.h | 4 -- .../Python/ScriptedPythonInterface.h | 18 ++++++++ 8 files changed, 43 insertions(+), 88 deletions(-) diff --git a/lldb/examples/python/scripted_process/scripted_process.py b/lldb/examples/python/scripted_process/scripted_process.py index e4d2521..5cd78ad 100644 --- a/lldb/examples/python/scripted_process/scripted_process.py +++ b/lldb/examples/python/scripted_process/scripted_process.py @@ -161,30 +161,25 @@ class ScriptedProcess(metaclass=ABCMeta): """ return lldb.SBError() - def resume(self): + def resume(self, should_stop=True): """ Simulate the scripted process resume. - Returns: - lldb.SBError: An `lldb.SBError` with error code 0. - """ - return lldb.SBError() - - @abstractmethod - def should_stop(self): - """ Check if the scripted process plugin should produce the stop event. - - Returns: - bool: True if scripted process should broadcast a stop event. - False otherwise. - """ - pass - - def stop(self): - """ Trigger the scripted process stop. + Args: + should_stop (bool): If True, resume will also force the process + state to stopped after running it. Returns: lldb.SBError: An `lldb.SBError` with error code 0. """ + process = self.target.GetProcess() + if not process: + error = lldb.SBError() + error.SetErrorString("Invalid process.") + return error + + process.ForceScriptedState(lldb.eStateRunning); + if (should_stop): + process.ForceScriptedState(lldb.eStateStopped); return lldb.SBError() @abstractmethod diff --git a/lldb/include/lldb/Interpreter/ScriptedProcessInterface.h b/lldb/include/lldb/Interpreter/ScriptedProcessInterface.h index 895fc5e..f37ba0f4 100644 --- a/lldb/include/lldb/Interpreter/ScriptedProcessInterface.h +++ b/lldb/include/lldb/Interpreter/ScriptedProcessInterface.h @@ -38,10 +38,6 @@ public: virtual Status Resume() { return Status("ScriptedProcess did not resume"); } - virtual bool ShouldStop() { return true; } - - virtual Status Stop() { return Status("ScriptedProcess did not stop"); } - virtual std::optional GetMemoryRegionContainingAddress(lldb::addr_t address, Status &error) { error.SetErrorString("ScriptedProcess have no memory region."); diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp index 23ff881..3c2e8f5 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp @@ -150,22 +150,15 @@ Status ScriptedProcess::DoLoadCore() { Status ScriptedProcess::DoLaunch(Module *exe_module, ProcessLaunchInfo &launch_info) { - /* FIXME: This doesn't reflect how lldb actually launches a process. - In reality, it attaches to debugserver, then resume the process. */ + LLDB_LOGF(GetLog(LLDBLog::Process), "ScriptedProcess::%s launching process", __FUNCTION__); + + /* MARK: This doesn't reflect how lldb actually launches a process. + In reality, it attaches to debugserver, then resume the process. + That's not true in all cases. If debugserver is remote, lldb + asks debugserver to launch the process for it. */ Status error = GetInterface().Launch(); - SetPrivateState(eStateRunning); - - if (error.Fail()) - return error; - - // TODO: Fetch next state from stopped event queue then send stop event - // const StateType state = SetThreadStopInfo(response); - // if (state != eStateInvalid) { - // SetPrivateState(state); - SetPrivateState(eStateStopped); - - return {}; + return error; } void ScriptedProcess::DidLaunch() { m_pid = GetInterface().GetProcessID(); } @@ -177,25 +170,9 @@ void ScriptedProcess::DidResume() { } Status ScriptedProcess::DoResume() { - Log *log = GetLog(LLDBLog::Process); - // FIXME: Fetch data from thread. - const StateType thread_resume_state = eStateRunning; - LLDB_LOGF(log, "ScriptedProcess::%s thread_resume_state = %s", __FUNCTION__, - StateAsCString(thread_resume_state)); - - bool resume = (thread_resume_state == eStateRunning); - assert(thread_resume_state == eStateRunning && "invalid thread resume state"); + LLDB_LOGF(GetLog(LLDBLog::Process), "ScriptedProcess::%s resuming process", __FUNCTION__); - Status error; - if (resume) { - LLDB_LOGF(log, "ScriptedProcess::%s sending resume", __FUNCTION__); - - SetPrivateState(eStateRunning); - SetPrivateState(eStateStopped); - error = GetInterface().Resume(); - } - - return error; + return GetInterface().Resume(); } Status ScriptedProcess::DoAttach(const ProcessAttachInfo &attach_info) { @@ -226,19 +203,6 @@ void ScriptedProcess::DidAttach(ArchSpec &process_arch) { process_arch = GetArchitecture(); } -Status ScriptedProcess::DoStop() { - Log *log = GetLog(LLDBLog::Process); - - if (GetInterface().ShouldStop()) { - SetPrivateState(eStateStopped); - LLDB_LOGF(log, "ScriptedProcess::%s Immediate stop", __FUNCTION__); - return {}; - } - - LLDB_LOGF(log, "ScriptedProcess::%s Delayed stop", __FUNCTION__); - return GetInterface().Stop(); -} - Status ScriptedProcess::DoDestroy() { return Status(); } bool ScriptedProcess::IsAlive() { return GetInterface().IsAlive(); } diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h index 9837f7f..594bd09 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h +++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h @@ -98,8 +98,6 @@ protected: ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, const ScriptedMetadata &scripted_metadata, Status &error); - Status DoStop(); - void Clear(); bool DoUpdateThreadList(ThreadList &old_thread_list, diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h index 365d499..3309220 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -201,6 +201,7 @@ template <> struct PythonFormat : PassthroughFormat {}; template <> struct PythonFormat : PassthroughFormat {}; template <> struct PythonFormat : PassthroughFormat {}; +template <> struct PythonFormat : PassthroughFormat {}; template <> struct PythonFormat : PassthroughFormat {}; template <> struct PythonFormat : PassthroughFormat {}; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp index 17fc2e5..c618a8f 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp @@ -81,21 +81,8 @@ Status ScriptedProcessPythonInterface::Launch() { } Status ScriptedProcessPythonInterface::Resume() { - return GetStatusFromMethod("resume"); -} - -bool ScriptedProcessPythonInterface::ShouldStop() { - Status error; - StructuredData::ObjectSP obj = Dispatch("is_alive", error); - - if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error)) - return {}; - - return obj->GetBooleanValue(); -} - -Status ScriptedProcessPythonInterface::Stop() { - return GetStatusFromMethod("stop"); + // When calling ScriptedProcess.Resume from lldb we should always stop. + return GetStatusFromMethod("resume", /*should_stop=*/true); } std::optional diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h index a96a55d..f56db42 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h @@ -37,10 +37,6 @@ public: Status Resume() override; - bool ShouldStop() override; - - Status Stop() override; - std::optional GetMemoryRegionContainingAddress(lldb::addr_t address, Status &error) override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h index a015bd1..7d76f06 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h @@ -113,6 +113,11 @@ protected: return {object}; } + python::PythonObject Transform(bool arg) { + // Boolean arguments need to be turned into python objects. + return python::PythonBoolean(arg); + } + python::PythonObject Transform(Status arg) { return python::ToSWIGWrapper(arg); } @@ -141,6 +146,19 @@ protected: original_arg = ExtractValueFromPythonObject(transformed_arg, error); } + template <> + void ReverseTransform(bool &original_arg, + python::PythonObject transformed_arg, Status &error) { + python::PythonBoolean boolean_arg = python::PythonBoolean( + python::PyRefType::Borrowed, transformed_arg.get()); + if (boolean_arg.IsValid()) + original_arg = boolean_arg.GetValue(); + else + error.SetErrorString( + llvm::formatv("{}: Invalid boolean argument.", LLVM_PRETTY_FUNCTION) + .str()); + } + template auto TransformTuple(const std::tuple &args, std::index_sequence) { -- 2.7.4