Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix interpreter validity check on Box #98847

Merged
merged 2 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,16 +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() => {
let unique = self.ecx.operand_field(value, 0)?;
let nonnull = self.ecx.operand_field(&unique, 0)?;
let ptr = self.ecx.operand_field(&nonnull, 0)?;
self.check_safe_pointer(&ptr, "box")?;

// Check other fields of Box
self.walk_value(value)?;
Ok(true)
}
ty::FnPtr(_sig) => {
let value = try_validation!(
self.ecx.read_scalar(value).and_then(|v| v.check_init()),
Expand Down Expand Up @@ -813,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 @@ -821,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
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Rust MIR: a lowered representation of Rust.
#![feature(trusted_step)]
#![feature(try_blocks)]
#![feature(yeet_expr)]
#![feature(is_some_with)]
#![recursion_limit = "256"]
#![allow(rustc::potential_query_instability)]

Expand Down