Does Rust allow multiple &mut references to static mut items?

Consider this code. coeff_state is a static mut item and this is a body of unsafe function:

        let previous_set_item = &mut coeff_state.set[prev as usize];
        let next_set_item = &mut coeff_state.set[next as usize];

This code compiles, even though it takes two mutable references to the same field (set is an array).

This code came out of c2rust and ton of refactoring and I've been slowly migrating it to safe rust, so I tried to extract the coeff_state into a variable like this:

        let state = &mut coeff_state;
        let previous_set_item = &mut state.set[prev as usize];
        let next_set_item = &mut state.set[next as usize];

This code suddendly doesn't compile, giving you a classic borrow checker error:

error[E0499]: cannot borrow `state.set[_]` as mutable more than once at a time
   --> engine/src/getcoeff.rs:421:29
    |
420 |         let previous_set_item = &mut state.set[prev as usize];
    |                                 ----------------------------- first mutable borrow occurs here
421 |         let next_set_item = &mut state.set[next as usize];
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second mutable borrow occurs here
422 |         set_item.constant = ((weight1 * previous_set_item.constant as i32 +
    |                                         -------------------------- first borrow later used here

I understand the error, it's obviously a problem, but I don't understand why the first version compiles fine. Does rust allow taking two mutable references to static mut item?

It does, but it can only do so because taking a reference to a static mut is unsafe in the first place. Actually using those references is often unsound, as you'd expect.

There is some desire to deprecate static mut entirely, replacing it with the usual UnsafeCell-based mechanisms for getting mutable access to shared objects: https://github.com/rust-lang/rust/issues/53639

Depending on what's going on, it may eventually be better to take raw references directly: https://github.com/rust-lang/rust/issues/64490 (see also https://github.com/rust-lang/rust/issues/73394).

1 Like

It's not allowed. The compiler doesn't stop you from doing it, but it's still undefined behavior. c2rust is wrong.

Behavior considered undefined

To be clear, I don't think this specific instance is a result from c2rust, I already rewrote it a lot, so it's probably my fault. c2rust gets around these issues by always using raw pointers everywhere.

1 Like

In the meantime, using the actual replacement of static mut here lets us choose the level of control we want:

mod lib {
    pub
    struct StaticMut<T> /* = */ (
        Cell<T>,
    );

    impl<T> StaticMut<T> {
        pub
        const
        fn new (value: T)
          -> StaticMut<T>
        { ... }

        /// # Safety
        /// There must not be any parallel accesses during `'uniq`.
        pub
        unsafe
        fn assume_unique_thread<'uniq> (self: &'uniq StaticMut<T>)
          -> &'uniq Cell<T> 
        {
            &self.0
        }
    }

    unsafe // Safety: No non-unsafe API
        impl<T : Sync> Sync for StaticMut<T>
        {}
}
use lib::StaticMut;

struct ThingWithSet {
    set: [i32; 42],
    // ...
}

static coeff_state: StaticMut<ThingWithSet> = { ... };

// ...

use ::cell_project::cell_project as cp; // renamed for ergonomics

let state: &Cell< ThingWithSet > = unsafe { coeff_state.assume_unique_thread() };
let at_set: &Cell<[i32]> = cp!(ThingWithSet, state.set);
let at_set: &[Cell<i32>] = at_set.as_slice_of_cells();

let previous_set_item = &at_set[prev as usize];
let next_set_item = &at_set[next as usize];

and you then use .get() and .set() to mutate those fields.

You still get references and checks (compared to using raw pointers), but you no longer have to worry about &mut-aliasing. This is of course only safe if operating from within a single thread each time (which obviously was also a requirement of static mut + raw pointers).

If you know there is no multi-threading in your code whatsoever, you can go full non-unsafe by using a thread_local! { static ... }.

Thanks for the detailed answer, but in my case I think the best thing to do is just get rid of the globals completely, which I've been trying to do for some time already (I got from 300 to 16 globals in last few weeks). There's no reason for these to be globals, it's just very oldschool C code.

Thanks for the answers :wink:

1 Like
   let previous_set_item = &mut state.set[prev as usize];
   let next_set_item = &mut state.set[next as usize];

This is OK if prev != next (and in case of static mut, if there are no other threads touching it), but the borrow checker is unable to prove that. It works based on interfaces, not values or symbolic execution, so it sees only calls to std::ops::IndexMut::index_mut that takes &mut self, which borrows all of state.set exclusively, twice.

If you need to borrow multiple elements from the same vec/array, there's split_at_mut (or just use unsafe).

1 Like

Another alternative depending on what types are actually involved is to use split_at_mut or split_first_mut. Under the covers split_at_mut uses unsafe, but the interface it shows you is safe.

1 Like

What I find weird is that while this syntax is clearly not enough to satisfy borrow checker on its own, somehow just state being static mut enables it to compile. Compare that with split_at_mut which has to go through raw pointers to compile. It seems like a unexpected footgun to me.

   let previous_set_item = &mut state.set[prev as usize];
   let next_set_item = &mut state.set[next as usize];
1 Like

That's a really good point. It's like the borrow checker doesn't notice that state is the same thing.

I think that's arguably a bug -- worth filing an issue at least with a minimal repro.

Well technically no. To perform the indexing you must first create a mutable reference to the entire array, which would overlap with the &mut you already have to a specific element.

3 Likes

Is the compiler treating this as subborrows, as if the two different array indices are two different fields of a struct?

I mean, that would just be a lint. The same problem exists if you access it across function boundaries, and then it certainly can't detect it.

No, I expect what it's doing is treating each mention of state as a different place.

So I suspect this would also compile:

    let previous_set_item = &mut state.set[0];
    let next_set_item = &mut state.set[0]; // UB because overlapping `&mut`s
    use(previous_set_item, next_set_item); // keep the borrows alive

True. Still feels like something that should be caught, because it would be if it were a local. But maybe it'd be better as a "we looked and the liveness range of the references and they overlap" deny-by-default lint than as a different MIR desugaring that could hard-error.

I'd definitely expect this to be the case - ie. treat references to globals the same way as references to locals but only across single function scope.

Still, I am not sure if I should open an issue about this. It seems like it's intentional choice - even very obvious &mut aliasing violations still compile eg. function calls like these foo(&mut state, &mut state.set).

One consideration is that a function-local lint might mislead people into thinking the cross-function version is okay, because the lint stops firing.

Once you've reached for static mut and unsafe, you should already be thinking hard about aliasing and reentrancy- and the lint would only catch the easy cases.

Further, the cross-function version often is okay in C and C++, and is okay in even more cases when you consider "UB but happens to work because the optimizer doesn't see it because it's cross-function" cases. So the lint might wind up flagging exactly the cases a C compiler would miscompile, and ignoring exactly the cases a C compiler would not break but that Rust might miscompile.

(I don't think this is actually an argument against adding the lint, just something to consider in its design.)

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.