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/tests/ScopedSignalHandler.h b/tests/ScopedSignalHandler.h
index 8998d0d..71f22dc 100644
--- a/tests/ScopedSignalHandler.h
+++ b/tests/ScopedSignalHandler.h
@@ -53,13 +53,13 @@
const int signal_number_;
};
-class ScopedSignalMask {
+class SignalMaskRestorer {
public:
- ScopedSignalMask() {
+ SignalMaskRestorer() {
sigprocmask(SIG_SETMASK, nullptr, &old_mask_);
}
- ~ScopedSignalMask() {
+ ~SignalMaskRestorer() {
sigprocmask(SIG_SETMASK, &old_mask_, nullptr);
}
diff --git a/tests/signal_test.cpp b/tests/signal_test.cpp
index 207c156..5cbec88 100644
--- a/tests/signal_test.cpp
+++ b/tests/signal_test.cpp
@@ -464,33 +464,45 @@
ASSERT_EQ(EINVAL, errno);
}
-TEST(signal, sighold_sigpause_sigrelse) {
- static int sigalrm_handler_call_count;
- auto sigalrm_handler = [](int) { sigalrm_handler_call_count++; };
- ScopedSignalHandler sigalrm{SIGALRM, sigalrm_handler};
- ScopedSignalMask mask;
+static void TestSigholdSigpauseSigrelse(int sig) {
+ static int signal_handler_call_count = 0;
+ ScopedSignalHandler ssh{sig, [](int) { signal_handler_call_count++; }};
+ SignalMaskRestorer mask_restorer;
sigset_t set;
- // sighold(SIGALRM) should add SIGALRM to the signal mask ...
- ASSERT_EQ(0, sighold(SIGALRM));
+ // sighold(SIGALRM/SIGRTMIN) should add SIGALRM/SIGRTMIN to the signal mask ...
+ ASSERT_EQ(0, sighold(sig));
ASSERT_EQ(0, sigprocmask(SIG_SETMASK, 0, &set));
- EXPECT_TRUE(sigismember(&set, SIGALRM));
+ EXPECT_TRUE(sigismember(&set, sig));
- // ... preventing our SIGALRM handler from running ...
- raise(SIGALRM);
- ASSERT_EQ(0, sigalrm_handler_call_count);
- // ... until sigpause(SIGALRM) temporarily unblocks it.
- ASSERT_EQ(-1, sigpause(SIGALRM));
+ // ... preventing our SIGALRM/SIGRTMIN handler from running ...
+ raise(sig);
+ ASSERT_EQ(0, signal_handler_call_count);
+ // ... until sigpause(SIGALRM/SIGRTMIN) temporarily unblocks it.
+ ASSERT_EQ(-1, sigpause(sig));
ASSERT_EQ(EINTR, errno);
- ASSERT_EQ(1, sigalrm_handler_call_count);
+ ASSERT_EQ(1, signal_handler_call_count);
- // But sigpause(SIGALRM) shouldn't permanently unblock SIGALRM.
- ASSERT_EQ(0, sigprocmask(SIG_SETMASK, 0, &set));
- EXPECT_TRUE(sigismember(&set, SIGALRM));
+ if (sig >= SIGRTMIN && sizeof(void*) == 8) {
+ // But sigpause(SIGALRM/SIGRTMIN) shouldn't permanently unblock SIGALRM/SIGRTMIN.
+ ASSERT_EQ(0, sigprocmask(SIG_SETMASK, 0, &set));
+ EXPECT_TRUE(sigismember(&set, sig));
- ASSERT_EQ(0, sigrelse(SIGALRM));
- ASSERT_EQ(0, sigprocmask(SIG_SETMASK, 0, &set));
- EXPECT_FALSE(sigismember(&set, SIGALRM));
+ // Whereas sigrelse(SIGALRM/SIGRTMIN) should.
+ ASSERT_EQ(0, sigrelse(sig));
+ ASSERT_EQ(0, sigprocmask(SIG_SETMASK, 0, &set));
+ EXPECT_FALSE(sigismember(&set, sig));
+ } else {
+ // sigismember won't work for SIGRTMIN on LP32.
+ }
+}
+
+TEST(signal, sighold_sigpause_sigrelse) {
+ TestSigholdSigpauseSigrelse(SIGALRM);
+}
+
+TEST(signal, sighold_sigpause_sigrelse_RT) {
+ TestSigholdSigpauseSigrelse(SIGRTMIN);
}
TEST(signal, sigset_EINVAL) {
@@ -499,23 +511,48 @@
ASSERT_EQ(EINVAL, errno);
}
-TEST(signal, sigset) {
- auto sigalrm_handler = [](int) { };
- ScopedSignalHandler sigalrm{SIGALRM, sigalrm_handler};
- ScopedSignalMask mask;
+TEST(signal, sigset_RT) {
+ static int signal_handler_call_count = 0;
+ auto signal_handler = [](int) { signal_handler_call_count++; };
+ ScopedSignalHandler ssh{SIGRTMIN, signal_handler};
+ SignalMaskRestorer mask_restorer;
- // block SIGALRM so the next sigset(SIGARLM) call will return SIG_HOLD
- sigset_t sigalrm_set;
- sigemptyset(&sigalrm_set);
- sigaddset(&sigalrm_set, SIGALRM);
- ASSERT_EQ(0, sigprocmask(SIG_BLOCK, &sigalrm_set, nullptr));
-
+ ASSERT_EQ(signal_handler, sigset(SIGRTMIN, SIG_HOLD));
+#if defined(__LP64__)
sigset_t set;
- ASSERT_EQ(SIG_HOLD, sigset(SIGALRM, sigalrm_handler));
+ ASSERT_EQ(0, sigprocmask(SIG_BLOCK, nullptr, &set));
+ ASSERT_TRUE(sigismember(&set, SIGRTMIN));
+#endif
+
+ ASSERT_EQ(SIG_HOLD, sigset(SIGRTMIN, signal_handler));
+ ASSERT_EQ(signal_handler, sigset(SIGRTMIN, signal_handler));
+ ASSERT_EQ(0, signal_handler_call_count);
+ raise(SIGRTMIN);
+ ASSERT_EQ(1, signal_handler_call_count);
+}
+
+TEST(signal, sigset) {
+ static int signal_handler_call_count = 0;
+ auto signal_handler = [](int) { signal_handler_call_count++; };
+ ScopedSignalHandler ssh{SIGALRM, signal_handler};
+ SignalMaskRestorer mask_restorer;
+
+ ASSERT_EQ(0, signal_handler_call_count);
+ raise(SIGALRM);
+ ASSERT_EQ(1, signal_handler_call_count);
+
+ // Block SIGALRM so the next sigset(SIGARLM) call will return SIG_HOLD.
+ sigset_t set;
+ sigemptyset(&set);
+ sigaddset(&set, SIGALRM);
+ ASSERT_EQ(0, sigprocmask(SIG_BLOCK, &set, nullptr));
+
+ sigemptyset(&set);
+ ASSERT_EQ(SIG_HOLD, sigset(SIGALRM, signal_handler));
ASSERT_EQ(0, sigprocmask(SIG_BLOCK, nullptr, &set));
EXPECT_FALSE(sigismember(&set, SIGALRM));
- ASSERT_EQ(sigalrm_handler, sigset(SIGALRM, SIG_IGN));
+ ASSERT_EQ(signal_handler, sigset(SIGALRM, SIG_IGN));
ASSERT_EQ(0, sigprocmask(SIG_BLOCK, nullptr, &set));
EXPECT_FALSE(sigismember(&set, SIGALRM));
diff --git a/tests/spawn_test.cpp b/tests/spawn_test.cpp
index d2e4ea1..dfce0dc 100644
--- a/tests/spawn_test.cpp
+++ b/tests/spawn_test.cpp
@@ -376,6 +376,7 @@
sigset_t just_SIGALRM;
sigemptyset(&just_SIGALRM);
sigaddset(&just_SIGALRM, SIGALRM);
+
ASSERT_EQ(0, posix_spawnattr_setsigdefault(&sa, &just_SIGALRM));
ASSERT_EQ(0, posix_spawnattr_setflags(&sa, POSIX_SPAWN_SETSIGDEF));
@@ -393,15 +394,18 @@
// child without first defaulting any caught signals (http://b/68707996).
static pid_t parent = getpid();
+ setpgid(0, 0);
+
pid_t pid = fork();
ASSERT_NE(-1, pid);
if (pid == 0) {
+ signal(SIGRTMIN, SIG_IGN);
for (size_t i = 0; i < 1024; ++i) {
- kill(0, SIGWINCH);
+ kill(0, SIGRTMIN);
usleep(10);
}
- return;
+ _exit(99);
}
// We test both with and without attributes, because they used to be
@@ -417,11 +421,15 @@
posix_spawnattr_t* attrs[] = { nullptr, &attr1, &attr2 };
- ScopedSignalHandler ssh(SIGWINCH, [](int) { ASSERT_EQ(getpid(), parent); });
+ // We use a real-time signal because that's a tricky case for LP32
+ // because our sigset_t was too small.
+ ScopedSignalHandler ssh(SIGRTMIN, [](int) { ASSERT_EQ(getpid(), parent); });
ExecTestHelper eth;
eth.SetArgs({"true", nullptr});
for (size_t i = 0; i < 128; ++i) {
posix_spawn(nullptr, "true", nullptr, attrs[i % 3], eth.GetArgs(), nullptr);
}
+
+ AssertChildExited(pid, 99);
}