Auto merge of rust-lang#71665 - RalfJung:miri-intern-no-ice, r=oli-obk

Miri interning: replace ICEs by proper errors

Fixes rust-lang#71316

I also did some refactoring, as I kept being confused by all the parameters to `intern_shallow`, some of which have invalid combinations (such as a mutable const). So instead `InternMode` now contains all the information that is needed and invalid combinations are ruled out by the type system.

Also I removed interpreter errors from interning. We already ignored almost all errors, and the `ValidationFailure` errors that we handled separately actually cannot ever happen here. The only interpreter failure that was actually reachable was the UB on dangling pointers -- and arguably, a dangling raw pointer is not UB, so the error was not even correct. It's just that the rest of the compiler does not like "dangling" `AllocId`.

It should be possible to review the 3 commits separately.

r? @oli-obk
Cc @rust-lang/wg-const-eval

src/librustc_mir/const_eval/eval_queries.rs

@@ -66,7 +66,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
         intern_kind,
         ret,
         body.ignore_interior_mut_in_const_validation,
-    )?;
+    );
 
     debug!("eval_body_using_ecx done: {:?}", *ret);
     Ok(ret)

src/librustc_mir/const_eval/mod.rs

@@ -53,7 +53,7 @@ pub(crate) fn const_caller_location(
     let mut ecx = mk_eval_cx(tcx, DUMMY_SP, ty::ParamEnv::reveal_all(), false);
 
     let loc_place = ecx.alloc_caller_location(file, line, col);
-    intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place, false).unwrap();
+    intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place, false);
     ConstValue::Scalar(loc_place.ptr)
 }
 

src/librustc_mir/interpret/eval_context.rs

@@ -871,6 +871,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // Our result will later be validated anyway, and there seems no good reason
         // to have to fail early here.  This is also more consistent with
         // `Memory::get_static_alloc` which has to use `const_eval_raw` to avoid cycles.
+        // FIXME: We can hit delay_span_bug if this is an invalid const, interning finds
+        // that problem, but we never run validation to show an error. Can we ensure
+        // this does not happen?
         let val = self.tcx.const_eval_raw(param_env.and(gid))?;
         self.raw_const_to_mplace(val)
     }

src/librustc_mir/transform/const_prop.rs

@@ -709,8 +709,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             )) => l.is_bits() && r.is_bits(),
             interpret::Operand::Indirect(_) if mir_opt_level >= 2 => {
                 let mplace = op.assert_mem_place(&self.ecx);
-                intern_const_alloc_recursive(&mut self.ecx, InternKind::ConstProp, mplace, false)
-                    .expect("failed to intern alloc");
+                intern_const_alloc_recursive(&mut self.ecx, InternKind::ConstProp, mplace, false);
                 true
             }
             _ => false,

src/test/ui/consts/dangling-alloc-id-ice.rs

@@ -1,11 +1,13 @@
 // https://github.com/rust-lang/rust/issues/55223
+#![allow(const_err)]
 
 union Foo<'a> {
     y: &'a (),
     long_live_the_unit: &'static (),
 }
 
