[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)

[Jason Molenda via lldb-commits]

================
@@ -578,20 +605,47 @@ Status ProcessMachCore::DoLoadCore() {
 
   SetCanJIT(false);
 
+  // If we have an executable binary in the Target already,
+  // use that to set the Target's ArchSpec.
+  //
+  // Don't initialize the ArchSpec based on the corefile's cputype/cpusubtype
+  // here, the corefile creator may not know the correct subtype of the code
+  // that is executing, initialize the Target to that, and if the
+  // main binary has Python code which initializes based on the Target arch,
+  // get the wrong subtype value.
+  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+    LLDB_LOGF(log,
+              "ProcessMachCore::%s: Was given binary + corefile, setting "
+              "target ArchSpec to binary to start",
+              __FUNCTION__);
+    GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
+  }
+
   CreateMemoryRegions();
 
   LoadBinariesAndSetDYLD();
 
   CleanupMemoryRegionPermissions();
 
-  ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
+  exe_module_sp = GetTarget().GetExecutableModule();
----------------
jasonmolenda wrote:

Before we call `LoadBinariesAndSetDYLD()`, the only way we have an executable module is if the user provided it on the commandline along with the corefile.  After that method call, we may have found it by metadata in the corefile or by an exhaustive search for a properly-aligned dyld or kernel.  I'm open to having this in a method, but it's also a tiny method "if we have an executable binary, set the Target's arch to that ArchSpec" - the logging I added here is the largest part.  

The real opportunity for reducing a copy of code would be in ProcessMachCore::LoadBinariesViaExhaustiveSearch where it does the "if we have an executable module, set arch, else use the corefile's arch" but we already know we don't have an executable module.  We checked it before we got here in DoLoadCore, and if we'd added an executable module at this point, we wouldn't be doing the exhaustive search.  This bit could be reduced to simply "if target has no arch, set it to the corefile's arch", I thought of that last night.

https://github.com/llvm/llvm-project/pull/136065