[lldb][Mach-O corefiles] Don't init Target arch to corefile (#136065) #10516
+46
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
) 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 (cherry picked from commit 1ab9e53)
|
@swift-ci test |
|
@swift-ci test windows |
JDevlieghere
approved these changes
Apr 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch is making three changes, when loading a Mach-O corefile:
At the start of
DoLoadCore, if a binary was provided in addition to the corefile, initialize the Target's ArchSpec.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.
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
(cherry picked from commit 1ab9e53)