[LLVM][rtsan] Add nosanitize_realtime instrumentation

[cjappl]

If a function has the attribute nosanitize_realtime we insert a call to __rtsan_off to disable rtsan for this call stack. At all return points we re-enable the stack by placing __rtsan_on.

[cjappl]

CC @davidtrevelyan

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Chris Apple (cjappl)

Changes

If a function has the attribute nosanitize_realtime we insert a call to __rtsan_off to disable rtsan for this call stack. At all return points we re-enable the stack by placing __rtsan_on.

---

Full diff: https://github.com/llvm/llvm-project/pull/106125.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp (+13)
• (added) llvm/test/Instrumentation/RealtimeSanitizer/rtsan_nosanitize.ll (+25)

diff --git a/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
index 7854cf4f2c625f..1707ec298d6d28 100644
--- a/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp
@@ -19,6 +19,8 @@
 
 #include "llvm/Transforms/Instrumentation/RealtimeSanitizer.h"
 
+#include <cassert>
+
 using namespace llvm;
 
 static void insertCallBeforeInstruction(Function &Fn, Instruction &Instruction,
@@ -51,6 +53,7 @@ RealtimeSanitizerPass::RealtimeSanitizerPass(
 PreservedAnalyses RealtimeSanitizerPass::run(Function &F,
                                              AnalysisManager<Function> &AM) {
   if (F.hasFnAttribute(Attribute::SanitizeRealtime)) {
+    assert(!F.hasFnAttribute(Attribute::NoSanitizeRealtime));
     insertCallAtFunctionEntryPoint(F, "__rtsan_realtime_enter");
     insertCallAtAllFunctionExitPoints(F, "__rtsan_realtime_exit");
 
@@ -59,5 +62,15 @@ PreservedAnalyses RealtimeSanitizerPass::run(Function &F,
     return PA;
   }
 
+  if (F.hasFnAttribute(Attribute::NoSanitizeRealtime)) {
+    assert(!F.hasFnAttribute(Attribute::SanitizeRealtime));
+    insertCallAtFunctionEntryPoint(F, "__rtsan_off");
+    insertCallAtAllFunctionExitPoints(F, "__rtsan_on");
+
+    PreservedAnalyses PA;
+    PA.preserveSet<CFGAnalyses>();
+    return PA;
+  }
+
   return PreservedAnalyses::all();
 }
diff --git a/llvm/test/Instrumentation/RealtimeSanitizer/rtsan_nosanitize.ll b/llvm/test/Instrumentation/RealtimeSanitizer/rtsan_nosanitize.ll
new file mode 100644
index 00000000000000..5a465df749da65
--- /dev/null
+++ b/llvm/test/Instrumentation/RealtimeSanitizer/rtsan_nosanitize.ll
@@ -0,0 +1,25 @@
+; RUN: opt < %s -passes=rtsan -S | FileCheck %s
+
+define void @nosanitized_function() #0 {
+  %1 = alloca ptr, align 8
+  %2 = call ptr @malloc(i64 noundef 2) #3
+  store ptr %2, ptr %1, align 8
+  ret void
+}
+
+declare ptr @malloc(i64 noundef) #1
+
+define noundef i32 @main() #2 {
+  %1 = alloca i32, align 4
+  store i32 0, ptr %1, align 4
+  call void @nosanitized_function() #4
+  ret i32 0
+}
+
+attributes #0 = { nosanitize_realtime }
+
+; CHECK-LABEL: @nosanitized_function()
+; CHECK-NEXT: call{{.*}}@__rtsan_off
+
+; CHECK: call{{.*}}@__rtsan_on
+; CHECK-NEXT: ret{{.*}}void

[vitalybuka]

Why do we need NoSanitizeRealtime, when Address and Memory sanitizers can exist without them?

[cjappl]

The main reason this is necessary is we need to have a way to turn off the rtsan stack for all function calls below the function with the nosanitize attribute.

If you mark a function as needing the real-time sanitizer, all functions invoked by that function are subject to it's constraints (no allocations, no locks, no system calls). This is enforced via a stack, which is popped or pushed via __rtsan_realtime_enter and __rtsan_realtime_exit, inserted in this file.

When you mark a function as not needing to be inspected by the realtime sanitizer, it must change the stack such so these constraints are no longer in place. The nosanitize function AS WELL AS functions invoked by it can allocate, take locks, or call system calls.

Address and Memory sanitizer don't have this hierarchical model.

To make the example concrete...
Realtime sanitizer

int ChildWithViolation() {
  int* v = new int[100]; // SHOULD BE VIOLATION, can't alloc in nonblocking context
}

__attribute__((no_sanitize("realtime")))
int Parent() {
  return ChildWithViolation(); // does not throw an error, because we are within a no_sanitize("realtime") context
}

int main() [[clang::nonblocking]]{
  Parent();
}

Marking parent with no_sanitize("realtime") disables the realtime sanitizer for this function and all functions this function calls.

Address sanitizer, does not work like this, if you try to do something similar:

int ChildWithViolation() {
  int* v = new int[100];
  return v[100];
}

__attribute__((no_sanitize("address")))
int Parent() {
  return ChildWithViolation();
}

int main() {
  Parent();

 	return 0;
}

> ./build/helloWorld
=================================================================
==24510==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000107915bd0 at pc 0x000104ea3e14 bp 0x00016af5e950 sp 0x00016af5e948
READ of size 4 at 0x000107915bd0 thread T0
    #0 0x104ea3e10 in ChildWithViolation() main.cpp:17
    #1 0x104ea3e34 in Parent() main.cpp:22
    #2 0x104ea3e58 in main main.cpp:26
    #3 0x1958960dc  (<unknown module>)
...

The nosanitize attribute only turns it off for that particular function, not "down the stack"

[cjappl]

I guess another way to say the same thing as the previous comment:

ASan instruments and inspects every function in a binary. Asan disables on a per function basis.

RTSan instruments and inspects only in a specific context. RTSan disables on a per context basis.

This is the root of the difference and why we have to disable in this way, we are keeping state that tracks if we are in the context our sanitizer cares about.

[vitalybuka]

Why it needs to be attribute?

I think it's unusual, not sure if there is example at all, when fn attribute affects callees.

__lsan::ScopedDisabler achieves the same without this confusion using language and even can be more precise, by limiting itself to a code block.

[cjappl]

Thanks for pointing that out, I wasn't aware it existed!

Pros of ScopedDisabler:

• Enables finer-grained disabling within a function.
• Avoids modifying LLVM attributes or Clang, simplifying the implementation.
• Precedent exists with lsan.

Cons of ScopedDisabler:

• Doesn't follow existing patterns for disabling interceptors, unlike asan and others.
• Users need to recreate build system infrastructure for conditional compilation with -fsanitize=realtime.

The biggest concern is that second "con" -- users would need to manually include out-of-project headers and manage build conditions, leading to some potential issues:

1. Duplication of effort across users.
2. Difficulty in locating the required header files. (and conditionally including them if the sanitizer is enabled, leading to more complex build systems)
3. Risk of users re-implementing ScopedDisabler in their codebases.

The nosanitize attribute is comparatively simpler, which "just works" without extra steps. I would prefer whatever solution we go forward with, it uses a construct that would be as non-invasive as the nosanitize approach to avoid burdening users with additional setup. Are there other solutions that provide this level of ease to the end user?

For finer-grained interception, we've advised extracting code into new functions and applying the nosanitize attribute. This approach is a little more invasive but clearly indicates that the code is treated differently.

[vitalybuka]

Thanks for pointing that out, I wasn't aware it existed!

Pros of ScopedDisabler:

• Enables finer-grained disabling within a function.
• Avoids modifying LLVM attributes or Clang, simplifying the implementation.
• Precedent exists with lsan.

Cons of ScopedDisabler:

• Doesn't follow existing patterns for disabling interceptors, unlike asan and others.
• Users need to recreate build system infrastructure for conditional compilation with -fsanitize=realtime.

The biggest concern is that second "con" -- users would need to manually include out-of-project headers and manage build conditions, leading to some potential issues:

conditions should not be a problem if you check __feature in rtsan header.
So it could be included unconditionally, and just compile into no-op without RTSan
we should do the same for all sanitizers, we have some inconsistency there.

1. Duplication of effort across users.

Code modification is needed. It should be either attribute or C++ object either way, so I don't see much difference in duplication.

2. Difficulty in locating the required header files. (and conditionally including them if the sanitizer is enabled, leading to more complex build systems)

Should be unconditional. Header installed with clang.
However with g++ it may need a condition.

But I don't think it's a big issue. We used to preprocessor in C++.

3. Risk of users re-implementing ScopedDisabler in their codebases.

Why it's a problem? They can do so even you go with existing attribute. I am 100% sure that ScopedDisabler like will be needed, as function scope may affect to much.

Also inliner and other passes can be affected by attribute. Regular C++ code will be more predictable for end user.

The nosanitize attribute is comparatively simpler, which "just works" without extra steps. I would prefer whatever solution we go forward with, it uses a construct that would be as non-invasive as the nosanitize approach to avoid burdening users with additional setup. Are there other solutions that provide this level of ease to the end user?

lsan has larger user base then rtsan, at least for now, and it was never an issue. Attribute approach can work for lsan as well.

For finer-grained interception, we've advised extracting code into new functions and applying the nosanitize attribute. This approach is a little more invasive but clearly indicates that the code is treated differently.

I am not aware of context for this, can you point me there? How attribute is better here then ScopedDisabler?

So unless we have strong reason, I would prefer we solve the same problem in all component consistently.

[cjappl]

conditions should not be a problem if you check __feature in rtsan head.
So it could be included unconditionally, and just compile into no-op without RTSan
we should do the same for all sanitizers, we have some inconsistency there.

Could you elaborate on this, perhaps with a small code example? I think if this were possible my concern would be alleviated!

[cjappl]

(actually, I think I have it figured out, going to test this for a little bit and get back to you. Thanks for the guidance)

[cjappl]

This works great for us @vitalybuka ! Thanks for explaining this to me. You will see some follow up reviews today that accomplish this goal.

Closing this review.

[cjappl]

That review is posted here: #106736