[compiler-rt] [GWP-ASan] Various test fixes. (PR #94938)

Mitch Phillips via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 10 01:52:54 PDT 2024


https://github.com/hctim created https://github.com/llvm/llvm-project/pull/94938

When running some tests with --gtest_repeat=100 --gtest_shuffle, I
encountered some problems because the allocator wasn't torn down
completely, and the singleton pointer ended up pointing to a
use-after-scope'd object.

This patch has a couple of fixes and niceties:
 1. Removing the once-init stuff from tests, now that it's implicitly
    done in GuardedPoolAllocator::installAtFork() anyway.
 2. Calling uninitTestOnly() in the late_init test.
 3. Resetting the HasReportedBadPoolAccess when the signal handlers are
    installed (allowing for --gtest_repeat w/ recoverable mode).
 4. Adding a check and resetting the singleton pointer in
    uninitTestOnly().

>From 9a1ede202bce250f973f44bec68662f6a25e9fea Mon Sep 17 00:00:00 2001
From: Mitch Phillips <mitchp at google.com>
Date: Mon, 10 Jun 2024 10:48:58 +0200
Subject: [PATCH] [GWP-ASan] Various test fixes.

When running some tests with --gtest_repeat=100 --gtest_shuffle, I
encountered some problems because the allocator wasn't torn down
completely, and the singleton pointer ended up pointing to a
use-after-scope'd object.

This patch has a couple of fixes and niceties:
 1. Removing the once-init stuff from tests, now that it's implicitly
    done in GuardedPoolAllocator::installAtFork() anyway.
 2. Calling uninitTestOnly() in the late_init test.
 3. Resetting the HasReportedBadPoolAccess when the signal handlers are
    installed (allowing for --gtest_repeat w/ recoverable mode).
 4. Adding a check and resetting the singleton pointer in
    uninitTestOnly().
---
 .../lib/gwp_asan/guarded_pool_allocator.cpp        |  3 +++
 .../lib/gwp_asan/optional/segv_handler_posix.cpp   |  1 +
 compiler-rt/lib/gwp_asan/tests/harness.cpp         |  9 ---------
 compiler-rt/lib/gwp_asan/tests/harness.h           | 14 --------------
 compiler-rt/lib/gwp_asan/tests/late_init.cpp       |  1 +
 5 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp b/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp
index 9017ab7cf7ac0..5f41e1eb8a4b4 100644
--- a/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp
+++ b/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp
@@ -57,6 +57,8 @@ void GuardedPoolAllocator::init(const options::Options &Opts) {
   Check(Opts.MaxSimultaneousAllocations >= 0,
         "GWP-ASan Error: MaxSimultaneousAllocations is < 0.");
 
+  Check(SingletonPtr == nullptr,
+        "There's already a live GuardedPoolAllocator!");
   SingletonPtr = this;
   Backtrace = Opts.Backtrace;
 
@@ -158,6 +160,7 @@ void GuardedPoolAllocator::uninitTestOnly() {
     FreeSlots = nullptr;
   }
   *getThreadLocals() = ThreadLocalPackedVariables();
+  SingletonPtr = nullptr;
 }
 
 // Note, minimum backing allocation size in GWP-ASan is always one page, and
diff --git a/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp b/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
index 198db5cb074cb..ae4493723a47e 100644
--- a/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
+++ b/compiler-rt/lib/gwp_asan/optional/segv_handler_posix.cpp
@@ -257,6 +257,7 @@ void installSignalHandlers(gwp_asan::GuardedPoolAllocator *GPA, Printf_t Printf,
   Action.sa_flags = SA_SIGINFO;
   sigaction(SIGSEGV, &Action, &PreviousHandler);
   SignalHandlerInstalled = true;
+  HasReportedBadPoolAccess = false;
 }
 
 void uninstallSignalHandlers() {
diff --git a/compiler-rt/lib/gwp_asan/tests/harness.cpp b/compiler-rt/lib/gwp_asan/tests/harness.cpp
index 7ed408bedc2c3..95fa98f203989 100644
--- a/compiler-rt/lib/gwp_asan/tests/harness.cpp
+++ b/compiler-rt/lib/gwp_asan/tests/harness.cpp
@@ -10,15 +10,6 @@
 
 #include <string>
 
-namespace gwp_asan {
-namespace test {
-bool OnlyOnce() {
-  static int x = 0;
-  return !x++;
-}
-} // namespace test
-} // namespace gwp_asan
-
 // Optnone to ensure that the calls to these functions are not optimized away,
 // as we're looking for them in the backtraces.
 __attribute__((optnone)) char *
diff --git a/compiler-rt/lib/gwp_asan/tests/harness.h b/compiler-rt/lib/gwp_asan/tests/harness.h
index ae39a443bd0a0..c96f846996d35 100644
--- a/compiler-rt/lib/gwp_asan/tests/harness.h
+++ b/compiler-rt/lib/gwp_asan/tests/harness.h
@@ -39,10 +39,6 @@ namespace test {
 // `optional/printf_sanitizer_common.cpp` which supplies the __sanitizer::Printf
 // for this purpose.
 Printf_t getPrintfFunction();
-
-// First call returns true, all the following calls return false.
-bool OnlyOnce();
-
 }; // namespace test
 }; // namespace gwp_asan
 
@@ -57,10 +53,7 @@ class DefaultGuardedPoolAllocator : public ::testing::Test {
 public:
   void SetUp() override {
     gwp_asan::options::Options Opts;
-    Opts.setDefaults();
     MaxSimultaneousAllocations = Opts.MaxSimultaneousAllocations;
-
-    Opts.InstallForkHandlers = gwp_asan::test::OnlyOnce();
     GPA.init(Opts);
   }
 
@@ -78,12 +71,8 @@ class CustomGuardedPoolAllocator : public ::testing::Test {
   InitNumSlots(decltype(gwp_asan::options::Options::MaxSimultaneousAllocations)
                    MaxSimultaneousAllocationsArg) {
     gwp_asan::options::Options Opts;
-    Opts.setDefaults();
-
     Opts.MaxSimultaneousAllocations = MaxSimultaneousAllocationsArg;
     MaxSimultaneousAllocations = MaxSimultaneousAllocationsArg;
-
-    Opts.InstallForkHandlers = gwp_asan::test::OnlyOnce();
     GPA.init(Opts);
   }
 
@@ -100,10 +89,7 @@ class BacktraceGuardedPoolAllocator
 public:
   void SetUp() override {
     gwp_asan::options::Options Opts;
-    Opts.setDefaults();
-
     Opts.Backtrace = gwp_asan::backtrace::getBacktraceFunction();
-    Opts.InstallForkHandlers = gwp_asan::test::OnlyOnce();
     GPA.init(Opts);
 
     // In recoverable mode, capture GWP-ASan logs to an internal buffer so that
diff --git a/compiler-rt/lib/gwp_asan/tests/late_init.cpp b/compiler-rt/lib/gwp_asan/tests/late_init.cpp
index 8a947270c2d65..bebe1de8133fd 100644
--- a/compiler-rt/lib/gwp_asan/tests/late_init.cpp
+++ b/compiler-rt/lib/gwp_asan/tests/late_init.cpp
@@ -22,4 +22,5 @@ TEST(LateInit, CheckLateInitIsOK) {
 
   GPA.init(Opts);
   EXPECT_TRUE(GPA.shouldSample());
+  GPA.uninitTestOnly();
 }



More information about the llvm-commits mailing list