tests and comments related to the tail-expression problem

RalfJung · RalfJung · commit 02c0cfe002fe · 2023-10-28T17:59:58.000+02:00
diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs
@@ -302,10 +302,7 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
     is_static: bool,
 ) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> {
     // `is_static` just means "in static", it could still be a promoted!
-    debug_assert_eq!(
-        is_static,
-        ecx.tcx.static_mutability(cid.instance.def_id()).is_some()
-    );
+    debug_assert_eq!(is_static, ecx.tcx.static_mutability(cid.instance.def_id()).is_some());
 
     let res = ecx.load_mir(cid.instance.def, cid.promoted);
     match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body)) {
diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs
@@ -246,7 +246,14 @@ pub fn intern_const_alloc_recursive<
     // that we are starting in, and all other allocations that we are encountering recursively.
     let (base_mutability, inner_mutability) = match intern_kind {
         InternKind::Constant | InternKind::Promoted => {
-            // Completely immutable.
+            // Completely immutable. Interning anything mutably here can only lead to unsoundness,
+            // since all consts are conceptually independent values but share the same underlying
+            // memory.
+            // This means we do intern allocations caused by the "tail expression" / "outer scope"
+            // rule as immutable. We rely on const-checking ensuring that no mutable memory can
+            // occur there: mutable references and interior mutability must be rejected even when
+            // the corresponding feature gates are enabled, so only `&Freeze` references remain, and
+            // hence interning immutably is sound.
             (Mutability::Not, Mutability::Not)
         }
         InternKind::Static(Mutability::Not) => {
@@ -257,13 +264,14 @@ pub fn intern_const_alloc_recursive<
                 } else {
                     Mutability::Mut
                 },
-                // Inner allocations are never mutable.
+                // Inner allocations are never mutable. They can only arise via the "tail
+                // expression" / "outer scope" rule, and we treat them consistently with `const`.
                 Mutability::Not,
             )
         }
         InternKind::Static(Mutability::Mut) => {
             // Just make everything mutable. We accept code like
-            // `static mut X = &mut 42`, so even inner allocations need to be mutable.
+            // `static mut X = &mut [42]`, so even inner allocations need to be mutable.
             (Mutability::Mut, Mutability::Mut)
         }
     };
