Validate before reporting interning errors.

validation produces much higher quality errors and already handles most of the cases

Author: oli-obk

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -10,20 +10,21 @@ use rustc_middle::traits::Reveal;
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self,Ty,TyCtxt};
+use rustc_session::lint;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::Span;
 use rustc_target::abi::{self,Abi};
 
 usesuper::{CanAccessMutGlobal,CompileTimeEvalContext,CompileTimeInterpreter};
 usecrate::const_eval::CheckAlignment;
-usecrate::errors;
 usecrate::errors::ConstEvalError;
-usecrate::interpret::eval_nullary_intrinsic;
+usecrate::errors::{self,DanglingPtrInFinal};
 usecrate::interpret::{
  create_static_alloc, intern_const_alloc_recursive,CtfeValidationMode,GlobalId,Immediate,
 InternKind,InterpCx,InterpError,InterpResult,MPlaceTy,MemoryKind,OpTy,RefTracking,
 StackPopCleanup,
 };
+usecrate::interpret::{eval_nullary_intrinsic,InternResult};
 usecrate::CTRL_C_RECEIVED;
 
 // Returns a pointer to where the result lives
@@ -89,11 +90,33 @@ fn eval_body_using_ecx<'mir, 'tcx, R: InterpretationResult<'tcx>>(
 }
 
 // Intern the result
-intern_const_alloc_recursive(ecx, intern_kind,&ret)?;
+let intern_result = intern_const_alloc_recursive(ecx, intern_kind,&ret);
 
 // Since evaluation had no errors, validate the resulting constant.
 const_validate_mplace(&ecx,&ret, cid)?;
 
+// Only report this after validation, as validaiton produces much better diagnostics.
+// FIXME: ensure validation always reports this and stop making interning care about it.
+
+ifletErr(InternResult{ found_bad_mutable_pointer, found_dangling_pointer }) = intern_result {
+if found_dangling_pointer {
+returnErr(ecx
+.tcx
+.dcx()
+.emit_err(DanglingPtrInFinal{span: ecx.tcx.span,kind: intern_kind })
+.into());
+}elseif found_bad_mutable_pointer {
+// only report mutable pointers if there were no dangling pointers
+let err_diag = errors::MutablePtrInFinal{span: ecx.tcx.span,kind: intern_kind };
+ ecx.tcx.emit_node_span_lint(
+ lint::builtin::CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
+ ecx.best_lint_scope(),
+ err_diag.span,
+ err_diag,
+)
+}
+}
+
 Ok(R::make_result(ret, ecx))
 }
 

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -16,19 +16,17 @@
 use hir::def::DefKind;
 use rustc_ast::Mutability;
 use rustc_data_structures::fx::{FxHashSet,FxIndexMap};
-use rustc_errors::ErrorGuaranteed;
 use rustc_hir as hir;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
 use rustc_middle::mir::interpret::{ConstAllocation,CtfeProvenance,InterpResult};
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::layout::TyAndLayout;
-use rustc_session::lint;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::sym;
 
 usesuper::{AllocId,Allocation,InterpCx,MPlaceTy,Machine,MemoryKind,PlaceTy};
 usecrate::const_eval;
-usecrate::errors::{DanglingPtrInFinal,MutablePtrInFinal,NestedStaticInThreadLocal};
+usecrate::errors::NestedStaticInThreadLocal;
 
 pubtraitCompileTimeMachine<'mir,'tcx:'mir,T> = Machine<
 'mir,
@@ -134,6 +132,19 @@ pub enum InternKind {
 Promoted,
 }
 
+#[derive(Default,Debug)]
+pubstructInternResult{
+pubfound_bad_mutable_pointer:bool,
+pubfound_dangling_pointer:bool,
+}
+
+implInternResult{
+fnhas_errors(&self) -> bool{
+letSelf{ found_bad_mutable_pointer, found_dangling_pointer } = *self;
+ found_bad_mutable_pointer || found_dangling_pointer
+}
+}
+
 /// Intern `ret` and everything it references.
 ///
 /// This *cannot raise an interpreter error*. Doing so is left to validation, which
@@ -149,7 +160,7 @@ pub fn intern_const_alloc_recursive<
 ecx:&mutInterpCx<'mir,'tcx,M>,
 intern_kind:InternKind,
 ret:&MPlaceTy<'tcx>,
