Are mutable static primitives actually `unsafe` if single-threaded?
Asked Answered
M

4

13

I'm developing for a single-core embedded chip. In C & C++ it's common to statically-define mutable values that can be used globally. The Rust equivalent is roughly this:

static mut MY_VALUE: usize = 0;

pub fn set_value(val: usize) {
    unsafe { MY_VALUE = val }
}

pub fn get_value() -> usize {
    unsafe { MY_VALUE }
}

Now anywhere can call the free functions get_value and set_value.

I think that this should be entirely safe in single-threaded embedded Rust, but I've not been able to find a definitive answer. I'm only interested in types that don't require allocation or destruction (like the primitive in the example here).

The only gotcha I can see is with the compiler or processor reordering accesses in unexpected ways (which could be solves using the volatile access methods), but is that unsafe per se?


Edit:

The book suggests that this is safe so long as we can guarantee no multi-threaded data races (obviously the case here)

With mutable data that is globally accessible, it’s difficult to ensure there are no data races, which is why Rust considers mutable static variables to be unsafe.

The docs are phrased less definitively, suggesting that data races are only one way this can be unsafe but not expanding on other examples

accessing mutable statics can cause undefined behavior in a number of ways, for example due to data races in a multithreaded context

The nomicon suggests that this should be safe so long as you don't somehow dereference a bad pointer.

