Only have one copy of the kernel_sigset_t hack, and add more tests.
Change-Id: I377522fcba6fb4b5fd2754ab15b091014bd7c16f
diff --git a/libc/Android.mk b/libc/Android.mk
index 45297b5..360bdc1 100644
--- a/libc/Android.mk
+++ b/libc/Android.mk
@@ -222,7 +222,6 @@
bionic/signame.c \
bionic/sigsetmask.c \
bionic/sigsuspend.c \
- bionic/sigwait.c \
bionic/sleep.c \
bionic/statfs.c \
bionic/strcoll.c \
@@ -281,10 +280,12 @@
bionic/__memcpy_chk.cpp \
bionic/__memmove_chk.cpp \
bionic/__memset_chk.cpp \
+ bionic/pthread_sigmask.cpp \
bionic/raise.cpp \
bionic/__set_errno.cpp \
bionic/setlocale.cpp \
bionic/signalfd.cpp \
+ bionic/sigwait.cpp \
bionic/__strcat_chk.cpp \
bionic/__strcpy_chk.cpp \
bionic/strerror.cpp \
diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c
index b685f2d..92ee4aa 100644
--- a/libc/bionic/pthread.c
+++ b/libc/bionic/pthread.c
@@ -2099,58 +2099,6 @@
return ret;
}
-/* Despite the fact that our kernel headers define sigset_t explicitly
- * as a 32-bit integer, the kernel system call really expects a 64-bit
- * bitmap for the signal set, or more exactly an array of two-32-bit
- * values (see $KERNEL/arch/$ARCH/include/asm/signal.h for details).
- *
- * Unfortunately, we cannot fix the sigset_t definition without breaking
- * the C library ABI, so perform a little runtime translation here.
- */
-typedef union {
- sigset_t bionic;
- uint32_t kernel[2];
-} kernel_sigset_t;
-
-/* this is a private syscall stub */
-extern int __rt_sigprocmask(int, const kernel_sigset_t *, kernel_sigset_t *, size_t);
-
-int pthread_sigmask(int how, const sigset_t *set, sigset_t *oset)
-{
- /* pthread_sigmask must return the error code, but the syscall
- * will set errno instead and return 0/-1
- */
- int ret, old_errno = errno;
-
- /* We must convert *set into a kernel_sigset_t */
- kernel_sigset_t in_set, *in_set_ptr;
- kernel_sigset_t out_set;
-
- in_set.kernel[0] = in_set.kernel[1] = 0;
- out_set.kernel[0] = out_set.kernel[1] = 0;
-
- /* 'in_set_ptr' is the second parameter to __rt_sigprocmask. It must be NULL
- * if 'set' is NULL to ensure correct semantics (which in this case would
- * be to ignore 'how' and return the current signal set into 'oset'.
- */
- if (set == NULL) {
- in_set_ptr = NULL;
- } else {
- in_set.bionic = *set;
- in_set_ptr = &in_set;
- }
-
- ret = __rt_sigprocmask(how, in_set_ptr, &out_set, sizeof(kernel_sigset_t));
- if (ret < 0)
- ret = errno;
-
- if (oset)
- *oset = out_set.bionic;
-
- errno = old_errno;
- return ret;
-}
-
int pthread_getcpuclockid(pthread_t tid, clockid_t *clockid)
{
diff --git a/libc/bionic/pthread_sigmask.cpp b/libc/bionic/pthread_sigmask.cpp
new file mode 100644
index 0000000..9c9dc3e
--- /dev/null
+++ b/libc/bionic/pthread_sigmask.cpp
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2008 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <errno.h>
+#include <pthread.h>
+#include <signal.h>
+
+#include <private/kernel_sigset_t.h>
+
+extern "C" int __rt_sigprocmask(int, const kernel_sigset_t*, kernel_sigset_t*, size_t);
+
+int pthread_sigmask(int how, const sigset_t* iset, sigset_t* oset) {
+ int old_errno = errno;
+
+ // 'in_set_ptr' is the second parameter to __rt_sigprocmask. It must be NULL
+ // if 'set' is NULL to ensure correct semantics (which in this case would
+ // be to ignore 'how' and return the current signal set into 'oset').
+ kernel_sigset_t in_set;
+ kernel_sigset_t* in_set_ptr = NULL;
+ if (iset != NULL) {
+ in_set.set(iset);
+ in_set_ptr = &in_set;
+ }
+
+ kernel_sigset_t out_set;
+ int result = __rt_sigprocmask(how, in_set_ptr, &out_set, sizeof(out_set));
+ if (result < 0) {
+ result = errno;
+ }
+
+ if (oset != NULL) {
+ *oset = out_set.bionic;
+ }
+
+ errno = old_errno;
+ return result;
+}
diff --git a/libc/bionic/signalfd.cpp b/libc/bionic/signalfd.cpp
index 51fae83..b7e6474 100644
--- a/libc/bionic/signalfd.cpp
+++ b/libc/bionic/signalfd.cpp
@@ -28,27 +28,11 @@
#include <sys/signalfd.h>
-/* Despite the fact that our kernel headers define sigset_t explicitly
- * as a 32-bit integer, the kernel system call really expects a 64-bit
- * bitmap for the signal set, or more exactly an array of two-32-bit
- * values (see $KERNEL/arch/$ARCH/include/asm/signal.h for details).
- *
- * Unfortunately, we cannot fix the sigset_t definition without breaking
- * the C library ABI, so perform a little runtime translation here.
- */
-typedef union {
- sigset_t bionic;
- uint32_t kernel[2];
-} kernel_sigset_t;
+#include <private/kernel_sigset_t.h>
-extern "C" int signalfd4(int fd, kernel_sigset_t *mask, size_t sizemask, int flags);
+extern "C" int signalfd4(int fd, kernel_sigset_t* mask, size_t sizemask, int flags);
-int signalfd(int fd, const sigset_t *mask, int flags)
-{
- kernel_sigset_t in_set;
- in_set.kernel[0] = in_set.kernel[1] = 0;
-
- in_set.bionic = *mask;
-
- return signalfd4(fd, &in_set, sizeof(in_set), flags);
+int signalfd(int fd, const sigset_t* mask, int flags) {
+ kernel_sigset_t in_set(mask);
+ return signalfd4(fd, &in_set, sizeof(in_set), flags);
}
diff --git a/libc/bionic/sigwait.c b/libc/bionic/sigwait.c
deleted file mode 100644
index 1e90c41..0000000
--- a/libc/bionic/sigwait.c
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- * Copyright (C) 2008 The Android Open Source Project
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * * Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in
- * the documentation and/or other materials provided with the
- * distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-#include <errno.h>
-#include <signal.h>
-#include <string.h>
-#include <time.h>
-
-int __rt_sigtimedwait(const sigset_t *uthese, siginfo_t *uinfo, const struct timespec *uts, size_t sigsetsize);
-
-/* ok, this is really subtle: <asm/signal.h> defines sigset_t differently
- * when you're in the kernel or in the C library.
- *
- * in the kernel, this is an array of 2 32-bit unsigned longs
- * in the C library, this is a single 32-bit unsigned long
- *
- * moreover, the kernel implementation of rt_sigtimedwait doesn't
- * accept anything except kernel-sized signal sets (probably a bug !)
- *
- * we thus need to create a fake kernel sigset !!
- */
-
-int sigwait(const sigset_t *set, int *sig)
-{
- int ret;
-#ifdef __mips__
- /* use a union to get rid of aliasing warnings. On MIPS sigset_t is 128 bits */
- union {
- sigset_t kernel_sigset;
- sigset_t dummy_sigset;
- } u;
- u.dummy_sigset = *set;
-#else
- /* use a union to get rid of aliasing warnings */
- union {
- unsigned long kernel_sigset[2];
- sigset_t dummy_sigset;
- } u;
-
- u.kernel_sigset[0] = *set;
- u.kernel_sigset[1] = 0; /* no real-time signals supported ? */
-#endif
- for (;;)
- {
- /* __rt_sigtimedwait can return EAGAIN or EINTR, we need to loop
- * around them since sigwait is only allowed to return EINVAL
- */
- ret = __rt_sigtimedwait ( &u.dummy_sigset, NULL, NULL, sizeof(u.kernel_sigset));
- if (ret >= 0)
- break;
-
- if (errno != EAGAIN && errno != EINTR)
- return errno;
- }
-
- *sig = ret;
- return 0;
-}
-
diff --git a/libc/bionic/sigwait.cpp b/libc/bionic/sigwait.cpp
new file mode 100644
index 0000000..1546fd6
--- /dev/null
+++ b/libc/bionic/sigwait.cpp
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2008 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <errno.h>
+#include <signal.h>
+#include <string.h>
+#include <time.h>
+
+#include <private/kernel_sigset_t.h>
+
+extern "C" int __rt_sigtimedwait(const sigset_t* uthese, siginfo_t* uinfo, const struct timespec* uts, size_t sigsetsize);
+
+int sigwait(const sigset_t* set, int* sig) {
+ kernel_sigset_t sigset(set);
+ while (true) {
+ // __rt_sigtimedwait can return EAGAIN or EINTR, we need to loop
+ // around them since sigwait is only allowed to return EINVAL.
+ int result = __rt_sigtimedwait(sigset.get(), NULL, NULL, sizeof(sigset));
+ if (result >= 0) {
+ *sig = result;
+ return 0;
+ }
+
+ if (errno != EAGAIN && errno != EINTR) {
+ return errno;
+ }
+ }
+}
diff --git a/libc/private/kernel_sigset_t.h b/libc/private/kernel_sigset_t.h
new file mode 100644
index 0000000..733a842
--- /dev/null
+++ b/libc/private/kernel_sigset_t.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2013 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef LIBC_PRIVATE_KERNEL_SIGSET_T_H_
+#define LIBC_PRIVATE_KERNEL_SIGSET_T_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.
+// 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.
+union kernel_sigset_t {
+ kernel_sigset_t() {
+ clear();
+ }
+
+ kernel_sigset_t(const sigset_t* value) {
+ clear();
+ set(value);
+ }
+
+ void clear() {
+ memset(this, 0, sizeof(*this));
+ }
+
+ void set(const sigset_t* value) {
+ bionic = *value;
+ }
+
+ sigset_t* get() {
+ return &bionic;
+ }
+
+ sigset_t bionic;
+#ifndef __mips__
+ uint32_t kernel[2];
+#endif
+};
+
+#endif
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index da945b4..3e144db 100644
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -126,3 +126,31 @@
EXPECT_EXIT(TestBug37410(), ::testing::ExitedWithCode(0), "");
}
#endif
+
+static void* SignalHandlerFn(void* arg) {
+ sigset_t wait_set;
+ sigfillset(&wait_set);
+ return reinterpret_cast<void*>(sigwait(&wait_set, reinterpret_cast<int*>(arg)));
+}
+
+TEST(pthread, pthread_sigmask) {
+ // Block SIGUSR1.
+ sigset_t set;
+ sigemptyset(&set);
+ sigaddset(&set, SIGUSR1);
+ ASSERT_EQ(0, pthread_sigmask(SIG_BLOCK, &set, NULL));
+
+ // Spawn a thread that calls sigwait and tells us what it received.
+ pthread_t signal_thread;
+ int received_signal = -1;
+ ASSERT_EQ(0, pthread_create(&signal_thread, NULL, SignalHandlerFn, &received_signal));
+
+ // Send that thread SIGUSR1.
+ pthread_kill(signal_thread, SIGUSR1);
+
+ // See what it got.
+ void* join_result;
+ ASSERT_EQ(0, pthread_join(signal_thread, &join_result));
+ ASSERT_EQ(SIGUSR1, received_signal);
+ ASSERT_EQ(0, reinterpret_cast<int>(join_result));
+}
diff --git a/tests/signal_test.cpp b/tests/signal_test.cpp
index 1292568..b100372 100644
--- a/tests/signal_test.cpp
+++ b/tests/signal_test.cpp
@@ -101,3 +101,27 @@
ASSERT_EQ(-1, raise(-1));
ASSERT_EQ(EINVAL, errno);
}
+
+static void HandleSIGALRM(int signal_number) {
+ ASSERT_EQ(SIGALRM, signal_number);
+}
+
+TEST(signal, sigwait) {
+ struct sigaction action;
+ sigemptyset(&action.sa_mask);
+ action.sa_flags = 0;
+ action.sa_handler = HandleSIGALRM;
+ sigaction(SIGALRM, &action, NULL);
+
+ sigset_t wait_set;
+ sigemptyset(&wait_set);
+ sigaddset(&wait_set, SIGALRM);
+
+ alarm(1);
+
+ int received_signal;
+ errno = 0;
+ ASSERT_EQ(0, sigwait(&wait_set, &received_signal));
+ ASSERT_EQ(0, errno);
+ ASSERT_EQ(SIGALRM, received_signal);
+}