Pre-RFC: Deprecate then remove `static mut`

language design
81

You're talking generalities here, disregarding the context in which I replied. The original comment was that it was still possible to make an error in a single thread by changing the value at two points in the stack.

That error could be made for any mutable variable; for example when it's passed as an argument in a recursive function (to elaborate on the OP). Should the language constraint all changes to mutable variables by forcing to use synchronizers because someone might make that sort of mistake?

Besides, using a mutex wouldn't necessarily detect the problem. It would still require to keep the lock while calling the subfunctions.

I'm only trying to be practical, and to propose not to make that changes in all situations, to limit the consequences this could have. Anyway, I'll stop here. I can only point at the problem.

82

See my post above to understand why it's not the case, IMO.

83

The issue isn't that it causes a logic error, it's that it causes undefined behavior. You cannot accidentally do the same thing with &mut, the compiler checks that for you, but with static mut you have to unsafely assert to the compiler that you won't cause aliasing which is very hard to check. Switching from a static mut _: T to a static _: SyncUnsafeCell<T> makes the unsafe code more explicit so that errors in aliasing are more obvious to readers (and the APIs it provides push you away from using borrowing at all, and instead operating on the raw pointers directly, which also improve the readability for this sort of code).

4 Likes
84

Rust's rules include "one(mutable) XOR any(shared)" references. Two mutable borrows of the same object (each taken independently) in the call stack violates this rule.

If this is the part you refer to, deadlocks are "fine" in Rust. You don't encounter UB (because you make no forward progress).

85

I assume you mean like

fn recursive(x: &mut Vec<i32>, ...) {
    ...
    recursive(x, ...);
    ...
}

That is critically different in two ways:

  • it's explicit that the recursive call depends on x, so the programmer has an opportunity to notice that x will be modified and think about the consequences, and
  • the borrow checker sees that x is being reborrowed here and can check that there are no overlapping &muts.

Neither of these properties is true of static mut.

7 Likes
86

Yeah, the benefit such as it is is not really that that SyncUnsafeCell is safer and less footgunny than static mut (which is what the proposed pre-RFC suggests), it's almost entirely just about adding salt to the unsafe variants to encourage the safe versions.

To me this suggests that unless #![no_std] is set, the warning should suggest Mutex/RwLock/Atomic ahead of SyncUnsafeCell. And the RFC itself should probably explicitly mention that UnsafeCell/SyncUnsafeCell is FFI-compatible with T rather than just imply it, clarifying that the breakage is only crates with a pub static mut API.

2 Likes
87

I'd like to signal that I will begin writing the RFC with what's been said and the results of the survey in mind soon. If you have further concerns, please voice them now.

3 Likes
88

EDIT: I agree with the 2nd bullet but not the first.

It's a global variable, so I agree it doesn't appear in function arguments, but it's explicitly indicated as mutable and only modified in unsafe blocks, so the programmer cannot miss it. Adding UnsafeCell won't make it more obvious - it may even be less visible, since the code will be more cluttered - which was my point.

It's not undefined behaviour in synchronous code, unless I'm missing something like the compiler being confused about mutable statics (would it optimize as if the variable were immutable?). Normally, it's just the old, plain logic flaw. To illustrate what I mean with a trivial example: assuming the static variable won't change when calling a function and reading it too early:

static mut OPTION_SIZE: usize = 0;
...
let quantity_ref = &OPTION_SIZE;
init_global_values(); // sets OPTION_SIZE, should have been done earlier
for i in 0..*quantity_ref { ... }
89

See this Rust Playground which contains UB; I've created two &mut COUNT references that are both alive at the same time. Miri correctly detects this, although it says that "the Stacked Borrows rules it violated are still experimental".

I disagree; the code in my Rust Playground link is a minimized version of something I see experienced C developers who are new to Rust write on a not-infrequent basis. They've internalised that static is required for globals, that mut is required to mutate things (instead of const in C), and that unsafe is short-hand for "I'm doing something that the compiler can't check, but trust me, I'm an expert", and they're transliterating what they'd write in C into Rust.

It needs to be static mut, because it's a mutable global, and unsafe just means "trust me", but they're writing something that they "know" (from C) is good code. The extra effort of (a) having to find UnsafeCell in the first place and (b) having to write a lot of extra boilerplate for a "simple" global clues them into the idea that this is not what they wanted.

And that is basically all this comes down to - is there enough ceremony around static mut specifically to warn programmers away from it, or do we need to add more so that they don't get surprised by it?

4 Likes
90

No, it is not. As far as the borrow checker is concerned, each usage of a static mut is independent of all others. @farnz already gave a demo, but here is an even shorter one:

static mut COUNT: u32 = 0;
fn main() {
    let r1 = unsafe { &mut COUNT };
    let r2 = unsafe { &mut COUNT };
    *r1 += 1;
    *r2 += 1;
}

This program compiles fine, but has UB. Now, the borrow checker could be taught to consider mutable borrows of COUNT within the same function to be exclusive just as if it was a local variables — but then, static mut unsoundness could be hidden from the compiler just by spreading it out among multiple functions. So, this would not help in the case where it is most needed (when the program is complex).

11 Likes
91

Thanks a lot for the clarification. :slight_smile:

I suppose (again) that UB is "undefined behaviour", and not "unsafe block", since it's ambiguous here.

