interpret: reset provenance on typed copies

Author: RalfJung

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -94,7 +94,7 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
 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)?;
+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.
@@ -391,7 +391,7 @@ fn eval_in_interpreter<'tcx, R: InterpretationResult<'tcx>>(
 
 #[inline(always)]
 fnconst_validate_mplace<'tcx>(
-ecx:&InterpCx<'tcx,CompileTimeMachine<'tcx>>,
+ecx:&mutInterpCx<'tcx,CompileTimeMachine<'tcx>>,
 mplace:&MPlaceTy<'tcx>,
 cid:GlobalId<'tcx>,
 ) -> Result<(),ErrorHandled>{

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -8,9 +8,8 @@
 
 use std::assert_matches::assert_matches;
 use std::borrow::Cow;
-use std::cell::Cell;
 use std::collections::VecDeque;
-use std::{fmt, ptr};
+use std::{fmt,mem,ptr};
 
 use rustc_ast::Mutability;
 use rustc_data_structures::fx::{FxHashSet,FxIndexMap};
@@ -118,7 +117,7 @@ pub struct Memory<'tcx, M: Machine<'tcx>> {
 /// This stores whether we are currently doing reads purely for the purpose of validation.
 /// Those reads do not trigger the machine's hooks for memory reads.
 /// Needless to say, this must only be set with great care!
-validation_in_progress:Cell<bool>,
+validation_in_progress:bool,
 }
 
 /// A reference to some allocation that was already bounds-checked for the given region
@@ -145,7 +144,7 @@ impl<'tcx, M: Machine<'tcx>> Memory<'tcx, M> {
 alloc_map:M::MemoryMap::default(),
 extra_fn_ptr_map:FxIndexMap::default(),
 dead_alloc_map:FxIndexMap::default(),
-validation_in_progress:Cell::new(false),
+validation_in_progress:false,
 }
 }
 
@@ -682,15 +681,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 // We want to call the hook on *all* accesses that involve an AllocId, including zero-sized
 // accesses. That means we cannot rely on the closure above or the `Some` branch below. We
 // do this after `check_and_deref_ptr` to ensure some basic sanity has already been checked.
