Clarify which architectures do/don't need sa_restorer.
In particular: 32-bit x86 doesn't need sa_restorer either.
I still don't fully understand why arm32 and x86-64 do, so I've left the
comments in those .S files alone. I haven't (knowingly) tested
compiler-rt since we switched from libgcc (which is what the comments
refer to), but I have tested libunwindstack since we switched from
libunwind, and that does fail existing bionic tests for unwinds through
signal frames --- I just don't know why, or whether there's a better fix.
Anyway, local testing suggests that the 32-bit x86 code is vestigial, so
let's get rid of it.
Test: treehugger
Change-Id: I3e2616f736d27a8463814356e5adb52fd76a90cc
diff --git a/libc/bionic/sigaction.cpp b/libc/bionic/sigaction.cpp
index 1cdb021..a84886b 100644
--- a/libc/bionic/sigaction.cpp
+++ b/libc/bionic/sigaction.cpp
@@ -39,26 +39,25 @@
extern "C" int __rt_sigaction(int, const struct __kernel_sigaction*, struct __kernel_sigaction*, size_t);
int sigaction(int signal, const struct sigaction* bionic_new_action, struct sigaction* bionic_old_action) {
- __kernel_sigaction kernel_new_action;
+ __kernel_sigaction kernel_new_action = {};
if (bionic_new_action != nullptr) {
kernel_new_action.sa_flags = bionic_new_action->sa_flags;
kernel_new_action.sa_handler = bionic_new_action->sa_handler;
// Don't filter signals here; if the caller asked for everything to be blocked, we should obey.
kernel_new_action.sa_mask = bionic_new_action->sa_mask;
-#if defined(SA_RESTORER)
+#if defined(__x86_64__)
+ // riscv64 doesn't have sa_restorer. For arm64 and 32-bit x86, unwinding
+ // works best if you just let the kernel supply the default restorer
+ // from [vdso]. gdb doesn't care, but libgcc needs the nop that the
+ // kernel includes before the actual code. (We could add that ourselves,
+ // but why bother?)
+ // TODO: why do arm32 and x86-64 need this to unwind through signal handlers?
kernel_new_action.sa_restorer = bionic_new_action->sa_restorer;
-#if defined(__aarch64__)
- // arm64 has sa_restorer, but unwinding works best if you just let the
- // kernel supply the default restorer from [vdso]. gdb doesn't care, but
- // libgcc needs the nop that the kernel includes before the actual code.
- // (We could add that ourselves, but why bother?)
-#else
if (!(kernel_new_action.sa_flags & SA_RESTORER)) {
kernel_new_action.sa_flags |= SA_RESTORER;
kernel_new_action.sa_restorer = &__restore_rt;
}
#endif
-#endif
}
__kernel_sigaction kernel_old_action;
@@ -90,10 +89,11 @@
// by extracting the implementation of sigaction64 to a static function.
static int __sigaction64(int signal, const struct sigaction64* bionic_new,
struct sigaction64* bionic_old) {
- struct sigaction64 kernel_new;
+ struct sigaction64 kernel_new = {};
if (bionic_new) {
kernel_new = *bionic_new;
-#if defined(SA_RESTORER)
+#if defined(__arm__)
+ // (See sa_restorer comment in sigaction() above.)
if (!(kernel_new.sa_flags & SA_RESTORER)) {
kernel_new.sa_flags |= SA_RESTORER;
kernel_new.sa_restorer = (kernel_new.sa_flags & SA_SIGINFO) ? &__restore_rt : &__restore;
@@ -110,9 +110,8 @@
int sigaction(int signal, const struct sigaction* bionic_new, struct sigaction* bionic_old) {
// The 32-bit ABI is broken. struct sigaction includes a too-small sigset_t,
// so we have to translate to struct sigaction64 first.
- struct sigaction64 kernel_new;
+ struct sigaction64 kernel_new = {};
if (bionic_new) {
- kernel_new = {};
kernel_new.sa_flags = bionic_new->sa_flags;
kernel_new.sa_handler = bionic_new->sa_handler;
#if defined(SA_RESTORER)