Rewrite system(3) to use posix_spawn(3).
We saw crashes from pthread_exit+debuggerd on LP32
(https://issuetracker.google.com/72291624), and it seems like the
equivalent problem should exist with system(3). I fixed posix_spawn(3)
as part of that bug, so the easiest fix is probably to reuse that.
Bug: http://b/72470344
Test: ran tests
Change-Id: I05f838706f2b4a14ac3ee21292833e6c8579b0d4
diff --git a/libc/Android.bp b/libc/Android.bp
index 7454d03..20df52c 100644
--- a/libc/Android.bp
+++ b/libc/Android.bp
@@ -481,7 +481,6 @@
"upstream-openbsd/lib/libc/stdlib/reallocarray.c",
"upstream-openbsd/lib/libc/stdlib/remque.c",
"upstream-openbsd/lib/libc/stdlib/setenv.c",
- "upstream-openbsd/lib/libc/stdlib/system.c",
"upstream-openbsd/lib/libc/stdlib/tfind.c",
"upstream-openbsd/lib/libc/stdlib/tsearch.c",
"upstream-openbsd/lib/libc/string/strcasecmp.c",
@@ -1369,6 +1368,7 @@
"bionic/sys_time.cpp",
"bionic/sysinfo.cpp",
"bionic/syslog.cpp",
+ "bionic/system.cpp",
"bionic/system_property_api.cpp",
"bionic/system_property_set.cpp",
"bionic/tdestroy.cpp",
diff --git a/libc/bionic/system.cpp b/libc/bionic/system.cpp
new file mode 100644
index 0000000..7411ae5
--- /dev/null
+++ b/libc/bionic/system.cpp
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2018 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 <paths.h>
+#include <stdlib.h>
+#include <spawn.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "private/ScopedSignalBlocker.h"
+#include "private/ScopedSignalHandler.h"
+
+int system(const char* command) {
+ // "The system() function shall always return non-zero when command is NULL."
+ // http://pubs.opengroup.org/onlinepubs/9699919799/functions/system.html
+ if (command == nullptr) return 1;
+
+ ScopedSignalBlocker sigchld_blocker(SIGCHLD);
+ ScopedSignalHandler sigint_ignorer(SIGINT, SIG_IGN);
+ ScopedSignalHandler sigquit_ignorer(SIGQUIT, SIG_IGN);
+
+ sigset64_t default_mask = {};
+ if (sigint_ignorer.old_action_.sa_handler != SIG_IGN) sigaddset64(&default_mask, SIGINT);
+ if (sigquit_ignorer.old_action_.sa_handler != SIG_IGN) sigaddset64(&default_mask, SIGQUIT);
+
+ static constexpr int flags = POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK;
+ posix_spawnattr_t attributes;
+ if ((errno = posix_spawnattr_init(&attributes))) return -1;
+ if ((errno = posix_spawnattr_setsigdefault64(&attributes, &default_mask))) return -1;
+ if ((errno = posix_spawnattr_setsigmask64(&attributes, &sigchld_blocker.old_set_))) return -1;
+ if ((errno = posix_spawnattr_setflags(&attributes, flags))) return -1;
+
+ const char* argv[] = { "sh", "-c", command, nullptr };
+ pid_t child;
+ if ((errno = posix_spawn(&child, _PATH_BSHELL, nullptr, &attributes,
+ const_cast<char**>(argv), environ)) != 0) {
+ return -1;
+ }
+
+ posix_spawnattr_destroy(&attributes);
+
+ int status;
+ pid_t pid = TEMP_FAILURE_RETRY(waitpid(child, &status, 0));
+ return (pid == -1 ? -1 : status);
+}
diff --git a/libc/private/ScopedSignalBlocker.h b/libc/private/ScopedSignalBlocker.h
index 7582068..d1cf629 100644
--- a/libc/private/ScopedSignalBlocker.h
+++ b/libc/private/ScopedSignalBlocker.h
@@ -14,8 +14,7 @@
* limitations under the License.
*/
-#ifndef SCOPED_SIGNAL_BLOCKER_H
-#define SCOPED_SIGNAL_BLOCKER_H
+#pragma once
#include <signal.h>
@@ -23,10 +22,18 @@
class ScopedSignalBlocker {
public:
+ // Block all signals.
explicit ScopedSignalBlocker() {
sigset64_t set;
sigfillset64(&set);
- sigprocmask64(SIG_SETMASK, &set, &old_set_);
+ sigprocmask64(SIG_BLOCK, &set, &old_set_);
+ }
+
+ // Block just the specified signal.
+ explicit ScopedSignalBlocker(int signal) {
+ sigset64_t set = {};
+ sigaddset64(&set, signal);
+ sigprocmask64(SIG_BLOCK, &set, &old_set_);
}
~ScopedSignalBlocker() {
@@ -37,10 +44,7 @@
sigprocmask64(SIG_SETMASK, &old_set_, nullptr);
}
- private:
sigset64_t old_set_;
DISALLOW_COPY_AND_ASSIGN(ScopedSignalBlocker);
};
-
-#endif
diff --git a/libc/private/ScopedSignalHandler.h b/libc/private/ScopedSignalHandler.h
new file mode 100644
index 0000000..dd5823f
--- /dev/null
+++ b/libc/private/ScopedSignalHandler.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2012 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.
+ */
+
+#pragma once
+
+#include <signal.h>
+
+class ScopedSignalHandler {
+ public:
+ ScopedSignalHandler(int signal_number, void (*handler)(int), int sa_flags = 0)
+ : signal_number_(signal_number) {
+ action_ = { .sa_flags = sa_flags, .sa_handler = handler };
+ sigaction64(signal_number_, &action_, &old_action_);
+ }
+
+ ScopedSignalHandler(int signal_number, void (*action)(int, siginfo_t*, void*),
+ int sa_flags = SA_SIGINFO)
+ : signal_number_(signal_number) {
+ action_ = { .sa_flags = sa_flags, .sa_sigaction = action };
+ sigaction64(signal_number_, &action_, &old_action_);
+ }
+
+ ScopedSignalHandler(int signal_number) : signal_number_(signal_number) {
+ sigaction64(signal_number, nullptr, &old_action_);
+ }
+
+ ~ScopedSignalHandler() {
+ sigaction64(signal_number_, &old_action_, nullptr);
+ }
+
+ struct sigaction64 action_;
+ struct sigaction64 old_action_;
+ const int signal_number_;
+};
diff --git a/libc/upstream-openbsd/lib/libc/stdlib/system.c b/libc/upstream-openbsd/lib/libc/stdlib/system.c
deleted file mode 100644
index de32d43..0000000
--- a/libc/upstream-openbsd/lib/libc/stdlib/system.c
+++ /dev/null
@@ -1,78 +0,0 @@
-/* $OpenBSD: system.c,v 1.11 2015/10/23 04:44:41 guenther Exp $ */
-/*
- * Copyright (c) 1988 The Regents of the University of California.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. 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.
- * 3. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS 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 REGENTS 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 <sys/types.h>
-#include <sys/wait.h>
-#include <errno.h>
-#include <signal.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <paths.h>
-
-extern char **environ;
-
-int
-system(const char *command)
-{
- pid_t pid, cpid;
- struct sigaction intsave, quitsave;
- sigset_t mask, omask;
- int pstat;
- char *argp[] = {"sh", "-c", NULL, NULL};
-
- if (!command) /* just checking... */
- return(1);
-
- argp[2] = (char *)command;
-
- sigemptyset(&mask);
- sigaddset(&mask, SIGCHLD);
- sigprocmask(SIG_BLOCK, &mask, &omask);
- switch (cpid = vfork()) {
- case -1: /* error */
- sigprocmask(SIG_SETMASK, &omask, NULL);
- return(-1);
- case 0: /* child */
- sigprocmask(SIG_SETMASK, &omask, NULL);
- execve(_PATH_BSHELL, argp, environ);
- _exit(127);
- }
-
- sigaction(SIGINT, NULL, &intsave);
- sigaction(SIGQUIT, NULL, &quitsave);
- do {
- pid = waitpid(cpid, &pstat, 0);
- } while (pid == -1 && errno == EINTR);
- sigprocmask(SIG_SETMASK, &omask, NULL);
- sigaction(SIGINT, &intsave, NULL);
- sigaction(SIGQUIT, &quitsave, NULL);
- return (pid == -1 ? -1 : pstat);
-}
-DEF_STRONG(system);
diff --git a/tests/ScopedSignalHandler.h b/tests/ScopedSignalHandler.h
deleted file mode 100644
index 36bbf10..0000000
--- a/tests/ScopedSignalHandler.h
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- * Copyright (C) 2012 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 _BIONIC_TESTS_SCOPED_SIGNAL_HANDLER_H
-#define _BIONIC_TESTS_SCOPED_SIGNAL_HANDLER_H
-
-#include <signal.h>
-#include <string.h>
-
-#if defined(__GLIBC__)
-#define posix_spawnattr_getsigdefault64 posix_spawnattr_getsigdefault
-#define posix_spawnattr_getsigmask64 posix_spawnattr_getsigmask
-#define posix_spawnattr_setsigdefault64 posix_spawnattr_setsigdefault
-#define posix_spawnattr_setsigmask64 posix_spawnattr_setsigmask
-#define pthread_sigmask64 pthread_sigmask
-#define sigaction64 sigaction
-#define sigaddset64 sigaddset
-#define sigdelset64 sigdelset
-#define sigemptyset64 sigemptyset
-#define sigfillset64 sigfillset
-#define sigismember64 sigismember
-#define sigpending64 sigpending
-#define sigprocmask64 sigprocmask
-#define sigset64_t sigset_t
-#define sigsuspend64 sigsuspend
-#define sigtimedwait64 sigtimedwait
-#define sigwait64 sigwait
-#define sigwaitinfo64 sigwaitinfo
-#endif
-
-class ScopedSignalHandler {
- public:
- ScopedSignalHandler(int signal_number, void (*handler)(int), int sa_flags = 0)
- : signal_number_(signal_number) {
- memset(&action_, 0, sizeof(action_));
- action_.sa_flags = sa_flags;
- action_.sa_handler = handler;
- sigaction64(signal_number_, &action_, &old_action_);
- }
-
- ScopedSignalHandler(int signal_number, void (*action)(int, siginfo_t*, void*),
- int sa_flags = SA_SIGINFO)
- : signal_number_(signal_number) {
- memset(&action_, 0, sizeof(action_));
- action_.sa_flags = sa_flags;
- action_.sa_sigaction = action;
- sigaction64(signal_number_, &action_, &old_action_);
- }
-
- ScopedSignalHandler(int signal_number) : signal_number_(signal_number) {
- sigaction64(signal_number, nullptr, &old_action_);
- }
-
- ~ScopedSignalHandler() {
- sigaction64(signal_number_, &old_action_, NULL);
- }
-
- private:
- struct sigaction64 action_;
- struct sigaction64 old_action_;
- const int signal_number_;
-};
-
-class SignalMaskRestorer {
- public:
- SignalMaskRestorer() {
- sigprocmask64(SIG_SETMASK, nullptr, &old_mask_);
- }
-
- ~SignalMaskRestorer() {
- sigprocmask64(SIG_SETMASK, &old_mask_, nullptr);
- }
-
- private:
- sigset64_t old_mask_;
-};
-
-#endif // _BIONIC_TESTS_SCOPED_SIGNAL_HANDLER_H
diff --git a/tests/SignalUtils.h b/tests/SignalUtils.h
new file mode 100644
index 0000000..ece28ba
--- /dev/null
+++ b/tests/SignalUtils.h
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2012 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.
+ */
+
+#pragma once
+
+#include <signal.h>
+#include <string.h>
+
+#if defined(__GLIBC__)
+#define posix_spawnattr_getsigdefault64 posix_spawnattr_getsigdefault
+#define posix_spawnattr_getsigmask64 posix_spawnattr_getsigmask
+#define posix_spawnattr_setsigdefault64 posix_spawnattr_setsigdefault
+#define posix_spawnattr_setsigmask64 posix_spawnattr_setsigmask
+#define pthread_sigmask64 pthread_sigmask
+#define sigaction64 sigaction
+#define sigaddset64 sigaddset
+#define sigdelset64 sigdelset
+#define sigemptyset64 sigemptyset
+#define sigfillset64 sigfillset
+#define sigismember64 sigismember
+#define sigpending64 sigpending
+#define sigprocmask64 sigprocmask
+#define sigset64_t sigset_t
+#define sigsuspend64 sigsuspend
+#define sigtimedwait64 sigtimedwait
+#define sigwait64 sigwait
+#define sigwaitinfo64 sigwaitinfo
+#endif
+
+#include "private/ScopedSignalHandler.h"
+
+class SignalMaskRestorer {
+ public:
+ SignalMaskRestorer() {
+ sigprocmask64(SIG_SETMASK, nullptr, &old_mask_);
+ }
+
+ ~SignalMaskRestorer() {
+ sigprocmask64(SIG_SETMASK, &old_mask_, nullptr);
+ }
+
+ private:
+ sigset64_t old_mask_;
+};
diff --git a/tests/fenv_test.cpp b/tests/fenv_test.cpp
index 9fc7d96..d27808f 100644
--- a/tests/fenv_test.cpp
+++ b/tests/fenv_test.cpp
@@ -16,7 +16,6 @@
#include <gtest/gtest.h>
-#include "ScopedSignalHandler.h"
#include "utils.h"
#include <fenv.h>
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index dd7879c..523ed63 100644
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -40,7 +40,7 @@
#include "private/bionic_constants.h"
#include "private/bionic_macros.h"
#include "BionicDeathTest.h"
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
#include "utils.h"
TEST(pthread, pthread_key_create) {
diff --git a/tests/semaphore_test.cpp b/tests/semaphore_test.cpp
index 24a2dbe..b216ec9 100644
--- a/tests/semaphore_test.cpp
+++ b/tests/semaphore_test.cpp
@@ -24,7 +24,7 @@
#include <unistd.h>
#include "private/bionic_constants.h"
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
TEST(semaphore, sem_init) {
sem_t s;
diff --git a/tests/setjmp_test.cpp b/tests/setjmp_test.cpp
index 4f5e60c..a49e910 100644
--- a/tests/setjmp_test.cpp
+++ b/tests/setjmp_test.cpp
@@ -19,7 +19,7 @@
#include <setjmp.h>
#include <stdlib.h>
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
TEST(setjmp, setjmp_smoke) {
int value;
diff --git a/tests/signal_test.cpp b/tests/signal_test.cpp
index b37d06f..53c91a2 100644
--- a/tests/signal_test.cpp
+++ b/tests/signal_test.cpp
@@ -24,7 +24,7 @@
#include <gtest/gtest.h>
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
static int SIGNAL_MIN() {
return 1; // Signals start at 1 (SIGHUP), not 0.
diff --git a/tests/spawn_test.cpp b/tests/spawn_test.cpp
index 84df16d..86175f9 100644
--- a/tests/spawn_test.cpp
+++ b/tests/spawn_test.cpp
@@ -20,7 +20,7 @@
#include <fcntl.h>
#include <gtest/gtest.h>
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
#include "utils.h"
#include <android-base/file.h>
diff --git a/tests/stack_unwinding_test.cpp b/tests/stack_unwinding_test.cpp
index bb58ae4..0ff6f30 100644
--- a/tests/stack_unwinding_test.cpp
+++ b/tests/stack_unwinding_test.cpp
@@ -29,7 +29,7 @@
#include <unistd.h>
#include <unwind.h>
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
#define noinline __attribute__((__noinline__))
#define __unused __attribute__((__unused__))
diff --git a/tests/sys_signalfd_test.cpp b/tests/sys_signalfd_test.cpp
index 67a72e0..179fcf9 100644
--- a/tests/sys_signalfd_test.cpp
+++ b/tests/sys_signalfd_test.cpp
@@ -36,7 +36,7 @@
#include <thread>
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
static void TestSignalFd(int fd, int signal) {
ASSERT_NE(-1, fd) << strerror(errno);
diff --git a/tests/time_test.cpp b/tests/time_test.cpp
index ff427a6..3196f34 100644
--- a/tests/time_test.cpp
+++ b/tests/time_test.cpp
@@ -26,7 +26,7 @@
#include <unistd.h>
#include <atomic>
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
#include "utils.h"
#include "private/bionic_constants.h"
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index 912ea0c..db4859b 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -17,7 +17,7 @@
#include <gtest/gtest.h>
#include "BionicDeathTest.h"
-#include "ScopedSignalHandler.h"
+#include "SignalUtils.h"
#include "TemporaryFile.h"
#include "utils.h"