Skip to content

[clang][rtsan] Add sanitize_realtime_unsafe attr to [[clang::blocking]] function IR #111055

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

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

davidtrevelyan
Copy link
Contributor

Clang CodeGen for [[clang::blocking]] with RTSan

Follows #106754 and #109543. This is the final patch for the feature.

Motivation

Calls to system library functions such as malloc are easy for RealtimeSanitizer to intercept. If such a call is made in a [[clang::nonblocking]] function (a real-time context), RealtimeSanitizer will error. Real-time programmers also write their own blocking (real-time unsafe) functions that may or may not call intercepted functions. We wish to introduce a mechanism whereby RealtimeSanitizer can error on calls to these, too, if called within a real-time context.

At the same time as introducing [[clang::nonblocking]], the [[clang::blocking]] attribute was also introduced. With the function effects warnings (as errors) activated, blocking functions cannot be called from non-blocking functions, and this is enforced at compile time. The purpose of this series of PRs is to introduce similar functionality into RealtimeSanitizer, so that it can make the equivalent check at run time.

Implementation

We recently merged the sanitize_realtime_unsafe LLVM function attribute into main, as well as the LLVM pass to notify the sanitizer runtime of the blocking call. Our final step is to switch on the feature by updating Clang's CodeGen. This patch just adds the sanitize_realtime_unsafe attribute to the IR for functions attributed with [[clang::blocking]].

Once the feature is switched on, RealtimeSanitizer will error if any calls to functions attributed with [[clang::blocking]] are made from [[clang::nonblocking]] functions.

Integration Roadmap

The above functionality is currently split into three patches.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Oct 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-clang

Author: None (davidtrevelyan)

Changes

Clang CodeGen for [[clang::blocking]] with RTSan

Follows #106754 and #109543. This is the final patch for the feature.

Motivation

Calls to system library functions such as malloc are easy for RealtimeSanitizer to intercept. If such a call is made in a [[clang::nonblocking]] function (a real-time context), RealtimeSanitizer will error. Real-time programmers also write their own blocking (real-time unsafe) functions that may or may not call intercepted functions. We wish to introduce a mechanism whereby RealtimeSanitizer can error on calls to these, too, if called within a real-time context.

At the same time as introducing [[clang::nonblocking]], the [[clang::blocking]] attribute was also introduced. With the function effects warnings (as errors) activated, blocking functions cannot be called from non-blocking functions, and this is enforced at compile time. The purpose of this series of PRs is to introduce similar functionality into RealtimeSanitizer, so that it can make the equivalent check at run time.

Implementation

We recently merged the sanitize_realtime_unsafe LLVM function attribute into main, as well as the LLVM pass to notify the sanitizer runtime of the blocking call. Our final step is to switch on the feature by updating Clang's CodeGen. This patch just adds the sanitize_realtime_unsafe attribute to the IR for functions attributed with [[clang::blocking]].

Once the feature is switched on, RealtimeSanitizer will error if any calls to functions attributed with [[clang::blocking]] are made from [[clang::nonblocking]] functions.

Integration Roadmap

The above functionality is currently split into three patches.


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+2)
  • (modified) clang/test/CodeGen/rtsan_attribute_inserted.c (+7-3)
  • (modified) clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c (+4-2)
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 24723e392c2a3a..e1fd9b72b8d7b2 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -850,6 +850,8 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
       for (const FunctionEffectWithCondition &Fe : FD->getFunctionEffects()) {
         if (Fe.Effect.kind() == FunctionEffect::Kind::NonBlocking)
           Fn->addFnAttr(llvm::Attribute::SanitizeRealtime);
+        else if (Fe.Effect.kind() == FunctionEffect::Kind::Blocking)
+          Fn->addFnAttr(llvm::Attribute::SanitizeRealtimeUnsafe);
       }
 
   // Apply fuzzing attribute to the function.
diff --git a/clang/test/CodeGen/rtsan_attribute_inserted.c b/clang/test/CodeGen/rtsan_attribute_inserted.c
index 05a1d9a8c2047a..b21ecb6b6b06a9 100644
--- a/clang/test/CodeGen/rtsan_attribute_inserted.c
+++ b/clang/test/CodeGen/rtsan_attribute_inserted.c
@@ -1,7 +1,11 @@
 // RUN: %clang_cc1  -triple x86_64-unknown-linux -fsanitize=realtime %s -emit-llvm -o - %s | FileCheck %s
 
 float process(float *a) [[clang::nonblocking]] { return *a; }
-
-// CHECK-LABEL: @process{{.*}}#0 {
+// CHECK: @process{{.*}} #0 {
 // CHECK: attributes #0 = {