-const FOO: &() = { //~ ERROR any use of this value will cause an error
+const FOO: &() = { //~ ERROR it is undefined behavior to use this value
+//~^ ERROR encountered dangling pointer in final constant
     let y = ();
     unsafe { Foo { y: &y }.long_live_the_unit }
 };

src/test/ui/consts/dangling-alloc-id-ice.stderr

@@ -1,13 +1,25 @@
-error: any use of this value will cause an error
-  --> $DIR/dangling-alloc-id-ice.rs:8:1
+error: encountered dangling pointer in final constant
+  --> $DIR/dangling-alloc-id-ice.rs:9:1
    |
 LL | / const FOO: &() = {
+LL | |
 LL | |     let y = ();
 LL | |     unsafe { Foo { y: &y }.long_live_the_unit }
 LL | | };
-   | |__^ encountered dangling pointer in final constant
+   | |__^
+
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/dangling-alloc-id-ice.rs:9:1
+   |
+LL | / const FOO: &() = {
+LL | |
+LL | |     let y = ();
+LL | |     unsafe { Foo { y: &y }.long_live_the_unit }
+LL | | };
+   | |__^ type validation failed: encountered a dangling reference (use-after-free)
    |
-   = note: `#[deny(const_err)]` on by default
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 
+For more information about this error, try `rustc --explain E0080`.

src/test/ui/consts/dangling_raw_ptr.rs

@@ -1,4 +1,4 @@
-const FOO: *const u32 = { //~ ERROR any use of this value will cause an error
+const FOO: *const u32 = { //~ ERROR encountered dangling pointer in final constant
     let x = 42;
     &x
 };

src/test/ui/consts/dangling_raw_ptr.stderr

@@ -1,13 +1,11 @@
-error: any use of this value will cause an error
+error: encountered dangling pointer in final constant
   --> $DIR/dangling_raw_ptr.rs:1:1
    |
 LL | / const FOO: *const u32 = {
 LL | |     let x = 42;
 LL | |     &x
 LL | | };
-   | |__^ encountered dangling pointer in final constant
-   |
-   = note: `#[deny(const_err)]` on by default
+   | |__^
 
 error: aborting due to previous error
 

src/test/ui/consts/miri_unleashed/mutable_references.rs

@@ -17,12 +17,11 @@ struct Foo<T>(T);
 // this is fine for the same reason as `BAR`.
 static BOO: &mut Foo<()> = &mut Foo(());
 
+// interior mutability is fine
 struct Meh {
     x: &'static UnsafeCell<i32>,
 }
-
 unsafe impl Sync for Meh {}
-
 static MEH: Meh = Meh {
     x: &UnsafeCell::new(42),
 };

src/test/ui/consts/miri_unleashed/mutable_references.stderr

@@ -1,5 +1,5 @@
 error[E0594]: cannot assign to `*OH_YES`, as `OH_YES` is an immutable static item
-  --> $DIR/mutable_references.rs:37:5
+  --> $DIR/mutable_references.rs:36:5
    |
 LL |     *OH_YES = 99;
    |     ^^^^^^^^^^^^ cannot assign
@@ -22,12 +22,12 @@ help: skipping check for `const_mut_refs` feature
 LL | static BOO: &mut Foo<()> = &mut Foo(());
    |                            ^^^^^^^^^^^^
 help: skipping check that does not even have a feature gate
-  --> $DIR/mutable_references.rs:27:8
+  --> $DIR/mutable_references.rs:26:8
    |
 LL |     x: &UnsafeCell::new(42),
    |        ^^^^^^^^^^^^^^^^^^^^
 help: skipping check for `const_mut_refs` feature
-  --> $DIR/mutable_references.rs:31:27
+  --> $DIR/mutable_references.rs:30:27
    |
 LL | static OH_YES: &mut i32 = &mut 42;
    |                           ^^^^^^^

src/test/ui/consts/miri_unleashed/mutable_references_err.rs

@@ -0,0 +1,37 @@
+// compile-flags: -Zunleash-the-miri-inside-of-you
+
+#![allow(const_err)]
+
+use std::cell::UnsafeCell;
+
+// this test ensures that our mutability story is sound
+
+struct Meh {
+    x: &'static UnsafeCell<i32>,
+}
+unsafe impl Sync for Meh {}
+
+// the following will never be ok! no interior mut behind consts, because
+// all allocs interned here will be marked immutable.
+const MUH: Meh = Meh { //~ ERROR: mutable memory (`UnsafeCell`) is not allowed in constant
+    x: &UnsafeCell::new(42),
+};
+
+struct Synced {
+    x: UnsafeCell<i32>,
+}
+unsafe impl Sync for Synced {}
+
+// Make sure we also catch this behind a type-erased `dyn Trait` reference.
+const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
+//~^ ERROR: mutable memory (`UnsafeCell`) is not allowed in constant
+
+// Make sure we also catch mutable references.
+const BLUNT: &mut i32 = &mut 42;
+//~^ ERROR: mutable memory (`&mut`) is not allowed in constant
+
+fn main() {
+    unsafe {
+        *MUH.x.get() = 99;
+    }
+}

src/test/ui/consts/miri_unleashed/mutable_references_err.stderr

@@ -0,0 +1,40 @@
+error: mutable memory (`UnsafeCell`) is not allowed in constant
+  --> $DIR/mutable_references_err.rs:16:1
+   |
+LL | / const MUH: Meh = Meh {
+LL | |     x: &UnsafeCell::new(42),
+LL | | };
+   | |__^
+
+error: mutable memory (`UnsafeCell`) is not allowed in constant
+  --> $DIR/mutable_references_err.rs:26:1
+   |
+LL | const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: mutable memory (`&mut`) is not allowed in constant
+  --> $DIR/mutable_references_err.rs:30:1
+   |
+LL | const BLUNT: &mut i32 = &mut 42;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: skipping const checks
+   |
+help: skipping check that does not even have a feature gate
+  --> $DIR/mutable_references_err.rs:17:8
+   |
+LL |     x: &UnsafeCell::new(42),
+   |        ^^^^^^^^^^^^^^^^^^^^
+help: skipping check that does not even have a feature gate
+  --> $DIR/mutable_references_err.rs:26:27
+   |
+LL | const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: skipping check for `const_mut_refs` feature
+  --> $DIR/mutable_references_err.rs:30:25
+   |
+LL | const BLUNT: &mut i32 = &mut 42;
+   |                         ^^^^^^^
+
+error: aborting due to 3 previous errors; 1 warning emitted
+