-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[rtsan] Handle attributed IR function declarations #169577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[rtsan] Handle attributed IR function declarations #169577
Conversation
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-compiler-rt-sanitizer Author: None (davidtrevelyan) ChangesAddresses #169377. Previously, the RealtimeSanitizer pass only handled attributed function definitions in IR, and attributed function declarations caused it to crash. To fix the issue, we must check whether the IR function is empty before attempting to do any manipulation of its instructions. This PR:
Full diff: https://github.com/llvm/llvm-project/pull/169577.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
index 5ef6ffb58a7c1..adb0e518c6074 100644
--- a/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
@@ -61,23 +61,21 @@ static void insertCallAtAllFunctionExitPoints(Function &Fn,
insertCallBeforeInstruction(Fn, I, InsertFnName, FunctionArgs);
}
-static PreservedAnalyses rtsanPreservedCFGAnalyses() {
- PreservedAnalyses PA;
- PA.preserveSet<CFGAnalyses>();
- return PA;
-}
+static void runSanitizeRealtime(Function &Fn) {
+ if (Fn.empty())
+ return;
-static PreservedAnalyses runSanitizeRealtime(Function &Fn) {
insertCallAtFunctionEntryPoint(Fn, "__rtsan_realtime_enter", {});
insertCallAtAllFunctionExitPoints(Fn, "__rtsan_realtime_exit", {});
- return rtsanPreservedCFGAnalyses();
}
-static PreservedAnalyses runSanitizeRealtimeBlocking(Function &Fn) {
+static void runSanitizeRealtimeBlocking(Function &Fn) {
+ if (Fn.empty())
+ return;
+
IRBuilder<> Builder(&Fn.front().front());
Value *Name = Builder.CreateGlobalString(demangle(Fn.getName()));
insertCallAtFunctionEntryPoint(Fn, "__rtsan_notify_blocking_call", {Name});
- return rtsanPreservedCFGAnalyses();
}
PreservedAnalyses RealtimeSanitizerPass::run(Module &M,
diff --git a/llvm/test/Instrumentation/RealtimeSanitizer/rtsan_attrib_declare.ll b/llvm/test/Instrumentation/RealtimeSanitizer/rtsan_attrib_declare.ll
new file mode 100644
index 0000000000000..a5336b9e9e491
--- /dev/null
+++ b/llvm/test/Instrumentation/RealtimeSanitizer/rtsan_attrib_declare.ll
@@ -0,0 +1,12 @@
+; RUN: opt < %s -passes='rtsan' -S | FileCheck %s
+
+declare void @declared_realtime_function() sanitize_realtime #0
+
+declare void @declared_blocking_function() sanitize_realtime_blocking #0
+
+; RealtimeSanitizer pass should ignore attributed functions that are just declarations
+; CHECK: declared_realtime_function
+; CHECK-EMPTY:
+; CHECK: declared_blocking_function
+; CHECK-EMPTY:
+
|
Addresses llvm#169377. Previously, the RealtimeSanitizer pass only handled attributed function definitions in IR, and attributed function declarations caused it to crash. To fix the issue, we must check whether the IR function is empty before attempting to do any manipulation of its instructions.
8877924 to
36ac559
Compare
cjappl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable enough, just have some questions before I approve
| if (Fn.empty()) | ||
| return; | ||
|
|
||
| static PreservedAnalyses runSanitizeRealtime(Function &Fn) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is not returning these preserved analyses going to bite us elsewhere? I remember most of the sanitizer transform passes (or transform passes generally) returning these.
| } | ||
| static void runSanitizeRealtime(Function &Fn) { | ||
| if (Fn.empty()) | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there precedence in other sanitizer passes for doing this? (Curious how they solved this problem)
|
Looks good! doesn't crash anymore and the rtsan tests i added to rustc also still pass. |
You can use this format. |
Addresses #169377.
Previously, the RealtimeSanitizer pass only handled attributed function definitions in IR, and we have recently found that attributed function declarations caused it to crash. To fix the issue, we must check whether the IR function is empty before attempting to do any manipulation of its instructions.
This PR:
Functions areempty()in each relevant RTSan pass routinertsanPreservedCFGAnalysesfrom the pass, whose result was unused and which would otherwise have complicated the fix