[lldb/Target] Delay image loading after corefile process creation

This patch is a follow-up to db223b7. Similarly to it, it changes
the timing of binary image loading for the ProcessMachCore plugin.

This issue came up after getting reports of scripting resources that
would fail to execute because they relied on data provided by the corefile
process (i.e. for reading memory). However, rior to this change, the
scripting resource loading would happen as part of the binary image
loading, which in turns happened before the process finished being created.

This patch address that issue by delaying the binary image loading phase
until we receive the corefile process stop event event, ensuring that the
process is fully formed.

Signed-off-by: Med Ismail Bennani <[email protected]>

Author: medismailben

lldb/include/lldb/Target/Process.h

@@ -614,6 +614,8 @@ class Process : public std::enable_shared_from_this<Process>,
     return error;
   }
 
+  virtual void DidLoadCore() {}
+
   /// The "ShadowListener" for a process is just an ordinary Listener that 
   /// listens for all the Process event bits.  It's convenient because you can
   /// specify it in the LaunchInfo or AttachInfo, so it will get events from

lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp

@@ -570,8 +570,6 @@ Status ProcessMachCore::DoLoadCore() {
 
   CreateMemoryRegions();
 
-  LoadBinariesAndSetDYLD();
-
   CleanupMemoryRegionPermissions();
 
   AddressableBits addressable_bits = core_objfile->GetAddressableBits();
@@ -580,6 +578,8 @@ Status ProcessMachCore::DoLoadCore() {
   return error;
 }
 
+void ProcessMachCore::DidLoadCore() { LoadBinariesAndSetDYLD(); }
+
 lldb_private::DynamicLoader *ProcessMachCore::GetDynamicLoader() {
   if (m_dyld_up.get() == nullptr)
     m_dyld_up.reset(DynamicLoader::FindPlugin(this, m_dyld_plugin_name));

lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

@@ -46,6 +46,8 @@ class ProcessMachCore : public lldb_private::PostMortemProcess {
   // Creating a new process, or attaching to an existing one
   lldb_private::Status DoLoadCore() override;
 
+  void DidLoadCore() override;
+
   lldb_private::DynamicLoader *GetDynamicLoader() override;
 
   // PluginInterface protocol

lldb/source/Target/Process.cpp

@@ -2639,19 +2639,6 @@ Status Process::LoadCore() {
     else
       StartPrivateStateThread();
 
-    DynamicLoader *dyld = GetDynamicLoader();
-    if (dyld)
-      dyld->DidAttach();
-
-    GetJITLoaders().DidAttach();
-
-    SystemRuntime *system_runtime = GetSystemRuntime();
-    if (system_runtime)
-      system_runtime->DidAttach();
-
-    if (!m_os_up)
-      LoadOperatingSystemPlugin(false);
-
     // We successfully loaded a core file, now pretend we stopped so we can
     // show all of the threads in the core file and explore the crashed state.
     SetPrivateState(eStateStopped);
@@ -2668,7 +2655,23 @@ Status Process::LoadCore() {
                 StateAsCString(state));
       error.SetErrorString(
           "Did not get stopped event after loading the core file.");
+    } else {
+      DidLoadCore();
+
+      DynamicLoader *dyld = GetDynamicLoader();
+      if (dyld)
+        dyld->DidAttach();
+
+      GetJITLoaders().DidAttach();
+
+      SystemRuntime *system_runtime = GetSystemRuntime();
+      if (system_runtime)
+        system_runtime->DidAttach();
+
+      if (!m_os_up)
+        LoadOperatingSystemPlugin(false);
     }
+
     RestoreProcessEvents();
   }
   return error;

lldb/test/API/functionalities/script-resource-loading/Makefile

@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp
+
+override ARCH := $(shell uname -m)
+
+include Makefile.rules

lldb/test/API/functionalities/script-resource-loading/TestScriptResourceLoading.py

@@ -0,0 +1,63 @@
+"""
+Test loading python scripting resource from corefile
+"""
+
+import os, tempfile
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbtest
+
+
+class ScriptResourceLoadingTestCase(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def create_stack_skinny_corefile(self, file):
+        self.build()
+        target, process, thread, _ = lldbutil.run_to_source_breakpoint(
+            self, "// break", lldb.SBFileSpec("main.cpp")
+        )
+        self.assertTrue(process.IsValid(), "Process is invalid.")
+        # FIXME: Use SBAPI to save the process corefile.
+        self.runCmd("process save-core -s stack  " + file)
+        self.assertTrue(os.path.exists(file), "No stack-only corefile found.")
+        self.assertTrue(self.dbg.DeleteTarget(target), "Couldn't delete target")
+
+    def move_blueprint_to_dsym(self, blueprint_name):
+        blueprint_origin_path = os.path.join(self.getSourceDir(), blueprint_name)
+        dsym_bundle = self.getBuildArtifact("a.out.dSYM")
+        blueprint_destination_path = os.path.join(
+            dsym_bundle, "Contents", "Resources", "Python"
+        )
+        if not os.path.exists(blueprint_destination_path):
+            os.mkdir(blueprint_destination_path)
+
+        blueprint_destination_path = os.path.join(
+            blueprint_destination_path, "a_out.py"
+        )
+        shutil.copy(blueprint_origin_path, blueprint_destination_path)
+
+    @skipUnlessDarwin
+    def test_script_resource_loading(self):
+        """
+        Test that we're able to load the python scripting resource from
+        corefile dSYM bundle.
+
+        """
+        self.build()
+
+        self.runCmd("settings set target.load-script-from-symbol-file true")
+        self.move_blueprint_to_dsym("my_scripting_resource.py")
+
+        corefile_process = None
+        with tempfile.NamedTemporaryFile() as file:
+            self.create_stack_skinny_corefile(file.name)
+            corefile_target = self.dbg.CreateTarget(None)
+            corefile_process = corefile_target.LoadCore(
+                self.getBuildArtifact(file.name)
+            )
+        self.assertTrue(corefile_process, PROCESS_IS_VALID)
+        self.expect("command script list", substrs=["test_script_resource_loading"])
+        self.runCmd("test_script_resource_loading")

lldb/test/API/functionalities/script-resource-loading/main.cpp

@@ -0,0 +1,5 @@
+int foo() {
+  return 42; // break
+}
+
+int main() { return foo(); }

lldb/test/API/functionalities/script-resource-loading/my_scripting_resource.py

@@ -0,0 +1,15 @@
+import sys, lldb
+
+
+def test_script_resource_loading(debugger, command, exe_ctx, result, dict):
+    if not exe_ctx.target.process.IsValid():
+        result.SetError("invalid process")
+    process = exe_ctx.target.process
+    if not len(process):
+        result.SetError("invalid thread count")
+
+
+def __lldb_init_module(debugger, dict):
+    debugger.HandleCommand(
+        "command script add -o -f a_out.test_script_resource_loading test_script_resource_loading"
+    )