-
Notifications
You must be signed in to change notification settings - Fork 1
Attribute to disable RADSan #6
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
Changes from all commits
063cb7e
b1b6a4f
b788cf7
50c7218
590cd43
2423f12
7b2bde3
63d23ee
229d02a
cfa1dc8
15f8e6b
c6ed788
3c5e415
b8e2a46
6ce7046
5d43609
a87c83e
2f252df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // RUN: %clangxx -fsanitize=realtime %s -o %t | ||
| // RUN: not %run %t 2>&1 | FileCheck %s | ||
| // UNSUPPORTED: ios | ||
|
|
||
| #include <pthread.h> | ||
| #include <stdlib.h> | ||
|
|
||
| __attribute__((no_sanitize("realtime"))) | ||
| void bypassedLock(pthread_mutex_t& Mutex) { | ||
| pthread_mutex_lock(&Mutex); | ||
| } | ||
|
|
||
| __attribute__((no_sanitize("realtime"))) | ||
| void bypassedUnlock(pthread_mutex_t& Mutex) { | ||
| pthread_mutex_unlock(&Mutex); | ||
| } | ||
|
|
||
| void violationLock(pthread_mutex_t& Mutex) { | ||
| pthread_mutex_lock(&Mutex); | ||
| } | ||
|
|
||
| [[clang::realtime]] void process(pthread_mutex_t& Mutex) { | ||
| bypassedLock(Mutex); | ||
| bypassedUnlock(Mutex); | ||
|
|
||
| violationLock(Mutex); | ||
| bypassedUnlock(Mutex); | ||
| } | ||
|
|
||
| int main() { | ||
| pthread_mutex_t Mutex; | ||
| pthread_mutex_init(&Mutex, NULL); | ||
|
|
||
| process(Mutex); | ||
| return 0; | ||
| // CHECK: {{.*Real-time violation.*pthread_mutex_lock.*}} | ||
| // CHECK: {{.*violationLock*}} | ||
| // CHECK-NOT: {{.*pthread_mutex_unlock.*}} | ||
| // CHECK-NOT: {{.*bypassedUnlock.*}} | ||
| // CHECK-NOT: {{.*bypassedLock.*}} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1556,7 +1556,7 @@ exit: | |
| ; CHECK: select <2 x i1> <i1 true, i1 false>, <2 x i8> <i8 2, i8 3>, <2 x i8> <i8 3, i8 2> | ||
|
|
||
| call void @f.nobuiltin() builtin | ||
| ; CHECK: call void @f.nobuiltin() #51 | ||
| ; CHECK: call void @f.nobuiltin() #52 | ||
|
|
||
| call fastcc noalias ptr @f.noalias() noinline | ||
| ; CHECK: call fastcc noalias ptr @f.noalias() #12 | ||
|
|
@@ -1980,6 +1980,8 @@ declare void @f.nosanitize_bounds() nosanitize_bounds | |
| declare void @f.allockind() allockind("alloc,uninitialized") | ||
| ; CHECK: declare void @f.allockind() #50 | ||
|
|
||
| declare void @f.nosanitize_realtime() nosanitize_realtime | ||
| ; CHECK: declare void @f.nosanitize_realtime() #51 | ||
|
|
||
| ; CHECK: declare nofpclass(snan) float @nofpclass_snan(float nofpclass(snan)) | ||
| declare nofpclass(snan) float @nofpclass_snan(float nofpclass(snan)) | ||
|
|
@@ -2102,7 +2104,8 @@ define float @nofpclass_callsites(float %arg) { | |
| ; CHECK: attributes #48 = { allocsize(1,0) } | ||
| ; CHECK: attributes #49 = { nosanitize_bounds } | ||
| ; CHECK: attributes #50 = { allockind("alloc,uninitialized") } | ||
| ; CHECK: attributes #51 = { builtin } | ||
| ; CHECK: attributes #51 = { nosanitize_realtime } | ||
| ; CHECK: attributes #52 = { builtin } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did I do 51 instead of 52? These orderings are very specific, it seems. I could not get them to play nicely if I appended this, so I put it at 51. It seems like it compiles a bunch of little functions and checks the output is sane. All of the rest of them seem to be copy/paste but builtin is different somehow. This may be the reason it is last and should remain last |
||
|
|
||
| ;; Metadata | ||
|
|
||
|
|
||
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.
Alright, this feels like a kind of hack here.
Basically the idea is what I described in the original description of this PR. On line 1573, if the realtime sanitizer is disabled, it is turned off. That sounds stupid to type out, but it's important.
If the realtime sanitizer is turned off, the block on line 1573 is never tripped, and we never get the opportunity to add the bypass call in.
Our options:
The psuedocode (implemented here) feels hacky:
This feels a little hacky because it's essentially saying "Do not ACTUALLY turn off the RADSan if the flag tells you to". This means that while SanOpts will be accurate for all other sanitizers, it will not be accurate for RADSan.
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.
After having a think, this doesn't feel that bad to me. Fundamentally what "no sanitize" means for a RADSan-sanitized function is different to what "no sanitize" means in other sanitizers. For RADSan, it's not really "turning it off" - it just means that we have to do something different: inserting the
radsan_off()andradsan_on()calls! I'm not sure there's another way, as you imply as well. With a clear enough comment, I feel like this is a reasonable approach!