avoid raising interpreter errors from interning

Author: RalfJung

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/intern.rs

@@ -5,9 +5,8 @@
 
 use super::validity::RefTracking;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_errors::ErrorReported;
 use rustc_hir as hir;
-use rustc_middle::mir::interpret::{ErrorHandled, InterpResult};
+use rustc_middle::mir::interpret::InterpResult;
 use rustc_middle::ty::{self, query::TyCtxtAt, Ty};
 
 use rustc_ast::ast::Mutability;
@@ -64,6 +63,7 @@ enum InternMode {
 struct IsStaticOrFn;
 
 fn mutable_memory_in_const(tcx: TyCtxtAt<'_>, kind: &str) {
+    // FIXME: show this in validation instead so we can point at where in the value the error is?
     tcx.sess.span_err(tcx.span, &format!("mutable memory ({}) is not allowed in constant", kind));
 }
 
@@ -79,7 +79,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
     alloc_id: AllocId,
     mode: InternMode,
     ty: Option<Ty<'tcx>>,
-) -> InterpResult<'tcx, Option<IsStaticOrFn>> {
+) -> Option<IsStaticOrFn> {
     trace!("intern_shallow {:?} with {:?}", alloc_id, mode);
     // remove allocation
     let tcx = ecx.tcx;
@@ -97,7 +97,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
             }
             // treat dangling pointers like other statics
             // just to stop trying to recurse into them
-            return Ok(Some(IsStaticOrFn));
+            return Some(IsStaticOrFn);
         }
     };
     // This match is just a canary for future changes to `MemoryKind`, which most likely need
@@ -136,7 +136,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
     let alloc = tcx.intern_const_alloc(alloc);
     leftover_allocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc));
     tcx.set_alloc_id_memory(alloc_id, alloc);
-    Ok(None)
+    None
 }
 
 impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> InternVisitor<'rt, 'mir, 'tcx, M> {
@@ -145,7 +145,7 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> InternVisitor<'rt, 'mir
         alloc_id: AllocId,
         mode: InternMode,
         ty: Option<Ty<'tcx>>,
-    ) -> InterpResult<'tcx, Option<IsStaticOrFn>> {
+    ) -> Option<IsStaticOrFn> {
         intern_shallow(
             self.ecx,
             self.leftover_allocations,
@@ -213,7 +213,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
                 if let Scalar::Ptr(vtable) = mplace.meta.unwrap_meta() {
                     // Explicitly choose const mode here, since vtables are immutable, even
                     // if the reference of the fat pointer is mutable.
-                    self.intern_shallow(vtable.alloc_id, InternMode::ConstInner, None)?;
+                    self.intern_shallow(vtable.alloc_id, InternMode::ConstInner, None);
                 } else {
                     // Let validation show the error message, but make sure it *does* error.
                     tcx.sess.delay_span_bug(
@@ -277,7 +277,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
                         InternMode::ConstInner
                     }
                 };
-                match self.intern_shallow(ptr.alloc_id, ref_mode, Some(referenced_ty))? {
+                match self.intern_shallow(ptr.alloc_id, ref_mode, Some(referenced_ty)) {
                     // No need to recurse, these are interned already and statics may have
                     // cycles, so we don't want to recurse there
                     Some(IsStaticOrFn) => {}
@@ -304,12 +304,18 @@ pub enum InternKind {
     ConstProp,
 }
 
+/// Intern `ret` and everything it references.
+///
+/// This *cannot raise an interpreter error*.  Doing so is left to validation, which
+/// trakcs where in the value we are and thus can show much better error messages.
+/// Any errors here would anyway be turned into `const_err` lints, whereas validation failures
+/// are hard errors.
 pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
     ecx: &mut InterpCx<'mir, 'tcx, M>,
     intern_kind: InternKind,
     ret: MPlaceTy<'tcx>,
     ignore_interior_mut_in_const: bool,
-) -> InterpResult<'tcx>
+)
 where
     'tcx: 'mir,
 {
@@ -338,7 +344,7 @@ where
         ret.ptr.assert_ptr().alloc_id,
         base_intern_mode,
         Some(ret.layout.ty),
-    )?;
+    );
 
     ref_tracking.track((ret, base_intern_mode), || ());
 
@@ -422,13 +428,16 @@ where
                 }
             }
         } else if ecx.memory.dead_alloc_map.contains_key(&alloc_id) {
-            // dangling pointer
-            throw_ub_format!("encountered dangling pointer in final constant")
+            // Codegen does not like dangling pointers, and generally `tcx` assumes that
+            // all allocations referenced anywhere actually exist. So, make sure we error here.
+            ecx.tcx.sess.span_err(
+                ecx.tcx.span,
+                "encountered dangling pointer in final constant",
+            );
         } else if ecx.tcx.get_global_alloc(alloc_id).is_none() {
-            // We have hit an `AllocId` that is neither in local or global memory and isn't marked
-            // as dangling by local memory.
+                // We have hit an `AllocId` that is neither in local or global memory and isn't
+                // marked as dangling by local memory.  That should be impossible.
             span_bug!(ecx.tcx.span, "encountered unknown alloc id {:?}", alloc_id);
         }
     }
-    Ok(())
 }

src/librustc_mir/transform/const_prop.rs

@@ -702,8 +702,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