Maggee answered 24/5, 2022 at 20:39 Comment(10)
The term "unsafe" is incorrect here. Yes, it is unsafe, because Rust requires unsafe to access it. You probably meant unsound.Ellieellinger
@ChayimFriedman I thought it was common to use "unsafe" in a context meaning "this piece of unsafe-wrapped code is not a valid safe abstraction". I'm confident I've seen it used in that context before many times. To me "unsound" could include broader correctness concerns. But please do point me to a ref if I've got these mixed up.Maggee
I don't remember seeing it, but anyway this is exactly the meaning of soundness (in Rust): cannot be abused in safe code to execute undefined behavior.Ellieellinger
Soundness is a logical property. A block of Rust code is unsafe if the compiler can not prove that it is sound.Ulyanovsk
Here is the official description of unsafe vs unsound in the Rust documentation: doc.rust-lang.org/beta/reference/… `Unstring
I think the most crucial error in your train of thought is that single-threaded microcontrollers do not care about concurrency problems. Interrupts are also a form of concurrency which cause the exact same problems as multithreading, and they exist on almost every microcontroller.Unstring
@finomnis Thanks, clearly I need to better my reading skills as I was already on that page :P Also, excellent point about interrupts.Maggee
The book suggests that this is safe so long as we can guarantee no multi-threaded data races - no, the passage you quote only says data races, not multi-threaded. Rust does not have a formally specified memory model yet, but in both C and C++, the language level definition of "data race" does not in fact require racy accesses to be from two different threads. This is a very common misunderstanding about concurrency: it is a part of the language model, not just a fact about what the hardware does.Bering
@Bering I'm not interested in litigating whether you think my use of "thread" there (and later in the section I was quoting) is precisely correct according to your precise idea of what that word must mean.Maggee
The book section you quote does not use the word "thread" at all, as you seem to think it does.Bering
U
13

Be aware as there is no such thing as single-threaded code as long as interrupts are enabled. So even for microcontrollers, mutable statics are unsafe.

If you really can guarantee single-threaded access, your assumption is correct that accessing primitive types should be safe. That's why the Cell type exists, which allows mutability of primitive types with the exception that it is not Sync (meaning it explicitely prevents threaded access).

That said, to create a safe static variable, it needs to implement Sync for exactly the reason mentioned above; which Cell doesn't do, for obvious reasons.

To actually have a mutable global variable with a primitive type without using an unsafe block, I personally would use an Atomic. Atomics do not allocate and are available in the core library, meaning they work on microcontrollers.

use core::sync::atomic::{AtomicUsize, Ordering};

static MY_VALUE: AtomicUsize = AtomicUsize::new(0);

pub fn set_value(val: usize) {
    MY_VALUE.store(val, Ordering::Relaxed)
}

pub fn get_value() -> usize {
    MY_VALUE.load(Ordering::Relaxed)
}

fn main() {
    println!("{}", get_value());
    set_value(42);
    println!("{}", get_value());
}

Atomics with Relaxed are zero-overhead on almost all architectures.

Unstring answered 24/5, 2022 at 21:31 Comment(13)
My understanding was that atomics with Ordering::Relaxed basically compile to ordinary mov on most architectures, so is this actually any different or does it simply avoid needing unsafe blocks?Maggee
Having tested this for my specific case at hand (armv4) unfortunately Ordering::Relaxed is my own option. I get linker errors with acqurie/release (presumably because of lack of relevant intrinsics, but that's getting off-topic for this question).Maggee
Be aware that Atomics might not work if no respective mov command exists on the architecture, for example AtomicU128 is not lock-free on a 32-bit system and might therefore fail to compile.Unstring
There is another fact about Cell that makes it sound besides the fact it is !Sync: it does not allow the user to get any references to the inside from a shared reference to the cell itself. To allow such access would make it unsound, even apart from thread safety.Bering
(That is the case both in your and the OP's examples, but it is not made explicit anywhere)Bering
For certain applications, it would really be nice to have #![no_shared] or similar that is analogous to #![no_std] but only enforces no shared data concurrency. In HPC, it is common to use message passing instead of threads and in some cases I am finding that meeting unwanted no race requirements is a hinderance. I don't know if such a thing is possible in practice but it would make my life so much easier!Alis
@Alis I'm unsure what you mean with that. There's plenty of ways to share data between threads that would work for HPC using the existing primitives. What would such a flag do? What exactly would it allow? How would it achieve that without breaking Rust's fundamental soundness guarantees?Unstring
@Unstring Thanks for your interest. From what I can gather, some rust soundness guarantees are superfluous yet costly if my code is 100% single-threaded. In my case, I use a global static for the pseudo-random number generator state and it is extremely performance critical, so direct modification is needed. I may experiment with some other patterns, but implementing this made me wonder if there was a way to say this code is safe without threads and interrupt callbacks. Perhaps Cell is what I need but it does appear to require a copy-update-write pattern.Alis
@Alis Why not Atomic? Either way, no code is ever 100% single-threaded. How do you get that guarantee? Also, why is copy-update-write a problem?Unstring
@Unstring Ya, that's what I meant by "I don't know if such a thing is possible in practice". Nonetheless, I've run millions of hours of simulations in single-threaded C++ and never had any issue ever with concurrent access to static data, so it feels a bit pedantic needing to appease rust in code I know I will never use with threads. Of course, the real answer is I need to do more benchmarking because none of this may matter in the end.Alis
Also, in your argumentation, be aware that there's a difference between unsafe and unsound. Soundness is the real issue here - Rust defines that any code that doesn't require unsafe is unable to cause undefined behaviour. And a mutable static can be used to cause undefined behaviour, making it unsound to expose such an API.Unstring
Also, this is a common problem in embedded (where statics are commonly used), which is why embedded frameworks/RTOSes like RTIC usually provide their own way of creating scoped &'static muts.Unstring
UB is trivially possible in single threaded code with static mut. It's still unsafe. Just create 2 mutable references to the static or a mutable reference and a shared reference (Rust will allow this). So this is a bit misleading. (Though I will assume by your answer about "accessing it", you didn't mean taking references to it)Cerement
E
4

In this case it's not unsound, but you still should avoid it because it is too easy to misuse it in a way that is UB.

Instead, use a wrapper around UnsafeCell that is Sync:

pub struct SyncCell<T>(UnsafeCell<T>);

unsafe impl<T> Sync for SyncCell<T> {}

impl<T> SyncCell<T> {
    pub const fn new(v: T) -> Self { Self(UnsafeCell::new(v)); }

    pub unsafe fn set(&self, v: T) { *self.0.get() = v; }
}

impl<T: Copy> SyncCell<T> {
    pub unsafe fn get(&self) -> T { *self.0.get() }
}

If you use nightly, you can use SyncUnsafeCell.

Ellieellinger answered 24/5, 2022 at 20:54 Comment(5)
Is there any advantage to using UnsafeCell over Cell here?Agueweed
@Agueweed No, not really. UnsafeCell is the basic interior mutability primitive all other primitives are based on. But you can wrap Cell and unsafe impl Sync for it.Ellieellinger
Would you mind elaborating how this is sound? To me, your code very much sounds like it contains a race condition... There is nothing preventing two threads to simultaneously call "set" or "set" and "get", so i don't think this struct should be "sync"Unstring
@Unstring The OP is talking about a scenraio where everything is single threaded.Ellieellinger
I guess he then has to manually make sure not to call this from within an interrupt handier?Unstring
U
2

Mutable statics are unsafe in general because they circumvent the normal borrow checker rules that enforce either exactly 1 mutable borrow exists or any number of immutable borrows exist (including 0), which allows you to write code which causes undefined behavior. For instance, the following compiles and prints 2 2:

static mut COUNTER: i32 = 0;

fn main() {
    unsafe {
        let mut_ref1 = &mut COUNTER;
        let mut_ref2 = &mut COUNTER;
        *mut_ref1 += 1;
        *mut_ref2 += 1;
        println!("{mut_ref1} {mut_ref2}");
    }
}

However we have two mutable references to the same location in memory existing concurrently, which is UB.

I believe the code that you posted there is safe, but I generally would not recommend using static mut. Use an atomic, SyncUnsafeCell/UnsafeCell, a wrapper around a Cell that implements Sync which is safe since your environment is single-threaded, or honestly just about anything else. static mut is wildly unsafe and its use is highly discouraged.

Ulyanovsk answered 24/5, 2022 at 20:48 Comment(5)
Thanks. This answers the question of the title, but not the code in the body of my question (which doesn't allow such aliasing by construction).Maggee
Edited to reply to your code example specificallyUlyanovsk
"static mut is wildly unsafe..." perhaps my question might have been better posed as a "so long as I use primitives and avoid aliasing and threading, is it unsafe?". Maybe I'll write a new clearer question later.Maggee
Of course I would normally use atomics/mutexes/etc., but not every platform supports those and not every usecase can bear the overhead. I will look more into UsafeCell et alMaggee
I mean yes if you do everything correctly (ensure the borrow rules are upheld, which broadly covers everything you've mentioned) then it's sound, tautologically. The point is that it's extremely easy to mess that up, and other abstractions (such as UnsafeCell and SyncUnsafeCell which Chayim mentioned) are available to make the unsafety more explicit, and have documentation outlining the contract you're opting-into more explicitly. My personal recommendation would be to base your solution off of Chayim's answer or do something similar by just wrapping Cell since your type is Copy.Ulyanovsk
A
0

In order to sidestep the issue of exactly how mutable statics can be used safely in single-threaded code, another option is to use thread-local storage:

use std::cell::Cell;

thread_local! (static MY_VALUE: Cell<usize> = {
    Cell::new(0)
});

pub fn set_value(val: usize) {
    MY_VALUE.with(|cell| cell.set(val))
}

pub fn get_value() -> usize {
    MY_VALUE.with(|cell| cell.get())
}
Agueweed answered 25/5, 2022 at 10:22 Comment(2)
(a) Cell is much better than RefCell for that. (b) The OP said they are developing for emebedded, so they likely have neither std nor thread locals.Ellieellinger
@ChayimFriedman, two good points, thanks! I fixed (a) and accept (b) as a limitation. I still think this is useful for non-embedded.Agueweed

© 2022 - 2024 — McMap. All rights reserved.