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

Miri: Refactor to_scalar_ptr out of existence #66147

Merged
merged 4 commits into from
Nov 7, 2019
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
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

match (&src_pointee_ty.kind, &dest_pointee_ty.kind) {
(&ty::Array(_, length), &ty::Slice(_)) => {
let ptr = self.read_immediate(src)?.to_scalar_ptr()?;
let ptr = self.read_immediate(src)?.to_scalar()?;
// u64 cast is from usize to u64, which is always good
let val = Immediate::new_slice(
ptr,
Expand All @@ -279,7 +279,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
(_, &ty::Dynamic(ref data, _)) => {
// Initial cast from sized to dyn trait
let vtable = self.get_vtable(src_pointee_ty, data.principal())?;
let ptr = self.read_immediate(src)?.to_scalar_ptr()?;
let ptr = self.read_immediate(src)?.to_scalar()?;
let val = Immediate::new_dyn_trait(ptr, vtable);
self.write_immediate(val, dest)
}
Expand Down
22 changes: 10 additions & 12 deletions src/librustc_mir/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,20 +192,18 @@ for
let ty = mplace.layout.ty;
if let ty::Ref(_, referenced_ty, mutability) = ty.kind {
let value = self.ecx.read_immediate(mplace.into())?;
let mplace = self.ecx.ref_to_mplace(value)?;
// Handle trait object vtables
if let Ok(meta) = value.to_meta() {
if let ty::Dynamic(..) =
self.ecx.tcx.struct_tail_erasing_lifetimes(
referenced_ty, self.ecx.param_env).kind
{
if let Ok(vtable) = meta.unwrap().to_ptr() {
// explitly choose `Immutable` here, since vtables are immutable, even
// if the reference of the fat pointer is mutable
self.intern_shallow(vtable.alloc_id, Mutability::Immutable, None)?;
}
if let ty::Dynamic(..) =
self.ecx.tcx.struct_tail_erasing_lifetimes(
referenced_ty, self.ecx.param_env).kind
{
if let Ok(vtable) = mplace.meta.unwrap().to_ptr() {
// explitly choose `Immutable` here, since vtables are immutable, even
// if the reference of the fat pointer is mutable
self.intern_shallow(vtable.alloc_id, Mutability::Immutable, None)?;
}
}
let mplace = self.ecx.ref_to_mplace(value)?;
// Check if we have encountered this pointer+layout combination before.
// Only recurse for allocation-backed pointers.
if let Scalar::Ptr(ptr) = mplace.ptr {
Expand All @@ -230,7 +228,7 @@ for
ty::Array(_, n)
if n.eval_usize(self.ecx.tcx.tcx, self.ecx.param_env) == 0 => {}
ty::Slice(_)
if value.to_meta().unwrap().unwrap().to_usize(self.ecx)? == 0 => {}
if mplace.meta.unwrap().to_usize(self.ecx)? == 0 => {}
_ => bug!("const qualif failed to prevent mutable references"),
}
},
Expand Down
20 changes: 0 additions & 20 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,26 +82,6 @@ impl<'tcx, Tag> Immediate<Tag> {
Immediate::ScalarPair(a, b) => Ok((a.not_undef()?, b.not_undef()?))
}
}

/// Converts the immediate into a pointer (or a pointer-sized integer).
/// Throws away the second half of a ScalarPair!
#[inline]
pub fn to_scalar_ptr(self) -> InterpResult<'tcx, Scalar<Tag>> {
match self {
Immediate::Scalar(ptr) |
Immediate::ScalarPair(ptr, _) => ptr.not_undef(),
}
}

/// Converts the value into its metadata.
/// Throws away the first half of a ScalarPair!
#[inline]
pub fn to_meta(self) -> InterpResult<'tcx, Option<Scalar<Tag>>> {
Ok(match self {
Immediate::Scalar(_) => None,
Immediate::ScalarPair(_, meta) => Some(meta.not_undef()?),
})
}
}

// ScalarPair needs a type to interpret, so we often have an immediate and a type together
Expand Down
12 changes: 9 additions & 3 deletions src/librustc_mir/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,17 +287,23 @@ where
&self,
val: ImmTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
let pointee_type = val.layout.ty.builtin_deref(true).unwrap().ty;
let pointee_type = val.layout.ty.builtin_deref(true)
.expect("`ref_to_mplace` called on non-ptr type")
.ty;
let layout = self.layout_of(pointee_type)?;
let (ptr, meta) = match *val {
Immediate::Scalar(ptr) => (ptr.not_undef()?, None),
Immediate::ScalarPair(ptr, meta) => (ptr.not_undef()?, Some(meta.not_undef()?)),
};

