rust-embedded / cortex-m-semihosting

Semihosting for ARM Cortex-M processors
Apache License 2.0
40 stars 19 forks source link

`interrupt::free` is not enough to access `static mut` safely #49

Closed jonas-schievink closed 3 years ago

jonas-schievink commented 4 years ago

At least in the presence of multiple cores sharing the same set of statics. Problematic code:

https://github.com/rust-embedded/cortex-m-semihosting/blob/7a961f0fbe6eb1b29a7ebde4bad4b9cf5f842b31/src/export.rs#L9-L19

jonas-schievink commented 4 years ago

This point might be moot. There's not really a way to solve this, and all other foundational crates also use interrupt::free. I don't usually work with multi-core chips, but I suppose we need another solution for them (are there any devices/use cases where you'd share statics between cores?).

jonas-schievink commented 4 years ago

This may be resolved by https://github.com/rust-embedded/wg/pull/388

japaric commented 4 years ago

This may be resolved by rust-embedded/wg#388

I don't see how that solves the problem. hprintln!, which calls this function, is global and can be called from any core. It's not even protected by bare_metal::Mutex at the moment and if it were with the linked RFC it would simply stop to compile.

One potential solution would be use AtomicUsize to store the file descriptor and use something like usize::max_value to mean None. That would synchronize the open syscall between cores but then more than one core would end up using the same file descriptor. What's the observed behavior of using the same file descriptor from different cores in parallel? I personally don't know because I have never gotten multi-core debugging working (OpenOCD / pyOCD / JLink only worked on one core at a time when I tried them on multi-core Cortex-M SoCs)

The other option is to have a core-local variable to store the file descriptor then each core gets its own file descriptor. Again, I don't know whether parallel semihosting writes using different file descriptors actually works in multi-core context. Leaving that aside, the problem is that there's no support in the Cortex-M ISA to differentiate one core from the other, unlike say RISC-V which has a "thread local pointer" (don't remember the exact name) register for this purpose.

adamgreig commented 3 years ago

This is closed by virtue of our decision to basically not support SMP.