-// CHECK-SAME: {{.*sanitize_realtime.*}}
+// CHECK-SAME: {{.*sanitize_realtime .*}}
+
+int spinlock(int *a) [[clang::blocking]] { return *a; }
+// CHECK: @spinlock{{.*}} #1 {
+// CHECK: attributes #1 = {
+// CHECK-SAME: {{.*sanitize_realtime_unsafe .*}}
diff --git a/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c b/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c
index 43ad6ed1a429ee..0f43007c5e4c16 100644
--- a/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c
+++ b/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
 
 float process(float *a) [[clang::nonblocking]] { return *a; }
+int spinlock(int *a) [[clang::blocking]] { return *a; }
 
-// Without the -fsanitize=realtime flag, we shouldn't attach the attribute.
-// CHECK-NOT: {{.*sanitize_realtime.*}}
+// Without the -fsanitize=realtime flag, we shouldn't attach the attributes.
+// CHECK-NOT: {{.*sanitize_realtime .*}}
+// CHECK-NOT: {{.*sanitize_realtime_unsafe .*}}

@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-clang-codegen

Author: None (davidtrevelyan)

Changes

Clang CodeGen for [[clang::blocking]] with RTSan

Follows #106754 and #109543. This is the final patch for the feature.

Motivation

Calls to system library functions such as malloc are easy for RealtimeSanitizer to intercept. If such a call is made in a [[clang::nonblocking]] function (a real-time context), RealtimeSanitizer will error. Real-time programmers also write their own blocking (real-time unsafe) functions that may or may not call intercepted functions. We wish to introduce a mechanism whereby RealtimeSanitizer can error on calls to these, too, if called within a real-time context.

At the same time as introducing [[clang::nonblocking]], the [[clang::blocking]] attribute was also introduced. With the function effects warnings (as errors) activated, blocking functions cannot be called from non-blocking functions, and this is enforced at compile time. The purpose of this series of PRs is to introduce similar functionality into RealtimeSanitizer, so that it can make the equivalent check at run time.

Implementation

We recently merged the sanitize_realtime_unsafe LLVM function attribute into main, as well as the LLVM pass to notify the sanitizer runtime of the blocking call. Our final step is to switch on the feature by updating Clang's CodeGen. This patch just adds the sanitize_realtime_unsafe attribute to the IR for functions attributed with [[clang::blocking]].

Once the feature is switched on, RealtimeSanitizer will error if any calls to functions attributed with [[clang::blocking]] are made from [[clang::nonblocking]] functions.

Integration Roadmap

The above functionality is currently split into three patches.


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+2)
  • (modified) clang/test/CodeGen/rtsan_attribute_inserted.c (+7-3)
  • (modified) clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c (+4-2)
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 24723e392c2a3a..e1fd9b72b8d7b2 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -850,6 +850,8 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
       for (const FunctionEffectWithCondition &Fe : FD->getFunctionEffects()) {
         if (Fe.Effect.kind() == FunctionEffect::Kind::NonBlocking)
           Fn->addFnAttr(llvm::Attribute::SanitizeRealtime);
+        else if (Fe.Effect.kind() == FunctionEffect::Kind::Blocking)
+          Fn->addFnAttr(llvm::Attribute::SanitizeRealtimeUnsafe);
       }
 
   // Apply fuzzing attribute to the function.
diff --git a/clang/test/CodeGen/rtsan_attribute_inserted.c b/clang/test/CodeGen/rtsan_attribute_inserted.c
index 05a1d9a8c2047a..b21ecb6b6b06a9 100644
--- a/clang/test/CodeGen/rtsan_attribute_inserted.c
+++ b/clang/test/CodeGen/rtsan_attribute_inserted.c
@@ -1,7 +1,11 @@
 // RUN: %clang_cc1  -triple x86_64-unknown-linux -fsanitize=realtime %s -emit-llvm -o - %s | FileCheck %s
 
 float process(float *a) [[clang::nonblocking]] { return *a; }
-
-// CHECK-LABEL: @process{{.*}}#0 {
+// CHECK: @process{{.*}} #0 {
 // CHECK: attributes #0 = {
-// CHECK-SAME: {{.*sanitize_realtime.*}}
+// CHECK-SAME: {{.*sanitize_realtime .*}}
+
+int spinlock(int *a) [[clang::blocking]] { return *a; }
+// CHECK: @spinlock{{.*}} #1 {
+// CHECK: attributes #1 = {
+// CHECK-SAME: {{.*sanitize_realtime_unsafe .*}}
diff --git a/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c b/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c
index 43ad6ed1a429ee..0f43007c5e4c16 100644
--- a/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c
+++ b/clang/test/CodeGen/rtsan_no_attribute_sanitizer_disabled.c
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
 
 float process(float *a) [[clang::nonblocking]] { return *a; }
