pa-fazil
/
android_bionic
mirror of https://github.com/fazilsheik96/android_bionic.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix setjmp.bug_152210274 with HWASan/stack MTE enabled. 

This test was flaky because there was no synchronization between the end
of the test function scope and the jumper threads, resulting in a racy
use-after-scope when accessing the thread array. Fix it by joining the
threads before leaving the test function scope.

Replace the thread array access, which can race with the store to the
thread array in the main thread, with an access to a pre-initialized
variable, which acts as a kind of enforcement that the threads are done
before leaving the test.

Bug: 227390656
Change-Id: Icfbb0d7cae66c12e5ce31072c34529e3c5fdf563
This commit is contained in:
Peter Collingbourne 2023-03-21 22:20:22 -07:00
parent d2e55dd05b
commit 25a7c3fd57
1 changed files with 11 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
tests/setjmp_test.cpp
View File

@ -278,7 +278,6 @@ TEST(setjmp, setjmp_stack) {
} }
TEST(setjmp, bug_152210274) { TEST(setjmp, bug_152210274) {
SKIP_WITH_HWASAN; // b/227390656
// Ensure that we never have a mangled value in the stack pointer. // Ensure that we never have a mangled value in the stack pointer.
#if defined(__BIONIC__) #if defined(__BIONIC__)
struct sigaction sa = {.sa_flags = SA_SIGINFO, .sa_sigaction = [](int, siginfo_t*, void*) {}}; struct sigaction sa = {.sa_flags = SA_SIGINFO, .sa_sigaction = [](int, siginfo_t*, void*) {}};
@ -299,15 +298,19 @@ TEST(setjmp, bug_152210274) {
perror("setjmp"); perror("setjmp");
abort(); abort();
} }
if (*static_cast<pid_t*>(arg) == 100) longjmp(buf, 1); // This will never be true, but the compiler doesn't know that, so the
// setjmp won't be removed by DCE. With HWASan/MTE this also acts as a
// kind of enforcement that the threads are done before leaving the test.
if (*static_cast<size_t*>(arg) != 123) longjmp(buf, 1);
} }
return nullptr; return nullptr;
}; };
pthread_t threads[kNumThreads];
pid_t tids[kNumThreads] = {}; pid_t tids[kNumThreads] = {};
size_t var = 123;
for (size_t i = 0; i < kNumThreads; ++i) { for (size_t i = 0; i < kNumThreads; ++i) {
pthread_t t; ASSERT_EQ(0, pthread_create(&threads[i], nullptr, jumper, &var));
ASSERT_EQ(0, pthread_create(&t, nullptr, jumper, &tids[i])); tids[i] = pthread_gettid_np(threads[i]);
tids[i] = pthread_gettid_np(t);
} }
// Start the interrupter thread. // Start the interrupter thread.
@ -327,6 +330,9 @@ TEST(setjmp, bug_152210274) {
pthread_t t; pthread_t t;
ASSERT_EQ(0, pthread_create(&t, nullptr, interrupter, tids)); ASSERT_EQ(0, pthread_create(&t, nullptr, interrupter, tids));
pthread_join(t, nullptr); pthread_join(t, nullptr);
for (size_t i = 0; i < kNumThreads; i++) {
pthread_join(threads[i], nullptr);
}
#else #else
GTEST_SKIP() << "tests uses functions not in glibc"; GTEST_SKIP() << "tests uses functions not in glibc";
#endif #endif