[lldb] Fix assert in ScriptedProcess destructor

[medismailben]

This patch should fix a test failure in Expr/TestIRMemoryMapWindows.test:

https://lab.llvm.org/buildbot/#/builders/219/builds/6786

The problem here is that since 7991412 landed, all the ScriptInterpreter::CreateScripted*Interface now return a nullptr when using the base ScriptInterpreter instance, instead of ScriptInterpreterPython for instance.

This nullptr is actually well handled in the various places where we create a Scripted Interface, however, because of the way to instanciate a process, the process plugin manager have to iterate over every process plugin and call the CreateInstance static function that should instanciate the right object.

So in the ScriptedProcess case, because we are getting a nullptr when trying to create a ScriptedProcessInterface, we try to discard the process object, which calls the Process destructor, which in turns calls the ScriptedProcess plugin IsAlive method. That method will fire an assertion if the scripted interface pointer is not allocated.

This patch address that issue by setting a flag when destroying the ScriptedProcess object, and checks that flag when calling IsAlive.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

Changes

This patch should fix a test failure in Expr/TestIRMemoryMapWindows.test:

https://lab.llvm.org/buildbot/#/builders/219/builds/6786

The problem here is that since 7991412 landed, all the ScriptInterpreter::CreateScripted*Interface now return a nullptr when using the base ScriptInterpreter instance, instead of ScriptInterpreterPython for instance.

This nullptr is actually well handled in the various places where we create a Scripted Interface, however, because of the way to instanciate a process, the process plugin manager have to iterate over every process plugin and call the CreateInstance static function that should instanciate the right object.

So in the ScriptedProcess case, because we are getting a nullptr when trying to create a ScriptedProcessInterface, we try to discard the process object, which calls the Process destructor, which in turns calls the ScriptedProcess plugin IsAlive method. That method will fire an assertion if the scripted interface pointer is not allocated.

This patch address that issue by setting a flag when destroying the ScriptedProcess object, and checks that flag when calling IsAlive.

---

Full diff: https://github.com/llvm/llvm-project/pull/71744.diff

2 Files Affected:

• (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp (+4-1)
• (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.h (+1)

diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
index f2a647c1b0bea4c..11c681b957e7af8 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -129,6 +129,7 @@ ScriptedProcess::ScriptedProcess(lldb::TargetSP target_sp,
 }
 
 ScriptedProcess::~ScriptedProcess() {
+  m_is_finalizing = true;
   Clear();
   // We need to call finalize on the process before destroying ourselves to
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
@@ -212,7 +213,9 @@ void ScriptedProcess::DidAttach(ArchSpec &process_arch) {
 
 Status ScriptedProcess::DoDestroy() { return Status(); }
 
-bool ScriptedProcess::IsAlive() { return GetInterface().IsAlive(); }
+bool ScriptedProcess::IsAlive() {
+  return !m_is_finalizing && GetInterface().IsAlive();
+}
 
 size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                                      Status &error) {
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
index 0335364b4010b22..c4b4633a14c7abb 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -129,6 +129,7 @@ class ScriptedProcess : public Process {
   // Member variables.
   const ScriptedMetadata m_scripted_metadata;
   lldb::ScriptedProcessInterfaceUP m_interface_up;
+  bool m_is_finalizing = false;
 };
 
 } // namespace lldb_private

[bulbazord]

🚢

[DavidSpickett]

This did the job, https://lab.llvm.org/buildbot/#/builders/219/builds/6810, thanks.