From c537245de8918fea19af117cab0f2bee2f1b6f6a Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Wed, 24 Nov 2021 13:45:20 -0800 Subject: [PATCH] Revert "Reraise signals via rt_tgsigqueueinfo(2) on Linux." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Causes test failures on older versions of Android (e.g. Marshmallow). Also reverts follow-up CL "Fix dead-code warning in util/posix/signals.cc". This reverts commits ab9a87fb5463e5d1579e16bacb1f79d0dd71119b and 04431eccfe878570b1c74a5b376d96b4c9c7e0e8. Bug: 1272877 Change-Id: Id9ef420516c932147b6c8b67d9f4daf9d31d9b03 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3300986 Reviewed-by: Peter Boström Commit-Queue: Peter Collingbourne --- util/posix/signals.cc | 20 ----- util/posix/signals_test.cc | 164 +++++++++++-------------------------- 2 files changed, 49 insertions(+), 135 deletions(-) diff --git a/util/posix/signals.cc b/util/posix/signals.cc index 5df5a08d..b90e8f65 100644 --- a/util/posix/signals.cc +++ b/util/posix/signals.cc @@ -22,10 +22,6 @@ #include "base/cxx17_backports.h" #include "base/logging.h" -#if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS) -#include -#endif - namespace crashpad { namespace { @@ -284,21 +280,6 @@ void Signals::RestoreHandlerAndReraiseSignalOnReturn( _exit(kFailureExitCode); } - // If we can raise a signal with siginfo on this platform, do so. This ensures - // that we preserve the siginfo information for asynchronous signals (i.e. - // signals that do not re-raise autonomously), such as signals delivered via - // kill() and asynchronous hardware faults such as SEGV_MTEAERR, which would - // otherwise be lost when re-raising the signal via raise(). -#if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS) - int retval = syscall(SYS_rt_tgsigqueueinfo, - getpid(), - syscall(SYS_gettid), - siginfo->si_signo, - siginfo); - if (retval != 0) { - _exit(kFailureExitCode); - } -#else // Explicitly re-raise the signal if it will not re-raise itself. Because // signal handlers normally execute with their signal blocked, this raise() // cannot immediately deliver the signal. Delivery is deferred until the @@ -308,7 +289,6 @@ void Signals::RestoreHandlerAndReraiseSignalOnReturn( if (!WillSignalReraiseAutonomously(siginfo) && raise(sig) != 0) { _exit(kFailureExitCode); } -#endif // defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS) } // static diff --git a/util/posix/signals_test.cc b/util/posix/signals_test.cc index cb29ff50..2bd55801 100644 --- a/util/posix/signals_test.cc +++ b/util/posix/signals_test.cc @@ -34,48 +34,15 @@ #include "test/scoped_temp_dir.h" #include "util/posix/scoped_mmap.h" -#if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS) -#include -#include - -#if defined(ARCH_CPU_ARM64) -#ifndef HWCAP2_MTE -#define HWCAP2_MTE (1 << 18) -#endif -#ifndef SEGV_MTEAERR -#define SEGV_MTEAERR 8 -#endif -#ifndef PROT_MTE -#define PROT_MTE 0x20 -#endif -#ifndef PR_SET_TAGGED_ADDR_CTRL -#define PR_SET_TAGGED_ADDR_CTRL 55 -#endif -#ifndef PR_TAGGED_ADDR_ENABLE -#define PR_TAGGED_ADDR_ENABLE (1UL << 0) -#endif -#ifndef PR_MTE_TCF_ASYNC -#define PR_MTE_TCF_ASYNC (1UL << 2) -#endif -#endif // defined(ARCH_CPU_ARM64) -#endif // defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS) - namespace crashpad { namespace test { namespace { constexpr int kUnexpectedExitStatus = 3; -struct TestableSignal { - int sig, code; -}; - // Keep synchronized with CauseSignal(). -std::vector TestableSignals() { - std::vector signals; - signals.push_back({SIGABRT, 0}); - signals.push_back({SIGALRM, 0}); - signals.push_back({SIGBUS, 0}); +bool CanCauseSignal(int sig) { + return sig == SIGABRT || sig == SIGALRM || sig == SIGBUS || /* According to DDI0487D (Armv8 Architecture Reference Manual) the expected * behavior for division by zero (Section 3.4.8) is: "... results in a * zero being written to the destination register, without any @@ -83,30 +50,24 @@ std::vector TestableSignals() { * This applies to Armv8 (and not earlier) for both 32bit and 64bit app code. */ #if defined(ARCH_CPU_X86_FAMILY) - signals.push_back({SIGFPE, 0}); + sig == SIGFPE || #endif + #if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARMEL) - signals.push_back({SIGILL, 0}); + sig == SIGILL || #endif // defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARMEL) - signals.push_back({SIGPIPE, 0}); - signals.push_back({SIGSEGV, 0}); -#if (defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS)) && \ - defined(ARCH_CPU_ARM64) - if (getauxval(AT_HWCAP2) & HWCAP2_MTE) { - signals.push_back({SIGSEGV, SEGV_MTEAERR}); - } -#endif + sig == SIGPIPE || sig == SIGSEGV || #if defined(OS_APPLE) - signals.push_back({SIGSYS, 0}); + sig == SIGSYS || #endif // OS_APPLE #if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARM64) - signals.push_back({SIGTRAP, 0}); + sig == SIGTRAP || #endif // defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARM64) - return signals; + false; } -// Keep synchronized with TestableSignals(). -void CauseSignal(int sig, int code) { +// Keep synchronized with CanCauseSignal(). +void CauseSignal(int sig) { switch (sig) { case SIGABRT: { abort(); @@ -203,37 +164,8 @@ void CauseSignal(int sig, int code) { } case SIGSEGV: { - switch (code) { - case 0: { - volatile int* i = nullptr; - *i = 0; - break; - } -#if (defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS)) && \ - defined(ARCH_CPU_ARM64) - case SEGV_MTEAERR: { - ScopedMmap mapping; - if (!mapping.ResetMmap(nullptr, - getpagesize(), - PROT_READ | PROT_WRITE | PROT_MTE, - MAP_PRIVATE | MAP_ANON, - -1, - 0)) { - _exit(kUnexpectedExitStatus); - } - if (prctl(PR_SET_TAGGED_ADDR_CTRL, - PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_ASYNC, - 0, - 0, - 0) != 0) { - _exit(kUnexpectedExitStatus); - } - mapping.addr_as()[1ULL << 56] = 0; - break; - } -#endif // (defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_CHROMEOS)) && - // defined(ARCH_CPU_ARM64) - } + volatile int* i = nullptr; + *i = 0; break; } @@ -286,10 +218,9 @@ class SignalsTest : public Multiprocess { }; static constexpr int kExitingHandlerExitStatus = 2; - SignalsTest(TestType test_type, SignalSource signal_source, int sig, int code) + SignalsTest(TestType test_type, SignalSource signal_source, int sig) : Multiprocess(), sig_(sig), - code_(code), test_type_(test_type), signal_source_(signal_source) {} @@ -368,7 +299,7 @@ class SignalsTest : public Multiprocess { switch (signal_source_) { case SignalSource::kCause: - CauseSignal(sig_, code_); + CauseSignal(sig_); break; case SignalSource::kRaise: raise(sig_); @@ -379,7 +310,6 @@ class SignalsTest : public Multiprocess { } int sig_; - int code_; TestType test_type_; SignalSource signal_source_; static Signals::OldActions old_actions_; @@ -422,28 +352,32 @@ TEST(Signals, WillSignalReraiseAutonomously) { } TEST(Signals, Cause_DefaultHandler) { - for (TestableSignal s : TestableSignals()) { - SCOPED_TRACE(base::StringPrintf( - "sig %d (%s), code %d", s.sig, strsignal(s.sig), s.code)); + for (int sig = 1; sig < NSIG; ++sig) { + SCOPED_TRACE(base::StringPrintf("sig %d (%s)", sig, strsignal(sig))); + + if (!CanCauseSignal(sig)) { + continue; + } SignalsTest test(SignalsTest::TestType::kDefaultHandler, SignalsTest::SignalSource::kCause, - s.sig, - s.code); - test.SetExpectedChildTermination(Multiprocess::kTerminationSignal, s.sig); + sig); + test.SetExpectedChildTermination(Multiprocess::kTerminationSignal, sig); test.Run(); } } TEST(Signals, Cause_HandlerExits) { - for (TestableSignal s : TestableSignals()) { - SCOPED_TRACE(base::StringPrintf( - "sig %d (%s), code %d", s.sig, strsignal(s.sig), s.code)); + for (int sig = 1; sig < NSIG; ++sig) { + SCOPED_TRACE(base::StringPrintf("sig %d (%s)", sig, strsignal(sig))); + + if (!CanCauseSignal(sig)) { + continue; + } SignalsTest test(SignalsTest::TestType::kHandlerExits, SignalsTest::SignalSource::kCause, - s.sig, - s.code); + sig); test.SetExpectedChildTermination(Multiprocess::kTerminationNormal, SignalsTest::kExitingHandlerExitStatus); test.Run(); @@ -451,28 +385,32 @@ TEST(Signals, Cause_HandlerExits) { } TEST(Signals, Cause_HandlerReraisesToDefault) { - for (TestableSignal s : TestableSignals()) { - SCOPED_TRACE(base::StringPrintf( - "sig %d (%s), code %d", s.sig, strsignal(s.sig), s.code)); + for (int sig = 1; sig < NSIG; ++sig) { + SCOPED_TRACE(base::StringPrintf("sig %d (%s)", sig, strsignal(sig))); + + if (!CanCauseSignal(sig)) { + continue; + } SignalsTest test(SignalsTest::TestType::kHandlerReraisesToDefault, SignalsTest::SignalSource::kCause, - s.sig, - s.code); - test.SetExpectedChildTermination(Multiprocess::kTerminationSignal, s.sig); + sig); + test.SetExpectedChildTermination(Multiprocess::kTerminationSignal, sig); test.Run(); } } TEST(Signals, Cause_HandlerReraisesToPrevious) { - for (TestableSignal s : TestableSignals()) { - SCOPED_TRACE(base::StringPrintf( - "sig %d (%s), code %d", s.sig, strsignal(s.sig), s.code)); + for (int sig = 1; sig < NSIG; ++sig) { + SCOPED_TRACE(base::StringPrintf("sig %d (%s)", sig, strsignal(sig))); + + if (!CanCauseSignal(sig)) { + continue; + } SignalsTest test(SignalsTest::TestType::kHandlerReraisesToPrevious, SignalsTest::SignalSource::kCause, - s.sig, - s.code); + sig); test.SetExpectedChildTermination(Multiprocess::kTerminationNormal, SignalsTest::kExitingHandlerExitStatus); test.Run(); @@ -489,8 +427,7 @@ TEST(Signals, Raise_DefaultHandler) { SignalsTest test(SignalsTest::TestType::kDefaultHandler, SignalsTest::SignalSource::kRaise, - sig, - 0); + sig); test.SetExpectedChildTermination(Multiprocess::kTerminationSignal, sig); test.Run(); } @@ -506,8 +443,7 @@ TEST(Signals, Raise_HandlerExits) { SignalsTest test(SignalsTest::TestType::kHandlerExits, SignalsTest::SignalSource::kRaise, - sig, - 0); + sig); test.SetExpectedChildTermination(Multiprocess::kTerminationNormal, SignalsTest::kExitingHandlerExitStatus); test.Run(); @@ -539,8 +475,7 @@ TEST(Signals, Raise_HandlerReraisesToDefault) { SignalsTest test(SignalsTest::TestType::kHandlerReraisesToDefault, SignalsTest::SignalSource::kRaise, - sig, - 0); + sig); test.SetExpectedChildTermination(Multiprocess::kTerminationSignal, sig); test.Run(); } @@ -571,8 +506,7 @@ TEST(Signals, Raise_HandlerReraisesToPrevious) { SignalsTest test(SignalsTest::TestType::kHandlerReraisesToPrevious, SignalsTest::SignalSource::kRaise, - sig, - 0); + sig); test.SetExpectedChildTermination(Multiprocess::kTerminationNormal, SignalsTest::kExitingHandlerExitStatus); test.Run();