[compiler-rt] [compiler-rt][rtsan] NFC: Refactor context helper functions (PR #106869)

via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 31 13:06:53 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Chris Apple (cjappl)

<details>
<summary>Changes</summary>

Pull `ExpectNotRealtime` and `PrintDiagnostics` into their own functions, away from Context.

Why?
- Currently PrintDiagnostics does not depend on any private method of Context
- We want to change soon to allow for detection of infinite loops and user defined blocking functions
    - These changes will affect these two functions, but shouldn't affect Context.
    - We will need to add an argument, or otherwise change these methods to say "why" we expect not realtime in this situation.

Overall, I think this refactor decreases coupling, and increases our flexibility going forward.

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


4 Files Affected:

- (modified) compiler-rt/lib/rtsan/rtsan.cpp (+1-2) 
- (modified) compiler-rt/lib/rtsan/rtsan_context.cpp (+6-6) 
- (modified) compiler-rt/lib/rtsan/rtsan_context.h (+9-4) 
- (modified) compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp (+7-7) 


``````````diff
diff --git a/compiler-rt/lib/rtsan/rtsan.cpp b/compiler-rt/lib/rtsan/rtsan.cpp
index b2c4616b5fd0dc..1388ce66cbde22 100644
--- a/compiler-rt/lib/rtsan/rtsan.cpp
+++ b/compiler-rt/lib/rtsan/rtsan.cpp
@@ -69,8 +69,7 @@ SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_enable() {
 SANITIZER_INTERFACE_ATTRIBUTE void
 __rtsan_expect_not_realtime(const char *intercepted_function_name) {
   __rtsan_ensure_initialized();
-  __rtsan::GetContextForThisThread().ExpectNotRealtime(
-      intercepted_function_name);
+  ExpectNotRealtime(GetContextForThisThread(), intercepted_function_name);
 }
 
 } // extern "C"
diff --git a/compiler-rt/lib/rtsan/rtsan_context.cpp b/compiler-rt/lib/rtsan/rtsan_context.cpp
index f761ddedce6727..330cc008cbc71f 100644
--- a/compiler-rt/lib/rtsan/rtsan_context.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_context.cpp
@@ -71,13 +71,13 @@ void __rtsan::Context::BypassPush() { bypass_depth++; }
 
 void __rtsan::Context::BypassPop() { bypass_depth--; }
 
-void __rtsan::Context::ExpectNotRealtime(
-    const char *intercepted_function_name) {
-  if (InRealtimeContext() && !IsBypassed()) {
-    BypassPush();
+void __rtsan::ExpectNotRealtime(Context &context,
+                                const char *intercepted_function_name) {
+  if (context.InRealtimeContext() && !context.IsBypassed()) {
+    context.BypassPush();
     PrintDiagnostics(intercepted_function_name);
     InvokeViolationDetectedAction();
-    BypassPop();
+    context.BypassPop();
   }
 }
 
@@ -85,7 +85,7 @@ bool __rtsan::Context::InRealtimeContext() const { return realtime_depth > 0; }
 
 bool __rtsan::Context::IsBypassed() const { return bypass_depth > 0; }
 
-void __rtsan::Context::PrintDiagnostics(const char *intercepted_function_name) {
+void __rtsan::PrintDiagnostics(const char *intercepted_function_name) {
   fprintf(stderr,
           "Real-time violation: intercepted call to real-time unsafe function "
           "`%s` in real-time context! Stack trace:\n",
diff --git a/compiler-rt/lib/rtsan/rtsan_context.h b/compiler-rt/lib/rtsan/rtsan_context.h
index 515bb8ade1eb97..bccc088c500cae 100644
--- a/compiler-rt/lib/rtsan/rtsan_context.h
+++ b/compiler-rt/lib/rtsan/rtsan_context.h
@@ -22,17 +22,22 @@ class Context {
   void BypassPush();
   void BypassPop();
 
-  void ExpectNotRealtime(const char *intercepted_function_name);
-
-private:
   bool InRealtimeContext() const;
   bool IsBypassed() const;
-  void PrintDiagnostics(const char *intercepted_function_name);
 
+  Context(const Context &) = delete;
+  Context(Context &&) = delete;
+  Context &operator=(const Context &) = delete;
+  Context &operator=(Context &&) = delete;
+
+private:
   int realtime_depth{0};
   int bypass_depth{0};
 };
 
 Context &GetContextForThisThread();
 
+void ExpectNotRealtime(Context &context, const char *intercepted_function_name);
+void PrintDiagnostics(const char *intercepted_function_name);
+
 } // namespace __rtsan
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp
index b7e73236a14cc6..cad8cf2d539960 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_context.cpp
@@ -16,21 +16,21 @@ TEST(TestRtsanContext, CanCreateContext) { __rtsan::Context context{}; }
 
 TEST(TestRtsanContext, ExpectNotRealtimeDoesNotDieBeforeRealtimePush) {
   __rtsan::Context context{};
-  context.ExpectNotRealtime("do_some_stuff");
+  ExpectNotRealtime(context, "do_some_stuff");
 }
 
 TEST(TestRtsanContext, ExpectNotRealtimeDoesNotDieAfterPushAndPop) {
   __rtsan::Context context{};
   context.RealtimePush();
   context.RealtimePop();
-  context.ExpectNotRealtime("do_some_stuff");
+  ExpectNotRealtime(context, "do_some_stuff");
 }
 
 TEST(TestRtsanContext, ExpectNotRealtimeDiesAfterRealtimePush) {
   __rtsan::Context context{};
 
   context.RealtimePush();
-  EXPECT_DEATH(context.ExpectNotRealtime("do_some_stuff"), "");
+  EXPECT_DEATH(ExpectNotRealtime(context, "do_some_stuff"), "");
 }
 
 TEST(TestRtsanContext,
@@ -42,7 +42,7 @@ TEST(TestRtsanContext,
   context.RealtimePush();
   context.RealtimePop();
   context.RealtimePop();
-  EXPECT_DEATH(context.ExpectNotRealtime("do_some_stuff"), "");
+  EXPECT_DEATH(ExpectNotRealtime(context, "do_some_stuff"), "");
 }
 
 TEST(TestRtsanContext, ExpectNotRealtimeDoesNotDieAfterBypassPush) {
@@ -50,7 +50,7 @@ TEST(TestRtsanContext, ExpectNotRealtimeDoesNotDieAfterBypassPush) {
 
   context.RealtimePush();
   context.BypassPush();
-  context.ExpectNotRealtime("do_some_stuff");
+  ExpectNotRealtime(context, "do_some_stuff");
 }
 
 TEST(TestRtsanContext,
@@ -63,7 +63,7 @@ TEST(TestRtsanContext,
   context.BypassPush();
   context.BypassPop();
   context.BypassPop();
-  context.ExpectNotRealtime("do_some_stuff");
+  ExpectNotRealtime(context, "do_some_stuff");
   context.BypassPop();
-  EXPECT_DEATH(context.ExpectNotRealtime("do_some_stuff"), "");
+  EXPECT_DEATH(ExpectNotRealtime(context, "do_some_stuff"), "");
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/106869


More information about the llvm-commits mailing list