Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler-rt/lib/rtsan/rtsan_assertions.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ void ExpectNotRealtime(Context &context, const DiagnosticsInfo &info,
if (context.InRealtimeContext() && !context.IsBypassed()) {
ScopedBypass sb{context};

if (IsFunctionSuppressed(info.func_name))
return;

__sanitizer::BufferedStackTrace stack;

// We use the unwind_on_fatal flag here because of precedent with other
Expand Down
1 change: 1 addition & 0 deletions compiler-rt/lib/rtsan/rtsan_checks.inc
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@
// SummaryKind should be a string literal.

RTSAN_CHECK(CallStackContains, "call-stack-contains")
RTSAN_CHECK(FunctionNameIs, "function-name-is")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review the new name @davidtrevelyan and @fmayer - I chose one with a verb at the end to mirror call-stack-contains.

Other candidates I considered:

  • unsafe-function-name-is
  • function-name
  • function-name-matches

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like function-name-is if the intention is only to allow users to provide an exact function name match, character for character. But I think that would be difficult to use - how is the user supposed to reason about namespaces, mangling, parentheses on the end, overloads, etc.? I think at the minute I prefer function-name-matches with regex supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool that works for me. At least as written, we use the suppression built-in simple regexes (which I think is good, and we should keep it). I'll wait to see what fmayer says.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function-name-matches SGTM if it's a regex / glob

13 changes: 13 additions & 0 deletions compiler-rt/lib/rtsan/rtsan_suppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,16 @@ bool __rtsan::IsStackTraceSuppressed(const StackTrace &stack) {
}
return false;
}

bool __rtsan::IsFunctionSuppressed(const char *function_name) {
if (suppression_ctx == nullptr)
return false;

const char *flag_name = ConvertTypeToFlagName(ErrorType::FunctionNameIs);

if (!suppression_ctx->HasSuppressionType(flag_name))
return false;

Suppression *s;
return suppression_ctx->Match(function_name, flag_name, &s);
}
1 change: 1 addition & 0 deletions compiler-rt/lib/rtsan/rtsan_suppressions.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ namespace __rtsan {

void InitializeSuppressions();
bool IsStackTraceSuppressed(const __sanitizer::StackTrace &stack);
bool IsFunctionSuppressed(const char *function_name);

} // namespace __rtsan
25 changes: 20 additions & 5 deletions compiler-rt/test/rtsan/stack_suppressions.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %clangxx -fsanitize=realtime %s -o %t
// RUN: %env_rtsan_opts=halt_on_error=false %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NOSUPPRESSIONS
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this check here to make sure that these would actually cause errors before suppressing them

// RUN: %env_rtsan_opts=suppressions='%s.supp' not %run %t 2>&1 | FileCheck %s
// UNSUPPORTED: ios

Expand All @@ -8,8 +9,11 @@
#include <stdlib.h>
#include <unistd.h>

#include <atomic>
#include <vector>

std::atomic<int> cas_atomic{0};

void *MallocViolation() { return malloc(10); }

void VectorViolations() {
Expand All @@ -22,13 +26,18 @@ void VectorViolations() {
v.reserve(10);
}

void BlockFunc() [[clang::blocking]] { usleep(1); }
void BlockFunc() [[clang::blocking]] {
int expected = 0;
while (!cas_atomic.compare_exchange_weak(expected, 1)) {
expected = cas_atomic.load();
}
}

void *process() [[clang::nonblocking]] {
void *ptr = MallocViolation();
VectorViolations();
BlockFunc();
free(ptr);
void *ptr = MallocViolation(); // Suppressed call-stack-contains
VectorViolations(); // Suppressed call-stack-contains with regex
BlockFunc(); // Suppressed function-name-is
free(ptr); // Suppressed function-name-is

// This is the one that should abort the program
// Everything else is suppressed
Expand All @@ -51,3 +60,9 @@ int main() {
// CHECK-NOT: vector
// CHECK-NOT: free
// CHECK-NOT: BlockFunc

// CHECK-NOSUPPRESSIONS: malloc
// CHECK-NOSUPPRESSIONS: vector
// CHECK-NOSUPPRESSIONS: free
// CHECK-NOSUPPRESSIONS: BlockFunc
// CHECK-NOSUPPRESSIONS: usleep
5 changes: 3 additions & 2 deletions compiler-rt/test/rtsan/stack_suppressions.cpp.supp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
call-stack-contains:MallocViolation
call-stack-contains:std::*vector
call-stack-contains:free
call-stack-contains:BlockFunc

function-name-is:free
function-name-is:BlockFunc
Loading