[Lldb-commits] [lldb] a3d4f73 - [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes
Med Ismail Bennani via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 6 16:03:22 PST 2023
Author: Med Ismail Bennani
Date: 2023-02-06T16:02:51-08:00
New Revision: a3d4f739eea357c702754246442a2568f2bace81
URL: https://github.com/llvm/llvm-project/commit/a3d4f739eea357c702754246442a2568f2bace81
DIFF: https://github.com/llvm/llvm-project/commit/a3d4f739eea357c702754246442a2568f2bace81.diff
LOG: [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes
This patch should address a bug when a user have multiple scripted
processes in the same debugging session.
In order for the scripted process plugin to be able to call into the
scripted object instance methods to fetch the necessary data to
reconstruct its state, the scripted process plugin calls into a
scripted process interface, that has a reference to the created script
object instance.
However, prior to this patch, we only had a single instance of the
scripted process interface, living the script interpreter. So every time
a new scripted process plugin was created, it would overwrite the script
object instance that was held by the single scripted process interface
in the script interpreter.
That would cause all the method calls made to the scripted process
interface to be dispatched by the last instanciated script object
instance, which is wrong.
In order to prevent that, this patch moves the scripted process
interface reference to be help by the scripted process plugin itself.
rdar://104882562
Differential Revision: https://reviews.llvm.org/D143308
Signed-off-by: Med Ismail Bennani <medismail.bennani at gmail.com>
Added:
Modified:
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/include/lldb/Interpreter/ScriptedInterface.h
lldb/source/Interpreter/ScriptInterpreter.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.h
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 4d073995defb2..4917b7a508aed 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -147,8 +147,6 @@ class ScriptInterpreter : public PluginInterface {
ScriptInterpreter(
Debugger &debugger, lldb::ScriptLanguage script_lang,
- lldb::ScriptedProcessInterfaceUP scripted_process_interface_up =
- std::make_unique<ScriptedProcessInterface>(),
lldb::ScriptedPlatformInterfaceUP scripted_platform_interface_up =
std::make_unique<ScriptedPlatformInterface>());
@@ -570,8 +568,8 @@ class ScriptInterpreter : public PluginInterface {
lldb::ScriptLanguage GetLanguage() { return m_script_lang; }
- ScriptedProcessInterface &GetScriptedProcessInterface() {
- return *m_scripted_process_interface_up;
+ virtual lldb::ScriptedProcessInterfaceUP CreateScriptedProcessInterface() {
+ return std::make_unique<ScriptedProcessInterface>();
}
ScriptedPlatformInterface &GetScriptedPlatformInterface() {
@@ -589,7 +587,6 @@ class ScriptInterpreter : public PluginInterface {
protected:
Debugger &m_debugger;
lldb::ScriptLanguage m_script_lang;
- lldb::ScriptedProcessInterfaceUP m_scripted_process_interface_up;
lldb::ScriptedPlatformInterfaceUP m_scripted_platform_interface_up;
};
diff --git a/lldb/include/lldb/Interpreter/ScriptedInterface.h b/lldb/include/lldb/Interpreter/ScriptedInterface.h
index da4977af123da..31064de7b765f 100644
--- a/lldb/include/lldb/Interpreter/ScriptedInterface.h
+++ b/lldb/include/lldb/Interpreter/ScriptedInterface.h
@@ -30,6 +30,10 @@ class ScriptedInterface {
StructuredData::DictionarySP args_sp,
StructuredData::Generic *script_obj = nullptr) = 0;
+ StructuredData::GenericSP GetScriptObjectInstance() {
+ return m_object_instance_sp;
+ }
+
template <typename Ret>
static Ret ErrorWithMessage(llvm::StringRef caller_name,
llvm::StringRef error_msg, Status &error,
diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
index 2722666439bff..60b676d2a5cd0 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -29,10 +29,8 @@ using namespace lldb_private;
ScriptInterpreter::ScriptInterpreter(
Debugger &debugger, lldb::ScriptLanguage script_lang,
- lldb::ScriptedProcessInterfaceUP scripted_process_interface_up,
lldb::ScriptedPlatformInterfaceUP scripted_platform_interface_up)
: m_debugger(debugger), m_script_lang(script_lang),
- m_scripted_process_interface_up(std::move(scripted_process_interface_up)),
m_scripted_platform_interface_up(
std::move(scripted_platform_interface_up)) {}
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
index d7d87c793c4d1..06b35068b3849 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -47,11 +47,6 @@ bool ScriptedProcess::IsScriptLanguageSupported(lldb::ScriptLanguage language) {
return llvm::is_contained(supported_languages, language);
}
-void ScriptedProcess::CheckInterpreterAndScriptObject() const {
- lldbassert(m_interpreter && "Invalid Script Interpreter.");
- lldbassert(m_script_object_sp && "Invalid Script Object.");
-}
-
lldb::ProcessSP ScriptedProcess::CreateInstance(lldb::TargetSP target_sp,
lldb::ListenerSP listener_sp,
const FileSpec *file,
@@ -66,8 +61,7 @@ lldb::ProcessSP ScriptedProcess::CreateInstance(lldb::TargetSP target_sp,
auto process_sp = std::shared_ptr<ScriptedProcess>(
new ScriptedProcess(target_sp, listener_sp, scripted_metadata, error));
- if (error.Fail() || !process_sp || !process_sp->m_script_object_sp ||
- !process_sp->m_script_object_sp->IsValid()) {
+ if (error.Fail() || !process_sp || !process_sp->m_interface_up) {
LLDB_LOGF(GetLog(LLDBLog::Process), "%s", error.AsCString());
return nullptr;
}
@@ -92,17 +86,28 @@ ScriptedProcess::ScriptedProcess(lldb::TargetSP target_sp,
return;
}
- m_interpreter = target_sp->GetDebugger().GetScriptInterpreter();
+ ScriptInterpreter *interpreter =
+ target_sp->GetDebugger().GetScriptInterpreter();
- if (!m_interpreter) {
+ if (!interpreter) {
error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s",
__FUNCTION__,
"Debugger has no Script Interpreter");
return;
}
+ // Create process instance interface
+ m_interface_up = interpreter->CreateScriptedProcessInterface();
+ if (!m_interface_up) {
+ error.SetErrorStringWithFormat(
+ "ScriptedProcess::%s () - ERROR: %s", __FUNCTION__,
+ "Script interpreter couldn't create Scripted Process Interface");
+ return;
+ }
+
ExecutionContext exe_ctx(target_sp, /*get_process=*/false);
+ // Create process script object
StructuredData::GenericSP object_sp = GetInterface().CreatePluginObject(
m_scripted_metadata.GetClassName(), exe_ctx,
m_scripted_metadata.GetArgsSP());
@@ -113,8 +118,6 @@ ScriptedProcess::ScriptedProcess(lldb::TargetSP target_sp,
"Failed to create valid script object");
return;
}
-
- m_script_object_sp = object_sp;
}
ScriptedProcess::~ScriptedProcess() {
@@ -147,8 +150,6 @@ Status ScriptedProcess::DoLoadCore() {
Status ScriptedProcess::DoLaunch(Module *exe_module,
ProcessLaunchInfo &launch_info) {
- CheckInterpreterAndScriptObject();
-
/* FIXME: This doesn't reflect how lldb actually launches a process.
In reality, it attaches to debugserver, then resume the process. */
Status error = GetInterface().Launch();
@@ -168,14 +169,11 @@ Status ScriptedProcess::DoLaunch(Module *exe_module,
}
void ScriptedProcess::DidLaunch() {
- CheckInterpreterAndScriptObject();
m_pid = GetInterface().GetProcessID();
GetLoadedDynamicLibrariesInfos();
}
Status ScriptedProcess::DoResume() {
- CheckInterpreterAndScriptObject();
-
Log *log = GetLog(LLDBLog::Process);
// FIXME: Fetch data from thread.
const StateType thread_resume_state = eStateRunning;
@@ -198,8 +196,6 @@ Status ScriptedProcess::DoResume() {
}
Status ScriptedProcess::DoStop() {
- CheckInterpreterAndScriptObject();
-
Log *log = GetLog(LLDBLog::Process);
if (GetInterface().ShouldStop()) {
@@ -214,18 +210,10 @@ Status ScriptedProcess::DoStop() {
Status ScriptedProcess::DoDestroy() { return Status(); }
-bool ScriptedProcess::IsAlive() {
- if (m_interpreter && m_script_object_sp)
- return GetInterface().IsAlive();
- return false;
-}
+bool ScriptedProcess::IsAlive() { return GetInterface().IsAlive(); }
size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
Status &error) {
- if (!m_interpreter)
- return ScriptedInterface::ErrorWithMessage<size_t>(
- LLVM_PRETTY_FUNCTION, "No interpreter.", error);
-
lldb::DataExtractorSP data_extractor_sp =
GetInterface().ReadMemoryAtAddress(addr, size, error);
@@ -248,8 +236,6 @@ ArchSpec ScriptedProcess::GetArchitecture() {
Status ScriptedProcess::DoGetMemoryRegionInfo(lldb::addr_t load_addr,
MemoryRegionInfo ®ion) {
- CheckInterpreterAndScriptObject();
-
Status error;
if (auto region_or_err =
GetInterface().GetMemoryRegionContainingAddress(load_addr, error))
@@ -259,8 +245,6 @@ Status ScriptedProcess::DoGetMemoryRegionInfo(lldb::addr_t load_addr,
}
Status ScriptedProcess::GetMemoryRegions(MemoryRegionInfos ®ion_list) {
- CheckInterpreterAndScriptObject();
-
Status error;
lldb::addr_t address = 0;
@@ -286,22 +270,9 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list,
// This is supposed to get the current set of threads, if any of them are in
// old_thread_list then they get copied to new_thread_list, and then any
// actually new threads will get added to new_thread_list.
-
- CheckInterpreterAndScriptObject();
m_thread_plans.ClearThreadCache();
Status error;
- ScriptLanguage language = m_interpreter->GetLanguage();
-
- if (language != eScriptLanguagePython)
- return ScriptedInterface::ErrorWithMessage<bool>(
- LLVM_PRETTY_FUNCTION,
- llvm::Twine("ScriptInterpreter language (" +
- llvm::Twine(m_interpreter->LanguageToString(language)) +
- llvm::Twine(") not supported."))
- .str(),
- error);
-
StructuredData::DictionarySP thread_info_sp = GetInterface().GetThreadsInfo();
if (!thread_info_sp)
@@ -400,8 +371,6 @@ bool ScriptedProcess::GetProcessInfo(ProcessInstanceInfo &info) {
lldb_private::StructuredData::ObjectSP
ScriptedProcess::GetLoadedDynamicLibrariesInfos() {
- CheckInterpreterAndScriptObject();
-
Status error;
auto error_with_message = [&error](llvm::StringRef message) {
return ScriptedInterface::ErrorWithMessage<bool>(LLVM_PRETTY_FUNCTION,
@@ -486,8 +455,6 @@ ScriptedProcess::GetLoadedDynamicLibrariesInfos() {
}
lldb_private::StructuredData::DictionarySP ScriptedProcess::GetMetadata() {
- CheckInterpreterAndScriptObject();
-
StructuredData::DictionarySP metadata_sp = GetInterface().GetMetadata();
Status error;
@@ -499,7 +466,7 @@ lldb_private::StructuredData::DictionarySP ScriptedProcess::GetMetadata() {
}
void ScriptedProcess::UpdateQueueListIfNeeded() {
- CheckInterpreterAndScriptObject();
+ CheckScriptedInterface();
for (ThreadSP thread_sp : Threads()) {
if (const char *queue_name = thread_sp->GetQueueName()) {
QueueSP queue_sp = std::make_shared<Queue>(
@@ -510,12 +477,15 @@ void ScriptedProcess::UpdateQueueListIfNeeded() {
}
ScriptedProcessInterface &ScriptedProcess::GetInterface() const {
- return m_interpreter->GetScriptedProcessInterface();
+ CheckScriptedInterface();
+ return *m_interface_up;
}
void *ScriptedProcess::GetImplementation() {
- if (m_script_object_sp &&
- m_script_object_sp->GetType() == eStructuredDataTypeGeneric)
- return m_script_object_sp->GetAsGeneric()->GetValue();
+ StructuredData::GenericSP object_instance_sp =
+ GetInterface().GetScriptObjectInstance();
+ if (object_instance_sp &&
+ object_instance_sp->GetType() == eStructuredDataTypeGeneric)
+ return object_instance_sp->GetAsGeneric()->GetValue();
return nullptr;
}
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
index bf283d5bce5da..29676cace33b6 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -93,15 +93,16 @@ class ScriptedProcess : public Process {
private:
friend class ScriptedThread;
- void CheckInterpreterAndScriptObject() const;
+ inline void CheckScriptedInterface() const {
+ lldbassert(m_interface_up && "Invalid scripted process interface.");
+ }
+
ScriptedProcessInterface &GetInterface() const;
static bool IsScriptLanguageSupported(lldb::ScriptLanguage language);
// Member variables.
const ScriptedMetadata m_scripted_metadata;
- lldb_private::ScriptInterpreter *m_interpreter = nullptr;
- lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
- //@}
+ lldb::ScriptedProcessInterfaceUP m_interface_up;
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index ad0d26af88798..6877e9a14aa77 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -35,7 +35,7 @@ ScriptedThread::Create(ScriptedProcess &process,
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Invalid scripted process.");
- process.CheckInterpreterAndScriptObject();
+ process.CheckScriptedInterface();
auto scripted_thread_interface =
process.GetInterface().CreateScriptedThreadInterface();
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 7026815e120d2..3c0aa29071968 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -412,8 +412,6 @@ ScriptInterpreterPythonImpl::ScriptInterpreterPythonImpl(Debugger &debugger)
m_active_io_handler(eIOHandlerNone), m_session_is_active(false),
m_pty_secondary_is_open(false), m_valid_session(true), m_lock_count(0),
m_command_thread_state(nullptr) {
- m_scripted_process_interface_up =
- std::make_unique<ScriptedProcessPythonInterface>(*this);
m_scripted_platform_interface_up =
std::make_unique<ScriptedPlatformPythonInterface>(*this);
@@ -1484,6 +1482,11 @@ lldb::ValueObjectListSP ScriptInterpreterPythonImpl::GetRecognizedArguments(
return ValueObjectListSP();
}
+ScriptedProcessInterfaceUP
+ScriptInterpreterPythonImpl::CreateScriptedProcessInterface() {
+ return std::make_unique<ScriptedProcessPythonInterface>(*this);
+}
+
StructuredData::GenericSP
ScriptInterpreterPythonImpl::OSPlugin_CreatePluginObject(
const char *class_name, lldb::ProcessSP process_sp) {
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
index f4875bfb8d18a..98a05d816b20d 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -124,6 +124,8 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
GetRecognizedArguments(const StructuredData::ObjectSP &implementor,
lldb::StackFrameSP frame_sp) override;
+ lldb::ScriptedProcessInterfaceUP CreateScriptedProcessInterface() override;
+
StructuredData::GenericSP
OSPlugin_CreatePluginObject(const char *class_name,
lldb::ProcessSP process_sp) override;
diff --git a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
index 50ef5ffadd81a..1078db8ad52a4 100644
--- a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -98,8 +98,8 @@ def test_scripted_process_and_scripted_thread(self):
id, name stop reason and register context.
"""
self.build()
- target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
- self.assertTrue(target, VALID_TARGET)
+ target_0 = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+ self.assertTrue(target_0, VALID_TARGET)
os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
def cleanup():
@@ -115,12 +115,12 @@ def cleanup():
launch_info.SetScriptedProcessClassName("dummy_scripted_process.DummyScriptedProcess")
error = lldb.SBError()
- process = target.Launch(launch_info, error)
- self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
- self.assertEqual(process.GetProcessID(), 42)
- self.assertEqual(process.GetNumThreads(), 1)
+ process_0 = target_0.Launch(launch_info, error)
+ self.assertTrue(process_0 and process_0.IsValid(), PROCESS_IS_VALID)
+ self.assertEqual(process_0.GetProcessID(), 42)
+ self.assertEqual(process_0.GetNumThreads(), 1)
- py_impl = process.GetScriptedImplementation()
+ py_impl = process_0.GetScriptedImplementation()
self.assertTrue(py_impl)
self.assertTrue(isinstance(py_impl, dummy_scripted_process.DummyScriptedProcess))
self.assertFalse(hasattr(py_impl, 'my_super_secret_member'))
@@ -128,13 +128,37 @@ def cleanup():
self.assertTrue(hasattr(py_impl, 'my_super_secret_member'))
self.assertEqual(py_impl.my_super_secret_method(), 42)
+ # Try reading from target #0 process ...
+ addr = 0x500000000
+ message = "Hello, target 0"
+ buff = process_0.ReadCStringFromMemory(addr, len(message) + 1, error)
+ self.assertSuccess(error)
+ self.assertEqual(buff, message)
+
+ target_1 = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+ self.assertTrue(target_1, VALID_TARGET)
+ error = lldb.SBError()
+ process_1 = target_1.Launch(launch_info, error)
+ self.assertTrue(process_1 and process_1.IsValid(), PROCESS_IS_VALID)
+ self.assertEqual(process_1.GetProcessID(), 42)
+ self.assertEqual(process_1.GetNumThreads(), 1)
+
+ # ... then try reading from target #1 process ...
+ addr = 0x500000000
+ message = "Hello, target 1"
+ buff = process_1.ReadCStringFromMemory(addr, len(message) + 1, error)
+ self.assertSuccess(error)
+ self.assertEqual(buff, message)
+
+ # ... now, reading again from target #0 process to make sure the call
+ # gets dispatched to the right target.
addr = 0x500000000
- message = "Hello, world!"
- buff = process.ReadCStringFromMemory(addr, len(message) + 1, error)
+ message = "Hello, target 0"
+ buff = process_0.ReadCStringFromMemory(addr, len(message) + 1, error)
self.assertSuccess(error)
self.assertEqual(buff, message)
- thread = process.GetSelectedThread()
+ thread = process_0.GetSelectedThread()
self.assertTrue(thread, "Invalid thread.")
self.assertEqual(thread.GetThreadID(), 0x19)
self.assertEqual(thread.GetName(), "DummyScriptedThread.thread-1")
@@ -158,5 +182,5 @@ def cleanup():
self.assertEqual(idx, int(reg.value, 16))
self.assertTrue(frame.IsArtificial(), "Frame is not artificial")
- pc = frame.GetPCAddress().GetLoadAddress(target)
+ pc = frame.GetPCAddress().GetLoadAddress(target_0)
self.assertEqual(pc, 0x0100001b00)
diff --git a/lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py b/lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
index 05dfa17f3bbbc..94664114430ec 100644
--- a/lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ b/lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -21,10 +21,12 @@ def get_registers_for_thread(self, tid: int):
return {}
def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+ debugger = self.target.GetDebugger()
+ index = debugger.GetIndexOfTarget(self.target)
data = lldb.SBData().CreateDataFromCString(
self.target.GetByteOrder(),
self.target.GetCodeByteSize(),
- "Hello, world!")
+ "Hello, target " + str(index))
return data
More information about the lldb-commits
mailing list