static mut transform is unsound on multi-core systems
#411
Comments
|
cortex-m-rt only supports single-core systems |
|
Is it stated anywhere apart from this comment? |
|
Do the static mut transforms in the macros provide much value? I can't remember but I think we discussed removing them at some point, maybe involving #407/#250. Cortex-m's Doesn't the RP2040 ROM send the second core to sleep, so it only starts execution after being signalled once user code is running? |
|
Chiming in that this also applies to |
thejpster
changed the title
static mut transform is unsound on multi-core systems
|
It's unsound where the interrupt function can be re-entered. The NVIC usually prohibits this but on a multi-core system you need something at the HAL level to ensure this. You can imagine a mechanism that moves ownership of system interrupts from one core to another, hiding the regular NVIC API from the user. Then it would still be sound. |
newAM
added
the
I-unsound
You can also re-enable interrupts inside an interrupt and get the same problem. In spite of rp2040 being around for several years we haven't had a single report of anyone having issues with this. Rather than trying to make this safe, I think we should just document that you should not do anything that allows concurrent/recurrent execution of interrupt handlers and leave it at that, since the actual real-world usages of this behaviour are exceptionally rare, and even in the absence of the static mut transform are likely to be unsafe. |
I'm not sure if this is a strong argument. Having two instances of IMHO if we know that there are conditions that the user must hold up to avoid UB, there should be at least one |
|
It should go behind a flag, like the cs impl. Maybe even the same flag. |
On nightly, the static_mut_refs lint defaults to warn. With some of our examples using `#[deny(warnings)]`, this leads to the following error: ``` error: creating a mutable reference to mutable static is discouraged --> cortex-m-rt/examples/entry-static.rs:15:16 | 15 | static mut COUNT: u32 = 0; | ^^^^^ mutable reference to mutable static | = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html> = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives ``` With edition 2024, the lint will probably default to deny. (https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html) To avoid that, set #[allow(static_mut_refs)] locally in the macro expansion. This only silences the warning and doesn't answer the underlying question if we want to do that transform at all. See eg. rust-embedded#411 for discussion.
RP2040 starts execution on both cores. If entry function contains a "special" definition of
static mutvariable, each core takes a mutable reference to it, which is illegal from the Rust point of view.Here is the relevant piece of
#[entry]code:https://github.com/rust-embedded/cortex-m-rt/blob/ca4790f40738bcc770285c8080955a2077682078/macros/src/lib.rs#L87-L88
The text was updated successfully, but these errors were encountered: