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));