let mplace = MemPlace {
ptr: val.to_scalar_ptr()?,
ptr,
// We could use the run-time alignment here. For now, we do not, because
// the point of tracking the alignment here is to make sure that the *static*
// alignment information emitted with the loads is correct. The run-time
// alignment can only be more restrictive.
align: layout.align.abi,
meta: val.to_meta()?,
meta,
};
Ok(MPlaceTy { mplace, layout })
}
Expand Down
44 changes: 15 additions & 29 deletions src/librustc_mir/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,44 +388,31 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
}
}
ty::RawPtr(..) => {
// Check pointer part.
if self.ref_tracking_for_consts.is_some() {
// Integers/floats in CTFE: For consistency with integers, we do not
// accept undef.
let _ptr = try_validation!(value.to_scalar_ptr(),
"undefined address in raw pointer", self.path);
} else {
// Remain consistent with `usize`: Accept anything.
}

// Check metadata.
let meta = try_validation!(value.to_meta(),
"uninitialized data in wide pointer metadata", self.path);
let layout = self.ecx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?;
if layout.is_unsized() {
self.check_wide_ptr_meta(meta, layout)?;
// We are conservative with undef for integers, but try to
// actually enforce our current rules for raw pointers.
let place = try_validation!(self.ecx.ref_to_mplace(value),
"undefined pointer", self.path);
if place.layout.is_unsized() {
self.check_wide_ptr_meta(place.meta, place.layout)?;
}
}
_ if ty.is_box() || ty.is_region_ptr() => {
// Handle wide pointers.
// Check metadata early, for better diagnostics
let ptr = try_validation!(value.to_scalar_ptr(),
"undefined address in pointer", self.path);
let meta = try_validation!(value.to_meta(),
"uninitialized data in wide pointer metadata", self.path);
let layout = self.ecx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?;
if layout.is_unsized() {
self.check_wide_ptr_meta(meta, layout)?;
let place = try_validation!(self.ecx.ref_to_mplace(value),
"undefined pointer", self.path);
if place.layout.is_unsized() {
self.check_wide_ptr_meta(place.meta, place.layout)?;
}
// Make sure this is dereferencable and all.
let (size, align) = self.ecx.size_and_align_of(meta, layout)?
let (size, align) = self.ecx.size_and_align_of(place.meta, place.layout)?
// for the purpose of validity, consider foreign types to have
// alignment and size determined by the layout (size will be 0,
// alignment should take attributes into account).
.unwrap_or_else(|| (layout.size, layout.align.abi));
.unwrap_or_else(|| (place.layout.size, place.layout.align.abi));
let ptr: Option<_> = match
self.ecx.memory.check_ptr_access_align(
ptr,
place.ptr,
size,
Some(align),
CheckInAllocMsg::InboundsTest,
Expand All @@ -435,7 +422,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
Err(err) => {
info!(
"{:?} did not pass access check for size {:?}, align {:?}",
ptr, size, align
place.ptr, size, align
);
match err.kind {
err_unsup!(InvalidNullPointerUsage) =>
Expand All @@ -459,7 +446,6 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
};
// Recursive checking
if let Some(ref mut ref_tracking) = self.ref_tracking_for_consts {
let place = self.ecx.ref_to_mplace(value)?;
if let Some(ptr) = ptr { // not a ZST
// Skip validation entirely for some external statics
let alloc_kind = self.ecx.tcx.alloc_map.lock().get(ptr.alloc_id);
Expand Down Expand Up @@ -627,7 +613,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
// reject it. However, that's good: We don't inherently want
// to reject those pointers, we just do not have the machinery to
// talk about parts of a pointer.
// We also accept undef, for consistency with the type-based checks.
// We also accept undef, for consistency with the slow path.
match self.ecx.memory.get(ptr.alloc_id)?.check_bytes(
self.ecx,
ptr,
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/consts/const-eval/ub-wide-ptr.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-wide-ptr.rs:107:1
|
LL | const SLICE_LENGTH_UNINIT: &[u8] = unsafe { SliceTransmute { addr: 42 }.slice};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized data in wide pointer metadata
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered undefined pointer
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.

Expand Down Expand Up @@ -90,7 +90,7 @@ error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-wide-ptr.rs:133:1
|
LL | const RAW_SLICE_LENGTH_UNINIT: *const [u8] = unsafe { SliceTransmute { addr: 42 }.raw_slice};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized data in wide pointer metadata
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered undefined pointer
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.

Expand Down