-) -> Result<(),ErrorGuaranteed>{
+) -> Result<(),InternResult>{
 // We are interning recursively, and for mutability we are distinguishing the "root" allocation
 // that we are starting in, and all other allocations that we are encountering recursively.
 let(base_mutability, inner_mutability, is_static) = match intern_kind {
@@ -201,7 +212,7 @@ pub fn intern_const_alloc_recursive<
 // Whether we encountered a bad mutable pointer.
 // We want to first report "dangling" and then "mutable", so we need to delay reporting these
 // errors.
-letmutfound_bad_mutable_pointer = false;
+letmutresult = InternResult::default();
 
 // Keep interning as long as there are things to intern.
 // We show errors if there are dangling pointers, or mutable pointers in immutable contexts
@@ -251,7 +262,7 @@ pub fn intern_const_alloc_recursive<
 // on the promotion analysis not screwing up to ensure that it is sound to intern
 // promoteds as immutable.
 trace!("found bad mutable pointer");
- found_bad_mutable_pointer = true;
+result.found_bad_mutable_pointer = true;
 }
 if ecx.tcx.try_get_global_alloc(alloc_id).is_some(){
 // Already interned.
@@ -269,21 +280,15 @@ pub fn intern_const_alloc_recursive<
 // pointers before deciding which allocations can be made immutable; but for now we are
 // okay with losing some potential for immutability here. This can anyway only affect
 // `static mut`.
- todo.extend(intern_shallow(ecx, alloc_id, inner_mutability).map_err(|()| {
- ecx.tcx.dcx().emit_err(DanglingPtrInFinal{span: ecx.tcx.span,kind: intern_kind })
-})?);
-}
-if found_bad_mutable_pointer {
-let err_diag = MutablePtrInFinal{span: ecx.tcx.span,kind: intern_kind };
- ecx.tcx.emit_node_span_lint(
- lint::builtin::CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
- ecx.best_lint_scope(),
- err_diag.span,
- err_diag,
-)
+matchintern_shallow(ecx, alloc_id, inner_mutability){
+Ok(nested) => todo.extend(nested),
+Err(()) => {
+ ecx.tcx.dcx().delayed_bug("found dangling pointer during const interning");
+ result.found_dangling_pointer = true
+}
+}
 }
-
-Ok(())
+if result.has_errors(){Err(result)}else{Ok(())}
 }
 
 /// Intern `ret`. This function assumes that `ret` references no other allocation.

compiler/rustc_const_eval/src/interpret/mod.rs

@@ -23,6 +23,7 @@ pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in
 pubuseself::eval_context::{format_interp_error,Frame,FrameInfo,InterpCx,StackPopCleanup};
 pubuseself::intern::{
  intern_const_alloc_for_constprop, intern_const_alloc_recursive,HasStaticRootDefId,InternKind,
+InternResult,
 };
 pubuseself::machine::{compile_time_machine,AllocMap,Machine,MayLeak,StackPopJump};
 pubuseself::memory::{AllocKind,AllocRef,AllocRefMut,FnVal,Memory,MemoryKind};

tests/ui/consts/const-eval/heap/dealloc_intrinsic_dangling.rs

@@ -2,10 +2,15 @@
 #![feature(const_heap)]
 #![feature(const_mut_refs)]
 
+// Strip out raw byte dumps to make comparison platform-independent:
+//@ normalize-stderr-test "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
+//@ normalize-stderr-test "([0-9a-f][0-9a-f] |╾─*A(LLOC)?[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
+//@ normalize-stderr-test "HEX_DUMP\s*\n\s*HEX_DUMP" -> "HEX_DUMP"
+
 use std::intrinsics;
 
 const _X:&'staticu8 = unsafe{
-//~^ error: dangling pointer in final value of constant
+//~^ error: it is undefined behavior to use this value
 let ptr = intrinsics::const_allocate(4,4);
  intrinsics::const_deallocate(ptr,4,4);
 &*ptr

tests/ui/consts/const-eval/heap/dealloc_intrinsic_dangling.stderr

@@ -1,14 +1,19 @@
-error: encountered dangling pointer in final value of constant
- --> $DIR/dealloc_intrinsic_dangling.rs:7:1
+error[E0080]: it is undefined behavior to use this value
+ --> $DIR/dealloc_intrinsic_dangling.rs:12:1
  |
 LL | const _X: &'static u8 = unsafe {
- | ^^^^^^^^^^^^^^^^^^^^^
+ | ^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (use-after-free)
+ |
+ = 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.
+ = note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
+ HEX_DUMP
+ }
 
 error[E0080]: evaluation of constant value failed
- --> $DIR/dealloc_intrinsic_dangling.rs:18:5
+ --> $DIR/dealloc_intrinsic_dangling.rs:23:5
  |
 LL | *reference
- | ^^^^^^^^^^ memory access failed: ALLOC0 has been freed, so this pointer is dangling
+ | ^^^^^^^^^^ memory access failed: ALLOC1 has been freed, so this pointer is dangling
 
 error: aborting due to 2 previous errors
 