If it's "undefined behaviour", it indeed clarifies the misunderstanding. But I'd rather call it a side-effect, because the behaviour seems well defined. The two blocks aren't actually "alive" at the same time in the same sense as asynchronous code; they're executed one after the other, but they reference the same variable simultaneously, which is bad. As a result, COUNT is increased twice, and since it's referenced in each structure and not copied, that's the value displayed at the end: 2.

That's what I said in my previous post: the result is predictable and logical, unless the compiler makes the wrong assumption that it can optimize the code in some way; for example, by using two CPU registers caching the value COUNT, ultimately displaying a 1 or a 2 depending on the level of optimization, the compiler version, or who knows what else (I don't think it's the case).

Here, the programmer used unsafe blocks without doing a proper check and unfortunately doesn't benefit from the borrow checker. The same error could happen by duplicating &mut of any non-static variable in unsafe blocks (Miri will also detect that). For example, I had to do that when dealing with tree structures and a mutable iterator, but I knew that it was OK per design and double-checked with Miri. Then I checked again and made sure to add the safety comments.

If you replace the static variable by UnsafeCell (SyncUnsafeCell is currently unstable, and I don't think it would change anything in this single-threaded code), you get the same error:

static mut COUNT: UnsafeCell<u32> = UnsafeCell::new(0);
...
    fn main() {
        let mut foo = new_foo(unsafe { COUNT.get_mut() });
        let mut bar = new_bar(unsafe { COUNT.get_mut() });
...

=> Foo { thing: "new_foo", counter: 2 } Bar { thing: "new_bar", counter: 2 }

So, regarding what's obvious or not, or instructive or not, I think it's subjective and we'll have to agree to disagree. If C/C++ developers internalize a recipe for using static variables, they'll do the same with UnsafeCell, feel safe, and commit the same error. Not that I really think a C/C++ developer will be any different than a Rust developer here: both are familiar with pointers, and reading a code like that should switch on a big caution light in their mind. Changing a variable in uppercase should be a pretty obvious clue, too. Since the "solution" here doesn't guarantee safety, it's best to avoid making it look so, and rather count on programmers to keep on their toes around unsafe code as usual.

Maybe that makes me a wild, evil programmer, but I believe that the sane reaction would have been (a) examining how the static variable is used and (b) taking the appropriate decision:

  • is it only assigned once (from the program arguments, for ex)? => nothing special to do, but check that it's only used after being assigned.
  • is it assigned several times / complicated (like in the code you kindly provided)? => it seems more hairy, let's use RefCell.
  • is it a multithread or are there coroutines? => ... (maybe that's the use-case where the compiler should incite the use of proper synchronization)
92

Sorry, I meant that I agree with the 2nd bullet but not the first. I badly phrased it.

PS: As for undefined behaviour (if that's "UB"), I've already replied to @farnz: I don't think it's what we usually call undefined behaviour since it's the expected, reproducible result.

93

The two blocks are alive at the same time; the code is not executed (post-optimization) in linear order, but rather we want to allow the compiler to rearrange the code (within the limits of Rust's formal semantics) to get faster binaries. This means that (for example), the compiler can split *counter += 1 into let local = *counter; then insert some other code, then let local = local + 1;, then insert other code, then *counter = local;. And it can do this to both instances of *counter += 1, and interleave the two completely legally (since the existence of counter: &mut u32 implies semantically that there are no other references to the thing pointed to), resulting in counter being 1 when it should be 2.

The big thing for me here is that static, mut and unsafe are all things you'll find when you've decided you need a global, without having to read documentation. You can try and use this off the back of your C experience, without reading Rust's documentation in depth.

UnsafeCell needs you to read documentation and internalize a "recipe". But the documentation tells you why this is a bad idea, and thus helps you get it right.

5 Likes
94

Note that every instance of "UB" I can recall on this forum has always been "Undefined Behaviour". The name isn't great, but we basically inherited it from C++, where "UB" is commonly said as well -- see things like https://en.cppreference.com/w/cpp/language/ub (and its URL).

11 Likes
95

That would not be a "wrong assumption", it's the correct assumption: the compiler is free to (and does) optimize code by assuming that &mut references are unique. That assumption lies at the core of the design of Rust's references.

12 Likes
96

It's not just a matter of compiler optimizations. For example, this program will access the contents of freed memory even without optimizations:

9 Likes
97

Thanks for the explanation, @farnz @tczajka @comex. I see what you mean, now. :slight_smile:

98

Thank you all for participating in the survey, it's now closed.

You can see the results interactively here.

I've also opened an RFC PR with what's been said here and in the survey in mind at rfcs#3560, so we can continue to discuss this there.

Thank you all for your participation!

99

It seems some of the embedded concerns from the survey have not been addressed in the RFC.

I'm looking specifically at the points about more overhead due to lack of "placement new" like functionality in Rust causing unnecessary copies that don't always get optimised away, according to the survey-taker in question with static muy this gets optimised much better.

On desktop extra memory copies is usually just an annoyance. On embedded this can be the difference between a program that works and one that crashes due to running out of memory. So this is a very grave concern if true.

2 Likes
100

I don't understand, can you give a concrete example? The whole point of statics is they aren't getting copied.

I also don't know what placement new is.

1 Like