[llvm] [llvm][rtsan] Add transform pass for sanitize_realtime_unsafe (PR #109543)

[via llvm-commits]

================
@@ -17,47 +17,75 @@
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Module.h"
 
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/Transforms/Instrumentation/RealtimeSanitizer.h"
 
 using namespace llvm;
 
+static SmallVector<Type *> getArgTypes(ArrayRef<Value *> FunctionArgs) {
+  SmallVector<Type *> Types;
+  for (Value *Arg : FunctionArgs)
+    Types.push_back(Arg->getType());
+  return Types;
+}
+
 static void insertCallBeforeInstruction(Function &Fn, Instruction &Instruction,
-                                        const char *FunctionName) {
+                                        const char *FunctionName,
+                                        ArrayRef<Value *> FunctionArgs) {
   LLVMContext &Context = Fn.getContext();
-  FunctionType *FuncType = FunctionType::get(Type::getVoidTy(Context), false);
+  FunctionType *FuncType = FunctionType::get(Type::getVoidTy(Context),
+                                             getArgTypes(FunctionArgs), false);
   FunctionCallee Func =
       Fn.getParent()->getOrInsertFunction(FunctionName, FuncType);
   IRBuilder<> Builder{&Instruction};
-  Builder.CreateCall(Func, {});
+  Builder.CreateCall(Func, FunctionArgs);
 }
 
 static void insertCallAtFunctionEntryPoint(Function &Fn,
-                                           const char *InsertFnName) {
-
-  insertCallBeforeInstruction(Fn, Fn.front().front(), InsertFnName);
+                                           const char *InsertFnName,
+                                           ArrayRef<Value *> FunctionArgs) {
+  insertCallBeforeInstruction(Fn, Fn.front().front(), InsertFnName,
+                              FunctionArgs);
 }
 
 static void insertCallAtAllFunctionExitPoints(Function &Fn,
-                                              const char *InsertFnName) {
+                                              const char *InsertFnName,
+                                              ArrayRef<Value *> FunctionArgs) {
   for (auto &BB : Fn)
     for (auto &I : BB)
       if (isa<ReturnInst>(&I))
-        insertCallBeforeInstruction(Fn, I, InsertFnName);
+        insertCallBeforeInstruction(Fn, I, InsertFnName, FunctionArgs);
+}
+
+static PreservedAnalyses rtsanPreservedCFGAnalyses() {
+  PreservedAnalyses PA;
+  PA.preserveSet<CFGAnalyses>();
+  return PA;
+}
+
+static PreservedAnalyses runSanitizeRealtime(Function &Fn) {
+  insertCallAtFunctionEntryPoint(Fn, "__rtsan_realtime_enter", {});
+  insertCallAtAllFunctionExitPoints(Fn, "__rtsan_realtime_exit", {});
+  return rtsanPreservedCFGAnalyses();
+}
+
+static PreservedAnalyses runSanitizeRealtimeUnsafe(Function &Fn) {
+  IRBuilder<> Builder(&Fn.front().front());
+  Value *Name = Builder.CreateGlobalString(demangle(Fn.getName()));
----------------
davidtrevelyan wrote:

Thanks for this question and prompting us to look into this in more detail. I had indeed not considered the issue of binary size, so went away and thought about it a bit harder with @cjappl. There's lots to think about here, especially in terms of how we expect the sanitizer to be used.

We anticipate that the sanitizer users will _only be marking a handful of functions they are worried about_ with the `[[clang::blocking]]` attribute, which is what ultimately results in `sanitize_realtime_unsafe` being added in IR. We think this because any functions that do the normal bad real-time stuff like allocation will be picked up by the libc interceptors anyway. Adding the `[[clang::blocking]]` attribute is more for user functions that could block _entirely within user space_, like a spin mutex lock or similar busy-wait scenario. Conservatively estimating that a user has 10 concerning `[[blocking]]` functions, each with demangled symbol length 25 characters - I think that would be (at least ideally) of the order 250 bytes of extra binary size to store. The rtsan runtime library is (when stripped) currently about 750 kB already, so we think this is about a 0.03% size increment - and only for sanitized builds.

We tried the PC approach in a branch and confirmed it works, but we were sad to lose the function name for printing diagnostics in release builds, and also to have to do the extra run-time work to unwind the stack. Being able to run the sanitizer with release builds is one of our design goals - we'd like to achieve as-near-normal performance on the real-time thread as possible, so that users can meaningfully QA their real-time systems with RealtimeSanitizer enabled.

Given:

- the extra work at run time necessary to unwind the stack,
- the simpler run-time implementation when accepting a function name string,
- the loss of being able to print the blocking function name in release builds, and
- the almost-negligible expected binary increment,

... our moderate preference is currently to keep the approach how we have it outlined in this PR. However, this is of course caveated with the appreciation that we're very new here and inexperienced with the codebase, so I'm very keen to take as much advice as possible on this. Many thanks for your help so far - keen to hear your thoughts!


https://github.com/llvm/llvm-project/pull/109543