[compiler-rt] [rtsan] Decouple assertions from error actions (PR #109535)

via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 21 11:04:49 PDT 2024


https://github.com/davidtrevelyan updated https://github.com/llvm/llvm-project/pull/109535

>From 630a2cf005993bf454f067503cfea86890ee1355 Mon Sep 17 00:00:00 2001
From: David Trevelyan <david.trevelyan at gmail.com>
Date: Sat, 21 Sep 2024 10:35:00 -0600
Subject: [PATCH 1/2] [rtsan] Decouple assertions from error actions

---
 compiler-rt/lib/rtsan/CMakeLists.txt          |  1 -
 compiler-rt/lib/rtsan/rtsan.cpp               | 25 ++++++++++------
 compiler-rt/lib/rtsan/rtsan_assertions.cpp    | 28 -----------------
 compiler-rt/lib/rtsan/rtsan_assertions.h      | 13 ++++++--
 .../lib/rtsan/tests/rtsan_test_assertions.cpp | 30 ++++++++++---------
 5 files changed, 43 insertions(+), 54 deletions(-)
 delete mode 100644 compiler-rt/lib/rtsan/rtsan_assertions.cpp

diff --git a/compiler-rt/lib/rtsan/CMakeLists.txt b/compiler-rt/lib/rtsan/CMakeLists.txt
index 68f890050c6ea8..0fc3a3f8f48960 100644
--- a/compiler-rt/lib/rtsan/CMakeLists.txt
+++ b/compiler-rt/lib/rtsan/CMakeLists.txt
@@ -2,7 +2,6 @@ include_directories(..)
 
 set(RTSAN_CXX_SOURCES
   rtsan.cpp
-  rtsan_assertions.cpp
   rtsan_context.cpp
   rtsan_diagnostics.cpp
   rtsan_flags.cpp
diff --git a/compiler-rt/lib/rtsan/rtsan.cpp b/compiler-rt/lib/rtsan/rtsan.cpp
index 936f0b5b8cee39..2afdf3c76696e7 100644
--- a/compiler-rt/lib/rtsan/rtsan.cpp
+++ b/compiler-rt/lib/rtsan/rtsan.cpp
@@ -10,6 +10,7 @@
 
 #include <rtsan/rtsan.h>
 #include <rtsan/rtsan_assertions.h>
+#include <rtsan/rtsan_diagnostics.h>
 #include <rtsan/rtsan_flags.h>
 #include <rtsan/rtsan_interceptors.h>
 
@@ -28,6 +29,13 @@ static void SetInitialized() {
   atomic_store(&rtsan_initialized, 1, memory_order_release);
 }
 
+static auto PrintDiagnosticsAndDieAction(DiagnosticsInfo info) {
+  return [info]() {
+    __rtsan::PrintDiagnostics(info);
+    Die();
+  };
+}
+
 extern "C" {
 
 SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_init() {
@@ -74,22 +82,21 @@ SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_enable() {
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE void
-__rtsan_notify_intercepted_call(const char *intercepted_function_name) {
+__rtsan_notify_intercepted_call(const char *func_name) {
   __rtsan_ensure_initialized();
-
   GET_CALLER_PC_BP;
-  DiagnosticsInfo info = {InterceptedCallInfo{intercepted_function_name}, pc,
-                          bp};
-  ExpectNotRealtime(GetContextForThisThread(), info);
+  ExpectNotRealtime(
+      GetContextForThisThread(),
+      PrintDiagnosticsAndDieAction({InterceptedCallInfo{func_name}, pc, bp}));
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE void
-__rtsan_notify_blocking_call(const char *blocking_function_name) {
+__rtsan_notify_blocking_call(const char *func_name) {
   __rtsan_ensure_initialized();
-
   GET_CALLER_PC_BP;
-  DiagnosticsInfo info = {BlockingCallInfo{blocking_function_name}, pc, bp};
-  ExpectNotRealtime(GetContextForThisThread(), info);
+  ExpectNotRealtime(
+      GetContextForThisThread(),
+      PrintDiagnosticsAndDieAction({BlockingCallInfo{func_name}, pc, bp}));
 }
 
 } // extern "C"
diff --git a/compiler-rt/lib/rtsan/rtsan_assertions.cpp b/compiler-rt/lib/rtsan/rtsan_assertions.cpp
deleted file mode 100644
index 4aae85de5c52f1..00000000000000
--- a/compiler-rt/lib/rtsan/rtsan_assertions.cpp
+++ /dev/null
@@ -1,28 +0,0 @@
-//===--- rtsan_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
-//
-//===----------------------------------------------------------------------===//
-#include "rtsan/rtsan_assertions.h"
-
-#include "rtsan/rtsan.h"
-#include "rtsan/rtsan_diagnostics.h"
-
-using namespace __sanitizer;
-
-void __rtsan::ExpectNotRealtime(Context &context, const DiagnosticsInfo &info) {
-  CHECK(__rtsan_is_initialized());
-  if (context.InRealtimeContext() && !context.IsBypassed()) {
-    context.BypassPush();
-
-    PrintDiagnostics(info);
-    Die();
-    context.BypassPop();
-  }
-}
diff --git a/compiler-rt/lib/rtsan/rtsan_assertions.h b/compiler-rt/lib/rtsan/rtsan_assertions.h
index bc1235363669df..ac9e1d203d20be 100644
--- a/compiler-rt/lib/rtsan/rtsan_assertions.h
+++ b/compiler-rt/lib/rtsan/rtsan_assertions.h
@@ -12,10 +12,19 @@
 
 #pragma once
 
+#include "rtsan/rtsan.h"
 #include "rtsan/rtsan_context.h"
-#include "rtsan/rtsan_diagnostics.h"
 
 namespace __rtsan {
 
-void ExpectNotRealtime(Context &context, const DiagnosticsInfo &info);
+template <typename OnViolationAction>
+void ExpectNotRealtime(Context &context, OnViolationAction &&on_violation) {
+  CHECK(__rtsan_is_initialized());
+  if (context.InRealtimeContext() && !context.IsBypassed()) {
+    context.BypassPush();
+    on_violation();
+    context.BypassPop();
+  }
+}
+
 } // namespace __rtsan
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
index b6999eeb7746a2..5df3e026e8e5fe 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
@@ -13,9 +13,8 @@
 #include "rtsan_test_utilities.h"
 
 #include "rtsan/rtsan_assertions.h"
-#include "rtsan/rtsan_diagnostics.h"
 
-#include <gtest/gtest.h>
+#include <gmock/gmock.h>
 
 using namespace __rtsan;
 
@@ -24,30 +23,33 @@ class TestRtsanAssertions : public ::testing::Test {
   void SetUp() override { __rtsan_ensure_initialized(); }
 };
 
-DiagnosticsInfo FakeDiagnosticsInfo() {
-  DiagnosticsInfo info;
-  info.pc = 0;
-  info.bp = 0;
-  info.call_info = InterceptedCallInfo{"fake_function_name"};
-  return info;
+static void testExpectNotRealtime(__rtsan::Context &context,
+                                  bool expect_violation_callback) {
+  ::testing::MockFunction<void()> mock_on_violation;
+  EXPECT_CALL(mock_on_violation, Call).Times(expect_violation_callback ? 1 : 0);
+  ExpectNotRealtime(context, mock_on_violation.AsStdFunction());
 }
 
-TEST_F(TestRtsanAssertions, ExpectNotRealtimeDoesNotDieIfNotInRealtimeContext) {
+TEST_F(TestRtsanAssertions,
+       ExpectNotRealtimeDoesNotCallViolationActionIfNotInRealtimeContext) {
   __rtsan::Context context{};
   ASSERT_FALSE(context.InRealtimeContext());
-  ExpectNotRealtime(context, FakeDiagnosticsInfo());
+  testExpectNotRealtime(context, false);
 }
 
-TEST_F(TestRtsanAssertions, ExpectNotRealtimeDiesIfInRealtimeContext) {
+TEST_F(TestRtsanAssertions,
+       ExpectNotRealtimeCallsViolationActionIfInRealtimeContext) {
   __rtsan::Context context{};
   context.RealtimePush();
   ASSERT_TRUE(context.InRealtimeContext());
-  EXPECT_DEATH(ExpectNotRealtime(context, FakeDiagnosticsInfo()), "");
+  testExpectNotRealtime(context, true);
 }
 
-TEST_F(TestRtsanAssertions, ExpectNotRealtimeDoesNotDieIfRealtimeButBypassed) {
+TEST_F(TestRtsanAssertions,
+       ExpectNotRealtimeDoesNotCallViolationActionIfRealtimeButBypassed) {
   __rtsan::Context context{};
   context.RealtimePush();
   context.BypassPush();
-  ExpectNotRealtime(context, FakeDiagnosticsInfo());
+  ASSERT_TRUE(context.IsBypassed());
+  testExpectNotRealtime(context, false);
 }

>From 5425557c6a7b087da8c3a67710eb7f6bd6fb7672 Mon Sep 17 00:00:00 2001
From: David Trevelyan <david.trevelyan at gmail.com>
Date: Sat, 21 Sep 2024 11:56:25 -0600
Subject: [PATCH 2/2] [rtsan] Rename callback variable and test function

---
 compiler-rt/lib/rtsan/rtsan_assertions.h              | 4 ++--
 compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/compiler-rt/lib/rtsan/rtsan_assertions.h b/compiler-rt/lib/rtsan/rtsan_assertions.h
index ac9e1d203d20be..1a653d198ab88b 100644
--- a/compiler-rt/lib/rtsan/rtsan_assertions.h
+++ b/compiler-rt/lib/rtsan/rtsan_assertions.h
@@ -18,11 +18,11 @@
 namespace __rtsan {
 
 template <typename OnViolationAction>
-void ExpectNotRealtime(Context &context, OnViolationAction &&on_violation) {
+void ExpectNotRealtime(Context &context, OnViolationAction &&OnViolation) {
   CHECK(__rtsan_is_initialized());
   if (context.InRealtimeContext() && !context.IsBypassed()) {
     context.BypassPush();
-    on_violation();
+    OnViolation();
     context.BypassPop();
   }
 }
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
index 5df3e026e8e5fe..d65a855220ab03 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
@@ -23,7 +23,7 @@ class TestRtsanAssertions : public ::testing::Test {
   void SetUp() override { __rtsan_ensure_initialized(); }
 };
 
-static void testExpectNotRealtime(__rtsan::Context &context,
+static void expectViolationAction(__rtsan::Context &context,
                                   bool expect_violation_callback) {
   ::testing::MockFunction<void()> mock_on_violation;
   EXPECT_CALL(mock_on_violation, Call).Times(expect_violation_callback ? 1 : 0);
@@ -34,7 +34,7 @@ TEST_F(TestRtsanAssertions,
        ExpectNotRealtimeDoesNotCallViolationActionIfNotInRealtimeContext) {
   __rtsan::Context context{};
   ASSERT_FALSE(context.InRealtimeContext());
-  testExpectNotRealtime(context, false);
+  expectViolationAction(context, false);
 }
 
 TEST_F(TestRtsanAssertions,
@@ -42,7 +42,7 @@ TEST_F(TestRtsanAssertions,
   __rtsan::Context context{};
   context.RealtimePush();
   ASSERT_TRUE(context.InRealtimeContext());
-  testExpectNotRealtime(context, true);
+  expectViolationAction(context, true);
 }
 
 TEST_F(TestRtsanAssertions,
@@ -51,5 +51,5 @@ TEST_F(TestRtsanAssertions,
   context.RealtimePush();
   context.BypassPush();
   ASSERT_TRUE(context.IsBypassed());
-  testExpectNotRealtime(context, false);
+  expectViolationAction(context, false);
 }



More information about the llvm-commits mailing list