Merge "Only have one copy of the kernel_sigset_t hack, and add more tests."

libc/Android.mk

@@ -222,7 +222,6 @@ libc_common_src_files := \
 	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 @@ libc_bionic_src_files := \
     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 \

libc/bionic/pthread.c

@@ -2099,58 +2099,6 @@ int pthread_kill(pthread_t tid, int sig)
     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)
 {

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;
+}

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);
-
-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;
+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(mask);
   return signalfd4(fd, &in_set, sizeof(in_set), flags);
 }

libc/bionic/sigwait.cpp

@@ -25,59 +25,29 @@
  * 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);
+#include <private/kernel_sigset_t.h>
 
-/* 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 !!
- */
+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)
-{
-    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;
+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;
     }
 
-    *sig = ret;
-    return 0;
+    if (errno != EAGAIN && errno != EINTR) {
+      return errno;
+    }
+  }
 }
-

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

tests/pthread_test.cpp

@@ -126,3 +126,31 @@ TEST(pthread_DeathTest, pthread_bug_37410) {
   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));
+}

tests/signal_test.cpp

@@ -101,3 +101,27 @@ TEST(signal, raise_invalid) {
   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);
+}