[compiler-rt] [rtsan][NFC] Refactor where we unwind the stack (PR #111443)

[Chris Apple via llvm-commits]

https://github.com/cjappl updated https://github.com/llvm/llvm-project/pull/111443

>From 0dce7ac92b676b16251b4b154e0e5302140753e9 Mon Sep 17 00:00:00 2001
From: Chris Apple <cja-private at pm.me>
Date: Mon, 7 Oct 2024 14:04:21 -0700
Subject: [PATCH] [rtsan][NFC] Refactor where we unwind the stack

---
 compiler-rt/lib/rtsan/rtsan.cpp               | 58 ++++++++-----------
 compiler-rt/lib/rtsan/rtsan_assertions.h      | 16 ++++-
 .../lib/rtsan/tests/rtsan_test_assertions.cpp | 10 +++-
 3 files changed, 47 insertions(+), 37 deletions(-)

diff --git a/compiler-rt/lib/rtsan/rtsan.cpp b/compiler-rt/lib/rtsan/rtsan.cpp
index 9f4a9a5701636f..85c452cb3c393d 100644
--- a/compiler-rt/lib/rtsan/rtsan.cpp
+++ b/compiler-rt/lib/rtsan/rtsan.cpp
@@ -46,41 +46,33 @@ static InitializationState GetInitializationState() {
       atomic_load(&rtsan_initialized, memory_order_acquire));
 }
 
-static auto OnViolationAction(DiagnosticsInfo info) {
-  return [info]() {
-    IncrementTotalErrorCount();
+static auto OnViolationAction(const BufferedStackTrace &stack,
+                              const DiagnosticsInfo &info) {
+  IncrementTotalErrorCount();
 
-    BufferedStackTrace stack;
+  // If in the future we interop with other sanitizers, we will
+  // need to make our own stackdepot
+  StackDepotHandle handle = StackDepotPut_WithHandle(stack);
 
-    // We use the unwind_on_fatal flag here because of precedent with other
-    // sanitizers, this action is not necessarily fatal if halt_on_error=false
-    stack.Unwind(info.pc, info.bp, nullptr,
-                 common_flags()->fast_unwind_on_fatal);
+  const bool is_stack_novel = handle.use_count() == 0;
 
-    // If in the future we interop with other sanitizers, we will
-    // need to make our own stackdepot
-    StackDepotHandle handle = StackDepotPut_WithHandle(stack);
+  // Marked UNLIKELY as if user is runing with halt_on_error=false
+  // we expect a high number of duplicate stacks. We are willing
+  // To pay for the first insertion.
+  if (UNLIKELY(is_stack_novel)) {
+    IncrementUniqueErrorCount();
 
-    const bool is_stack_novel = handle.use_count() == 0;
+    PrintDiagnostics(info);
+    stack.Print();
 
-    // Marked UNLIKELY as if user is runing with halt_on_error=false
-    // we expect a high number of duplicate stacks. We are willing
-    // To pay for the first insertion.
-    if (UNLIKELY(is_stack_novel)) {
-      IncrementUniqueErrorCount();
+    handle.inc_use_count_unsafe();
+  }
 
-      PrintDiagnostics(info);
-      stack.Print();
-
-      handle.inc_use_count_unsafe();
-    }
-
-    if (flags().halt_on_error) {
-      if (flags().print_stats_on_exit)
-        PrintStatisticsSummary();
-      Die();
-    }
-  };
+  if (flags().halt_on_error) {
+    if (flags().print_stats_on_exit)
+      PrintStatisticsSummary();
+    Die();
+  }
 }
 
 extern "C" {
@@ -141,8 +133,8 @@ __rtsan_notify_intercepted_call(const char *func_name) {
   __rtsan_ensure_initialized();
   GET_CALLER_PC_BP;
   ExpectNotRealtime(GetContextForThisThread(),
-                    OnViolationAction({DiagnosticsInfoType::InterceptedCall,
-                                       func_name, pc, bp}));
+                    {DiagnosticsInfoType::InterceptedCall, func_name, pc, bp},
+                    OnViolationAction);
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE void
@@ -150,8 +142,8 @@ __rtsan_notify_blocking_call(const char *func_name) {
   __rtsan_ensure_initialized();
   GET_CALLER_PC_BP;
   ExpectNotRealtime(GetContextForThisThread(),
-                    OnViolationAction({DiagnosticsInfoType::BlockingCall,
-                                       func_name, pc, bp}));
+                    {DiagnosticsInfoType::BlockingCall, func_name, pc, bp},
+                    OnViolationAction);
 }
 
 } // extern "C"
diff --git a/compiler-rt/lib/rtsan/rtsan_assertions.h b/compiler-rt/lib/rtsan/rtsan_assertions.h
index 4646e750b6796e..745cbea0eb3a29 100644
--- a/compiler-rt/lib/rtsan/rtsan_assertions.h
+++ b/compiler-rt/lib/rtsan/rtsan_assertions.h
@@ -14,15 +14,27 @@
 
 #include "rtsan/rtsan.h"
 #include "rtsan/rtsan_context.h"
+#include "rtsan/rtsan_diagnostics.h"
+
+#include "sanitizer_common/sanitizer_stacktrace.h"
 
 namespace __rtsan {
 
 template <typename OnViolationAction>
-void ExpectNotRealtime(Context &context, OnViolationAction &&OnViolation) {
+void ExpectNotRealtime(Context &context, const DiagnosticsInfo &info,
+                       OnViolationAction &&OnViolation) {
   CHECK(__rtsan_is_initialized());
   if (context.InRealtimeContext() && !context.IsBypassed()) {
     ScopedBypass sb{context};
-    OnViolation();
+
+    __sanitizer::BufferedStackTrace stack;
+
+    // We use the unwind_on_fatal flag here because of precedent with other
+    // sanitizers, this action is not necessarily fatal if halt_on_error=false
+    stack.Unwind(info.pc, info.bp, nullptr,
+                 __sanitizer::common_flags()->fast_unwind_on_fatal);
+
+    OnViolation(stack, info);
   }
 }
 
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
index 3b279989a49cb3..5e54225025fbd0 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
@@ -14,8 +14,11 @@
 
 #include "rtsan/rtsan_assertions.h"
 
+#include "sanitizer_common/sanitizer_stacktrace.h"
+
 #include <gmock/gmock.h>
 
+using namespace __sanitizer;
 using namespace __rtsan;
 
 class TestRtsanAssertions : public ::testing::Test {
@@ -25,9 +28,12 @@ class TestRtsanAssertions : public ::testing::Test {
 
 static void ExpectViolationAction(Context &context,
                                   bool expect_violation_callback) {
-  ::testing::MockFunction<void()> mock_on_violation;
+  ::testing::MockFunction<void(const BufferedStackTrace &stack,
+                               const DiagnosticsInfo &info)>
+      mock_on_violation;
   EXPECT_CALL(mock_on_violation, Call).Times(expect_violation_callback ? 1 : 0);
-  ExpectNotRealtime(context, mock_on_violation.AsStdFunction());
+  DiagnosticsInfo info{};
+  ExpectNotRealtime(context, info, mock_on_violation.AsStdFunction());
 }
 
 TEST_F(TestRtsanAssertions,