-
Notifications
You must be signed in to change notification settings - Fork 29
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
rtt_init*
macros are unsound
#3
Comments
Great catch, thanks for reporting :) Should be easy enough to fix thanks to your suggestions. |
This one is a bit annoying to fix due to the fact we can't rely on the availability of atomics on all platforms. I suppose I could live with atomics by default and a platform feature flag (is it just me or does like every crate have a "cortex-m" flag for instance?) like "no-atomics" that disables atomics and makes everything unsafe instead. I'd rather not have a requirement for everybody to use "unsafe". |
Yeah, this won't work on M0 targets indeed. But I think having a sane default is a good trade-off with a flag to disable it :) |
As an update, Rust now has |
Somewhat tangential, but using the |
rtt_init!
The
rtt_init!
macro lets you effectively alias a mutable reference to memory. Example below:Both
c0
variables have mutable access to the same chunk of memory and each variable can be accessed from a different priority level (SysTick
can preemptmain
): this alone is a data race and UB in paper (in practice, you may need to callwrite
to make the code explode)Suggested fix: either make
rtt_init!
unsafe
to call OR makertt_init!
returnSome
only once. The latter is the owned singleton pattern (described in its zero-sized reference form in this blog post). Basically you want something like this:rtt_init_default!
This is unsound for the same reason
rtt_init!
is unsound: you should not be able to aliasChannel
s in safe code.Fix: fixing
rtt_init!
fixes this -- just don't makertt_init
unsafe to call butrtt_init_default
safe to call; both should be unsafe OR both should return anOption
rtt_init_print!
This one also lets you run into data races in safe code. See below:
Here the exception (
SysTick
) can preemptrprintln!
and re-initialize the channel (overwrite static variables) used byrprintln!
. The main issue here is the unsafe code inrtt_init!
running more than once and/or running in parallel / concurrently to the execution ofrprintln!
.Suggested fix: this macro should panic if called more than once OR should be no-op if called more than once
cc @Yatekii
The text was updated successfully, but these errors were encountered: