Static checks for &mut in final value of constant with #![feature(const_mut_refs)]

[ecstatic-morse]

On stable, we prevent users from creating an &mut that points to memory inside a const by forbidding the creation of mutable references during const-eval. This limitation is only temporary, see #57349. We have a feature flag, const_mut_refs, that allows users to create mutable references, but no attempt is made to prevent &mut from escaping into the final value of a const like so:

#![feature(const_mut_refs)]
const FOO: &mut i32 = &mut 4;

fn main() {
    *FOO = 2;
}

This errors on the current nightly, and if there were a feature gate that allowed it, we would get an ICE:

error: internal compiler error: src/librustc_mir/interpret/intern.rs:238: const qualif failed to prevent mutable references

I think we've not yet settled on the semantics we want. We're allowing them in const fn and relying on the borrow checker to prevent references from escaping.

[ecstatic-morse]

static mut is also an interesting case. Should the following compile? Perhaps, although with my proposal the user would be forced to wrap the initializer of FOO_MUT in a const fn. This suggests to me that checking for &mut in the type of a const is not the correct solution.

static mut FOO: i32 = 4;
const FOO_MUT: &mut i32 = unsafe { &mut FOO };

fn main() {
    *FOO_MUT = 2;
}

[ecstatic-morse]

Label race XD

[ecstatic-morse]

cc @rust-lang/wg-const-eval

[RalfJung]

static mut FOO: i32 = 4;
const FOO_MUT: &mut i32 = unsafe { &mut FOO };

fn main() {
    *FOO_MUT = 2;
}

Another interesting question here could be aliasing. Usually mutable references are unique. Does that mean that the following is UB because uniqueness got violated (FOO was accessed through a different place between creation and use of FOO_MUT)?

static mut FOO: i32 = 4;
const FOO_MUT: &mut i32 = unsafe { &mut FOO };

fn main() { unsafe {
    FOO += 5;
    *FOO_MUT = 2;
} }

[RalfJung]

(Also strangely I did not get emailed about the ping above.)

[ecstatic-morse]

Ah, that's a good point. In that case, I think we can ignore that example as an argument against the type visitor approach.

Perhaps this is a better example. I believe that things at the top level of a const initializer have a static storage duration. I'm not actually sure whether this is user-observable and guaranteed since rvalue static promotion is pretty aggressive inside constants. NULL and A currently compile on stable and, assuming that I'm right that neither A nor B dangle, reading from either one is not UB. B, however, must fail to compile, but a type-based check on the final value of a constant would reject NULL as well.

// Compile on stable and work as expected.
const NULL: *mut i32 = ptr::null_mut(); 
const A: *const i32 = &4;

// Must not compile.
const B: *mut i32 = &mut 4;

fn main() {
    println!("{}", unsafe { *A });
    // unsafe { *B = 4 } // Bad news
}

[JohnTitor]

Triage: the ICE no longer occurs with the latest nightly by #71665.

[RalfJung]

The bug is not primarily about the ICE though, it is about what static checks we need in place to avoid mis-behaved &mut constants.

[RalfJung]

What actually happens currently is that interning detects the mutable reference in a const and complains about it -- good.

I wonder if validation should do that check, instead... but currently validation is a separate query so there is always the risk that it does not run. :/ Things would be so much easier if the raw const query would always do validation and thus ensure that only validated values ever enter the rest of the compiler...

[oli-obk]

Things would be so much easier if the raw const query would always do validation and thus ensure that only validated values ever enter the rest of the compiler...

If we make validation and interning stop when encountering GlobalAlloc::Static, we can do this.

[RalfJung]

@oli-obk That would be awesome! I laid down my thoughts in #72396. If that works, I'd be really happy. :D

[oli-obk]

Another fun bug with static mut and const_mut_refs:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=baf2a1bbbf7ba1b5a1c9d7015c2d5b2c

#![feature(const_mut_refs)]
pub mod a {
    #[no_mangle]
    pub static mut FOO: &mut [i32] = &mut [42];
}

pub mod b {
    #[no_mangle]
    pub static mut BAR: &mut [i32] = unsafe { &mut *crate::a::FOO };
}

fn main() {
    unsafe {
        assert_eq!(a::FOO.as_ptr(), b::BAR.as_ptr());
    }
}

The assertion fails, even though I belive that it should most definitely not. Only base allocations of statics have a guaranteed address and won't get deduplicated.

[RalfJung]

@oli-obk Shouldn't that be a separate bug? That has nothing to do with static checks or so, this seems like a codegen issue. &mut *crate::a::FOO should return the original pointer.

[oli-obk]

It's indeed a separate bug, but also a blocker for this feature gate