interpret: reset padding during validation

Author: RalfJung

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -1,16 +1,16 @@
-use std::borrow::Borrow;
+use std::borrow::{Borrow,Cow};
 use std::fmt;
 use std::hash::Hash;
 use std::ops::ControlFlow;
 
 use rustc_ast::Mutability;
-use rustc_data_structures::fx::{FxIndexMap,IndexEntry};
+use rustc_data_structures::fx::{FxHashMap,FxIndexMap,IndexEntry};
 use rustc_hir::def_id::{DefId,LocalDefId};
 use rustc_hir::{selfas hir,LangItem,CRATE_HIR_ID};
 use rustc_middle::mir::AssertMessage;
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::layout::{FnAbiOf,TyAndLayout};
-use rustc_middle::ty::{self,TyCtxt};
+use rustc_middle::ty::{self,Ty,TyCtxt};
 use rustc_middle::{bug, mir};
 use rustc_span::symbol::{sym,Symbol};
 use rustc_span::Span;
@@ -24,8 +24,8 @@ use crate::fluent_generated as fluent;
 usecrate::interpret::{
 self, compile_time_machine, err_ub, throw_exhaust, throw_inval, throw_ub_custom, throw_unsup,
  throw_unsup_format,AllocId,AllocRange,ConstAllocation,CtfeProvenance,FnArg,Frame,
-GlobalAlloc,ImmTy,InterpCx,InterpResult,MPlaceTy,OpTy,Pointer,PointerArithmetic,Scalar,
-StackPopCleanup,
+GlobalAlloc,ImmTy,InterpCx,InterpResult,MPlaceTy,OpTy,Pointer,PointerArithmetic,
+RangeSet,Scalar,StackPopCleanup,
 };
 
 /// When hitting this many interpreted terminators we emit a deny by default lint
@@ -65,6 +65,9 @@ pub struct CompileTimeMachine<'tcx> {
 /// storing the result in the given `AllocId`.
 /// Used to prevent reads from a static's base allocation, as that may allow for self-initialization loops.
 pub(crate)static_root_ids:Option<(AllocId,LocalDefId)>,
+
+/// A cache of "data range" computations for unions (i.e., the offsets of non-padding bytes).
+union_data_ranges:FxHashMap<Ty<'tcx>,RangeSet>,
 }
 
 #[derive(Copy,Clone)]
@@ -99,6 +102,7 @@ impl<'tcx> CompileTimeMachine<'tcx> {
  can_access_mut_global,
  check_alignment,
 static_root_ids:None,
+union_data_ranges:FxHashMap::default(),
 }
 }
 }
@@ -766,6 +770,19 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
 }
 Ok(())
 }
+
+fncached_union_data_range<'e>(
+ecx:&'emutInterpCx<'tcx,Self>,
+ty:Ty<'tcx>,
+compute_range:implFnOnce() -> RangeSet,
+) -> Cow<'e,RangeSet>{
+if ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks{
+Cow::Borrowed(ecx.machine.union_data_ranges.entry(ty).or_insert_with(compute_range))
+}else{
+// Don't bother caching, we're only doing one validation at the end anyway.
+Cow::Owned(compute_range())
+}
+}
 }
 
 // Please do not add any code below the above `Machine` trait impl. I (oli-obk) plan more cleanups

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -10,6 +10,7 @@ use rustc_apfloat::{Float, FloatConvert};
 use rustc_ast::{InlineAsmOptions,InlineAsmTemplatePiece};
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::layout::TyAndLayout;
+use rustc_middle::ty::Ty;
 use rustc_middle::{mir, ty};
 use rustc_span::def_id::DefId;
 use rustc_span::Span;
@@ -19,7 +20,7 @@ use rustc_target::spec::abi::Abi as CallAbi;
 usesuper::{
  throw_unsup, throw_unsup_format,AllocBytes,AllocId,AllocKind,AllocRange,Allocation,
 ConstAllocation,CtfeProvenance,FnArg,Frame,ImmTy,InterpCx,InterpResult,MPlaceTy,
-MemoryKind,Misalignment,OpTy,PlaceTy,Pointer,Provenance,CTFE_ALLOC_SALT,
+MemoryKind,Misalignment,OpTy,PlaceTy,Pointer,Provenance,RangeSet,CTFE_ALLOC_SALT,
 };
 
 /// Data returned by [`Machine::after_stack_pop`], and consumed by
