Skip to content

Commit 0dce7ac

Browse files
committed
[rtsan][NFC] Refactor where we unwind the stack
1 parent 75103aa commit 0dce7ac

File tree

3 files changed

+47
-37
lines changed

3 files changed

+47
-37
lines changed

compiler-rt/lib/rtsan/rtsan.cpp

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,41 +46,33 @@ static InitializationState GetInitializationState() {
4646
atomic_load(&rtsan_initialized, memory_order_acquire));
4747
}
4848

49-
static auto OnViolationAction(DiagnosticsInfo info) {
50-
return [info]() {
51-
IncrementTotalErrorCount();
49+
static auto OnViolationAction(const BufferedStackTrace &stack,
50+
const DiagnosticsInfo &info) {
51+
IncrementTotalErrorCount();
5252

53-
BufferedStackTrace stack;
53+
// If in the future we interop with other sanitizers, we will
54+
// need to make our own stackdepot
55+
StackDepotHandle handle = StackDepotPut_WithHandle(stack);
5456

55-
// We use the unwind_on_fatal flag here because of precedent with other
56-
// sanitizers, this action is not necessarily fatal if halt_on_error=false
57-
stack.Unwind(info.pc, info.bp, nullptr,
58-
common_flags()->fast_unwind_on_fatal);
57+
const bool is_stack_novel = handle.use_count() == 0;
5958

60-
// If in the future we interop with other sanitizers, we will
61-
// need to make our own stackdepot
62-
StackDepotHandle handle = StackDepotPut_WithHandle(stack);
59+
// Marked UNLIKELY as if user is runing with halt_on_error=false
60+
// we expect a high number of duplicate stacks. We are willing
61+
// To pay for the first insertion.
62+
if (UNLIKELY(is_stack_novel)) {
63+
IncrementUniqueErrorCount();
6364

64-
const bool is_stack_novel = handle.use_count() == 0;
65+
PrintDiagnostics(info);
66+
stack.Print();
6567

66-
// Marked UNLIKELY as if user is runing with halt_on_error=false
67-
// we expect a high number of duplicate stacks. We are willing
68-
// To pay for the first insertion.
69-
if (UNLIKELY(is_stack_novel)) {
70-
IncrementUniqueErrorCount();
68+
handle.inc_use_count_unsafe();
69+
}
7170

72-
PrintDiagnostics(info);
73-
stack.Print();
74-
75-
handle.inc_use_count_unsafe();
76-
}
77-
78-
if (flags().halt_on_error) {
79-
if (flags().print_stats_on_exit)
80-
PrintStatisticsSummary();
81-
Die();
82-
}
83-
};
71+
if (flags().halt_on_error) {
72+
if (flags().print_stats_on_exit)
73+
PrintStatisticsSummary();
74+
Die();
75+
}
8476
}
8577

8678
extern "C" {
@@ -141,17 +133,17 @@ __rtsan_notify_intercepted_call(const char *func_name) {
141133
__rtsan_ensure_initialized();
142134
GET_CALLER_PC_BP;
143135
ExpectNotRealtime(GetContextForThisThread(),
144-
OnViolationAction({DiagnosticsInfoType::InterceptedCall,
145-
func_name, pc, bp}));
136+
{DiagnosticsInfoType::InterceptedCall, func_name, pc, bp},
137+
OnViolationAction);
146138
}
147139

148140
SANITIZER_INTERFACE_ATTRIBUTE void
149141
__rtsan_notify_blocking_call(const char *func_name) {
150142
__rtsan_ensure_initialized();
151143
GET_CALLER_PC_BP;
152144
ExpectNotRealtime(GetContextForThisThread(),
153-
OnViolationAction({DiagnosticsInfoType::BlockingCall,
154-
func_name, pc, bp}));
145+
{DiagnosticsInfoType::BlockingCall, func_name, pc, bp},
146+
OnViolationAction);
155147
}
156148

157149
} // extern "C"

compiler-rt/lib/rtsan/rtsan_assertions.h

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,27 @@
1414

1515
#include "rtsan/rtsan.h"
1616
#include "rtsan/rtsan_context.h"
17+
#include "rtsan/rtsan_diagnostics.h"
18+
19+
#include "sanitizer_common/sanitizer_stacktrace.h"
1720

1821
namespace __rtsan {
1922

2023
template <typename OnViolationAction>
21-
void ExpectNotRealtime(Context &context, OnViolationAction &&OnViolation) {
24+
void ExpectNotRealtime(Context &context, const DiagnosticsInfo &info,
25+
OnViolationAction &&OnViolation) {
2226
CHECK(__rtsan_is_initialized());
2327
if (context.InRealtimeContext() && !context.IsBypassed()) {
2428
ScopedBypass sb{context};
25-
OnViolation();
29+
30+
__sanitizer::BufferedStackTrace stack;
31+
32+
// We use the unwind_on_fatal flag here because of precedent with other
33+
// sanitizers, this action is not necessarily fatal if halt_on_error=false
34+
stack.Unwind(info.pc, info.bp, nullptr,
35+
__sanitizer::common_flags()->fast_unwind_on_fatal);
36+
37+
OnViolation(stack, info);
2638
}
2739
}
2840

compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@
1414

1515
#include "rtsan/rtsan_assertions.h"
1616

17+
#include "sanitizer_common/sanitizer_stacktrace.h"
18+
1719
#include <gmock/gmock.h>
1820

21+
using namespace __sanitizer;
1922
using namespace __rtsan;
2023

2124
class TestRtsanAssertions : public ::testing::Test {
@@ -25,9 +28,12 @@ class TestRtsanAssertions : public ::testing::Test {
2528

2629
static void ExpectViolationAction(Context &context,
2730
bool expect_violation_callback) {
28-
::testing::MockFunction<void()> mock_on_violation;
31+
::testing::MockFunction<void(const BufferedStackTrace &stack,
32+
const DiagnosticsInfo &info)>
33+
mock_on_violation;
2934
EXPECT_CALL(mock_on_violation, Call).Times(expect_violation_callback ? 1 : 0);
30-
ExpectNotRealtime(context, mock_on_violation.AsStdFunction());
35+
DiagnosticsInfo info{};
36+
ExpectNotRealtime(context, info, mock_on_violation.AsStdFunction());
3137
}
3238

3339
TEST_F(TestRtsanAssertions,

0 commit comments

Comments
 (0)