Skip to content

Commit e0d8a0a

Browse files
markmentovaiCommit Bot
authored andcommitted
mac-arm64: Cope with signal handling quirks
On x86_64, it’s impossible for a signal handler distinguish between SIGBUS caused synchronously by a hardware fault and SIGBUS raised asynchronously by software. This remains true on arm64, and is expanded to include both SIGILL and SIGSEGV. Bug: crashpad:345 Test: crashpad_util_test Signals.Raise_HandlerReraisesTo* Change-Id: I181ea35121048dc0c666e2346340e698220ca650 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2386463 Commit-Queue: Mark Mentovai <mark@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org>
1 parent 5412beb commit e0d8a0a

File tree

2 files changed

+39
-28
lines changed

2 files changed

+39
-28
lines changed

util/posix/signals.cc

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -189,22 +189,25 @@ bool Signals::WillSignalReraiseAutonomously(const siginfo_t* siginfo) {
189189
// pointer), will not reoccur on their own when returning from the signal
190190
// handler.
191191
//
192-
// Unfortunately, on macOS, when SIGBUS is received asynchronously via kill(),
193-
// siginfo->si_code makes it appear as though it was actually received via a
194-
// hardware fault. See 10.12.3 xnu-3789.41.3/bsd/dev/i386/unix_signal.c
195-
// sendsig(). Asynchronous SIGBUS will not re-raise itself autonomously, but
196-
// this function (acting on information from the kernel) behaves as though it
197-
// will. This isn’t ideal, but asynchronous SIGBUS is an unexpected condition.
198-
// The alternative, to never treat SIGBUS as autonomously re-raising, is a bad
199-
// idea because the explicit re-raise would lose properties associated with
200-
// the the original signal, which are valuable for debugging and are visible
201-
// to a Mach exception handler. Since SIGBUS is normally received
202-
// synchronously in response to a hardware fault, don’t sweat the unexpected
203-
// asynchronous case.
192+
// Unfortunately, on macOS, when SIGBUS (on all CPUs) and SIGILL and SIGSEGV
193+
// (on arm64) is received asynchronously via kill(), siginfo->si_code makes it
194+
// appear as though it was actually received via a hardware fault. See 10.15.6
195+
// xnu-6153.141.1/bsd/dev/i386/unix_signal.c sendsig() and 10.15.6
196+
// xnu-6153.141.1/bsd/dev/arm/unix_signal.c sendsig(). Received
197+
// asynchronously, these signals will not re-raise themselves autonomously,
198+
// but this function (acting on information from the kernel) behaves as though
199+
// they will. This isn’t ideal, but these signals occurring asynchronously is
200+
// an unexpected condition. The alternative, to never treat these signals as
201+
// autonomously re-raising, is a bad idea because the explicit re-raise would
202+
// lose properties associated with the the original signal, which are valuable
203+
// for debugging and are visible to a Mach exception handler. Since these
204+
// signals are normally received synchronously in response to a hardware
205+
// fault, don’t sweat the unexpected asynchronous case.
204206
//
205-
// SIGSEGV on macOS originating from a general protection fault is a more
206-
// difficult case: si_code is cleared, making the signal appear asynchronous.
207-
// See 10.12.3 xnu-3789.41.3/bsd/dev/i386/unix_signal.c sendsig().
207+
// SIGSEGV on macOS on x86[_64] originating from a general protection fault is
208+
// a more difficult case: si_code is cleared, making the signal appear
209+
// asynchronous. See 10.15.6 xnu-6153.141.1/bsd/dev/i386/unix_signal.c
210+
// sendsig().
208211
const int sig = siginfo->si_signo;
209212
const int code = siginfo->si_code;
210213

util/posix/signals_test.cc

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ bool CanCauseSignal(int sig) {
5151
#endif // !defined(ARCH_CPU_ARM64)
5252
#if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARMEL)
5353
sig == SIGILL ||
54-
#endif // defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARMEL
54+
#endif // defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARMEL)
5555
sig == SIGPIPE ||
5656
sig == SIGSEGV ||
5757
#if defined(OS_APPLE)
@@ -466,12 +466,16 @@ TEST(Signals, Raise_HandlerReraisesToDefault) {
466466
}
467467

468468
#if defined(OS_APPLE)
469-
if (sig == SIGBUS) {
470-
// Signal handlers can’t distinguish between SIGBUS arising out of a
471-
// hardware fault and SIGBUS raised asynchronously.
472-
// Signals::RestoreHandlerAndReraiseSignalOnReturn() assumes that SIGBUS
473-
// comes from a hardware fault, but this test uses raise(), so the
474-
// re-raise test must be skipped.
469+
if (sig == SIGBUS
470+
#if defined(ARCH_CPU_ARM64)
471+
|| sig == SIGILL || sig == SIGSEGV
472+
#endif // defined(ARCH_CPU_ARM64)
473+
) {
474+
// Signal handlers can’t distinguish between these signals arising out of
475+
// hardware faults and raised asynchronously.
476+
// Signals::RestoreHandlerAndReraiseSignalOnReturn() assumes that they
477+
// come from hardware faults, but this test uses raise(), so the re-raise
478+
// test must be skipped.
475479
continue;
476480
}
477481
#endif // defined(OS_APPLE)
@@ -493,12 +497,16 @@ TEST(Signals, Raise_HandlerReraisesToPrevious) {
493497
}
494498

495499
#if defined(OS_APPLE)
496-
if (sig == SIGBUS) {
497-
// Signal handlers can’t distinguish between SIGBUS arising out of a
498-
// hardware fault and SIGBUS raised asynchronously.
499-
// Signals::RestoreHandlerAndReraiseSignalOnReturn() assumes that SIGBUS
500-
// comes from a hardware fault, but this test uses raise(), so the
501-
// re-raise test must be skipped.
500+
if (sig == SIGBUS
501+
#if defined(ARCH_CPU_ARM64)
502+
|| sig == SIGILL || sig == SIGSEGV
503+
#endif // defined(ARCH_CPU_ARM64)
504+
) {
505+
// Signal handlers can’t distinguish between these signals arising out of
506+
// hardware faults and raised asynchronously.
507+
// Signals::RestoreHandlerAndReraiseSignalOnReturn() assumes that they
508+
// come from hardware faults, but this test uses raise(), so the re-raise
509+
// test must be skipped.
502510
continue;
503511
}
504512
#endif // defined(OS_APPLE)

0 commit comments

Comments
 (0)