Fix UnwindTest repeatability.
- Rewrite some of the UnwindTest tests to properly wait for the process
to be ready.
- Add a TestScopedPidReaper to make sure that fork process get killed even
if the test fails. Add this to all tests that fail.
- Create a quiesce function to be used by all of the tests that will
wait after attaching to a process.
Bug: 65287279
Test: Ran unit tests on hikey960 board and on host repeatedly.
Change-Id: I57084120396f34d8dfb852f3d814bef2056f1b54
diff --git a/libunwindstack/tests/MemoryRemoteTest.cpp b/libunwindstack/tests/MemoryRemoteTest.cpp
index f8965b2..a66d0c5 100644
--- a/libunwindstack/tests/MemoryRemoteTest.cpp
+++ b/libunwindstack/tests/MemoryRemoteTest.cpp
@@ -22,7 +22,6 @@
#include <sys/mman.h>
#include <sys/ptrace.h>
#include <sys/types.h>
-#include <time.h>
#include <unistd.h>
#include <vector>
@@ -34,32 +33,18 @@
#include <unwindstack/Memory.h>
#include "MemoryFake.h"
+#include "TestUtils.h"
namespace unwindstack {
class MemoryRemoteTest : public ::testing::Test {
protected:
- static uint64_t NanoTime() {
- struct timespec t = { 0, 0 };
- clock_gettime(CLOCK_MONOTONIC, &t);
- return static_cast<uint64_t>(t.tv_sec * NS_PER_SEC + t.tv_nsec);
- }
-
static bool Attach(pid_t pid) {
if (ptrace(PTRACE_ATTACH, pid, 0, 0) == -1) {
return false;
}
- uint64_t start = NanoTime();
- siginfo_t si;
- while (TEMP_FAILURE_RETRY(ptrace(PTRACE_GETSIGINFO, pid, 0, &si)) < 0 && errno == ESRCH) {
- if ((NanoTime() - start) > 10 * NS_PER_SEC) {
- printf("%d: Failed to stop after 10 seconds.\n", pid);
- return false;
- }
- usleep(30);
- }
- return true;
+ return TestQuiescePid(pid);
}
static bool Detach(pid_t pid) {
@@ -79,6 +64,7 @@
exit(1);
}
ASSERT_LT(0, pid);
+ TestScopedPidReaper reap(pid);
ASSERT_TRUE(Attach(pid));
@@ -91,9 +77,6 @@
}
ASSERT_TRUE(Detach(pid));
-
- kill(pid, SIGKILL);
- ASSERT_EQ(pid, wait(nullptr));
}
TEST_F(MemoryRemoteTest, read_fail) {
@@ -111,6 +94,7 @@
exit(1);
}
ASSERT_LT(0, pid);
+ TestScopedPidReaper reap(pid);
ASSERT_TRUE(Attach(pid));
@@ -132,9 +116,6 @@
ASSERT_EQ(0, munmap(src, pagesize));
ASSERT_TRUE(Detach(pid));
-
- kill(pid, SIGKILL);
- ASSERT_EQ(pid, wait(nullptr));
}
TEST_F(MemoryRemoteTest, read_overflow) {
@@ -152,6 +133,7 @@
exit(1);
}
ASSERT_LT(0, pid);
+ TestScopedPidReaper reap(pid);
ASSERT_TRUE(Attach(pid));
@@ -162,9 +144,6 @@
ASSERT_FALSE(remote.Read(0, dst.data(), 100));
ASSERT_TRUE(Detach(pid));
-
- kill(pid, SIGKILL);
- ASSERT_EQ(pid, wait(nullptr));
}
} // namespace unwindstack
diff --git a/libunwindstack/tests/TestUtils.h b/libunwindstack/tests/TestUtils.h
new file mode 100644
index 0000000..8c31aa6
--- /dev/null
+++ b/libunwindstack/tests/TestUtils.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2017 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 _LIBUNWINDSTACK_TESTS_TEST_UTILS_H
+#define _LIBUNWINDSTACK_TESTS_TEST_UTILS_H
+
+#include <signal.h>
+#include <sys/ptrace.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+namespace unwindstack {
+
+class TestScopedPidReaper {
+ public:
+ TestScopedPidReaper(pid_t pid) : pid_(pid) {}
+ ~TestScopedPidReaper() {
+ kill(pid_, SIGKILL);
+ waitpid(pid_, nullptr, 0);
+ }
+
+ private:
+ pid_t pid_;
+};
+
+inline bool TestQuiescePid(pid_t pid) {
+ siginfo_t si;
+ bool ready = false;
+ // Wait for up to 5 seconds.
+ for (size_t i = 0; i < 5000; i++) {
+ if (ptrace(PTRACE_GETSIGINFO, pid, 0, &si) == 0) {
+ ready = true;
+ break;
+ }
+ usleep(1000);
+ }
+ return ready;
+}
+
+} // namespace unwindstack
+
+#endif // _LIBUNWINDSTACK_TESTS_TEST_UTILS_H
diff --git a/libunwindstack/tests/UnwindTest.cpp b/libunwindstack/tests/UnwindTest.cpp
index a0c45bd..a4f920a 100644
--- a/libunwindstack/tests/UnwindTest.cpp
+++ b/libunwindstack/tests/UnwindTest.cpp
@@ -15,10 +15,9 @@
*/
#include <errno.h>
-#include <string.h>
-
#include <signal.h>
#include <stdint.h>
+#include <string.h>
#include <sys/ptrace.h>
#include <sys/syscall.h>
#include <unistd.h>
@@ -39,14 +38,24 @@
#include <unwindstack/Regs.h>
#include <unwindstack/RegsGetLocal.h>
+#include "TestUtils.h"
+
namespace unwindstack {
-static std::atomic_bool g_ready(false);
-static volatile bool g_ready_for_remote = false;
-static volatile bool g_signal_ready_for_remote = false;
-static std::atomic_bool g_finish(false);
+static std::atomic_bool g_ready;
+static volatile bool g_ready_for_remote;
+static volatile bool g_signal_ready_for_remote;
+static std::atomic_bool g_finish;
static std::atomic_uintptr_t g_ucontext;
+static void ResetGlobals() {
+ g_ready = false;
+ g_ready_for_remote = false;
+ g_signal_ready_for_remote = false;
+ g_finish = false;
+ g_ucontext = 0;
+}
+
static std::vector<const char*> kFunctionOrder{"InnerFunction", "MiddleFunction", "OuterFunction"};
static std::vector<const char*> kFunctionSignalOrder{"SignalInnerFunction", "SignalMiddleFunction",
@@ -156,48 +165,52 @@
MiddleFunction(local);
}
-TEST(UnwindTest, local) {
+class UnwindTest : public ::testing::Test {
+ public:
+ void SetUp() override { ResetGlobals(); }
+};
+
+TEST_F(UnwindTest, local) {
OuterFunction(true);
}
void WaitForRemote(pid_t pid, uint64_t addr, bool leave_attached, bool* completed) {
*completed = false;
// Need to sleep before attempting first ptrace. Without this, on the
- // host it becomes impossible to attach and ptrace set errno to EPERM.
+ // host it becomes impossible to attach and ptrace sets errno to EPERM.
usleep(1000);
- for (size_t i = 0; i < 100; i++) {
- ASSERT_EQ(0, ptrace(PTRACE_ATTACH, pid, 0, 0));
- for (size_t j = 0; j < 100; j++) {
- siginfo_t si;
- if (ptrace(PTRACE_GETSIGINFO, pid, 0, &si) == 0) {
- MemoryRemote memory(pid);
- // Read the remote value to see if we are ready.
- bool value;
- if (memory.Read(addr, &value, sizeof(value)) && value) {
- *completed = true;
- break;
- }
+ for (size_t i = 0; i < 1000; i++) {
+ if (ptrace(PTRACE_ATTACH, pid, 0, 0) == 0) {
+ ASSERT_TRUE(TestQuiescePid(pid))
+ << "Waiting for process to quiesce failed: " << strerror(errno);
+
+ MemoryRemote memory(pid);
+ // Read the remote value to see if we are ready.
+ bool value;
+ if (memory.Read(addr, &value, sizeof(value)) && value) {
+ *completed = true;
}
- usleep(1000);
+ if (!*completed || !leave_attached) {
+ ASSERT_EQ(0, ptrace(PTRACE_DETACH, pid, 0, 0));
+ }
+ if (*completed) {
+ break;
+ }
+ } else {
+ ASSERT_EQ(ESRCH, errno) << "ptrace attach failed with unexpected error: " << strerror(errno);
}
- if (leave_attached && *completed) {
- break;
- }
- ASSERT_EQ(0, ptrace(PTRACE_DETACH, pid, 0, 0));
- if (*completed) {
- break;
- }
- usleep(1000);
+ usleep(5000);
}
}
-TEST(UnwindTest, remote) {
+TEST_F(UnwindTest, remote) {
pid_t pid;
if ((pid = fork()) == 0) {
OuterFunction(false);
exit(0);
}
ASSERT_NE(-1, pid);
+ TestScopedPidReaper reap(pid);
bool completed;
WaitForRemote(pid, reinterpret_cast<uint64_t>(&g_ready_for_remote), true, &completed);
@@ -210,13 +223,11 @@
VerifyUnwind(pid, &maps, regs.get(), kFunctionOrder);
- ASSERT_EQ(0, ptrace(PTRACE_DETACH, pid, 0, 0));
-
- kill(pid, SIGKILL);
- ASSERT_EQ(pid, wait(nullptr));
+ ASSERT_EQ(0, ptrace(PTRACE_DETACH, pid, 0, 0))
+ << "ptrace detach failed with unexpected error: " << strerror(errno);
}
-TEST(UnwindTest, from_context) {
+TEST_F(UnwindTest, from_context) {
std::atomic_int tid(0);
std::thread thread([&]() {
tid = syscall(__NR_gettid);
@@ -263,10 +274,6 @@
}
static void RemoteThroughSignal(unsigned int sa_flags) {
- g_ready = false;
- g_signal_ready_for_remote = false;
- g_finish = false;
-
pid_t pid;
if ((pid = fork()) == 0) {
struct sigaction act, oldact;
@@ -279,6 +286,7 @@
exit(0);
}
ASSERT_NE(-1, pid);
+ TestScopedPidReaper reap(pid);
bool completed;
WaitForRemote(pid, reinterpret_cast<uint64_t>(&g_ready_for_remote), false, &completed);
@@ -294,17 +302,15 @@
VerifyUnwind(pid, &maps, regs.get(), kFunctionSignalOrder);
- ASSERT_EQ(0, ptrace(PTRACE_DETACH, pid, 0, 0));
-
- kill(pid, SIGKILL);
- ASSERT_EQ(pid, wait(nullptr));
+ ASSERT_EQ(0, ptrace(PTRACE_DETACH, pid, 0, 0))
+ << "ptrace detach failed with unexpected error: " << strerror(errno);
}
-TEST(UnwindTest, remote_through_signal) {
+TEST_F(UnwindTest, remote_through_signal) {
RemoteThroughSignal(0);
}
-TEST(UnwindTest, remote_through_signal_sa_siginfo) {
+TEST_F(UnwindTest, remote_through_signal_sa_siginfo) {
RemoteThroughSignal(SA_SIGINFO);
}