Skip to content

[rtsan] Prune rtsan context and assertions tests #109503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

davidtrevelyan
Copy link
Contributor

Disentangles (and simplifies) integration-like tests for __rtsan::ExpectNotRealtime and __rtsan::Context into simpler unit tests. None of the tests are new, but their assertions have changed to reflect the more direct testing strategy.

@davidtrevelyan
Copy link
Contributor Author

@cjappl for review

@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (davidtrevelyan)

Changes

Disentangles (and simplifies) integration-like tests for __rtsan::ExpectNotRealtime and __rtsan::Context into simpler unit tests. None of the tests are new, but their assertions have changed to reflect the more direct testing strategy.


Full diff: https://github.com/llvm/llvm-project/pull/109503.diff

3 Files Affected:

  • (modified) compiler-rt/lib/rtsan/tests/CMakeLists.txt (+1)
  • (added) compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp (+42)
  • (modified) compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp (+55-34)
diff --git a/compiler-rt/lib/rtsan/tests/CMakeLists.txt b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
index 0529917bc895ce..139eea785fcdca 100644
--- a/compiler-rt/lib/rtsan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/rtsan/tests/CMakeLists.txt
@@ -21,6 +21,7 @@ set(RTSAN_INST_TEST_SOURCES
 
 set(RTSAN_NOINST_TEST_SOURCES
     ../rtsan_preinit.cpp
+    rtsan_test_assertions.cpp
     rtsan_test_context.cpp
     rtsan_test_main.cpp)
 
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
new file mode 100644
index 00000000000000..cdf2ac32170043
--- /dev/null
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
@@ -0,0 +1,42 @@
+//===--- rtsan_test_assertions.cpp - Realtime Sanitizer ---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Part of the RealtimeSanitizer runtime library test suite
+//
+//===----------------------------------------------------------------------===//
+
+#include "rtsan_test_utilities.h"
+
+#include "rtsan/rtsan_assertions.h"
+
+#include <gtest/gtest.h>
+
+class TestRtsanAssertions : public ::testing::Test {
+protected:
+  void SetUp() override { __rtsan_ensure_initialized(); }
+};
+
+TEST_F(TestRtsanAssertions, ExpectNotRealtimeDoesNotDieIfNotInRealtimeContext) {
+  __rtsan::Context context{};
+  ASSERT_FALSE(context.InRealtimeContext());
+  ExpectNotRealtime(context, "fake_function_name");
+}
+
+TEST_F(TestRtsanAssertions, ExpectNotRealtimeDiesIfInRealtimeContext) {
+  __rtsan::Context context{};
+  context.RealtimePush();
+  ASSERT_TRUE(context.InRealtimeContext());
+  EXPECT_DEATH(ExpectNotRealtime(context, "fake_function_name"), "");
+}
+
+TEST_F(TestRtsanAssertions, ExpectNotRealtimeDoesNotDieIfRealtimeButBypassed) {
+  __rtsan::Context context{};
+  context.RealtimePush();
+  context.BypassPush();
+  ExpectNotRealtime(context, "fake_function_name");
+}
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp
index a7a96161cf2b54..9ba33185bde28e 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp
@@ -11,66 +11,87 @@
 #include "rtsan_test_utilities.h"
 
 #include "rtsan/rtsan.h"
-#include "rtsan/rtsan_assertions.h"
 #include "rtsan/rtsan_context.h"
 
 #include <gtest/gtest.h>
 
-class TestRtsanContext : public ::testing::Test {
+using namespace ::testing;
+
+class TestRtsanContext : public Test {
 protected:
   void SetUp() override { __rtsan_ensure_initialized(); }
 };
 
-TEST_F(TestRtsanContext, ExpectNotRealtimeDoesNotDieBeforeRealtimePush) {
+TEST_F(TestRtsanContext, IsNotRealtimeAfterDefaultConstruction) {
   __rtsan::Context context{};
-  ExpectNotRealtime(context, "do_some_stuff");
+  EXPECT_THAT(context.InRealtimeContext(), Eq(false));
 }
 
-TEST_F(TestRtsanContext, ExpectNotRealtimeDoesNotDieAfterPushAndPop) {
+TEST_F(TestRtsanContext, IsRealtimeAfterRealtimePush) {
   __rtsan::Context context{};
   context.RealtimePush();
-  context.RealtimePop();
-  ExpectNotRealtime(context, "do_some_stuff");
+  EXPECT_THAT(context.InRealtimeContext(), Eq(true));
 }
 
-TEST_F(TestRtsanContext, ExpectNotRealtimeDiesAfterRealtimePush) {
+TEST_F(TestRtsanContext, IsNotRealtimeAfterRealtimePushAndPop) {
   __rtsan::Context context{};
-
   context.RealtimePush();
-  EXPECT_DEATH(ExpectNotRealtime(context, "do_some_stuff"), "");
+  ASSERT_THAT(context.InRealtimeContext(), Eq(true));
+  context.RealtimePop();
+  EXPECT_THAT(context.InRealtimeContext(), Eq(false));
 }
 
-TEST_F(TestRtsanContext,
-       ExpectNotRealtimeDiesAfterRealtimeAfterMorePushesThanPops) {
+TEST_F(TestRtsanContext, RealtimeContextStateIsStatefullyTracked) {
   __rtsan::Context context{};
-
-  context.RealtimePush();
-  context.RealtimePush();
-  context.RealtimePush();
-  context.RealtimePop();
-  context.RealtimePop();
-  EXPECT_DEATH(ExpectNotRealtime(context, "do_some_stuff"), "");
+  auto const expect_rt = [&context](bool is_rt) {
+    EXPECT_THAT(context.InRealtimeContext(), Eq(is_rt));
+  };
+  expect_rt(false);
+  context.RealtimePush(); // depth 1
+  expect_rt(true);
+  context.RealtimePush(); // depth 2
+  expect_rt(true);
+  context.RealtimePop(); // depth 1
+  expect_rt(true);
+  context.RealtimePush(); // depth 2
+  expect_rt(true);
+  context.RealtimePop(); // depth 1
+  expect_rt(true);
+  context.RealtimePop(); // depth 0
+  expect_rt(false);
+  context.RealtimePush(); // depth 1
+  expect_rt(true);
 }
 
-TEST_F(TestRtsanContext, ExpectNotRealtimeDoesNotDieAfterBypassPush) {
+TEST_F(TestRtsanContext, IsNotBypassedAfterDefaultConstruction) {
   __rtsan::Context context{};
+  EXPECT_THAT(context.IsBypassed(), Eq(false));
+}
 
-  context.RealtimePush();
+TEST_F(TestRtsanContext, IsBypassedAfterBypassPush) {
+  __rtsan::Context context{};
   context.BypassPush();
-  ExpectNotRealtime(context, "do_some_stuff");
+  EXPECT_THAT(context.IsBypassed(), Eq(true));
 }
 
-TEST_F(TestRtsanContext,
-       ExpectNotRealtimeDoesNotDieIfBypassDepthIsGreaterThanZero) {
+TEST_F(TestRtsanContext, BypassedStateIsStatefullyTracked) {
   __rtsan::Context context{};
-
-  context.RealtimePush();
-  context.BypassPush();
-  context.BypassPush();
-  context.BypassPush();
-  context.BypassPop();
-  context.BypassPop();
-  ExpectNotRealtime(context, "do_some_stuff");
-  context.BypassPop();
-  EXPECT_DEATH(ExpectNotRealtime(context, "do_some_stuff"), "");
+  auto const expect_bypassed = [&context](bool is_bypassed) {
+    EXPECT_THAT(context.IsBypassed(), Eq(is_bypassed));
+  };
+  expect_bypassed(false);
+  context.BypassPush(); // depth 1
+  expect_bypassed(true);
+  context.BypassPush(); // depth 2
+  expect_bypassed(true);
+  context.BypassPop(); // depth 1
+  expect_bypassed(true);
+  context.BypassPush(); // depth 2
+  expect_bypassed(true);
+  context.BypassPop(); // depth 1
+  expect_bypassed(true);
+  context.BypassPop(); // depth 0
+  expect_bypassed(false);
+  context.BypassPush(); // depth 1
+  expect_bypassed(true);
 }

@cjappl cjappl merged commit cf85b33 into llvm:main Sep 21, 2024
10 checks passed
@cjappl cjappl deleted the rtsan/unit-test-simplification branch September 21, 2024 02:55
@cjappl cjappl changed the title [rtsan][NFC] Prune rtsan context and assertions tests [rtsan] Prune rtsan context and assertions tests Sep 21, 2024
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 21, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building compiler-rt at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/5890

Here is the relevant piece of the build log for the reference
Step 10 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: sanitizer/ptr_outside_alloc_2.c' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 2
/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang -fopenmp    -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa -O3 /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/sanitizer/ptr_outside_alloc_2.c -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/ptr_outside_alloc_2.c.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa -O3 /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/sanitizer/ptr_outside_alloc_2.c -o /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/ptr_outside_alloc_2.c.tmp -Xoffload-linker -lc -Xoffload-linker -lm /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# RUN: at line 3
/home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/not --crash env -u LLVM_DISABLE_SYMBOLIZATION OFFLOAD_TRACK_ALLOCATION_TRACES=1 /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/ptr_outside_alloc_2.c.tmp 2>&1 | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/sanitizer/ptr_outside_alloc_2.c --check-prefixes=CHECK
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/not --crash env -u LLVM_DISABLE_SYMBOLIZATION OFFLOAD_TRACK_ALLOCATION_TRACES=1 /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/sanitizer/Output/ptr_outside_alloc_2.c.tmp
# executed command: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/sanitizer/ptr_outside_alloc_2.c --check-prefixes=CHECK
# .---command stderr------------
# | /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/sanitizer/ptr_outside_alloc_2.c:21:11: error: CHECK: expected string not found in input
# | // CHECK: OFFLOAD ERROR: Memory access fault by GPU {{.*}} (agent 0x{{.*}}) at virtual address [[PTR:0x[0-9a-z]*]]. Reasons: {{.*}}
# |           ^
# | <stdin>:1:1: note: scanning from here
# | AMDGPU error: Error in hsa_amd_memory_pool_allocate: HSA_STATUS_ERROR_OUT_OF_RESOURCES: The runtime failed to allocate the necessary resources. This error may also occur when the core runtime library needs to spawn threads or create internal OS-specific events.
# | ^
# | 
# | Input file: <stdin>
# | Check file: /home/ompworker/bbot/openmp-offload-libc-amdgpu-runtime/llvm.src/offload/test/sanitizer/ptr_outside_alloc_2.c
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |           1: AMDGPU error: Error in hsa_amd_memory_pool_allocate: HSA_STATUS_ERROR_OUT_OF_RESOURCES: The runtime failed to allocate the necessary resources. This error may also occur when the core runtime library needs to spawn threads or create internal OS-specific events. 
# | check:21     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
# |           2: AMDGPU error: Error in hsa_amd_memory_pool_allocate: HSA_STATUS_ERROR_OUT_OF_RESOURCES: The runtime failed to allocate the necessary resources. This error may also occur when the core runtime library needs to spawn threads or create internal OS-specific events. 
# | check:21     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           3: "PluginInterface" error: Failure to allocate device memory: Failed to allocate from memory manager 
# | check:21     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           4: omptarget error: Call to getTargetPointer returned null pointer (device failure or illegal mapping). 
# | check:21     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           5: omptarget error: Call to targetDataBegin failed, abort target. 
# | check:21     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           6: omptarget error: Failed to process data before launching the kernel. 
# | check:21     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           .
# |           .
# |           .
# | >>>>>>
# `-----------------------------
# error: command failed with exit status: 1

--

********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants