[Lldb-commits] [lldb] [lldb][Mach-O corefiles] Don't init Target arch to corefile (PR #136065)
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 17 14:09:36 PDT 2025
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/136065
>From 41274f7632d13ab7ec83a420c6ad6258e6fe8c36 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Wed, 16 Apr 2025 17:18:02 -0700
Subject: [PATCH 1/3] [lldb][Mach-O corefiles] Don't init Target arch to
corefile
This patch is making three changes, when loading a Mach-O
corefile:
1. At the start of `DoLoadCore`, if a binary was provided in addition
to the corefile, initialize the Target's ArchSpec.
2. Before ProcessMachCore does its "exhaustive search" fallback,
looking through the corefile contents for a userland dyld or mach
kernel, we must make sure the Target has an ArchSpec, or methods
that check the address word size, or initialize a DataExtractor
based on the Target arch will not succeed.
3. Add logging when setting the Target's arch listing exactly what
that setting was based on -- the corefile itself, or the main binary.
Jonas landed a change last August (started with a patch from me)
which removed the Target ArchSpec initialization at the start of
DoLoadCore, in a scenario where the corefile had arch armv7 and
the main binary had arch armv7em (Cortex-M), and there was python
code in the main binary's dSYM which sets the operating system threads
provider based on the Target arch. It did different things for
armv7 or armv7em, and so it would fail.
Jonas' patch removed any ArchSpec setting at the start of DoLoadCore,
so we wouldn't have an incorrect arch value, but that broke the
exhaustive search for kernel binaries, because we didn't have an
address word size or endianness.
This patch should navigate the needs of both use cases.
I spent a good bit of time trying to construct a test to capture all
of these requirements -- but it turns out to be a good bit difficult,
encompassing both a genuine kernel corefiles and a microcontroller
firmware corefiles.
rdar://146821929
---
.../Process/mach-core/ProcessMachCore.cpp | 58 ++++++++++++++++++-
1 file changed, 56 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 281f3a0db8f69..17362b2d2855d 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -426,6 +426,32 @@ void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
std::vector<addr_t> dylds_found;
std::vector<addr_t> kernels_found;
+ // To do an exhaustive search, we'll need to create data extractors
+ // to get correctly sized/endianness fields, and if the Target still
+ // doesn't have an architecture set, we need to seed it from either
+ // the main binary (if we were given one) or the corefile cputype/cpusubtype.
+ if (!GetTarget().GetArchitecture().IsValid()) {
+ Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
+ 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());
+ } else {
+ // The corefile's architecture is our best starting point.
+ ArchSpec arch(m_core_module_sp->GetArchitecture());
+ if (arch.IsValid()) {
+ LLDB_LOGF(log,
+ "ProcessMachCore::%s: Setting target ArchSpec based on "
+ "corefile mach-o cputype/cpusubtype",
+ __FUNCTION__);
+ GetTarget().SetArchitecture(arch);
+ }
+ }
+ }
+
const size_t num_core_aranges = m_core_aranges.GetSize();
for (size_t i = 0; i < num_core_aranges; ++i) {
const VMRangeToFileOffset::Entry *entry = m_core_aranges.GetEntryAtIndex(i);
@@ -569,6 +595,7 @@ Status ProcessMachCore::DoLoadCore() {
error = Status::FromErrorString("invalid core module");
return error;
}
+ Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
if (core_objfile == nullptr) {
@@ -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();
if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+ LLDB_LOGF(log,
+ "ProcessMachCore::%s: have executable binary in the Target "
+ "after metadata/scan. Setting Target's ArchSpec based on "
+ "that.",
+ __FUNCTION__);
GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
} else {
// The corefile's architecture is our best starting point.
ArchSpec arch(m_core_module_sp->GetArchitecture());
- if (arch.IsValid())
+ if (arch.IsValid()) {
+ LLDB_LOGF(log,
+ "ProcessMachCore::%s: Setting target ArchSpec based on "
+ "corefile mach-o cputype/cpusubtype",
+ __FUNCTION__);
GetTarget().SetArchitecture(arch);
+ }
}
AddressableBits addressable_bits = core_objfile->GetAddressableBits();
>From 6e1f6f01b4f992a3fcb43417ae2037b5940a320a Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Thu, 17 Apr 2025 10:12:59 -0700
Subject: [PATCH 2/3] Don't try to set the target arch to the exe module in the
ExhaustiveSearch - we would have done that already in DoLoadCore if it was
possible. Use the less-preferable corefile binary's cputype/cpusubtype to
seed Target so we can search.
---
.../Process/mach-core/ProcessMachCore.cpp | 28 ++++++-------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 17362b2d2855d..4f57c9ade6520 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -427,28 +427,18 @@ void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
std::vector<addr_t> kernels_found;
// To do an exhaustive search, we'll need to create data extractors
- // to get correctly sized/endianness fields, and if the Target still
- // doesn't have an architecture set, we need to seed it from either
- // the main binary (if we were given one) or the corefile cputype/cpusubtype.
+ // to get correctly sized/endianness fields. If we had a main binary
+ // already, we would have set the Target to that - so here we'll use
+ // the corefile's cputype/cpusubtype as the best guess.
if (!GetTarget().GetArchitecture().IsValid()) {
- Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Target));
- ModuleSP exe_module_sp = GetTarget().GetExecutableModule();
- if (exe_module_sp && exe_module_sp->GetArchitecture().IsValid()) {
+ // The corefile's architecture is our best starting point.
+ ArchSpec arch(m_core_module_sp->GetArchitecture());
+ if (arch.IsValid()) {
LLDB_LOGF(log,
- "ProcessMachCore::%s: Was given binary + corefile, setting "
- "target ArchSpec to binary to start",
+ "ProcessMachCore::%s: Setting target ArchSpec based on "
+ "corefile mach-o cputype/cpusubtype",
__FUNCTION__);
- GetTarget().SetArchitecture(exe_module_sp->GetArchitecture());
- } else {
- // The corefile's architecture is our best starting point.
- ArchSpec arch(m_core_module_sp->GetArchitecture());
- if (arch.IsValid()) {
- LLDB_LOGF(log,
- "ProcessMachCore::%s: Setting target ArchSpec based on "
- "corefile mach-o cputype/cpusubtype",
- __FUNCTION__);
- GetTarget().SetArchitecture(arch);
- }
+ GetTarget().SetArchitecture(arch);
}
}
>From e7a9d478d8116e294420df6cbcb98d5ba08ec5a1 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Thu, 17 Apr 2025 14:09:20 -0700
Subject: [PATCH 3/3] rm sp
---
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 4f57c9ade6520..39584bad2d745 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -429,7 +429,7 @@ void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
// To do an exhaustive search, we'll need to create data extractors
// to get correctly sized/endianness fields. If we had a main binary
// already, we would have set the Target to that - so here we'll use
- // the corefile's cputype/cpusubtype as the best guess.
+ // the corefile's cputype/cpusubtype as the best guess.
if (!GetTarget().GetArchitecture().IsValid()) {
// The corefile's architecture is our best starting point.
ArchSpec arch(m_core_module_sp->GetArchitecture());
More information about the lldb-commits
mailing list