@@ -326,7 +334,7 @@ pub fn intern_const_alloc_recursive<
     while let Some(alloc_id) = todo.pop() {
         if let Some((_, mut alloc)) = ecx.memory.alloc_map.remove(&alloc_id) {
             // This still needs to be interned. We keep the old logic around here to avoid changing
-            // rustc behavior; eventually this should all be removed in favor of ignroing all types
+            // rustc behavior; eventually this should all be removed in favor of ignoring all types
             // and interning everything with a simple `intern_shallow` loop.
             match intern_kind {
                 // Statics may point to mutable allocations.
diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs
@@ -387,6 +387,8 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
                 if self.local_has_storage_dead(local) {
                     self.check_op(ops::TransientMutBorrow(kind));
                 } else {
+                    // This check is *very important*. If we let this borrow leak into the final
+                    // value, it would be treated as read-only memory, which would be unsound!
                     self.check_op(ops::MutBorrow(kind));
                 }
             }
@@ -528,6 +530,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                             if self.local_has_storage_dead(place.local) {
                                 self.check_op(ops::TransientCellBorrow);
                             } else {
+                                // This check is *very important*. If we let this borrow leak into the final
+                                // value, it would be treated as read-only memory, which would be unsound!
                                 self.check_op(ops::CellBorrow);
                             }
                         }
diff --git a/tests/ui/consts/const-mut-refs/mut_ref_in_final.rs b/tests/ui/consts/const-mut-refs/mut_ref_in_final.rs
@@ -44,6 +44,23 @@ static mut FOO3: NotAMutex<&mut i32> = NotAMutex(UnsafeCell::new(&mut 42));
 // the enclosing scope rule.
 const BAR: NotAMutex<&i32> = NotAMutex(UnsafeCell::new(&42));
 
+struct SyncPtr<T> { x : *const T }
+unsafe impl<T> Sync for SyncPtr<T> {}
+
+// These pass the lifetime checks because of the "tail expression" / "outer scope" rule.
+// (This relies on `SyncPtr` being a curly brace struct.)
+// However, we intern the inner memory as read-only.
+// The resulting constant would pass all validation checks, so it is crucial that this gets rejected
+// by static const checks!
+static RAW_MUT_CAST_S: SyncPtr<i32> = SyncPtr { x : &mut 42 as *mut _ as *const _ };
+//~^ ERROR mutable references are not allowed
+static RAW_MUT_COERCE_S: SyncPtr<i32> = SyncPtr { x: &mut 0 };
+//~^ ERROR mutable references are not allowed
+const RAW_MUT_CAST_C: SyncPtr<i32> = SyncPtr { x : &mut 42 as *mut _ as *const _ };
+//~^ ERROR mutable references are not allowed
+const RAW_MUT_COERCE_C: SyncPtr<i32> = SyncPtr { x: &mut 0 };
+//~^ ERROR mutable references are not allowed
+
 fn main() {
     println!("{}", unsafe { *A });
     unsafe { *B = 4 } // Bad news
diff --git a/tests/ui/consts/const-mut-refs/mut_ref_in_final.stderr b/tests/ui/consts/const-mut-refs/mut_ref_in_final.stderr
@@ -54,7 +54,31 @@ LL | static mut FOO3: NotAMutex<&mut i32> = NotAMutex(UnsafeCell::new(&mut 42));
    |                                        |                              creates a temporary value which is freed while still in use
    |                                        using this value as a static requires that borrow lasts for `'static`
 
-error: aborting due to 6 previous errors
+error[E0764]: mutable references are not allowed in the final value of statics
+  --> $DIR/mut_ref_in_final.rs:55:53
+   |
+LL | static RAW_MUT_CAST_S: SyncPtr<i32> = SyncPtr { x : &mut 42 as *mut _ as *const _ };
+   |                                                     ^^^^^^^
+
+error[E0764]: mutable references are not allowed in the final value of statics
+  --> $DIR/mut_ref_in_final.rs:57:54
+   |
+LL | static RAW_MUT_COERCE_S: SyncPtr<i32> = SyncPtr { x: &mut 0 };
+   |                                                      ^^^^^^
+
+error[E0764]: mutable references are not allowed in the final value of constants
+  --> $DIR/mut_ref_in_final.rs:59:52
+   |
+LL | const RAW_MUT_CAST_C: SyncPtr<i32> = SyncPtr { x : &mut 42 as *mut _ as *const _ };
+   |                                                    ^^^^^^^
+
+error[E0764]: mutable references are not allowed in the final value of constants
+  --> $DIR/mut_ref_in_final.rs:61:53
+   |
+LL | const RAW_MUT_COERCE_C: SyncPtr<i32> = SyncPtr { x: &mut 0 };
+   |                                                     ^^^^^^
+
+error: aborting due to 10 previous errors
 
 Some errors have detailed explanations: E0716, E0764.
 For more information about an error, try `rustc --explain E0716`.
diff --git a/tests/ui/consts/miri_unleashed/mutable_references_err.32bit.stderr b/tests/ui/consts/miri_unleashed/mutable_references_err.32bit.stderr
@@ -70,6 +70,14 @@ error[E0080]: evaluation of constant value failed
 LL | const POINTS_TO_MUTABLE2: &i32 = unsafe { &*MUTABLE_REF };
    |                                           ^^^^^^^^^^^^^ constant accesses static
 
+error: unsupported untyped pointer in constant
+  --> $DIR/mutable_references_err.rs:57:1
+   |
+LL | const POINTS_TO_MUTABLE_INNER: *const i32 = &mut 42 as *mut _ as *const _;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: memory only reachable via raw pointers is not supported
+
 warning: skipping const checks
    |
 help: skipping check that does not even have a feature gate
@@ -142,7 +150,12 @@ help: skipping check that does not even have a feature gate
    |
 LL | const POINTS_TO_MUTABLE2: &i32 = unsafe { &*MUTABLE_REF };
    |                                             ^^^^^^^^^^^
+help: skipping check that does not even have a feature gate
+  --> $DIR/mutable_references_err.rs:57:45
+   |
+LL | const POINTS_TO_MUTABLE_INNER: *const i32 = &mut 42 as *mut _ as *const _;
+   |                                             ^^^^^^^
 
-error: aborting due to 7 previous errors; 1 warning emitted
+error: aborting due to 8 previous errors; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0080`.
diff --git a/tests/ui/consts/miri_unleashed/mutable_references_err.64bit.stderr b/tests/ui/consts/miri_unleashed/mutable_references_err.64bit.stderr
@@ -70,6 +70,14 @@ error[E0080]: evaluation of constant value failed
 LL | const POINTS_TO_MUTABLE2: &i32 = unsafe { &*MUTABLE_REF };
    |                                           ^^^^^^^^^^^^^ constant accesses static
 
+error: unsupported untyped pointer in constant
+  --> $DIR/mutable_references_err.rs:57:1
+   |
+LL | const POINTS_TO_MUTABLE_INNER: *const i32 = &mut 42 as *mut _ as *const _;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: memory only reachable via raw pointers is not supported
+
 warning: skipping const checks
    |
 help: skipping check that does not even have a feature gate
@@ -142,7 +150,12 @@ help: skipping check that does not even have a feature gate
    |
 LL | const POINTS_TO_MUTABLE2: &i32 = unsafe { &*MUTABLE_REF };
    |                                             ^^^^^^^^^^^
+help: skipping check that does not even have a feature gate
+  --> $DIR/mutable_references_err.rs:57:45
+   |
+LL | const POINTS_TO_MUTABLE_INNER: *const i32 = &mut 42 as *mut _ as *const _;
+   |                                             ^^^^^^^
 
-error: aborting due to 7 previous errors; 1 warning emitted
+error: aborting due to 8 previous errors; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0080`.
diff --git a/tests/ui/consts/miri_unleashed/mutable_references_err.rs b/tests/ui/consts/miri_unleashed/mutable_references_err.rs
@@ -55,6 +55,7 @@ const POINTS_TO_MUTABLE2: &i32 = unsafe { &*MUTABLE_REF };
 //~| accesses static
 
 const POINTS_TO_MUTABLE_INNER: *const i32 = &mut 42 as *mut _ as *const _;
+//~^ ERROR: unsupported untyped pointer in constant
 
 fn main() {
     unsafe {
diff --git a/tests/ui/consts/miri_unleashed/static-no-inner-mut.32bit.stderr b/tests/ui/consts/miri_unleashed/static-no-inner-mut.32bit.stderr
@@ -42,18 +42,6 @@ LL | static REFMUT2: &mut i32 = {let mut x = 0; &mut {x}};
                ╾ALLOC3╼                                     │ ╾──╼
            }
 
-error: encountered dangling pointer in final constant
-  --> $DIR/static-no-inner-mut.rs:28:1
-   |
-LL | static RAW_SYNC: SyncPtr<AtomicI32> = SyncPtr(&AtomicI32::new(42));
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: encountered dangling pointer in final constant
-  --> $DIR/static-no-inner-mut.rs:29:1
-   |
-LL | static RAW_MUT: SyncPtr<i32> = SyncPtr(&mut 42 as *mut _ as *const _);
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
 warning: skipping const checks
    |
 help: skipping check that does not even have a feature gate
@@ -76,12 +64,22 @@ help: skipping check that does not even have a feature gate
    |
 LL | static REFMUT2: &mut i32 = {let mut x = 0; &mut {x}};
    |                                            ^^^^^^^^
-help: skipping check for `const_mut_refs` feature
-  --> $DIR/static-no-inner-mut.rs:29:40
+help: skipping check that does not even have a feature gate
+  --> $DIR/static-no-inner-mut.rs:33:52
+   |
+LL | static RAW_SYNC: SyncPtr<AtomicI32> = SyncPtr { x: &AtomicI32::new(42) };
+   |                                                    ^^^^^^^^^^^^^^^^^^^
+help: skipping check that does not even have a feature gate
+  --> $DIR/static-no-inner-mut.rs:34:51
+   |
+LL | static RAW_MUT_CAST: SyncPtr<i32> = SyncPtr { x : &mut 42 as *mut _ as *const _ };
+   |                                                   ^^^^^^^
+help: skipping check that does not even have a feature gate
+  --> $DIR/static-no-inner-mut.rs:35:52
    |
-LL | static RAW_MUT: SyncPtr<i32> = SyncPtr(&mut 42 as *mut _ as *const _);
-   |                                        ^^^^^^^
+LL | static RAW_MUT_COERCE: SyncPtr<i32> = SyncPtr { x: &mut 0 };
+   |                                                    ^^^^^^
 
-error: aborting due to 6 previous errors; 1 warning emitted
+error: aborting due to 4 previous errors; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0080`.
diff --git a/tests/ui/consts/miri_unleashed/static-no-inner-mut.64bit.stderr b/tests/ui/consts/miri_unleashed/static-no-inner-mut.64bit.stderr
@@ -42,18 +42,6 @@ LL | static REFMUT2: &mut i32 = {let mut x = 0; &mut {x}};
                ╾ALLOC3╼                         │ ╾──────╼
            }
 
-error: encountered dangling pointer in final constant
-  --> $DIR/static-no-inner-mut.rs:28:1
-   |
-LL | static RAW_SYNC: SyncPtr<AtomicI32> = SyncPtr(&AtomicI32::new(42));
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: encountered dangling pointer in final constant
-  --> $DIR/static-no-inner-mut.rs:29:1
-   |
-LL | static RAW_MUT: SyncPtr<i32> = SyncPtr(&mut 42 as *mut _ as *const _);
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
 warning: skipping const checks
    |
 help: skipping check that does not even have a feature gate
@@ -76,12 +64,22 @@ help: skipping check that does not even have a feature gate
    |
 LL | static REFMUT2: &mut i32 = {let mut x = 0; &mut {x}};
    |                                            ^^^^^^^^
-help: skipping check for `const_mut_refs` feature
-  --> $DIR/static-no-inner-mut.rs:29:40
+help: skipping check that does not even have a feature gate
+  --> $DIR/static-no-inner-mut.rs:33:52
+   |
+LL | static RAW_SYNC: SyncPtr<AtomicI32> = SyncPtr { x: &AtomicI32::new(42) };
+   |                                                    ^^^^^^^^^^^^^^^^^^^
+help: skipping check that does not even have a feature gate
+  --> $DIR/static-no-inner-mut.rs:34:51
+   |
+LL | static RAW_MUT_CAST: SyncPtr<i32> = SyncPtr { x : &mut 42 as *mut _ as *const _ };
+   |                                                   ^^^^^^^
+help: skipping check that does not even have a feature gate
+  --> $DIR/static-no-inner-mut.rs:35:52
    |
-LL | static RAW_MUT: SyncPtr<i32> = SyncPtr(&mut 42 as *mut _ as *const _);
-   |                                        ^^^^^^^
+LL | static RAW_MUT_COERCE: SyncPtr<i32> = SyncPtr { x: &mut 0 };
+   |                                                    ^^^^^^
 
-error: aborting due to 6 previous errors; 1 warning emitted
+error: aborting due to 4 previous errors; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0080`.
diff --git a/tests/ui/consts/miri_unleashed/static-no-inner-mut.rs b/tests/ui/consts/miri_unleashed/static-no-inner-mut.rs
@@ -1,6 +1,6 @@
 // stderr-per-bitwidth
 // compile-flags: -Zunleash-the-miri-inside-of-you
-#![feature(const_refs_to_cell)]
+#![feature(const_refs_to_cell, const_mut_refs)]
 // All "inner" allocations that come with a `static` are interned immutably. This means it is
 // crucial that we do not accept any form of (interior) mutability there.
 
@@ -24,7 +24,12 @@ static REFMUT2: &mut i32 = {let mut x = 0; &mut {x}}; //~ERROR undefined behavio
 struct SyncPtr<T> { x : *const T }
 unsafe impl<T> Sync for SyncPtr<T> {}
 
-// This one does not get promoted, and then the reference is simply too short-lived.
+// All of these pass the lifetime checks because of the "tail expression" / "outer scope" rule.
+// (This relies on `SyncPtr` being a curly brace struct.)
+// Then they get interned immutably, which is not great.
+// `mut_ref_in_final.rs` and `std/cell.rs` ensure that we don't accept this even with the feature
+// fate, but for unleashed Miri there's not really any way we can reject them: it's just
+// non-dangling raw pointers.
 static RAW_SYNC: SyncPtr<AtomicI32> = SyncPtr { x: &AtomicI32::new(42) };
 static RAW_MUT_CAST: SyncPtr<i32> = SyncPtr { x : &mut 42 as *mut _ as *const _ };
 static RAW_MUT_COERCE: SyncPtr<i32> = SyncPtr { x: &mut 0 };
diff --git a/tests/ui/consts/refs-to-cell-in-final.rs b/tests/ui/consts/refs-to-cell-in-final.rs
@@ -0,0 +1,18 @@
+#![feature(const_refs_to_cell)]
+
+use std::cell::*;
+
+struct SyncPtr<T> { x : *const T }
+unsafe impl<T> Sync for SyncPtr<T> {}
+
+// These pass the lifetime checks because of the "tail expression" / "outer scope" rule.
+// (This relies on `SyncPtr` being a curly brace struct.)
+// However, we intern the inner memory as read-only.
+// The resulting constant would pass all validation checks, so it is crucial that this gets rejected
+// by static const checks!
+static RAW_SYNC_S: SyncPtr<Cell<i32>> = SyncPtr { x: &Cell::new(42) };
+//~^ ERROR: cannot refer to interior mutable data
+const RAW_SYNC_C: SyncPtr<Cell<i32>> = SyncPtr { x: &Cell::new(42) };
+//~^ ERROR: cannot refer to interior mutable data
+
+fn main() {}
diff --git a/tests/ui/consts/refs-to-cell-in-final.stderr b/tests/ui/consts/refs-to-cell-in-final.stderr
@@ -0,0 +1,17 @@
+error[E0492]: statics cannot refer to interior mutable data
+  --> $DIR/refs-to-cell-in-final.rs:13:54
+   |
+LL | static RAW_SYNC_S: SyncPtr<Cell<i32>> = SyncPtr { x: &Cell::new(42) };
+   |                                                      ^^^^^^^^^^^^^^ this borrow of an interior mutable value may end up in the final value
+   |
+   = help: to fix this, the value can be extracted to a separate `static` item and then referenced
+
+error[E0492]: constants cannot refer to interior mutable data
+  --> $DIR/refs-to-cell-in-final.rs:15:53
+   |
+LL | const RAW_SYNC_C: SyncPtr<Cell<i32>> = SyncPtr { x: &Cell::new(42) };
+   |                                                     ^^^^^^^^^^^^^^ this borrow of an interior mutable value may end up in the final value
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0492`.

Original file line number	Diff line number	Diff line change
`@@ -387,6 +387,8 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {`
`387`	`387`	`if self.local_has_storage_dead(local) {`
`388`	`388`	`self.check_op(ops::TransientMutBorrow(kind));`
`389`	`389`	`} else {`
	`390`	`+ // This check is very important. If we let this borrow leak into the final`
	`391`	`+ // value, it would be treated as read-only memory, which would be unsound!`
`390`	`392`	`self.check_op(ops::MutBorrow(kind));`
`391`	`393`	`}`
`392`	`394`	`}`
`@@ -528,6 +530,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {`
`528`	`530`	`if self.local_has_storage_dead(place.local) {`
`529`	`531`	`self.check_op(ops::TransientCellBorrow);`
`530`	`532`	`} else {`
	`533`	`+ // This check is very important. If we let this borrow leak into the final`
	`534`	`+ // value, it would be treated as read-only memory, which would be unsound!`
`531`	`535`	`self.check_op(ops::CellBorrow);`
`532`	`536`	`}`
`533`	`537`	`}`