const validation: remove check for mutable refs in final value of const

[RalfJung]

This check rejects code that is not necessarily UB, e.g. a mutable ref to a static mut that is very carefully used correctly. That led to us having to describe it in the Reference, which uncovered just how ad-hoc this check is (rust-lang/reference#2074).

Even without this check, we still reject things like

const C: &mut i32 = &mut 0;

This is rejected by const checking -- the part of the frontend that looks at the source code and says whether it is allowed in const context. In the Reference, this restriction is explained here.

So, the check during validation is just a safety net. And it is already a safety net with gaping holes since we only check &mut T, not &UnsafeCell<T>, due to the fact that we promote some immutable values that have !Freeze type so &!Freeze actually can occur in the final value of a const.

So... it may be time for me to acknowledge that the "mutable ref in final value of const" check is a cure that's worth than the disease. Nobody asked for that check, I just added it because I was worried about soundness issues when we allow mutable references in constants. Originally it was much stricter, but I had to slowly relax it to its current form to prevent t from firing on code we intend to allow. In the end there are only 3 tests left that trigger this error, and they are all just constants containing references to mutable statics -- not the safest code in the world, but also not so bad that we have to spend a lot of time devising a core language limitation and associated Reference wording to prevent it from ever happening.

So... @rust-lang/wg-const-eval @rust-lang/lang I propose that we allow code like this

static mut S: i32 = 3;
const C2: &'static mut i32 = unsafe { &mut * &raw mut S };

@theemathas would be great if you could try to poke a hole into this. ;)

[rustbot]

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

[rustbot]

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[theemathas]

In the end there are only 3 tests left that trigger this error, and they are all just constants containing references to mutable statics

All code that this PR newly allows involves &mut pointing to statics, right?

[theemathas]

With this PR, Miri detects no UB in this code. Is this intended?

use std::ptr;
static mut X: i32 = 1;
const Y: &mut i32 = unsafe { &mut *&raw mut X };
fn main() {
    let a = &Y;
    let b = &Y;
    println!("{a}");
    println!("{b}");
    unsafe {
        *ptr::from_ref(a).read() = 2;
        *ptr::from_ref(b).read() = 3;
        *ptr::from_ref(a).read() = 4;
        *ptr::from_ref(b).read() = 5;
    }
}

[theemathas]

However, Miri detects UB in the following code:

use std::ptr;
static mut X: i32 = 1;
const Y: &mut i32 = unsafe { &mut *&raw mut X };
fn main() {
    let a = &std::convert::identity(Y);
    let b = &Y;
    unsafe {
        *ptr::from_ref(b).read() = 3;
        *ptr::from_ref(a).read() = 4;
    }
}

error: Undefined Behavior: trying to retag from <287> for Unique permission at alloc1[0x0], but that tag does not exist in the borrow stack for this location       
  --> src\main.rs:10:10
   |
10 |         *ptr::from_ref(a).read() = 4;
   |          ^^^^^^^^^^^^^^^^^^^^^^^ this error occurs as part of retag at alloc1[0x0..0x4]
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental       
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <287> was created by a Unique retag at offsets [0x0..0x4]
  --> src\main.rs:5:14
   |
 5 |     let a = &std::convert::identity(Y);
   |              ^^^^^^^^^^^^^^^^^^^^^^^^^
help: <287> was later invalidated at offsets [0x0..0x4] by a Unique retag
  --> src\main.rs:9:10
   |
 9 |         *ptr::from_ref(b).read() = 3;
   |          ^^^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

[RalfJung]

All code that this PR newly allows involves &mut pointing to statics, right?

That should be the case, yes. The only other thing an &mut could point to is a temporary, and then either that gets lifetime extended and we should reject it during const checking, or it does not get lifetime extended and then this becomes a dangling reference that we detect.

With this PR, Miri detects no UB in this code. Is this intended?

We can't run the aliasing model when evaluating constants, so all pointers coming from constants are "equivalent" to Miri. These &Y get promoted so the values of a and b come from constants and Miri can't tell them apart.

The 2nd example passes Y around by-value, which leads to it being retagged, and then Miri's aliasing checks kick in.

[theemathas]

all pointers coming from constants are "equivalent" to Miri

Even if those are two aliasing &mut references being stored at different addresses?

[RalfJung]

If the address they are stored at is "global" memory (i.e. the memory backing a const or static), then yes. All pointers stored in global memory are considered to be "root" pointers for the aliasing model in Miri.