-
Notifications
You must be signed in to change notification settings - Fork 0
add attribute docs #2
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
Conversation
f8e534c to
304b1d7
Compare
|
I have reworked it a bit to remove "disable". I think i either addressed you comments or removed the text they were part of. I hope you are fine with exposing the disable functions for now. This isn't going on stable rust, so they could be removed once the attribute exists. |
| from a function not sanitized. So a function could be both sanitized and not sanitized in one program execution. | ||
| The sanitizer can be disabled using the external function `__rtsan_disable()`. It can be enabled again using | ||
| `__rtsan_enable()`. Entering a real-time context while the sanitizer is enabled is not possible. |
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.
Entering a real-time context while the sanitizer is enabled is not possible.
I think you meant to say the opposite here but, regardless, I think it's possibly not worth including - IMHO it's more confusing than helpful for the average user who isn't aware of the sanitizer's inner workings. If you really feel it should be included, I'd suggest maybe something like Entering a real-time context while the sanitizer is disabled does not reactivate the sanitization.
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.
Yeah i thought about that too, will remove it.
Is there a open llvm/clang (or even in one of your repos) about it?
Just that it's written down at least somewhere.
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.
Thanks for the reminder! Yes - just added one
Great - I think your changes are nice improvements! I've left a couple more minor comments but other than those it LGTM 👍 |
9d0f497 to
90c564e
Compare
90c564e to
95bdb34
Compare
1cdd36f to
7ea4dc6
Compare
…=jieyouxu Rehome 30 `tests/ui/issues/` tests to other subdirectories under `tests/ui/` [#1 of Batch #2] Part of rust-lang#133895 Methodology: 1. Refer to the previously written `tests/ui/SUMMARY.md` 2. Find an appropriate category for the test, using the original issue thread and the test contents. 3. Add the issue URL at the bottom (not at the top, as that would mess up stderr line numbers) 4. Rename the tests to make their purpose clearer Inspired by the methodology that `@Kivooeo` was using. r? `@jieyouxu`
…=jieyouxu Rehome 30 `tests/ui/issues/` tests to other subdirectories under `tests/ui/` [#2 of Batch #2] Part of rust-lang#133895 Methodology: 1. Refer to the previously written `tests/ui/SUMMARY.md` 2. Find an appropriate category for the test, using the original issue thread and the test contents. 3. Add the issue URL at the bottom (not at the top, as that would mess up stderr line numbers) 4. Rename the tests to make their purpose clearer Inspired by the methodology that `@Kivooeo` was using. r? `@jieyouxu`
…r=jieyouxu Rehome 30 `tests/ui/issues/` tests to other subdirectories under `tests/ui/` [#3 of Batch #2] Part of rust-lang#133895 Methodology: 1. Refer to the previously written `tests/ui/SUMMARY.md` 2. Find an appropriate category for the test, using the original issue thread and the test contents. 3. Add the issue URL at the bottom (not at the top, as that would mess up stderr line numbers) 4. Rename the tests to make their purpose clearer Inspired by the methodology that `@Kivooeo` was using. r? `@jieyouxu`
cjappl
left a comment
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.
Seems very close, left some suggestions on wording, ping me again and I'll review it ASAP
| while in a real-time context, it reports an error. | ||
| Besides "nonblocking" the attribute can also be used with "blocking" and "caller". | ||
| - "blocking" marks the function as having a non-deterministic execution time. When reaching such |
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.
Possibly:
"blocking" allows programmers to mark a function as having non-deterministic execution time, but not necessarily calling a system call. When calling a "blocking" function in a real-time context, a violation will be reported. A classic use case of this is a userland spinlock.
(adding an example, indicating that it's a way to show intent of what the API does)
…r=jieyouxu Rehome 30 `tests/ui/issues/` tests to other subdirectories under `tests/ui/` [rust-lang#4 of Batch #2] Part of rust-lang#133895 Methodology: 1. Refer to the previously written `tests/ui/SUMMARY.md` 2. Find an appropriate category for the test, using the original issue thread and the test contents. 3. Add the issue URL at the bottom (not at the top, as that would mess up stderr line numbers) 4. Rename the tests to make their purpose clearer Inspired by the methodology that `@Kivooeo` was using. r? `@jieyouxu`
| # RealtimeSanitizer | ||
| RealtimeSanitizer detects non-deterministic execution time calls in real-time contexts. | ||
| Function marked with the `#[sanitize(realtime = "nonblocking")]` attribute are considered real-time functions. | ||
| When RTSan detects a call to a function with non-deterministic execution time, like `malloc` or `free` |
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.
Suggestion: it'd be good to hint at the fact that RTSan doesn't actually infer that a function has a non-deterministic execution time, it instead has a list of functions that it knows about in advance. I'd suggest something like:
"When RTSan detects a call to a known function considered to have non-deterministic execution time, like malloc..."
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.
Don't know if that really makes sense. I think it kind of is a implementation detail and if it is in the clang docs.
I also couldn't think of a way to say this that i liked.
45be714 to
7450259
Compare
…r=jieyouxu Rehome 26 `tests/ui/issues/` tests to other subdirectories under `tests/ui/` [rust-lang#5 of Batch #2] Part of rust-lang#133895 Methodology: 1. Refer to the previously written `tests/ui/SUMMARY.md` 2. Find an appropriate category for the test, using the original issue thread and the test contents. 3. Add the issue URL at the bottom (not at the top, as that would mess up stderr line numbers) 4. Rename the tests to make their purpose clearer Inspired by the methodology that Kivooeo was using. r? ```@jieyouxu```
|
Sorry for all the rust commits that github shows here. If we continue to have discussions about the docs i would open a new PR. |
No description provided.