[rtsan][compiler-rt] Introduce __rtsan_notify_blocking_call #109529
Conversation
|
CC for review @davidtrevelyan |
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Chris Apple (cjappl) ChangesWhy?In llvm, we need to add a call to What is the other junk?We need to create an abstraction to allow for multiple violation types (currently an intercepted call, and a blocking call) and have it be easily extensible for the future, if we add other errors, or add more actions that operate on these errors. We think that a std::variant/visitor is the cleanest way to do this to allow for easy addition of more operations in the future Full diff: https://github.com/llvm/llvm-project/pull/109529.diff 8 Files Affected:
diff --git a/compiler-rt/lib/rtsan/rtsan.cpp b/compiler-rt/lib/rtsan/rtsan.cpp
index fe7247ec8e7bbc..936f0b5b8cee39 100644
--- a/compiler-rt/lib/rtsan/rtsan.cpp
+++ b/compiler-rt/lib/rtsan/rtsan.cpp
@@ -16,6 +16,7 @@
#include "sanitizer_common/sanitizer_atomic.h"
#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_mutex.h"
+#include "sanitizer_common/sanitizer_stacktrace.h"
using namespace __rtsan;
using namespace __sanitizer;
@@ -75,6 +76,20 @@ SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_enable() {
SANITIZER_INTERFACE_ATTRIBUTE void
__rtsan_notify_intercepted_call(const char *intercepted_function_name) {
__rtsan_ensure_initialized();
- ExpectNotRealtime(GetContextForThisThread(), intercepted_function_name);
+
+ GET_CALLER_PC_BP;
+ DiagnosticsInfo info = {InterceptedCallInfo{intercepted_function_name}, pc,
+ bp};
+ ExpectNotRealtime(GetContextForThisThread(), info);
}
+
+SANITIZER_INTERFACE_ATTRIBUTE void
+__rtsan_notify_blocking_call(const char *blocking_function_name) {
+ __rtsan_ensure_initialized();
+
+ GET_CALLER_PC_BP;
+ DiagnosticsInfo info = {BlockingCallInfo{blocking_function_name}, pc, bp};
+ ExpectNotRealtime(GetContextForThisThread(), info);
+}
+
} // extern "C"
diff --git a/compiler-rt/lib/rtsan/rtsan.h b/compiler-rt/lib/rtsan/rtsan.h
index b690f734e10327..37628ae2731f6d 100644
--- a/compiler-rt/lib/rtsan/rtsan.h
+++ b/compiler-rt/lib/rtsan/rtsan.h
@@ -45,4 +45,6 @@ SANITIZER_INTERFACE_ATTRIBUTE void __rtsan_enable();
SANITIZER_INTERFACE_ATTRIBUTE void
__rtsan_notify_intercepted_call(const char *intercepted_function_name);
+SANITIZER_INTERFACE_ATTRIBUTE void
+__rtsan_notify_blocking_call(const char *blocking_function_name);
} // extern "C"
diff --git a/compiler-rt/lib/rtsan/rtsan_assertions.cpp b/compiler-rt/lib/rtsan/rtsan_assertions.cpp
index ef996c92dc1e82..4aae85de5c52f1 100644
--- a/compiler-rt/lib/rtsan/rtsan_assertions.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_assertions.cpp
@@ -14,18 +14,14 @@
#include "rtsan/rtsan.h"
#include "rtsan/rtsan_diagnostics.h"
-#include "sanitizer_common/sanitizer_stacktrace.h"
-
using namespace __sanitizer;
-void __rtsan::ExpectNotRealtime(Context &context,
- const char *intercepted_function_name) {
+void __rtsan::ExpectNotRealtime(Context &context, const DiagnosticsInfo &info) {
CHECK(__rtsan_is_initialized());
if (context.InRealtimeContext() && !context.IsBypassed()) {
context.BypassPush();
- GET_CALLER_PC_BP;
- PrintDiagnostics(intercepted_function_name, pc, bp);
+ PrintDiagnostics(info);
Die();
context.BypassPop();
}
diff --git a/compiler-rt/lib/rtsan/rtsan_assertions.h b/compiler-rt/lib/rtsan/rtsan_assertions.h
index bc38a0f116eec2..bc1235363669df 100644
--- a/compiler-rt/lib/rtsan/rtsan_assertions.h
+++ b/compiler-rt/lib/rtsan/rtsan_assertions.h
@@ -13,7 +13,9 @@
#pragma once
#include "rtsan/rtsan_context.h"
+#include "rtsan/rtsan_diagnostics.h"
namespace __rtsan {
-void ExpectNotRealtime(Context &context, const char *intercepted_function_name);
+
+void ExpectNotRealtime(Context &context, const DiagnosticsInfo &info);
} // namespace __rtsan
diff --git a/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp b/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp
index d4c656606f3653..67e9e31b9dcb7a 100644
--- a/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp
@@ -34,8 +34,8 @@ namespace {
class Decorator : public __sanitizer::SanitizerCommonDecorator {
public:
Decorator() : SanitizerCommonDecorator() {}
- const char *FunctionName() { return Green(); }
- const char *Reason() { return Blue(); }
+ const char *FunctionName() const { return Green(); }
+ const char *Reason() const { return Blue(); }
};
} // namespace
@@ -46,18 +46,50 @@ static void PrintStackTrace(uptr pc, uptr bp) {
stack.Print();
}
-void __rtsan::PrintDiagnostics(const char *intercepted_function_name, uptr pc,
- uptr bp) {
+template <class... Ts> struct Overloaded : Ts... {
+ using Ts::operator()...;
+};
+// TODO: Remove below when c++20
+template <class... Ts> Overloaded(Ts...) -> Overloaded<Ts...>;
+
+void PrintError(Decorator &decorator, const DiagnosticsCallerInfo &info) {
+ const char *violation_type = std::visit(
+ Overloaded{
+ [](const InterceptedCallInfo &) { return "unsafe-library-call"; },
+ [](const BlockingCallInfo &) { return "blocking-call"; }},
+ info);
+
+ Printf("%s", decorator.Error());
+ Report("ERROR: RealtimeSanitizer: %s\n", violation_type);
+}
+
+void PrintReason(Decorator &decorator, const DiagnosticsCallerInfo &info) {
+ Printf("%s", decorator.Reason());
+
+ std::visit(
+ Overloaded{[decorator](const InterceptedCallInfo &call) {
+ Printf("Intercepted call to real-time unsafe function "
+ "`%s%s%s` in real-time context!",
+ decorator.FunctionName(),
+ call.intercepted_function_name_, decorator.Reason());
+ },
+ [decorator](const BlockingCallInfo &arg) {
+ Printf("Call to blocking function "
+ "`%s%s%s` in real-time context!",
+ decorator.FunctionName(), arg.blocking_function_name_,
+ decorator.Reason());
+ }},
+ info);
+
+ Printf("\n");
+}
+
+void __rtsan::PrintDiagnostics(const DiagnosticsInfo &info) {
ScopedErrorReportLock l;
Decorator d;
- Printf("%s", d.Error());
- Report("ERROR: RealtimeSanitizer: unsafe-library-call\n");
- Printf("%s", d.Reason());
- Printf("Intercepted call to real-time unsafe function "
- "`%s%s%s` in real-time context!\n",
- d.FunctionName(), intercepted_function_name, d.Reason());
-
+ PrintError(d, info.call_info);
+ PrintReason(d, info.call_info);
Printf("%s", d.Default());
- PrintStackTrace(pc, bp);
+ PrintStackTrace(info.pc, info.bp);
}
diff --git a/compiler-rt/lib/rtsan/rtsan_diagnostics.h b/compiler-rt/lib/rtsan/rtsan_diagnostics.h
index 1d6c3ccb7bc7eb..f6137913b1bd90 100644
--- a/compiler-rt/lib/rtsan/rtsan_diagnostics.h
+++ b/compiler-rt/lib/rtsan/rtsan_diagnostics.h
@@ -12,9 +12,31 @@
#pragma once
+#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_internal_defs.h"
+#include <string>
+#include <variant>
+
namespace __rtsan {
-void PrintDiagnostics(const char *intercepted_function_name,
- __sanitizer::uptr pc, __sanitizer::uptr bp);
+
+struct InterceptedCallInfo {
+ const char *intercepted_function_name_;
+};
+
+struct BlockingCallInfo {
+public:
+ const char *blocking_function_name_;
+};
+
+using DiagnosticsCallerInfo = std::variant<InterceptedCallInfo, BlockingCallInfo>;
+
+struct DiagnosticsInfo {
+ DiagnosticsCallerInfo call_info;
+
+ __sanitizer::uptr pc;
+ __sanitizer::uptr bp;
+};
+
+void PrintDiagnostics(const DiagnosticsInfo &info);
} // 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 cdf2ac32170043..7b254278814e2f 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_assertions.cpp
@@ -13,30 +13,41 @@
#include "rtsan_test_utilities.h"
#include "rtsan/rtsan_assertions.h"
+#include "rtsan/rtsan_diagnostics.h"
#include <gtest/gtest.h>
+using namespace __rtsan;
+
class TestRtsanAssertions : public ::testing::Test {
protected:
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;
+}
+
TEST_F(TestRtsanAssertions, ExpectNotRealtimeDoesNotDieIfNotInRealtimeContext) {
__rtsan::Context context{};
ASSERT_FALSE(context.InRealtimeContext());
- ExpectNotRealtime(context, "fake_function_name");
+ ExpectNotRealtime(context, FakeDiagnosticsInfo());
}
TEST_F(TestRtsanAssertions, ExpectNotRealtimeDiesIfInRealtimeContext) {
__rtsan::Context context{};
context.RealtimePush();
ASSERT_TRUE(context.InRealtimeContext());
- EXPECT_DEATH(ExpectNotRealtime(context, "fake_function_name"), "");
+ EXPECT_DEATH(ExpectNotRealtime(context, FakeDiagnosticsInfo()), "");
}
TEST_F(TestRtsanAssertions, ExpectNotRealtimeDoesNotDieIfRealtimeButBypassed) {
__rtsan::Context context{};
context.RealtimePush();
context.BypassPush();
- ExpectNotRealtime(context, "fake_function_name");
+ ExpectNotRealtime(context, FakeDiagnosticsInfo());
}
diff --git a/compiler-rt/test/rtsan/blocking_call.cpp b/compiler-rt/test/rtsan/blocking_call.cpp
new file mode 100644
index 00000000000000..47ce3d5544cbd6
--- /dev/null
+++ b/compiler-rt/test/rtsan/blocking_call.cpp
@@ -0,0 +1,34 @@
+// RUN: %clangxx -fsanitize=realtime %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// UNSUPPORTED: ios
+
+// Intent: Check that a function marked with [[clang::nonblocking]] cannot call a function that is blocking.
+
+#include <stdio.h>
+#include <stdlib.h>
+
+// TODO: Remove when [[blocking]] is implemented.
+extern "C" void __rtsan_notify_blocking_call(const char *function_name);
+
+void custom_blocking_function() {
+ // TODO: When [[blocking]] is implemented, don't call this directly.
+ __rtsan_notify_blocking_call(__func__);
+}
+
+void safe_call() {
+ // TODO: When [[blocking]] is implemented, don't call this directly.
+ __rtsan_notify_blocking_call(__func__);
+}
+
+void process() [[clang::nonblocking]] { custom_blocking_function(); }
+
+int main() {
+ safe_call(); // This shouldn't die, because it isn't in nonblocking context.
+ process();
+ return 0;
+ // CHECK-NOT: {{.*safe_call*}}
+ // CHECK: ==ERROR: RealtimeSanitizer: blocking-call
+ // CHECK-NEXT: Call to blocking function `custom_blocking_function` in real-time context!
+ // CHECK-NEXT: {{.*custom_blocking_function*}}
+ // CHECK-NEXT: {{.*process*}}
+}
|
| #include <stdlib.h> | ||
|
|
||
| // TODO: Remove when [[blocking]] is implemented. | ||
| extern "C" void __rtsan_notify_blocking_call(const char *function_name); |
There was a problem hiding this comment.
This test exists right now to nail down the output format, and make sure this works as intended. Shortly we will have the proper [[blocking]] attribute and this test will be simplified somewhat
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/4134 Here is the relevant piece of the build log for the reference |
|
Hi We are seeing a build failure of 'libclang_rt.rtsan_osx_dynamic.dylib' on our clang mac-x64 and mac-arm64 cross compile bots after this patch was landed: Failed builder task: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8736170060997779425/blamelist Could you take a look please? |
|
Looking now - will report back |
|
There seems to be something strange about this build environment. The ABI use is inconsistent, which is what the error is reporting (__1 vs __2): Are you building and linking with different version of the STL/C++ runtime across different components? Or different standards? We could fix this on our end by moving away from std::variant. However, I think the more holistic approach would be making sure the build environment is consistent. Us moving from Let me know what path you choose and what support you need, I am happy to help. |
Emmm, we also looked into this issue on our end and we don't think the root cause is mixture of v1 and v2 ABI in the build environment. We set the This issue did't show up before, probably due to the fact that rtsan didn't use any libcxx feature so missing dependency did cause any issue. |
|
Thanks for the additional info - very helpful. Looking into this to see what the other sanitizers do, I'll get back to you shortly |
|
See possible (?) fix #109715 |
|
Thanks everyone for the valuable input, I've drafted a PR for removing the |
|
This issue is hopefully resolved in #109786 @zeroomega please check and let us know. It has been merged to main |
That fixed our builder. Thanks. |
|
Thanks for the help gathering info - and reporting the issue in the first place. |
Why?
In llvm, we need to add a call to
__rtsan_notify_blocking_call()when a function is marked[[clang::blocking]]. This will produce a different error message than a call to an unsafe malloc etcWhat is the other junk?
We need to create an abstraction to allow for multiple violation types (currently an intercepted call, and a blocking call) and have it be easily extensible for the future, if we add other errors, or add more actions that operate on these errors.
We think that a std::variant/visitor is the cleanest way to do this to allow for easy addition of more operations in the future