+int spinlock(int *a) [[clang::blocking]] { return *a; }
 
-// Without the -fsanitize=realtime flag, we shouldn't attach the attribute.
-// CHECK-NOT: {{.*sanitize_realtime.*}}
+// Without the -fsanitize=realtime flag, we shouldn't attach the attributes.
+// CHECK-NOT: {{.*sanitize_realtime .*}}
+// CHECK-NOT: {{.*sanitize_realtime_unsafe .*}}

@davidtrevelyan
Copy link
Contributor Author

@cjappl for review (and please could you also request reviews from the key interested parties?)

PS I'll apply for membership after this PR so I hope this is the last time you'll have to do this for me :)

Copy link
Contributor

@cjappl cjappl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will wait to merge for a bit to let others chime in, but LGTM

@fmayer
Copy link
Contributor

fmayer commented Oct 3, 2024

With the function effects warnings (as errors) activated, blocking functions cannot be called from non-blocking functions, and this is enforced at compile time. The purpose of this series of PRs is to introduce similar functionality into RealtimeSanitizer, so that it can make the equivalent check at run time.

What is the reason we need to check something again at runtime that was already checked at compile-time? In case people didn't -Werror the warning?

@davidtrevelyan
Copy link
Contributor Author

With the function effects warnings (as errors) activated, blocking functions cannot be called from non-blocking functions, and this is enforced at compile time. The purpose of this series of PRs is to introduce similar functionality into RealtimeSanitizer, so that it can make the equivalent check at run time.

What is the reason we need to check something again at runtime that was already checked at compile-time? In case people didn't -Werror the warning?

Yes indeed - that's one of a few scenarios where we believe this is needed:

  • the user didn't compile with -Werror,
  • the user didn't compile with -Wfunction-effects (i.e. no checking at compile time happens),
  • the [[clang::blocking]] function is called deep within the call stack of a higher-level [[clang::nonblocking]] function, or maybe even
  • the [[clang::blocking]] function is pre-compiled in a different library to what the user is compiling.

RTSan differs from the performance constraints attributes in that it only flags violations that happen at run time, in contrast to flagging those that could happen at compile time. In this scenario, if a [[clang::blocking]] call exists somewhere in the code within a [[clang::nonblocking]] function, rtsan does not consider it a violation until it's called. Depending on the user's needs they may consider this either good or bad - there are pros and cons to it, of course. RTSan takes an "innocent until proven guilty" approach, whereas performance constraints are more pessimistically "guilty until proven innocent" - and we think both are very useful.

One of the design goals of the works was that these systems should be able to be used easily together, or separately, and that they should have analogous functionalities where possible. Hope that makes some sense!

@fmayer
Copy link
Contributor

fmayer commented Oct 3, 2024

With the function effects warnings (as errors) activated, blocking functions cannot be called from non-blocking functions, and this is enforced at compile time. The purpose of this series of PRs is to introduce similar functionality into RealtimeSanitizer, so that it can make the equivalent check at run time.

What is the reason we need to check something again at runtime that was already checked at compile-time? In case people didn't -Werror the warning?

Yes indeed - that's one of a few scenarios where we believe this is needed:

  • the user didn't compile with -Werror,
  • the user didn't compile with -Wfunction-effects (i.e. no checking at compile time happens),
  • the [[clang::blocking]] function is called deep within the call stack of a higher-level [[clang::nonblocking]] function, or maybe even
  • the [[clang::blocking]] function is pre-compiled in a different library to what the user is compiling.

RTSan differs from the performance constraints attributes in that it only flags violations that happen at run time, in contrast to flagging those that could happen at compile time. In this scenario, if a [[clang::blocking]] call exists somewhere in the code within a [[clang::nonblocking]] function, rtsan does not consider it a violation until it's called. Depending on the user's needs they may consider this either good or bad - there are pros and cons to it, of course. RTSan takes an "innocent until proven guilty" approach, whereas performance constraints are more pessimistically "guilty until proven innocent" - and we think both are very useful.

One of the design goals of the works was that these systems should be able to be used easily together, or separately, and that they should have analogous functionalities where possible. Hope that makes some sense!

Thanks for confirming. Optionally mention this somewhere in a comment in the code for future reference.

@cjappl cjappl merged commit 296a00b into llvm:main Oct 4, 2024
11 checks passed
@cjappl cjappl deleted the rtsan/blocking-3-codegen branch October 4, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants