drop perform_read_access (always read) in favor of zero_size

rust-lang · bors · Jun 29, 2023 · Nov 9, 2022 · Jun 2, 2023 · Jun 3, 2023
commit 65d60f9f11179bf0faa33f948131d454069b2be8
diff --git 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,28 128,27 @@ 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<Self> {
  let pointee = ty.builtin_deref(true).unwrap().ty;
  pointee.is_unpin(*cx.tcx, cx.param_env()).then_some(()).map(|()| {
  // Regular `Unpin` box, give it `noalias` but only a weak protector
  // 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)?;