Skip to content

Commit

Permalink
move Box mess handling into general visitor
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Jul 4, 2022
1 parent 7fc7780 commit d7edf66
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 34 deletions.
40 changes: 6 additions & 34 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,38 +593,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
self.check_safe_pointer(value, "reference")?;
Ok(true)
}
ty::Adt(def, ..) if def.is_box() => {
// Box is special, very special. We carefully assert all the assumptions we make
// here; if this needs to be adjusted, remember to also adjust all the other
// visitors -- in particular the Stacked Borrows retagging visitor in Miri.
// Did I mention that this is a gross hack? Anyway...

// `Box` has two fields: the pointer we care about, and the allocator.
assert_eq!(value.layout.fields.count(), 2, "`Box` must have exactly 2 fields");
let (unique_ptr, alloc) =
(self.ecx.operand_field(value, 0)?, self.ecx.operand_field(value, 1)?);
// Unfortunately there is some type junk in the way here: `unique_ptr` is a `Unique`...
// (which means another 2 fields, the second of which is a `PhantomData`)
assert_eq!(unique_ptr.layout.fields.count(), 2);
let (nonnull_ptr, phantom) = (
self.ecx.operand_field(&unique_ptr, 0)?,
self.ecx.operand_field(&unique_ptr, 1)?,
);
assert!(
phantom.layout.ty.ty_adt_def().is_some_and(|adt| adt.is_phantom_data()),
"2nd field of `Unique` should be PhantomData but is {:?}",
phantom.layout.ty,
);
// ... that contains a `NonNull`... (gladly, only a single field here)
assert_eq!(nonnull_ptr.layout.fields.count(), 1);
let raw_ptr = self.ecx.operand_field(&nonnull_ptr, 0)?; // the actual raw ptr
// ... whose only field finally is a raw ptr we can dereference.
self.check_safe_pointer(&raw_ptr, "box")?;
// The second `Box` field is the allocator, which we recursively check for validity
// like in regular structs.
self.walk_value(&alloc)?;
Ok(true)
}
ty::FnPtr(_sig) => {
let value = try_validation!(
self.ecx.read_scalar(value).and_then(|v| v.check_init()),
Expand Down Expand Up @@ -835,6 +803,12 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
Ok(())
}

#[inline]
fn visit_box(&mut self, op: &OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> {
self.check_safe_pointer(op, "box")?;
Ok(())
}

#[inline]
fn visit_value(&mut self, op: &OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> {
trace!("visit_value: {:?}, {:?}", *op, op.layout);
Expand All @@ -843,8 +817,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
if self.try_visit_primitive(op)? {
return Ok(());
}
// Sanity check: `builtin_deref` does not know any pointers that are not primitive.
assert!(op.layout.ty.builtin_deref(true).is_none());

// Special check preventing `UnsafeCell` in the inner part of constants
if let Some(def) = op.layout.ty.ty_adt_def() {
Expand Down
49 changes: 49 additions & 0 deletions compiler/rustc_const_eval/src/interpret/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ macro_rules! make_value_visitor {
{
Ok(())
}
/// Visits the given value as the pointer of a `Box`. There is nothing to recurse into.
/// The type of `v` will be a raw pointer, but this is a field of `Box<T>` and the
/// pointee type is the actual `T`.
#[inline(always)]
fn visit_box(&mut self, _v: &Self::V) -> InterpResult<'tcx>
{
Ok(())
}
/// Visits this value as an aggregate, you are getting an iterator yielding
/// all the fields (still in an `InterpResult`, you have to do error handling yourself).
/// Recurses into the fields.
Expand Down Expand Up @@ -221,6 +229,47 @@ macro_rules! make_value_visitor {
// Slices do not need special handling here: they have `Array` field
// placement with length 0, so we enter the `Array` case below which
// indirectly uses the metadata to determine the actual length.

// However, `Box`... let's talk about `Box`.
ty::Adt(def, ..) if def.is_box() => {
// `Box` is a hybrid primitive-library-defined type that one the one hand is
// a dereferenceable pointer, on the other hand has *basically arbitrary
// user-defined layout* since the user controls the 'allocator' field. So it
// cannot be treated like a normal pointer, since it does not fit into an
// `Immediate`. Yeah, it is quite terrible. But many visitors want to do
// something with "all boxed pointers", so we handle this mess for them.
//
// When we hit a `Box`, we do not do the usual `visit_aggregate`; instead,
// we (a) call `visit_box` on the pointer value, and (b) recurse on the
// allocator field. We also assert tons of things to ensure we do not miss
// any other fields.

// `Box` has two fields: the pointer we care about, and the allocator.
assert_eq!(v.layout().fields.count(), 2, "`Box` must have exactly 2 fields");
let (unique_ptr, alloc) =
(v.project_field(self.ecx(), 0)?, v.project_field(self.ecx(), 1)?);
// Unfortunately there is some type junk in the way here: `unique_ptr` is a `Unique`...
// (which means another 2 fields, the second of which is a `PhantomData`)
assert_eq!(unique_ptr.layout().fields.count(), 2);
let (nonnull_ptr, phantom) = (
unique_ptr.project_field(self.ecx(), 0)?,
unique_ptr.project_field(self.ecx(), 1)?,
);
assert!(
phantom.layout().ty.ty_adt_def().is_some_and(|adt| adt.is_phantom_data()),
"2nd field of `Unique` should be PhantomData but is {:?}",
phantom.layout().ty,
);
// ... that contains a `NonNull`... (gladly, only a single field here)
assert_eq!(nonnull_ptr.layout().fields.count(), 1);
let raw_ptr = nonnull_ptr.project_field(self.ecx(), 0)?; // the actual raw ptr
// ... whose only field finally is a raw ptr we can dereference.
self.visit_box(&raw_ptr)?;

// The second `Box` field is the allocator, which we recursively check for validity
// like in regular structs.
self.visit_field(v, 1, &alloc)?;
}
_ => {},
};

Expand Down

0 comments on commit d7edf66

Please sign in to comment.