-
Notifications
You must be signed in to change notification settings - Fork 12k
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
[LLVM][rtsan] Add sanitize_realtime_unsafe
attribute
#106754
[LLVM][rtsan] Add sanitize_realtime_unsafe
attribute
#106754
Conversation
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-ir Author: None (davidtrevelyan) ChangesIntroducing
|
Tagging @cjappl for review, as well as interested parties from similar past reviews: @vitalybuka @MaskRay @dougsonos @nikic @pcc Many thanks in advance for your time! |
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.
LGTM - please wait for another reviewer to sign off before merging
Weekly ping for reviewers @vitalybuka and @MaskRay - would really appreciate your oversight on this one! |
Leaving to @cjappl to land |
689f19b
to
9833759
Compare
…ealtime_blocking (#113155) # What This PR renames the newly-introduced llvm attribute `sanitize_realtime_unsafe` to `sanitize_realtime_blocking`. Likewise, sibling variables such as `SanitizeRealtimeUnsafe` are renamed to `SanitizeRealtimeBlocking` respectively. There are no other functional changes. # Why? - There are a number of problems that can cause a function to be real-time "unsafe", - we wish to communicate what problems rtsan detects and *why* they're unsafe, and - a generic "unsafe" attribute is, in our opinion, too broad a net - which may lead to future implementations that need extra contextual information passed through them in order to communicate meaningful reasons to users. - We want to avoid this situation and make the runtime library boundary API/ABI as simple as possible, and - we believe that restricting the scope of attributes to names like `sanitize_realtime_blocking` is an effective means of doing so. We also feel that the symmetry between `[[clang::blocking]]` and `sanitize_realtime_blocking` is easier to follow as a developer. # Concerns - I'm aware that the LLVM attribute `sanitize_realtime_unsafe` has been part of the tree for a few weeks now (introduced here: #106754). Given that it hasn't been released in version 20 yet, am I correct in considering this to not be a breaking change?
…ealtime_blocking (llvm#113155) # What This PR renames the newly-introduced llvm attribute `sanitize_realtime_unsafe` to `sanitize_realtime_blocking`. Likewise, sibling variables such as `SanitizeRealtimeUnsafe` are renamed to `SanitizeRealtimeBlocking` respectively. There are no other functional changes. # Why? - There are a number of problems that can cause a function to be real-time "unsafe", - we wish to communicate what problems rtsan detects and *why* they're unsafe, and - a generic "unsafe" attribute is, in our opinion, too broad a net - which may lead to future implementations that need extra contextual information passed through them in order to communicate meaningful reasons to users. - We want to avoid this situation and make the runtime library boundary API/ABI as simple as possible, and - we believe that restricting the scope of attributes to names like `sanitize_realtime_blocking` is an effective means of doing so. We also feel that the symmetry between `[[clang::blocking]]` and `sanitize_realtime_blocking` is easier to follow as a developer. # Concerns - I'm aware that the LLVM attribute `sanitize_realtime_unsafe` has been part of the tree for a few weeks now (introduced here: llvm#106754). Given that it hasn't been released in version 20 yet, am I correct in considering this to not be a breaking change?
…ealtime_blocking (llvm#113155) # What This PR renames the newly-introduced llvm attribute `sanitize_realtime_unsafe` to `sanitize_realtime_blocking`. Likewise, sibling variables such as `SanitizeRealtimeUnsafe` are renamed to `SanitizeRealtimeBlocking` respectively. There are no other functional changes. # Why? - There are a number of problems that can cause a function to be real-time "unsafe", - we wish to communicate what problems rtsan detects and *why* they're unsafe, and - a generic "unsafe" attribute is, in our opinion, too broad a net - which may lead to future implementations that need extra contextual information passed through them in order to communicate meaningful reasons to users. - We want to avoid this situation and make the runtime library boundary API/ABI as simple as possible, and - we believe that restricting the scope of attributes to names like `sanitize_realtime_blocking` is an effective means of doing so. We also feel that the symmetry between `[[clang::blocking]]` and `sanitize_realtime_blocking` is easier to follow as a developer. # Concerns - I'm aware that the LLVM attribute `sanitize_realtime_unsafe` has been part of the tree for a few weeks now (introduced here: llvm#106754). Given that it hasn't been released in version 20 yet, am I correct in considering this to not be a breaking change?
Introducing
sanitize_realtime_unsafe
attributeMotivation
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.Implementation
At the same time as introducing
[[clang::nonblocking]]
, the[[clang::blocking]]
attribute was also introduced. With the performance constraints attribute warnings (as errors) activated, blocking functions cannot be called from non-blocking functions, and this is enforced at compile time. The purpose of this PR is to introduce similar functionality into RealtimeSanitizer, so that it can make the equivalent check at run time.Using the same mechanism as adding the
sanitize_realtime
LLVM function attribute to the IR of[[clang::nonblocking]]
functions, we wish to add thesanitize_realtime_unsafe
attribute. If this attribute is detected, the RealtimeSanitizer pass inserts a call to__rtsan_expect_not_realtime
.Integration Roadmap
The above functionality is currently split into two patches.
sanitize_realtime_unsafe
LLVM attribute (this PR), and