[LLVM][rtsan] Add RealtimeSanitizer transform pass #101232
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-compiler-rt-sanitizer Author: Chris Apple (cjappl) ChangesSplit from #100596 at the request of the reviewers. (This review directly depends on that review being merged first) Introduce the RealtimeSanitizer transform, which inserts the rtsan_enter/exit functions at the appropriate places in an instrumented function. Full diff: https://github.com/llvm/llvm-project/pull/101232.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizer.h b/llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizer.h
new file mode 100644
index 0000000000000..9cf2448361608
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizer.h
@@ -0,0 +1,35 @@
+//===--------- Definition of the RealtimeSanitizer class ---------*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_REALTIMESANITIZER_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_REALTIMESANITIZER_H
+
+#include "llvm/IR/PassManager.h"
+#include "llvm/Transforms/Instrumentation/RealtimeSanitizerOptions.h"
+
+namespace llvm {
+
+struct RealtimeSanitizerOptions {};
+
+class RealtimeSanitizerPass : public PassInfoMixin<RealtimeSanitizerPass> {
+public:
+ RealtimeSanitizerPass(const RealtimeSanitizerOptions &Options);
+ PreservedAnalyses run(Function &F, AnalysisManager<Function> &AM);
+
+ static bool isRequired() { return true; }
+
+private:
+ RealtimeSanitizerOptions Options{};
+};
+
+} // namespace llvm
+
+#endif // LLVM_TRANSFORMS_INSTRUMENTATION_REALTIMESANITIZER_H
diff --git a/llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizerOptions.h b/llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizerOptions.h
new file mode 100644
index 0000000000000..35376a5647c60
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizerOptions.h
@@ -0,0 +1,17 @@
+//===--------- Definition of the RealtimeSanitizer options -------*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// This file defines data types used to set Realtime Sanitizer options.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_REALTIMESANITIZEROPTIONS_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_REALTIMESANITIZEROPTIONS_H
+
+namespace llvm {} // namespace llvm
+
+#endif
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 5dbb1e2f49871..faf3e25cb31d9 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -198,6 +198,7 @@
#include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h"
#include "llvm/Transforms/Instrumentation/PGOInstrumentation.h"
#include "llvm/Transforms/Instrumentation/PoisonChecking.h"
+#include "llvm/Transforms/Instrumentation/RealtimeSanitizer.h"
#include "llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h"
#include "llvm/Transforms/Instrumentation/SanitizerCoverage.h"
#include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
@@ -1207,6 +1208,11 @@ parseRegAllocFastPassOptions(PassBuilder &PB, StringRef Params) {
return Opts;
}
+Expected<RealtimeSanitizerOptions> parseRtSanPassOptions(StringRef Params) {
+ RealtimeSanitizerOptions Result;
+ return Result;
+}
+
} // namespace
/// Tests whether a pass name starts with a valid prefix for a default pipeline
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 3b92823cd283b..f795c7242f958 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -590,6 +590,10 @@ FUNCTION_PASS_WITH_PARAMS(
return WinEHPreparePass(DemoteCatchSwitchPHIOnly);
},
parseWinEHPrepareOptions, "demote-catchswitch-only")
+FUNCTION_PASS_WITH_PARAMS(
+ "rtsan", "RealtimeSanitizerPass",
+ [](RealtimeSanitizerOptions Opts) { return RealtimeSanitizerPass(Opts); },
+ parseRtSanPassOptions, "")
#undef FUNCTION_PASS_WITH_PARAMS
#ifndef LOOPNEST_PASS
diff --git a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt
index 4e3f9e27e0c34..deab37801ff1d 100644
--- a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt
+++ b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt
@@ -25,6 +25,7 @@ add_llvm_component_library(LLVMInstrumentation
ValueProfileCollector.cpp
ThreadSanitizer.cpp
HWAddressSanitizer.cpp
+ RealtimeSanitizer.cpp
ADDITIONAL_HEADER_DIRS
${LLVM_MAIN_INCLUDE_DIR}/llvm/Transforms
diff --git a/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
new file mode 100644
index 0000000000000..2fa2389c4984f
--- /dev/null
+++ b/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
@@ -0,0 +1,50 @@
+#include "llvm/IR/Analysis.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Module.h"
+
+#include "llvm/Transforms/Instrumentation/RealtimeSanitizer.h"
+
+using namespace llvm;
+
+namespace {
+
+void insertCallBeforeInstruction(Function &Fn, Instruction &Instruction,
+ const char *FunctionName) {
+ LLVMContext &Context = Fn.getContext();
+ FunctionType *FuncType = FunctionType::get(Type::getVoidTy(Context), false);
+ FunctionCallee Func =
+ Fn.getParent()->getOrInsertFunction(FunctionName, FuncType);
+ IRBuilder<> Builder{&Instruction};
+ Builder.CreateCall(Func, {});
+}
+
+void insertCallAtFunctionEntryPoint(Function &Fn, const char *InsertFnName) {
+
+ insertCallBeforeInstruction(Fn, Fn.front().front(), InsertFnName);
+}
+
+void insertCallAtAllFunctionExitPoints(Function &Fn, const char *InsertFnName) {
+ for (auto &BB : Fn) {
+ for (auto &I : BB) {
+ if (auto *RI = dyn_cast<ReturnInst>(&I)) {
+ insertCallBeforeInstruction(Fn, I, InsertFnName);
+ }
+ }
+ }
+}
+} // namespace
+
+RealtimeSanitizerPass::RealtimeSanitizerPass(
+ const RealtimeSanitizerOptions &Options)
+ : Options{Options} {}
+
+PreservedAnalyses RealtimeSanitizerPass::run(Function &F,
+ AnalysisManager<Function> &AM) {
+ if (F.hasFnAttribute(Attribute::NonBlocking)) {
+ insertCallAtFunctionEntryPoint(F, "__rtsan_realtime_enter");
+ insertCallAtAllFunctionExitPoints(F, "__rtsan_realtime_exit");
+ return PreservedAnalyses::none();
+ }
+
+ return PreservedAnalyses::all();
+}
diff --git a/llvm/test/Instrumentation/RealtimeSanitizer/rtsan.ll b/llvm/test/Instrumentation/RealtimeSanitizer/rtsan.ll
new file mode 100644
index 0000000000000..222df484273c7
--- /dev/null
+++ b/llvm/test/Instrumentation/RealtimeSanitizer/rtsan.ll
@@ -0,0 +1,35 @@
+; RUN: opt < %s -passes=rtsan -S | FileCheck %s
+
+; Function Attrs: mustprogress noinline nonblocking optnone ssp uwtable(sync)
+define void @violation() #0 {
+ %1 = alloca ptr, align 8
+ %2 = call ptr @malloc(i64 noundef 2) #3
+ store ptr %2, ptr %1, align 8
+ ret void
+}
+
+; Function Attrs: allocsize(0)
+declare ptr @malloc(i64 noundef) #1
+
+; Function Attrs: mustprogress noinline norecurse optnone ssp uwtable(sync)
+define noundef i32 @main() #2 {
+ %1 = alloca i32, align 4
+ store i32 0, ptr %1, align 4
+ call void @violation() #4
+ ret i32 0
+}
+
+attributes #0 = { mustprogress noinline nonblocking optnone ssp uwtable(sync) }
+attributes #1 = { allocsize(0) }
+attributes #2 = { mustprogress noinline norecurse optnone ssp uwtable(sync) }
+attributes #3 = { allocsize(0) }
+attributes #4 = { nonblocking }
+
+; RealtimeSanitizer pass should insert __rtsan_realtime_enter right after function definition
+; CHECK: define{{.*}}violation
+; CHECK-NEXT: call{{.*}}__rtsan_realtime_enter
+
+; RealtimeSanitizer pass should insert __rtsan_realtime_exit right before function return
+; CHECK: call{{.*}}@__rtsan_realtime_exit
+; CHECK-NEXT: ret{{.*}}void
+
|
|
CC Reviewers @vitalybuka @MaskRay This review should be green once #100596 is merged, as this depends on the attribute existing CC co-author @davidtrevelyan |
vitalybuka
reviewed
Jul 31, 2024
vitalybuka
reviewed
Jul 31, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vitalybuka
reviewed
Jul 31, 2024
| namespace { | ||
|
|
||
| void insertCallBeforeInstruction(Function &Fn, Instruction &Instruction, | ||
| const char *FunctionName) { |
There was a problem hiding this comment.
Do you expect Module specific features? Other sanitizers are ModulePasses already.
There was a problem hiding this comment.
At this time, no we do not expect to have Module specific features. A realtime or "nonblocking" context only deals with functions. We will insert appropriate calls at points within each function.
As far as I can tell, the FunctionPass will be all that we need.
There was a problem hiding this comment.
Prefer static for internal linkage functions
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces
vitalybuka
approved these changes
Jul 31, 2024
cjappl
force-pushed
the
rtsan_llvm_transform
branch
from
b181a91 to
aadb2db
Compare
|
Thanks for the approval and feedback. A reminder that this should not be merged before the attribute is settled in the other review (as I expect these tests to be red until then) |
MaskRay
reviewed
Jul 31, 2024
llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizerOptions.h
Outdated
Show resolved
Hide resolved
MaskRay
reviewed
Jul 31, 2024
MaskRay
reviewed
Jul 31, 2024
MaskRay
reviewed
Jul 31, 2024
MaskRay
reviewed
Jul 31, 2024
llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizer.h
Outdated
Show resolved
Hide resolved
|
Thanks @MaskRay ! Fixed all your comments in the latest version. |
MaskRay
reviewed
Jul 31, 2024
llvm/include/llvm/Transforms/Instrumentation/RealtimeSanitizer.h
Outdated
Show resolved
Hide resolved
MaskRay
reviewed
Jul 31, 2024
MaskRay
reviewed
Jul 31, 2024
MaskRay
approved these changes
Jul 31, 2024
|
Thanks for the reviews, now once the discussion on the attributes is finished in #100596 we can merge that, get green tests here and merge this. I will ping this review again when that other one is completely settled. If anyone has any thoughts on the name of the attribute, please chime in over there. |
cjappl
force-pushed
the
rtsan_llvm_transform
branch
from
d100061 to
6be30bb
Compare
|
@MaskRay @vitalybuka could I get a merge when you get a chance? Since your last review I:
Test being green shows that change took and everything is working as intended! Thanks as always. |
MaskRay
reviewed
Aug 8, 2024
| @@ -0,0 +1,46 @@ | |||
| #include "llvm/IR/Analysis.h" | |||
|
Thanks for the help @MaskRay , always sincerely appreciated. |
Np:) Glad to help. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/1270 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/2046 Here is the relevant piece of the build log for the reference: |
MaskRay
added a commit
that referenced
this pull request
Aug 8, 2024
Fixed in f865947 Therefore, we recommend that unused variables are not added in a patch, even if it will soon be used by the next patch. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/2657 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/1900 Here is the relevant piece of the build log for the reference: |
That makes sense!! Thanks for the speedy patch |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/2297 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/1104 Here is the relevant piece of the build log for the reference: |
Looking at the build log of this last one, it appears to be the same problem with the private member. This is fixed in main |
nikic
reviewed
Aug 9, 2024
| if (F.hasFnAttribute(Attribute::SanitizeRealtime)) { | ||
| insertCallAtFunctionEntryPoint(F, "__rtsan_realtime_enter"); | ||
| insertCallAtAllFunctionExitPoints(F, "__rtsan_realtime_exit"); | ||
| return PreservedAnalyses::none(); |
There was a problem hiding this comment.
Not super important, but this could preserveSet<CFGAnalyses>.
There was a problem hiding this comment.
Thanks! I can fix this in a follow up.
Do you know where I can find more info on what this means? I was pattern matching other transforms and don't have a deep knowledge of what these "preserved analyses" actually do for the pass.
Any additional info greatly appreciated!
There was a problem hiding this comment.
It means whether the analysis has to be re-evalulated after the pass (if needed by a pass afterwards), or whether the pass has not changed the result (or, for some analyses, it is possible to incrementally change them, in which case it could also have done that).
There was a problem hiding this comment.
Ok, if I can rephrase what you're saying for my own education:
"If a pass has changed the code in some way, we need to alert the analysis that it has been changed so it can be re-evaluated.
In my original code, I basically said the maximal 'this has changed EVERYTHING, please re-run EVERYTHING (preserve nothing)'. Where if I was being more judicious, I would have only said 'I changed the code, but the Control Flow Analysis is still valid, you do not have to re-run that portion, it is preserved'
"
Is my understanding correct?
Sincerely appreciate the information here, this area of the project is brand new to me and I will take any knowledge I can get.
nikic
pushed a commit
that referenced
this pull request
Aug 9, 2024
Follow on to #101232, as suggested in the comments, narrow the scope of the preserved analyses.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split from #100596 at the request of the reviewers. (This review directly depends on that review being merged first)
Introduce the RealtimeSanitizer transform, which inserts the rtsan_enter/exit functions at the appropriate places in an instrumented function.