rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

RFC: Implement RealtimeSanitizer (RTSan) support, add `#[realtime(nonblocking)]`, `#[realtime(blocking)]` attributes

Open cjappl opened this issue 1 year ago • 9 comments

RealtimeSanitizer is an approach to detecting real-time safety violations in timing critical code. It has been added to LLVM 20, and this RFC aims to leverage that work and add it to Rust.

See the document for more.

Thanks for considering, Chris (@cjappl), David (@davidtrevelyan), and Stephen (@steckes)

Rendered

cjappl avatar Jan 30 '25 23:01 cjappl

one objection to naming the attributes #[blocking]/#[nonblocking] is those are ambiguous since different people have different ideas of what to consider blocking or not based on which domain they're working in, e.g. for async network code reading a file from disk, writing to stdout/stderr, or allocating memory are often considered non-blocking. I'm mentioning this because this kind of annotation and checking has often been mentioned as something wanted for async. https://github.com/rust-lang/wg-async/issues/19

programmerjake avatar Jan 30 '25 23:01 programmerjake

I also agree that the naming is confusing, since while it is very similar to the goals of what async people want, I think that it's not quite the same.

Perhaps simply having a realtime attribute and having realtime(required) (must be executable in realtime context) and realtime(forbidden) (not allowed in realtime context) would be better, since it indicates what the attributes are actually doing.

clarfonthey avatar Jan 31 '25 04:01 clarfonthey

I think that seems reasonable. We are open to changing the attribute names. I like the sound of "realtime(required)" and "realtime(forbidden)"

Some other possibilities (spitballing):

For nonblocking:

  • realtime
  • realtime(required)
  • realtime(constrained)
  • rtsan(realtime)
  • rtsan(nonblocking) or realtime(nonblocking) - if we cared about parity with clang

For blocking:

  • realtime(unsafe) - unsafe may be too overloaded in rust for this one to fly
  • realtime(forbidden)
  • rtsan(blocking)

Any of these jump out to folks? Not sure if we want to do "rtsan" or "realtime" as the main attribute name, I like "realtime" better but it is less specific, for better or worse.

cjappl avatar Jan 31 '25 04:01 cjappl

  • rtsan(*)

I think we shouldn't name stuff after the verification mechanism, since that could be changed/extended to use something other than rtsan.

maybe name it hard_realtime() since that's where you'd want to disallow allocator calls and not merely sleep or println! or sometimes Mutex::lock

programmerjake avatar Jan 31 '25 06:01 programmerjake

  • rtsan(*)

I think we shouldn't name stuff after the verification mechanism, since that could be changed/extended to use something other than rtsan.

Seems reasonable to me! Thanks for the feedback, I agree.

maybe name it hard_realtime() since that's where you'd want to disallow allocator calls and not merely sleep or println! or sometimes Mutex::lock

Differentiating between harder and softer realtime is out of the scope of rtsan as it exists today, it only has one level which is "disallow all non-deterministic time calls", which is just generally "realtime", encompassing all levels from hard-soft. I think this would also open up a lot of debate as to what is allowed at each level of hardness. The solution we have gone with is to disallow everything, and let users opt-out via the scoped_disabler or suppression lists if they want to allow allocations or similar.

cjappl avatar Jan 31 '25 07:01 cjappl

I think we shouldn't name stuff after the verification mechanism, since that could be changed/extended to use something other than rtsan.

Seems reasonable to me! Thanks for the feedback, I agree.

in that case rtsan_scoped_disabler! should be renamed too -- maybe to something like assume_realtime_unchecked!?

programmerjake avatar Jan 31 '25 07:01 programmerjake

I think we shouldn't name stuff after the verification mechanism, since that could be changed/extended to use something other than rtsan.

Seems reasonable to me! Thanks for the feedback, I agree.

in that case rtsan_scoped_disabler! should be renamed too -- maybe to something like assume_realtime_unchecked!?

maybe something that is more similar to the no_sanitize which is already existing in Rust, like no_sanitize_realtime?

Just to be clear, I understand the ambiguity with async, but renaming those things means that the official RealtimeSanitizer documentation does not apply for Rust anymore. -> https://clang.llvm.org/docs/RealtimeSanitizer.html

It removes confusion in async but introduces confusion for people reading the LLVM docs. I am open for renaming, just want to ask again if everyone still thinks the same when reading the official docs.

steckes avatar Jan 31 '25 08:01 steckes

Seeing as it has been a week, I'm going to post a little ping/recap comment to encourage more conversation on this RFC.

What happened in the initial week:

  • We updated the attribute names to be #[realtime(nonblocking)] and #[realtime(blocking)] as there appeared to be general support. Discussion is still ongoing.
  • There is discussion on the naming of the rtsan_scoped_disabler! macro
  • We clarified some wording in the RFC, specifically around the use of the term "real-time unsafe" noting that unsafe is an important word to be specific with in rust-land.

Any more thoughts/ideas/input greatly appreciated. Thanks for the discussion so far.

CC: @steckes @davidtrevelyan

cjappl avatar Feb 10 '25 23:02 cjappl

Doing my weekly(-ish) ping to encourage more conversation.

Let us know if there is anything anyone has questions or thoughts.

CC: @steckes @davidtrevelyan

cjappl avatar Feb 23 '25 09:02 cjappl

Closing, implemented in a different form #147935

cjappl avatar Nov 08 '25 18:11 cjappl