jasonmolenda created this revision.
jasonmolenda added a reviewer: JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

In  https://reviews.llvm.org/D133680 I changed a lot of how ProcessMachCore 
scans a corefile to figure out what kind of debug session this is (kernel, 
firmware, userland, etc) and where the binaries are.   
`ProcessMachCore::LoadBinariesViaMetadata()` calls 
`ObjectFile::LoadCoreFileImages()`, and under there, it is possible that the 
Process DynamicLoader will be set in the process of its scans.  Then back in 
`LoadBinariesViaMetadata()` it would check to see if the dynamic loader had 
been set.  It would do this by calling `ProcessMachCore::GetDynamicLoader()`, 
but this method, if `m_dyld_up` is empty, will call the plugin interface 
`FindPlugin` method with the minimal amount of information we have so far --- 
and pick the wrong dynamic loader plugin for a userland process.

This broke userland corefile loading in lldb; no the DynamicLoaderMacOSX plugin 
is not loaded, so we don't load any of the binaries in the corfile.  It's not a 
very commonly used type of corefile on Darwin systems, so it slipped by.  The 
corefiles created by lldb's `process save-core` include a lot of metadata which 
make this work correctly for them, so it didn't get caught by the testsuite.

The fix is to change the call to `GetDynamicLoader()` which can *set* the 
dynamic loader, and instead look at the ivar.  This is the only call to this 
method inside ProcessMachCore's methods.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137807

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


Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -327,9 +327,11 @@
   // corefile
   core_objfile->LoadCoreFileImages(*this);
 
-  // LoadCoreFileImges may have set the dynamic loader; if we now have
-  // a dynamic loader, save its name so we don't un-set it later.
-  if (GetDynamicLoader())
+  // LoadCoreFileImges may have set the dynamic loader, e.g. in
+  // PlatformDarwinKernel::LoadPlatformBinaryAndSetup().
+  // If we now have a dynamic loader, save its name so we don't 
+  // un-set it later.
+  if (m_dyld_up.get())
     m_dyld_plugin_name = GetDynamicLoader()->GetPluginName();
 }
 


Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -327,9 +327,11 @@
   // corefile
   core_objfile->LoadCoreFileImages(*this);
 
-  // LoadCoreFileImges may have set the dynamic loader; if we now have
-  // a dynamic loader, save its name so we don't un-set it later.
-  if (GetDynamicLoader())
+  // LoadCoreFileImges may have set the dynamic loader, e.g. in
+  // PlatformDarwinKernel::LoadPlatformBinaryAndSetup().
+  // If we now have a dynamic loader, save its name so we don't 
+  // un-set it later.
+  if (m_dyld_up.get())
     m_dyld_plugin_name = GetDynamicLoader()->GetPluginName();
 }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to