From 65d60f9f11179bf0faa33f948131d454069b2be8 Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Mon, 19 Jun 2023 12:05:30 +0200 Subject: [PATCH] drop perform_read_access (always read) in favor of zero_size --- .../src/borrow_tracker/tree_borrows/mod.rs | 64 +++++++++++-------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 1c7a735779ba4..31cc03828f9b6 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -99,9 +99,9 @@ impl<'tcx> Tree { /// Policy for a new borrow. #[derive(Debug, Clone, Copy)] struct NewPermission { - /// Whether this borrow requires a read access on its parent. - /// `perform_read_access` is `true` for all pointers marked `dereferenceable`. - perform_read_access: bool, + /// Optionally ignore the actual size to do a zero-size reborrow. + /// If this is set then `dereferenceable` is not enforced. + zero_size: bool, /// Which permission should the pointer start with. initial_state: Permission, /// Whether this pointer is part of the arguments of a function call. @@ -128,20 +128,19 @@ impl<'tcx> NewPermission { // `&`s, which are excluded above. _ => return None, }; - // This field happens to be redundant since right now we always do a read, - // but it could be useful in the future. - let perform_read_access = true; let protector = (kind == RetagKind::FnEntry).then_some(ProtectorKind::StrongProtector); - Some(Self { perform_read_access, initial_state, protector }) + Some(Self { zero_size: false, initial_state, protector }) } - // Boxes are not handled by `from_ref_ty`, they need special behavior - // implemented here. - fn from_box_ty( + /// Compute permission for `Box`-like type (`Box` always, and also `Unique` if enabled). + /// These pointers allow deallocation so need a different kind of protector not handled + /// by `from_ref_ty`. + fn from_unique_ty( ty: Ty<'tcx>, kind: RetagKind, cx: &crate::MiriInterpCx<'_, 'tcx>, + zero_size: bool, ) -> Option { let pointee = ty.builtin_deref(true).unwrap().ty; pointee.is_unpin(*cx.tcx, cx.param_env()).then_some(()).map(|()| { @@ -149,7 +148,7 @@ impl<'tcx> NewPermission { // because it is valid to deallocate it within the function. let ty_is_freeze = ty.is_freeze(*cx.tcx, cx.param_env()); Self { - perform_read_access: true, + zero_size, initial_state: Permission::new_unique_2phase(ty_is_freeze), protector: (kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), } @@ -201,6 +200,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' Ok(()) }; + trace!("Reborrow of size {:?}", ptr_size); let (alloc_id, base_offset, parent_prov) = if ptr_size > Size::ZERO { this.ptr_get_alloc_id(place.ptr)? } else { @@ -276,8 +276,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let range = alloc_range(base_offset, ptr_size); let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); - if new_perm.perform_read_access { - // Count this reborrow as a read access + // All reborrows incur a (possibly zero-sized) read access to the parent + { let global = &this.machine.borrow_tracker.as_ref().unwrap(); let span = this.machine.current_span(); tree_borrows.perform_access( @@ -308,12 +308,19 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // We want a place for where the ptr *points to*, so we get one. let place = this.ref_to_mplace(val)?; - // Get a lower bound of the size of this place. - // (When `extern type` are involved, use the size of the known prefix.) - let size = this - .size_and_align_of_mplace(&place)? - .map(|(size, _)| size) - .unwrap_or(place.layout.size); + // Determine the size of the reborrow. + // For most types this is the entire size of the place, however + // - when `extern type` is involved we use the size of the known prefix, + // - if the pointer is not reborrowed (raw pointer) or if `zero_size` is set + // then we override the size to do a zero-length reborrow. + let reborrow_size = match new_perm { + Some(NewPermission { zero_size: false, .. }) => + this.size_and_align_of_mplace(&place)? + .map(|(size, _)| size) + .unwrap_or(place.layout.size), + _ => Size::from_bytes(0), + }; + trace!("Creating new permission: {:?} with size {:?}", new_perm, reborrow_size); // This new tag is not guaranteed to actually be used. // @@ -324,7 +331,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Compute the actual reborrow. - let reborrowed = this.tb_reborrow(&place, size, new_perm, new_tag)?; + let reborrowed = this.tb_reborrow(&place, reborrow_size, new_perm, new_tag)?; // Adjust pointer. let new_place = place.map_provenance(|p| { @@ -359,10 +366,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { val: &ImmTy<'tcx, Provenance>, ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); - let new_perm = if let &ty::Ref(_, pointee, mutability) = val.layout.ty.kind() { - NewPermission::from_ref_ty(pointee, mutability, kind, this) - } else { - None + let new_perm = match val.layout.ty.kind() { + &ty::Ref(_, pointee, mutability) => + NewPermission::from_ref_ty(pointee, mutability, kind, this), + _ => None, }; this.tb_retag_reference(val, new_perm) } @@ -408,7 +415,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx); + let new_perm = NewPermission::from_unique_ty( + place.layout.ty, + self.kind, + self.ecx, + /* zero_size */ false, + ); self.retag_ptr_inplace(place, new_perm) } @@ -486,7 +498,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // FIXME: do we truly want a 2phase borrow here? let new_perm = Some(NewPermission { initial_state: Permission::new_unique_2phase(/*freeze*/ false), - perform_read_access: true, + zero_size: false, protector: Some(ProtectorKind::StrongProtector), }); let val = this.tb_retag_reference(&val, new_perm)?;