-if !self.memory.validation_in_progress.get(){
+if !self.memory.validation_in_progress{
 ifletOk((alloc_id, ..)) = self.ptr_try_get_alloc_id(ptr, size_i64){
 M::before_alloc_read(self, alloc_id)?;
 }
 }
 
 ifletSome((alloc_id, offset, prov, alloc)) = ptr_and_alloc {
 let range = alloc_range(offset, size);
-if !self.memory.validation_in_progress.get(){
+if !self.memory.validation_in_progress{
 M::before_memory_read(
 self.tcx,
 &self.machine,
@@ -766,11 +765,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 let parts = self.get_ptr_access(ptr, size)?;
 ifletSome((alloc_id, offset, prov)) = parts {
 let tcx = self.tcx;
+let validation_in_progress = self.memory.validation_in_progress;
 // FIXME: can we somehow avoid looking up the allocation twice here?
 // We cannot call `get_raw_mut` inside `check_and_deref_ptr` as that would duplicate `&mut self`.
 let(alloc, machine) = self.get_alloc_raw_mut(alloc_id)?;
 let range = alloc_range(offset, size);
-M::before_memory_write(tcx, machine,&mut alloc.extra,(alloc_id, prov), range)?;
+if !validation_in_progress {
+M::before_memory_write(tcx, machine,&mut alloc.extra,(alloc_id, prov), range)?;
+}
 Ok(Some(AllocRefMut{ alloc, range,tcx:*tcx, alloc_id }))
 }else{
 Ok(None)
@@ -1014,16 +1016,16 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 ///
 /// We do this so Miri's allocation access tracking does not show the validation
 /// reads as spurious accesses.
-pubfnrun_for_validation<R>(&self,f:implFnOnce() -> R) -> R{
+pubfnrun_for_validation<R>(&mutself,f:implFnOnce(&mutSelf) -> R) -> R{
 // This deliberately uses `==` on `bool` to follow the pattern
 // `assert!(val.replace(new) == old)`.
 assert!(
-self.memory.validation_in_progress.replace(true) == false,
+mem::replace(&mutself.memory.validation_in_progress,true) == false,
 "`validation_in_progress` was already set"
 );
-let res = f();
+let res = f(self);
 assert!(
-self.memory.validation_in_progress.replace(false) == true,
+mem::replace(&mutself.memory.validation_in_progress,false) == true,
 "`validation_in_progress` was unset by someone else"
 );
  res
@@ -1115,6 +1117,10 @@ impl<'a, 'tcx, M: Machine<'tcx>> std::fmt::Debug for DumpAllocs<'a, 'tcx, M> {
 impl<'tcx,'a,Prov:Provenance,Extra,Bytes:AllocBytes>
 AllocRefMut<'a,'tcx,Prov,Extra,Bytes>
 {
+pubfnas_ref<'b>(&'bself) -> AllocRef<'b,'tcx,Prov,Extra,Bytes>{
+AllocRef{alloc:self.alloc,range:self.range,tcx:self.tcx,alloc_id:self.alloc_id}
+}
+
 /// `range` is relative to this allocation reference, not the base of the allocation.
 pubfnwrite_scalar(&mutself,range:AllocRange,val:Scalar<Prov>) -> InterpResult<'tcx>{
 let range = self.range.subrange(range);
@@ -1137,6 +1143,14 @@ impl<'tcx, 'a, Prov: Provenance, Extra, Bytes: AllocBytes>
 .write_uninit(&self.tcx,self.range)
 .map_err(|e| e.to_interp_error(self.alloc_id))?)
 }
+
+/// Remove all provenance in the reference range.
+pubfnclear_provenance(&mutself) -> InterpResult<'tcx>{
+Ok(self
+.alloc
+.clear_provenance(&self.tcx,self.range)
+.map_err(|e| e.to_interp_error(self.alloc_id))?)
+}
 }
 
 impl<'tcx,'a,Prov:Provenance,Extra,Bytes:AllocBytes>AllocRef<'a,'tcx,Prov,Extra,Bytes>{
@@ -1278,7 +1292,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 };
 let src_alloc = self.get_alloc_raw(src_alloc_id)?;
 let src_range = alloc_range(src_offset, size);
-assert!(!self.memory.validation_in_progress.get(),"we can't be copying during validation");
+assert!(!self.memory.validation_in_progress,"we can't be copying during validation");
 M::before_memory_read(
  tcx,
 &self.machine,

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -137,6 +137,20 @@ impl<Prov: Provenance> Immediate<Prov> {
 }
 }
 }
+
+pubfnclear_provenance<'tcx>(&mutself) -> InterpResult<'tcx>{
+matchself{
+Immediate::Scalar(s) => {
+ s.clear_provenance()?;
+}
+Immediate::ScalarPair(a, b) => {
+ a.clear_provenance()?;
+ b.clear_provenance()?;
+}
+Immediate::Uninit => {}
+}
+Ok(())
+}
 }
 
 // ScalarPair needs a type to interpret, so we often have an immediate and a type together

compiler/rustc_const_eval/src/interpret/place.rs

@@ -605,8 +605,9 @@ where
 ifM::enforce_validity(self, dest.layout()){
 // Data got changed, better make sure it matches the type!
 self.validate_operand(
-&dest.to_op(self)?,
+&dest.to_place(),
 M::enforce_validity_recursively(self, dest.layout()),
+/*reset_provenance*/true,
 )?;
 }
 
@@ -636,7 +637,7 @@ where
 /// Write an immediate to a place.
 /// If you use this you are responsible for validating that things got copied at the
 /// right type.
-fnwrite_immediate_no_validate(
+pub(super)fnwrite_immediate_no_validate(
 &mutself,
 src:Immediate<M::Provenance>,
 dest:&implWriteable<'tcx,M::Provenance>,
@@ -684,15 +685,7 @@ where
 
 match value {
 Immediate::Scalar(scalar) => {
-letAbi::Scalar(s) = layout.abielse{
-span_bug!(
-self.cur_span(),
-"write_immediate_to_mplace: invalid Scalar layout: {layout:#?}",
-)
-};
-let size = s.size(&tcx);
-assert_eq!(size, layout.size,"abi::Scalar size does not match layout size");
- alloc.write_scalar(alloc_range(Size::ZERO, size), scalar)
+ alloc.write_scalar(alloc_range(Size::ZERO, scalar.size()), scalar)
 }
 Immediate::ScalarPair(a_val, b_val) => {
 letAbi::ScalarPair(a, b) = layout.abielse{
@@ -702,16 +695,15 @@ where
  layout
 )
 };
-let(a_size, b_size) = (a.size(&tcx), b.size(&tcx));
-let b_offset = a_size.align_to(b.align(&tcx).abi);
+let b_offset = a.size(&tcx).align_to(b.align(&tcx).abi);
 assert!(b_offset.bytes() > 0);// in `operand_field` we use the offset to tell apart the fields
 
 // It is tempting to verify `b_offset` against `layout.fields.offset(1)`,
 // but that does not work: We could be a newtype around a pair, then the
 // fields do not match the `ScalarPair` components.
 
- alloc.write_scalar(alloc_range(Size::ZERO,a_size), a_val)?;
- alloc.write_scalar(alloc_range(b_offset,b_size), b_val)
+ alloc.write_scalar(alloc_range(Size::ZERO,a_val.size()), a_val)?;
+ alloc.write_scalar(alloc_range(b_offset,b_val.size()), b_val)
 }
 Immediate::Uninit => alloc.write_uninit(),
 }
@@ -736,6 +728,26 @@ where
 Ok(())
 }
 
+/// Remove all provenance in the given place.
+pubfnclear_provenance(
+&mutself,
+dest:&implWriteable<'tcx,M::Provenance>,
+) -> InterpResult<'tcx>{
+matchself.as_mplace_or_mutable_local(&dest.to_place())? {
+Right((local_val, _local_layout)) => {
+ local_val.clear_provenance()?;
+}
+Left(mplace) => {
+letSome(mut alloc) = self.get_place_alloc_mut(&mplace)? else{
+// Zero-sized access
+returnOk(());
+};
+ alloc.clear_provenance()?;
+}
+}
+Ok(())
+}
+
 /// Copies the data from an operand to a place.
 /// The layouts of the `src` and `dest` may disagree.
 /// Does not perform validation of the destination.
@@ -789,23 +801,30 @@ where
 allow_transmute:bool,
 validate_dest:bool,
 ) -> InterpResult<'tcx>{
-// Generally for transmutation, data must be valid both at the old and new type.
-// But if the types are the same, the 2nd validation below suffices.
-if src.layout().ty != dest.layout().ty && M::enforce_validity(self, src.layout()){
-self.validate_operand(
-&src.to_op(self)?,
-M::enforce_validity_recursively(self, src.layout()),
-)?;
-}
+// These are technically *two* typed copies: `src` is a not-yet-loaded value,
+// so we're going a typed copy at `src` type from there to some intermediate storage.
+// And then we're doing a second typed copy from that intermediate storage to `dest`.
+// But as an optimization, we only make a single direct copy here.
 
 // Do the actual copy.
 self.copy_op_no_validate(src, dest, allow_transmute)?;
 
 if validate_dest && M::enforce_validity(self, dest.layout()){
-// Data got changed, better make sure it matches the type!
+let dest = dest.to_place();
+// 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{
+self.validate_operand(
+&dest.transmute(src.layout(),self)?,
+M::enforce_validity_recursively(self, src.layout()),
+/*reset_provenance*/true,
+)?;
+}
 self.validate_operand(
-&dest.to_op(self)?,
+&dest,
 M::enforce_validity_recursively(self, dest.layout()),
+/*reset_provenance*/true,
 )?;
 }