tests/ui/consts/const-mut-refs/mut_ref_in_final_dynamic_check.rs

@@ -33,8 +33,8 @@ const fn helper_dangling() -> Option<&'static mut i32> { unsafe {
 // Undefined behaviour (dangling pointer), who doesn't love tests like this.
 Some(&mut*(&mut42as*muti32))
 }}
-constDANGLING:Option<&muti32> = helper_dangling();//~ ERROR encountered dangling pointer
-staticDANGLING_STATIC:Option<&muti32> = helper_dangling();//~ ERROR encountered dangling pointer
+constDANGLING:Option<&muti32> = helper_dangling();//~ ERROR it is undefined behavior to use this value
+staticDANGLING_STATIC:Option<&muti32> = helper_dangling();//~ ERROR it is undefined behavior to use this value
 
 // These are fine! Just statics pointing to mutable statics, nothing fundamentally wrong with this.
 staticMUT_STATIC:Option<&muti32> = helper();

tests/ui/consts/const-mut-refs/mut_ref_in_final_dynamic_check.stderr

@@ -31,17 +31,27 @@ LL | static INT2PTR_STATIC: Option<&mut i32> = helper_int2ptr();
  HEX_DUMP
  }
 
-error: encountered dangling pointer in final value of constant
+error[E0080]: it is undefined behavior to use this value
  --> $DIR/mut_ref_in_final_dynamic_check.rs:36:1
  |
 LL | const DANGLING: Option<&mut i32> = helper_dangling();
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered a dangling reference (use-after-free)
+ |
+ = 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.
+ = note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
+ HEX_DUMP
+ }
 
-error: encountered dangling pointer in final value of static
+error[E0080]: it is undefined behavior to use this value
  --> $DIR/mut_ref_in_final_dynamic_check.rs:37:1
  |
 LL | static DANGLING_STATIC: Option<&mut i32> = helper_dangling();
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered a dangling reference (use-after-free)
+ |
+ = 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.
+ = note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
+ HEX_DUMP
+ }
 
 error: aborting due to 5 previous errors
 

tests/ui/consts/dangling-alloc-id-ice.rs

@@ -1,12 +1,16 @@
 // https://github.com/rust-lang/rust/issues/55223
+// Strip out raw byte dumps to make comparison platform-independent:
+//@ normalize-stderr-test "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
+//@ normalize-stderr-test "([0-9a-f][0-9a-f] |╾─*A(LLOC)?[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
+//@ normalize-stderr-test "HEX_DUMP\s*\n\s*HEX_DUMP" -> "HEX_DUMP"
 
 unionFoo<'a>{
 y:&'a(),
 long_live_the_unit:&'static(),
 }
 
 constFOO:&() = {
-//~^ ERROR encountered dangling pointer in final value of constant
+//~^ ERROR it is undefined behavior to use this value
 let y = ();
 unsafe{Foo{y:&y }.long_live_the_unit}
 };

tests/ui/consts/dangling-alloc-id-ice.stderr

@@ -1,8 +1,14 @@
-error: encountered dangling pointer in final value of constant
- --> $DIR/dangling-alloc-id-ice.rs:8:1
+error[E0080]: it is undefined behavior to use this value
+ --> $DIR/dangling-alloc-id-ice.rs:12:1
  |
 LL | const FOO: &() = {
- | ^^^^^^^^^^^^^^
+ | ^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (use-after-free)
+ |
+ = 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.
+ = note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
+ HEX_DUMP
+ }
 
 error: aborting due to 1 previous error
 
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/dangling_raw_ptr.rs

@@ -1,8 +1,29 @@
-constFOO:*constu32 = {//~ ERROR encountered dangling pointer in final value of constant
+constFOO:*constu32 = {
+//~^ ERROR encountered dangling pointer in final value of constant
 let x = 42;
 &x
 };
 
-fnmain(){
-let x = FOO;
+unionUnion{
+ptr:*constu32,
 }
+
+constBAR:Union = {
+//~^ ERROR encountered dangling pointer in final value of constant
+let x = 42;
+Union{ptr:&x }
+};
+
+constBAZ:Union = {
+//~^ ERROR encountered dangling pointer in final value of constant
+let x = 42_u32;
+Union{ptr:&(&x as*constu32)as*const*constu32as_}
+};
+
+constFOOMP:*constu32 = {
+//~^ ERROR encountered dangling pointer in final value of constant
+let x = 42_u32;
+&(&x as*constu32)as*const*constu32as_
+};
+
+fnmain(){}