Better handling of sigset_t on LP32.
The main motivation here is that the sigprocmask in pthread_exit wasn't
actually blocking the real-time signals, and debuggerd (amongst other
things) is using them. I wasn't able to write a test that actually won
that race but I did write an equivalent one for posix_spawn.
This also fixes all the uses of sigset_t where the sigset_t isn't
exposed to the outside (which we can't easily fix because it would be
an ABI change).
Bug: https://issuetracker.google.com/72291624
Test: ran tests
Change-Id: Ib6eebebc5a7b0150079f1cb79593247917dcf750
diff --git a/libc/bionic/abort.cpp b/libc/bionic/abort.cpp
index f401cab..9f1c31f 100644
--- a/libc/bionic/abort.cpp
+++ b/libc/bionic/abort.cpp
@@ -32,6 +32,8 @@
#include <sys/syscall.h>
#include <unistd.h>
+#include "private/kernel_sigset_t.h"
+
// We call tgkill(2) directly instead of raise (or even the libc tgkill wrapper), to reduce the
// number of uninteresting stack frames at the top of a crash.
static inline __always_inline void inline_tgkill(pid_t pid, pid_t tid, int sig) {
@@ -60,10 +62,10 @@
// Don't block SIGABRT to give any signal handler a chance; we ignore
// any errors -- X311J doesn't allow abort to return anyway.
- sigset_t mask;
- sigfillset(&mask);
- sigdelset(&mask, SIGABRT);
- sigprocmask(SIG_SETMASK, &mask, NULL);
+ kernel_sigset_t mask;
+ mask.fill();
+ mask.clear(SIGABRT);
+ __rt_sigprocmask(SIG_SETMASK, &mask, nullptr, sizeof(mask));
inline_tgkill(pid, tid, SIGABRT);
@@ -74,7 +76,7 @@
sa.sa_flags = SA_RESTART;
sigemptyset(&sa.sa_mask);
sigaction(SIGABRT, &sa, &sa);
- sigprocmask(SIG_SETMASK, &mask, NULL);
+ __rt_sigprocmask(SIG_SETMASK, &mask, nullptr, sizeof(mask));
inline_tgkill(pid, tid, SIGABRT);
diff --git a/libc/bionic/pause.cpp b/libc/bionic/pause.cpp
index 94a16fb..2a0779a 100644
--- a/libc/bionic/pause.cpp
+++ b/libc/bionic/pause.cpp
@@ -30,13 +30,8 @@
#include "private/kernel_sigset_t.h"
-extern "C" int __rt_sigprocmask(int, const kernel_sigset_t*, kernel_sigset_t*, size_t);
-extern "C" int __rt_sigsuspend(const kernel_sigset_t*, size_t);
-
int pause() {
kernel_sigset_t mask;
- if (__rt_sigprocmask(SIG_SETMASK, NULL, &mask, sizeof(mask)) == -1) {
- return -1;
- }
+ if (__rt_sigprocmask(SIG_SETMASK, nullptr, &mask, sizeof(mask)) == -1) return -1;
return __rt_sigsuspend(&mask, sizeof(mask));
}
diff --git a/libc/bionic/posix_timers.cpp b/libc/bionic/posix_timers.cpp
index c46965f..e3bb112 100644
--- a/libc/bionic/posix_timers.cpp
+++ b/libc/bionic/posix_timers.cpp
@@ -74,8 +74,7 @@
static void* __timer_thread_start(void* arg) {
PosixTimer* timer = reinterpret_cast<PosixTimer*>(arg);
- kernel_sigset_t sigset;
- sigaddset(sigset.get(), TIMER_SIGNAL);
+ kernel_sigset_t sigset{TIMER_SIGNAL};
while (true) {
// Wait for a signal...
@@ -150,14 +149,13 @@
// We start the thread with TIMER_SIGNAL blocked by blocking the signal here and letting it
// inherit. If it tried to block the signal itself, there would be a race.
- kernel_sigset_t sigset;
- sigaddset(sigset.get(), TIMER_SIGNAL);
+ kernel_sigset_t sigset{TIMER_SIGNAL};
kernel_sigset_t old_sigset;
- pthread_sigmask(SIG_BLOCK, sigset.get(), old_sigset.get());
+ __rt_sigprocmask(SIG_BLOCK, &sigset, &old_sigset, sizeof(sigset));
int rc = pthread_create(&timer->callback_thread, &thread_attributes, __timer_thread_start, timer);
- pthread_sigmask(SIG_SETMASK, old_sigset.get(), NULL);
+ __rt_sigprocmask(SIG_SETMASK, &old_sigset, nullptr, sizeof(sigset));
if (rc != 0) {
free(timer);
diff --git a/libc/bionic/pthread_exit.cpp b/libc/bionic/pthread_exit.cpp
index 8b4c44e..f1b65fd 100644
--- a/libc/bionic/pthread_exit.cpp
+++ b/libc/bionic/pthread_exit.cpp
@@ -34,6 +34,7 @@
#include <sys/mman.h>
#include "private/bionic_defs.h"
+#include "private/ScopedSignalBlocker.h"
#include "pthread_internal.h"
extern "C" __noreturn void _exit_with_stack_teardown(void*, size_t);
@@ -63,6 +64,12 @@
}
}
+static void __pthread_unmap_tls(pthread_internal_t* thread) {
+ // Unmap the bionic TLS, including guard pages.
+ void* allocation = reinterpret_cast<char*>(thread->bionic_tls) - PTHREAD_GUARD_SIZE;
+ munmap(allocation, BIONIC_TLS_SIZE + 2 * PTHREAD_GUARD_SIZE);
+}
+
__BIONIC_WEAK_FOR_NATIVE_BRIDGE
void pthread_exit(void* return_value) {
// Call dtors for thread_local objects first.
@@ -96,10 +103,6 @@
thread->alternate_signal_stack = NULL;
}
- // Unmap the bionic TLS, including guard pages.
- void* allocation = reinterpret_cast<char*>(thread->bionic_tls) - PTHREAD_GUARD_SIZE;
- munmap(allocation, BIONIC_TLS_SIZE + 2 * PTHREAD_GUARD_SIZE);
-
ThreadJoinState old_state = THREAD_NOT_JOINED;
while (old_state == THREAD_NOT_JOINED &&
!atomic_compare_exchange_weak(&thread->join_state, &old_state, THREAD_EXITED_NOT_JOINED)) {
@@ -120,16 +123,15 @@
// That's not something we can do in C.
// We don't want to take a signal after we've unmapped the stack.
- // That's one last thing we can handle in C.
- sigset_t mask;
- sigfillset(&mask);
- sigprocmask(SIG_SETMASK, &mask, NULL);
-
+ // That's one last thing we can do before dropping to assembler.
+ ScopedSignalBlocker ssb;
+ __pthread_unmap_tls(thread);
_exit_with_stack_teardown(thread->attr.stack_base, thread->mmap_size);
}
}
// No need to free mapped space. Either there was no space mapped, or it is left for
// the pthread_join caller to clean up.
+ __pthread_unmap_tls(thread);
__exit(0);
}
diff --git a/libc/bionic/sigblock.c b/libc/bionic/sigblock.c
index 176bc13..cc47d5d 100644
--- a/libc/bionic/sigblock.c
+++ b/libc/bionic/sigblock.c
@@ -25,26 +25,18 @@
* OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
+
#include <signal.h>
-/* this function is called from the ARM assembly setjmp fragments */
-int
-sigblock(int mask)
-{
- int n;
- union {
- int the_mask;
- sigset_t the_sigset;
- } in, out;
+int sigblock(int mask) {
+ union {
+ int mask;
+ sigset_t set;
+ } in, out;
- sigemptyset(&in.the_sigset);
- in.the_mask = mask;
+ sigemptyset(&in.set);
+ in.mask = mask;
- n = sigprocmask(SIG_BLOCK, &in.the_sigset, &out.the_sigset);
- if (n)
- return n;
-
- return out.the_mask;
+ if (sigprocmask(SIG_BLOCK, &in.set, &out.set) == -1) return -1;
+ return out.mask;
}
-
-
diff --git a/libc/bionic/sighold.cpp b/libc/bionic/sighold.cpp
index e9c8ca1..2800d10 100644
--- a/libc/bionic/sighold.cpp
+++ b/libc/bionic/sighold.cpp
@@ -28,9 +28,11 @@
#include <signal.h>
+#include "private/kernel_sigset_t.h"
+
int sighold(int sig) {
- sigset_t set;
- if (sigemptyset(&set) == -1) return -1;
- if (sigaddset(&set, sig) == -1) return -1;
- return sigprocmask(SIG_BLOCK, &set, nullptr);
+ kernel_sigset_t set;
+ set.clear();
+ if (!set.set(sig)) return -1;
+ return __rt_sigprocmask(SIG_BLOCK, &set, nullptr, sizeof(set));
}
diff --git a/libc/bionic/sigpause.cpp b/libc/bionic/sigpause.cpp
index 8ba42d4..6b5d74a 100644
--- a/libc/bionic/sigpause.cpp
+++ b/libc/bionic/sigpause.cpp
@@ -28,9 +28,12 @@
#include <signal.h>
+#include "private/kernel_sigset_t.h"
+
int sigpause(int sig) {
- sigset_t set;
- if (sigprocmask(SIG_SETMASK, nullptr, &set) == -1) return -1;
- if (sigdelset(&set, sig) == -1) return -1;
- return sigsuspend(&set);
+ kernel_sigset_t set;
+ set.clear();
+ if (__rt_sigprocmask(SIG_SETMASK, nullptr, &set, sizeof(set)) == -1) return -1;
+ if (!set.clear(sig)) return -1;
+ return __rt_sigsuspend(&set, sizeof(set));
}
diff --git a/libc/bionic/sigpending.cpp b/libc/bionic/sigpending.cpp
index b6e503c..8a02de6 100644
--- a/libc/bionic/sigpending.cpp
+++ b/libc/bionic/sigpending.cpp
@@ -30,8 +30,6 @@
#include "private/kernel_sigset_t.h"
-extern "C" int __rt_sigpending(const kernel_sigset_t*, size_t);
-
int sigpending(sigset_t* bionic_set) {
kernel_sigset_t set;
int result = __rt_sigpending(&set, sizeof(set));
diff --git a/libc/bionic/sigprocmask.cpp b/libc/bionic/sigprocmask.cpp
index 61e2c63..c34e42e 100644
--- a/libc/bionic/sigprocmask.cpp
+++ b/libc/bionic/sigprocmask.cpp
@@ -32,8 +32,6 @@
#include "private/kernel_sigset_t.h"
-extern "C" int __rt_sigprocmask(int, const kernel_sigset_t*, kernel_sigset_t*, size_t);
-
int sigprocmask(int how, const sigset_t* bionic_new_set, sigset_t* bionic_old_set) {
kernel_sigset_t new_set;
kernel_sigset_t* new_set_ptr = NULL;
diff --git a/libc/bionic/sigrelse.cpp b/libc/bionic/sigrelse.cpp
index ab5554e..c4a484a 100644
--- a/libc/bionic/sigrelse.cpp
+++ b/libc/bionic/sigrelse.cpp
@@ -28,9 +28,11 @@
#include <signal.h>
+#include "private/kernel_sigset_t.h"
+
int sigrelse(int sig) {
- sigset_t set;
- if (sigemptyset(&set) == -1) return -1;
- if (sigaddset(&set, sig) == -1) return -1;
- return sigprocmask(SIG_UNBLOCK, &set, nullptr);
+ kernel_sigset_t set;
+ set.clear();
+ if (!set.set(sig)) return -1;
+ return __rt_sigprocmask(SIG_UNBLOCK, &set, nullptr, sizeof(set));
}
diff --git a/libc/bionic/sigset.cpp b/libc/bionic/sigset.cpp
index e3f3e72..52820f2 100644
--- a/libc/bionic/sigset.cpp
+++ b/libc/bionic/sigset.cpp
@@ -29,6 +29,8 @@
#include <signal.h>
#include <string.h>
+#include "private/kernel_sigset_t.h"
+
sighandler_t sigset(int sig, sighandler_t disp) {
struct sigaction new_sa;
if (disp != SIG_HOLD) {
@@ -38,19 +40,16 @@
}
struct sigaction old_sa;
- if (sigaction(sig, disp == SIG_HOLD ? nullptr : &new_sa, &old_sa) == -1) {
+ if (sigaction(sig, (disp == SIG_HOLD) ? nullptr : &new_sa, &old_sa) == -1) {
return SIG_ERR;
}
- sigset_t new_proc_mask;
- sigemptyset(&new_proc_mask);
- sigaddset(&new_proc_mask, sig);
-
- sigset_t old_proc_mask;
- if (sigprocmask(disp == SIG_HOLD ? SIG_BLOCK : SIG_UNBLOCK,
- &new_proc_mask, &old_proc_mask) == -1) {
+ kernel_sigset_t new_mask{sig};
+ kernel_sigset_t old_mask;
+ if (__rt_sigprocmask(disp == SIG_HOLD ? SIG_BLOCK : SIG_UNBLOCK, &new_mask, &old_mask,
+ sizeof(new_mask)) == -1) {
return SIG_ERR;
}
- return sigismember(&old_proc_mask, sig) ? SIG_HOLD : old_sa.sa_handler;
+ return old_mask.is_set(sig) ? SIG_HOLD : old_sa.sa_handler;
}
diff --git a/libc/bionic/sigsetmask.c b/libc/bionic/sigsetmask.c
index 7842bf1..6a3b1d2 100644
--- a/libc/bionic/sigsetmask.c
+++ b/libc/bionic/sigsetmask.c
@@ -25,26 +25,18 @@
* OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
+
#include <signal.h>
-/* called from setjmp assembly fragment */
-int
-sigsetmask(int mask)
-{
- int n;
+int sigsetmask(int mask) {
+ union {
+ int mask;
+ sigset_t set;
+ } in, out;
- union {
- int the_mask;
- sigset_t the_sigset;
- } in, out;
+ sigemptyset(&in.set);
+ in.mask = mask;
- sigemptyset(&in.the_sigset);
- in.the_mask = mask;
-
- n = sigprocmask(SIG_SETMASK, &in.the_sigset, &out.the_sigset);
- if (n)
- return n;
-
- return out.the_mask;
+ if (sigprocmask(SIG_SETMASK, &in.set, &out.set) == -1) return -1;
+ return out.mask;
}
-
diff --git a/libc/bionic/sigsuspend.cpp b/libc/bionic/sigsuspend.cpp
index fb846b8..1d89d4f 100644
--- a/libc/bionic/sigsuspend.cpp
+++ b/libc/bionic/sigsuspend.cpp
@@ -30,8 +30,6 @@
#include "private/kernel_sigset_t.h"
-extern "C" int __rt_sigsuspend(const kernel_sigset_t*, size_t);
-
int sigsuspend(const sigset_t* bionic_set) {
kernel_sigset_t set(bionic_set);
return __rt_sigsuspend(&set, sizeof(set));
diff --git a/libc/private/ScopedSignalBlocker.h b/libc/private/ScopedSignalBlocker.h
index 35d1c58..c3ab307 100644
--- a/libc/private/ScopedSignalBlocker.h
+++ b/libc/private/ScopedSignalBlocker.h
@@ -20,13 +20,14 @@
#include <signal.h>
#include "bionic_macros.h"
+#include "kernel_sigset_t.h"
class ScopedSignalBlocker {
public:
explicit ScopedSignalBlocker() {
- sigset_t set;
- sigfillset(&set);
- sigprocmask(SIG_BLOCK, &set, &old_set_);
+ kernel_sigset_t set;
+ set.fill();
+ __rt_sigprocmask(SIG_SETMASK, &set, &old_set_, sizeof(set));
}
~ScopedSignalBlocker() {
@@ -34,11 +35,11 @@
}
void reset() {
- sigprocmask(SIG_SETMASK, &old_set_, nullptr);
+ __rt_sigprocmask(SIG_SETMASK, &old_set_, nullptr, sizeof(old_set_));
}
private:
- sigset_t old_set_;
+ kernel_sigset_t old_set_;
DISALLOW_COPY_AND_ASSIGN(ScopedSignalBlocker);
};
diff --git a/libc/private/kernel_sigset_t.h b/libc/private/kernel_sigset_t.h
index 9415fcf..bdfb729 100644
--- a/libc/private/kernel_sigset_t.h
+++ b/libc/private/kernel_sigset_t.h
@@ -17,18 +17,27 @@
#ifndef LIBC_PRIVATE_KERNEL_SIGSET_T_H_
#define LIBC_PRIVATE_KERNEL_SIGSET_T_H_
+#include <errno.h>
#include <signal.h>
+#include <async_safe/log.h>
+
// Our sigset_t is wrong for ARM and x86. It's 32-bit but the kernel expects 64 bits.
-// This means we can't support real-time signals correctly until we can change the ABI.
+// This means we can't support real-time signals correctly without breaking the ABI.
// In the meantime, we can use this union to pass an appropriately-sized block of memory
-// to the kernel, at the cost of not being able to refer to real-time signals.
+// to the kernel, at the cost of not being able to refer to real-time signals when
+// initializing from a sigset_t on LP32.
union kernel_sigset_t {
+ public:
kernel_sigset_t() {
- clear();
}
- kernel_sigset_t(const sigset_t* value) {
+ explicit kernel_sigset_t(int signal_number) {
+ clear();
+ if (!set(signal_number)) async_safe_fatal("kernel_sigset_t(%d)", signal_number);
+ }
+
+ explicit kernel_sigset_t(const sigset_t* value) {
clear();
set(value);
}
@@ -37,7 +46,32 @@
__builtin_memset(this, 0, sizeof(*this));
}
+ bool clear(int signal_number) {
+ int bit = bit_of(signal_number);
+ if (bit == -1) return false;
+ bits[bit / LONG_BIT] &= ~(1UL << (bit % LONG_BIT));
+ return true;
+ }
+
+ void fill() {
+ __builtin_memset(this, 0xff, sizeof(*this));
+ }
+
+ bool is_set(int signal_number) {
+ int bit = bit_of(signal_number);
+ if (bit == -1) return false;
+ return ((bits[bit / LONG_BIT] >> (bit % LONG_BIT)) & 1) == 1;
+ }
+
+ bool set(int signal_number) {
+ int bit = bit_of(signal_number);
+ if (bit == -1) return false;
+ bits[bit / LONG_BIT] |= 1UL << (bit % LONG_BIT);
+ return true;
+ }
+
void set(const sigset_t* value) {
+ clear();
bionic = *value;
}
@@ -46,9 +80,21 @@
}
sigset_t bionic;
-#ifndef __mips__
- uint32_t kernel[2];
-#endif
+ unsigned long bits[_KERNEL__NSIG/LONG_BIT];
+
+ private:
+ int bit_of(int signal_number) {
+ int bit = signal_number - 1; // Signal numbers start at 1, but bit positions start at 0.
+ if (bit < 0 || bit >= static_cast<int>(8*sizeof(*this))) {
+ errno = EINVAL;
+ return -1;
+ }
+ return bit;
+ }
};
+extern "C" int __rt_sigpending(const kernel_sigset_t*, size_t);
+extern "C" int __rt_sigprocmask(int, const kernel_sigset_t*, kernel_sigset_t*, size_t);
+extern "C" int __rt_sigsuspend(const kernel_sigset_t*, size_t);
+
#endif