@@ -578,6 +579,15 @@ pub trait Machine<'tcx>: Sized {
 ecx:&InterpCx<'tcx,Self>,
 instance:Option<ty::Instance<'tcx>>,
 ) -> usize;
+
+fncached_union_data_range<'e>(
+_ecx:&'emutInterpCx<'tcx,Self>,
+_ty:Ty<'tcx>,
+compute_range:implFnOnce() -> RangeSet,
+) -> Cow<'e,RangeSet>{
+// Default to no caching.
+Cow::Owned(compute_range())
+}
 }
 
 /// A lot of the flexibility above is just needed for `Miri`, but all "compile-time" machines

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -1136,8 +1136,17 @@ impl<'tcx, 'a, Prov: Provenance, Extra, Bytes: AllocBytes>
 self.write_scalar(alloc_range(offset,self.tcx.data_layout().pointer_size), val)
 }
 
+/// Mark the given sub-range (relative to this allocation reference) as uninitialized.
+pubfnwrite_uninit(&mutself,range:AllocRange) -> InterpResult<'tcx>{
+let range = self.range.subrange(range);
+Ok(self
+.alloc
+.write_uninit(&self.tcx, range)
+.map_err(|e| e.to_interp_error(self.alloc_id))?)
+}
+
 /// Mark the entire referenced range as uninitialized
-pubfnwrite_uninit(&mutself) -> InterpResult<'tcx>{
+pubfnwrite_uninit_full(&mutself) -> InterpResult<'tcx>{
 Ok(self
 .alloc
 .write_uninit(&self.tcx,self.range)

compiler/rustc_const_eval/src/interpret/mod.rs

@@ -39,5 +39,5 @@ use self::place::{MemPlace, Place};
 pubuseself::projection::{OffsetMode,Projectable};
 pubuseself::stack::{Frame,FrameInfo,LocalState,StackPopCleanup,StackPopInfo};
 pub(crate)useself::util::create_static_alloc;
-pubuseself::validity::{CtfeValidationMode,RefTracking};
+pubuseself::validity::{CtfeValidationMode,RangeSet,RefTracking};
 pubuseself::visitor::ValueVisitor;

compiler/rustc_const_eval/src/interpret/place.rs

@@ -604,10 +604,11 @@ where
 
 ifM::enforce_validity(self, dest.layout()){
 // Data got changed, better make sure it matches the type!
+// Also needed to reset padding.
 self.validate_operand(
 &dest.to_place(),
 M::enforce_validity_recursively(self, dest.layout()),
-/*reset_provenance*/true,
+/*reset_provenance_and_padding*/true,
 )?;
 }
 
@@ -703,9 +704,11 @@ where
 // fields do not match the `ScalarPair` components.
 
  alloc.write_scalar(alloc_range(Size::ZERO, a_val.size()), a_val)?;
- alloc.write_scalar(alloc_range(b_offset, b_val.size()), b_val)
+ alloc.write_scalar(alloc_range(b_offset, b_val.size()), b_val)?;
+// We don't have to reset padding here, `write_immediate` will anyway do a validation run.
+Ok(())
 }
-Immediate::Uninit => alloc.write_uninit(),
+Immediate::Uninit => alloc.write_uninit_full(),
 }
 }
 
@@ -722,7 +725,7 @@ where
 // Zero-sized access
 returnOk(());
 };
- alloc.write_uninit()?;
+ alloc.write_uninit_full()?;
 }
 }
 Ok(())
@@ -814,17 +817,17 @@ where
 // Given that there were two typed copies, we have to ensure this is valid at both types,
 // and we have to ensure this loses provenance and padding according to both types.
 // But if the types are identical, we only do one pass.
-if src.layout().ty != dest.layout().ty{
+ifallow_transmute && src.layout().ty != dest.layout().ty{
 self.validate_operand(
 &dest.transmute(src.layout(),self)?,
 M::enforce_validity_recursively(self, src.layout()),
-/*reset_provenance*/true,
+/*reset_provenance_and_padding*/true,
 )?;
 }
 self.validate_operand(
 &dest,
 M::enforce_validity_recursively(self, dest.layout()),
-/*reset_provenance*/true,
+/*reset_provenance_and_padding*/true,
 )?;
 }