Fix ExceptionInfo() for an EXC_RESOURCE recovered from EXC_CRASH [crashpad/crashpad : main]
ZhengHao (Gerrit)
ZhengHao added 1 comment![]( "Open in Gerrit")
Hi, this fixes ExceptionSnapshotMac::Initialize() applying the EXC_RESOURCE/EXC_GUARD high-bit extraction to a code[0] recovered from an EXC_CRASH wrapper. That value retains only its low 20 bits, so the shift always yields 0 and ExceptionInfo() reports no payload.
Observed on macOS 26.2 where the kernel wrapped an EXC_RESOURCE in EXC_CRASH after a resource-limit SIGKILL. ExcCrashCouldContainException() already rejects the pairing but Initialize() only logs and continues. Added the observed code[0] to the existing test.
Related details
- Mark Mentovai
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mark Mentovai added 4 comments![]( "Open in Gerrit")
I think when I
Bug: 563877890You don’t need Bug: if you’re putting the same bug in Fixed:.
if (!recovered_from_exc_crash &&
(exception_ == EXC_RESOURCE || exception_ == EXC_GUARD)) {When I wrote this, `EXC_RESOURCE` and `EXC_GUARD` wouldn’t be wrapped in `EXC_CRASH`, so we had to handle them distinctly.
If that’s changed, it might be possible to simplify things considerably. For example, we can avoid registering as an `EXC_RESOURCE` handler, get rid of things like `crashpad::IsExceptionNonfatalResource`, and just take fatal resource exceptions wrapped in `EXC_CRASH`. We may need to suppress ReportCrash from claiming `EXC_RESOURCE` at the task level, such as by clearing it, as ReportCrash is a task-default `EXC_RESOURCE` handler.
Of course, doing so, you’d be limiting code[0] to the lower 20 bits, which is what your new comment states. And maybe that’s not enough for these exceptions.
Do you have thoughts?
// EXC_CRASH when a process is SIGKILLed for a resource limit, even though
// ExcCrashCouldContainException() rejects that pairing. Only the low 20`crashpad::ExcCrashCouldContainException` should be fixed.
In order to do so, you’ll find out where this changed by examining xnu’s change history, because the fix may wind up being OS version-specific.
Related details
- ZhengHao
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
ZhengHao (Gerrit)
ZhengHao added 4 comments![]( "Open in Gerrit")
You don’t need Bug: if you’re putting the same bug in Fixed:.
Done
You don’t need Bug: if you’re putting the same bug in Fixed:.
Done
if (!recovered_from_exc_crash &&
(exception_ == EXC_RESOURCE || exception_ == EXC_GUARD)) {When I wrote this, `EXC_RESOURCE` and `EXC_GUARD` wouldn’t be wrapped in `EXC_CRASH`, so we had to handle them distinctly.
If that’s changed, it might be possible to simplify things considerably. For example, we can avoid registering as an `EXC_RESOURCE` handler, get rid of things like `crashpad::IsExceptionNonfatalResource`, and just take fatal resource exceptions wrapped in `EXC_CRASH`. We may need to suppress ReportCrash from claiming `EXC_RESOURCE` at the task level, such as by clearing it, as ReportCrash is a task-default `EXC_RESOURCE` handler.
Of course, doing so, you’d be limiting code[0] to the lower 20 bits, which is what your new comment states. And maybe that’s not enough for these exceptions.
Do you have thoughts?
I'd lean against dropping it. Removing the handler doesn't recover the lost bits — a recovered code[0] only carries 20 bits — and it would lose the native 64-bit decoding path for direct non-fatal EXC_RESOURCE deliveries. Reading the real type from the corpse's crashinfo is a separate change.
// EXC_CRASH when a process is SIGKILLed for a resource limit, even though
// ExcCrashCouldContainException() rejects that pairing. Only the low 20`crashpad::ExcCrashCouldContainException` should be fixed.
In order to do so, you’ll find out where this changed by examining xnu’s change history, because the fix may wind up being OS version-specific.
Done. I checked proc_prepareexit() from xnu-1504 (macOS 10.7) through xnu-12377 (macOS 26.2): the packing and EXC_CRASH delivery are unchanged, and the kernel has always delivered a resource-limit SIGKILL this way. No version gate is needed. The EXC_GUARD < 13 gate stays.
Related details
- Mark Mentovai
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mark Mentovai added 1 comment![]( "Open in Gerrit")
if (!recovered_from_exc_crash &&
(exception_ == EXC_RESOURCE || exception_ == EXC_GUARD)) {ZhengHaoWhen I wrote this, `EXC_RESOURCE` and `EXC_GUARD` wouldn’t be wrapped in `EXC_CRASH`, so we had to handle them distinctly.
If that’s changed, it might be possible to simplify things considerably. For example, we can avoid registering as an `EXC_RESOURCE` handler, get rid of things like `crashpad::IsExceptionNonfatalResource`, and just take fatal resource exceptions wrapped in `EXC_CRASH`. We may need to suppress ReportCrash from claiming `EXC_RESOURCE` at the task level, such as by clearing it, as ReportCrash is a task-default `EXC_RESOURCE` handler.
Of course, doing so, you’d be limiting code[0] to the lower 20 bits, which is what your new comment states. And maybe that’s not enough for these exceptions.
Do you have thoughts?
I'd lean against dropping it. Removing the handler doesn't recover the lost bits — a recovered code[0] only carries 20 bits — and it would lose the native 64-bit decoding path for direct non-fatal EXC_RESOURCE deliveries. Reading the real type from the corpse's crashinfo is a separate change.
If we’re leaving it, I’ve got to ask:
Since the kernel can wrap `EXC_RESOURCE` and `EXC_GUARD` in `EXC_CRASH`, and since we’re registering as the handler for all three, is it possible that we’ll receive the same exception _twice_, once unwrapped and then again wrapped?
Related details
- ZhengHao
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
ZhengHao (Gerrit)
ZhengHao added 1 comment![]( "Open in Gerrit")
if (!recovered_from_exc_crash &&
(exception_ == EXC_RESOURCE || exception_ == EXC_GUARD)) {ZhengHaoWhen I wrote this, `EXC_RESOURCE` and `EXC_GUARD` wouldn’t be wrapped in `EXC_CRASH`, so we had to handle them distinctly.
If that’s changed, it might be possible to simplify things considerably. For example, we can avoid registering as an `EXC_RESOURCE` handler, get rid of things like `crashpad::IsExceptionNonfatalResource`, and just take fatal resource exceptions wrapped in `EXC_CRASH`. We may need to suppress ReportCrash from claiming `EXC_RESOURCE` at the task level, such as by clearing it, as ReportCrash is a task-default `EXC_RESOURCE` handler.
Of course, doing so, you’d be limiting code[0] to the lower 20 bits, which is what your new comment states. And maybe that’s not enough for these exceptions.
Do you have thoughts?
Mark MentovaiI'd lean against dropping it. Removing the handler doesn't recover the lost bits — a recovered code[0] only carries 20 bits — and it would lose the native 64-bit decoding path for direct non-fatal EXC_RESOURCE deliveries. Reading the real type from the corpse's crashinfo is a separate change.
If we’re leaving it, I’ve got to ask:
Since the kernel can wrap `EXC_RESOURCE` and `EXC_GUARD` in `EXC_CRASH`, and since we’re registering as the handler for all three, is it possible that we’ll receive the same exception _twice_, once unwrapped and then again wrapped?
No. The direct EXC_RESOURCE deliveries are non-fatal warnings that IsExceptionNonfatalResource() swallows without generating a report. The fatal kill goes through SIGKILL → proc_prepareexit() → EXC_CRASH, which is a separate delivery path.
Related details
- Mark Mentovai
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Mark Mentovai voted Code-Review+1![]( "Open in Gerrit")
- ZhengHao
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hewro Hewei (Gerrit)
Hewro Hewei voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- ZhengHao
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hewro Hewei (Gerrit)
Hewro Hewei voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
- ZhengHao
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
crashpad-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
crashpa...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
Fix ExceptionInfo() for an EXC_RESOURCE recovered from EXC_CRASH
When Initialize() recovers the original exception from an EXC_CRASH
wrapper, it still applies the EXC_RESOURCE/EXC_GUARD high-bit extraction
to code[0]. proc_prepareexit() packs only the low 20 bits of the
original code[0] into the wrapper, so the shift always yields 0 and
ExceptionInfo() reports nothing.
Skip the extraction for a recovered code[0], stop excluding EXC_RESOURCE
in ExcCrashCouldContainException(), and add the observed code[0]
0x9b4aaaa to the test data.
- M snapshot/mac/exception_snapshot_mac.cc
- M util/mach/exception_types.cc
- M util/mach/exception_types_test.cc
Code-Review: +1 by Mark Mentovai, +1 by Hewro Hewei
Review-Enforcement: Code-Review+1 by Mark Mentovai, Code-Review+1 by Hewro Hewei
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |