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

Initial work on Miri permissive-exposed-provenance #95826

Merged
merged 1 commit into from
May 14, 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
10 changes: 9 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_target::spec::abi::Abi;

use crate::interpret::{
self, compile_time_machine, AllocId, ConstAllocation, Frame, ImmTy, InterpCx, InterpResult,
OpTy, PlaceTy, Scalar, StackPopUnwind,
OpTy, PlaceTy, Pointer, Scalar, StackPopUnwind,
};

use super::error::*;
Expand Down Expand Up @@ -443,6 +443,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
Ok(())
}

#[inline(always)]
fn expose_ptr(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
_ptr: Pointer<AllocId>,
) -> InterpResult<'tcx> {
Err(ConstEvalErrKind::NeedsRfc("exposing pointers".to_string()).into())
}

#[inline(always)]
fn init_frame_extra(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
Expand Down
59 changes: 44 additions & 15 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

pub fn misc_cast(
&self,
&mut self,
src: &ImmTy<'tcx, M::PointerTag>,
cast_ty: Ty<'tcx>,
) -> InterpResult<'tcx, Immediate<M::PointerTag>> {
Expand Down Expand Up @@ -139,7 +139,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if let Some(discr) = src.layout.ty.discriminant_for_variant(*self.tcx, index) {
assert!(src.layout.is_zst());
let discr_layout = self.layout_of(discr.ty)?;
return Ok(self.cast_from_int_like(discr.val, discr_layout, cast_ty).into());

let scalar = Scalar::from_uint(discr.val, discr_layout.layout.size());
return Ok(self.cast_from_int_like(scalar, discr_layout, cast_ty)?.into());
}
}
Variants::Multiple { .. } => {}
Expand Down Expand Up @@ -170,38 +172,65 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

// # The remaining source values are scalar and "int-like".
let scalar = src.to_scalar()?;

// If we are casting from a pointer to something
// that is not a pointer, mark the pointer as exposed
if src.layout.ty.is_any_ptr() && !cast_ty.is_any_ptr() {
let ptr = self.scalar_to_ptr(scalar)?;

match ptr.into_pointer_or_addr() {
Ok(ptr) => {
M::expose_ptr(self, ptr)?;
}
Err(_) => {
// do nothing, exposing an invalid pointer
// has no meaning
}
};
}

// For all remaining casts, we either
// (a) cast a raw ptr to usize, or
// (b) cast from an integer-like (including bool, char, enums).
// In both cases we want the bits.
let bits = src.to_scalar()?.to_bits(src.layout.size)?;
Ok(self.cast_from_int_like(bits, src.layout, cast_ty).into())
Ok(self.cast_from_int_like(scalar, src.layout, cast_ty)?.into())
}

fn cast_from_int_like(
pub fn cast_from_int_like(
&self,
v: u128, // raw bits (there is no ScalarTy so we separate data+layout)
scalar: Scalar<M::PointerTag>, // input value (there is no ScalarTy so we separate data+layout)
src_layout: TyAndLayout<'tcx>,
cast_ty: Ty<'tcx>,
) -> Scalar<M::PointerTag> {
) -> InterpResult<'tcx, Scalar<M::PointerTag>> {
// Let's make sure v is sign-extended *if* it has a signed type.
let signed = src_layout.abi.is_signed(); // Also asserts that abi is `Scalar`.

let v = scalar.to_bits(src_layout.size)?;
let v = if signed { self.sign_extend(v, src_layout) } else { v };
trace!("cast_from_scalar: {}, {} -> {}", v, src_layout.ty, cast_ty);
use rustc_middle::ty::TyKind::*;
match *cast_ty.kind() {
Int(_) | Uint(_) | RawPtr(_) => {

Ok(match *cast_ty.kind() {
Int(_) | Uint(_) => {
let size = match *cast_ty.kind() {
Int(t) => Integer::from_int_ty(self, t).size(),
Uint(t) => Integer::from_uint_ty(self, t).size(),
RawPtr(_) => self.pointer_size(),
_ => bug!(),
};
let v = size.truncate(v);
Scalar::from_uint(v, size)
}

RawPtr(_) => {
assert!(src_layout.ty.is_integral());

let size = self.pointer_size();
let addr = u64::try_from(size.truncate(v)).unwrap();

let ptr = M::ptr_from_addr_cast(&self, addr);
if addr == 0 {
assert!(ptr.provenance.is_none(), "null pointer can never have an AllocId");
}
Scalar::from_maybe_pointer(ptr, self)
}

Float(FloatTy::F32) if signed => Scalar::from_f32(Single::from_i128(v as i128).value),
Float(FloatTy::F64) if signed => Scalar::from_f64(Double::from_i128(v as i128).value),
Float(FloatTy::F32) => Scalar::from_f32(Single::from_u128(v).value),
Expand All @@ -214,7 +243,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

// Casts to bool are not permitted by rustc, no need to handle them here.
_ => span_bug!(self.cur_span(), "invalid int to {:?} cast", cast_ty),
}
})
}

fn cast_from_float<F>(&self, f: F, dest_ty: Ty<'tcx>) -> Scalar<M::PointerTag>
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
trace!(
"deallocating local {:?}: {:?}",
local,
self.dump_alloc(ptr.provenance.unwrap().get_alloc_id())
// Locals always have a `alloc_id` (they are never the result of a int2ptr).
self.dump_alloc(ptr.provenance.unwrap().get_alloc_id().unwrap())
);
self.deallocate_ptr(ptr, None, MemoryKind::Stack)?;
};
Expand Down Expand Up @@ -1013,9 +1014,13 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
}
}

write!(fmt, ": {:?}", self.ecx.dump_allocs(allocs))
write!(
fmt,
": {:?}",
self.ecx.dump_allocs(allocs.into_iter().filter_map(|x| x).collect())
)
}
Place::Ptr(mplace) => match mplace.ptr.provenance.map(Provenance::get_alloc_id) {
Place::Ptr(mplace) => match mplace.ptr.provenance.and_then(Provenance::get_alloc_id) {
Some(alloc_id) => write!(
fmt,
"by align({}) ref {:?}: {:?}",
Expand Down
38 changes: 33 additions & 5 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,19 +286,36 @@ pub trait Machine<'mir, 'tcx>: Sized {
) -> Pointer<Self::PointerTag>;

/// "Int-to-pointer cast"
fn ptr_from_addr(
fn ptr_from_addr_cast(
ecx: &InterpCx<'mir, 'tcx, Self>,
addr: u64,
) -> Pointer<Option<Self::PointerTag>>;
carbotaniuman marked this conversation as resolved.
Show resolved Hide resolved

// FIXME: Transmuting an integer to a pointer should just always return a `None`
// provenance, but that causes problems with function pointers in Miri.
/// Hook for returning a pointer from a transmute-like operation on an addr.
fn ptr_from_addr_transmute(
ecx: &InterpCx<'mir, 'tcx, Self>,
addr: u64,
) -> Pointer<Option<Self::PointerTag>>;

/// Marks a pointer as exposed, allowing it's provenance
/// to be recovered. "Pointer-to-int cast"
fn expose_ptr(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
ptr: Pointer<Self::PointerTag>,
) -> InterpResult<'tcx>;

/// Convert a pointer with provenance into an allocation-offset pair
/// and extra provenance info.
///
/// The returned `AllocId` must be the same as `ptr.provenance.get_alloc_id()`.
carbotaniuman marked this conversation as resolved.
Show resolved Hide resolved
///
/// When this fails, that means the pointer does not point to a live allocation.
fn ptr_get_alloc(
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
ecx: &InterpCx<'mir, 'tcx, Self>,
ptr: Pointer<Self::PointerTag>,
) -> (AllocId, Size, Self::TagExtra);
) -> Option<(AllocId, Size, Self::TagExtra)>;

/// Called to initialize the "extra" state of an allocation and make the pointers
/// it contains (in relocations) tagged. The way we construct allocations is
Expand Down Expand Up @@ -480,17 +497,28 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
}

#[inline(always)]
fn ptr_from_addr(_ecx: &InterpCx<$mir, $tcx, Self>, addr: u64) -> Pointer<Option<AllocId>> {
fn ptr_from_addr_transmute(
_ecx: &InterpCx<$mir, $tcx, Self>,
addr: u64,
) -> Pointer<Option<AllocId>> {
Pointer::new(None, Size::from_bytes(addr))
}

#[inline(always)]
fn ptr_from_addr_cast(
_ecx: &InterpCx<$mir, $tcx, Self>,
addr: u64,
) -> Pointer<Option<AllocId>> {
Pointer::new(None, Size::from_bytes(addr))
carbotaniuman marked this conversation as resolved.
Show resolved Hide resolved
}

#[inline(always)]
fn ptr_get_alloc(
_ecx: &InterpCx<$mir, $tcx, Self>,
ptr: Pointer<AllocId>,
) -> (AllocId, Size, Self::TagExtra) {
) -> Option<(AllocId, Size, Self::TagExtra)> {
// We know `offset` is relative to the allocation, so we can use `into_parts`.
let (alloc_id, offset) = ptr.into_parts();
(alloc_id, offset, ())
Some((alloc_id, offset, ()))
}
}
20 changes: 13 additions & 7 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if reachable.insert(id) {
// This is a new allocation, add its relocations to `todo`.
if let Some((_, alloc)) = self.memory.alloc_map.get(id) {
todo.extend(alloc.relocations().values().map(|tag| tag.get_alloc_id()));
todo.extend(
alloc.relocations().values().filter_map(|tag| tag.get_alloc_id()),
);
}
}
}
Expand Down Expand Up @@ -805,7 +807,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> std::fmt::Debug for DumpAllocs<'a,
allocs_to_print: &mut VecDeque<AllocId>,
alloc: &Allocation<Tag, Extra>,
) -> std::fmt::Result {
for alloc_id in alloc.relocations().values().map(|tag| tag.get_alloc_id()) {
for alloc_id in alloc.relocations().values().filter_map(|tag| tag.get_alloc_id()) {
allocs_to_print.push_back(alloc_id);
}
write!(fmt, "{}", display_allocation(tcx, alloc))
Expand Down Expand Up @@ -1125,7 +1127,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Err(ptr) => ptr.into(),
Ok(bits) => {
let addr = u64::try_from(bits).unwrap();
let ptr = M::ptr_from_addr(&self, addr);
let ptr = M::ptr_from_addr_transmute(&self, addr);
if addr == 0 {
assert!(ptr.provenance.is_none(), "null pointer can never have an AllocId");
}
Expand Down Expand Up @@ -1165,10 +1167,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ptr: Pointer<Option<M::PointerTag>>,
) -> Result<(AllocId, Size, M::TagExtra), u64> {
match ptr.into_pointer_or_addr() {
Ok(ptr) => {
let (alloc_id, offset, extra) = M::ptr_get_alloc(self, ptr);
Ok((alloc_id, offset, extra))
}
Ok(ptr) => match M::ptr_get_alloc(self, ptr) {
Some((alloc_id, offset, extra)) => Ok((alloc_id, offset, extra)),
None => {
assert!(M::PointerTag::OFFSET_IS_ADDR);
let (_, addr) = ptr.into_parts();
Err(addr.bytes())
}
},
Err(addr) => Err(addr.bytes()),
}
}
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -695,17 +695,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Figure out which discriminant and variant this corresponds to.
Ok(match *tag_encoding {
TagEncoding::Direct => {
let scalar = tag_val.to_scalar()?;
// Generate a specific error if `tag_val` is not an integer.
// (`tag_bits` itself is only used for error messages below.)
let tag_bits = tag_val
.to_scalar()?
let tag_bits = scalar
.try_to_int()
.map_err(|dbg_val| err_ub!(InvalidTag(dbg_val)))?
.assert_bits(tag_layout.size);
// Cast bits from tag layout to discriminant layout.
// After the checks we did above, this cannot fail.
// After the checks we did above, this cannot fail, as
// discriminants are int-like.
let discr_val =
self.misc_cast(&tag_val, discr_layout.ty).unwrap().to_scalar().unwrap();
self.cast_from_int_like(scalar, tag_val.layout, discr_layout.ty).unwrap();
let discr_bits = discr_val.assert_bits(discr_layout.size);
// Convert discriminant to variant index, and catch invalid discriminants.
let index = match *op.layout.ty.kind() {
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_middle/src/mir/interpret/pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,11 @@ pub trait Provenance: Copy + fmt::Debug {
where
Self: Sized;

/// Provenance must always be able to identify the allocation this ptr points to.
/// If `OFFSET_IS_ADDR == false`, provenance must always be able to
/// identify the allocation this ptr points to (i.e., this must return `Some`).
/// Otherwise this function is best-effort (but must agree with `Machine::ptr_get_alloc`).
/// (Identifying the offset in that allocation, however, is harder -- use `Memory::ptr_get_alloc` for that.)
fn get_alloc_id(self) -> AllocId;
fn get_alloc_id(self) -> Option<AllocId>;
}

impl Provenance for AllocId {
Expand All @@ -147,8 +149,8 @@ impl Provenance for AllocId {
Ok(())
}

fn get_alloc_id(self) -> AllocId {
self
fn get_alloc_id(self) -> Option<AllocId> {
Some(self)
}
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ impl<'tcx, Tag: Provenance> Scalar<Tag> {
} else {
// We know `offset` is relative, since `OFFSET_IS_ADDR == false`.
let (tag, offset) = ptr.into_parts();
Err(Scalar::Ptr(Pointer::new(tag.get_alloc_id(), offset), sz))
// Because `OFFSET_IS_ADDR == false`, this unwrap can never fail.
Err(Scalar::Ptr(Pointer::new(tag.get_alloc_id().unwrap(), offset), sz))
}
}
}
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ use crate::MirPass;
use rustc_const_eval::interpret::{
self, compile_time_machine, AllocId, ConstAllocation, ConstValue, CtfeValidationMode, Frame,
ImmTy, Immediate, InterpCx, InterpResult, LocalState, LocalValue, MemPlace, MemoryKind, OpTy,
Operand as InterpOperand, PlaceTy, Scalar, ScalarMaybeUninit, StackPopCleanup, StackPopUnwind,
Operand as InterpOperand, PlaceTy, Pointer, Scalar, ScalarMaybeUninit, StackPopCleanup,
StackPopUnwind,
};

/// The maximum number of bytes that we'll allocate space for a local or the return value.
Expand Down Expand Up @@ -285,6 +286,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
Ok(())
}

#[inline(always)]
fn expose_ptr(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
_ptr: Pointer<AllocId>,
) -> InterpResult<'tcx> {
throw_machine_stop_str!("exposing pointers isn't supported in ConstProp")
}

#[inline(always)]
fn init_frame_extra(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_mir_transform/src/const_prop_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use crate::MirLint;
use rustc_const_eval::const_eval::ConstEvalErr;
use rustc_const_eval::interpret::{
self, compile_time_machine, AllocId, ConstAllocation, Frame, ImmTy, InterpCx, InterpResult,
LocalState, LocalValue, MemPlace, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Scalar,
ScalarMaybeUninit, StackPopCleanup, StackPopUnwind,
LocalState, LocalValue, MemPlace, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer,
Scalar, ScalarMaybeUninit, StackPopCleanup, StackPopUnwind,
};

/// The maximum number of bytes that we'll allocate space for a local or the return value.
Expand Down Expand Up @@ -280,6 +280,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
Ok(())
}

#[inline(always)]
fn expose_ptr(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
_ptr: Pointer<AllocId>,
) -> InterpResult<'tcx> {
throw_machine_stop_str!("exposing pointers isn't supported in ConstProp")
}

#[inline(always)]
fn init_frame_extra(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/miri_unleashed/ptr_arith.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ static CMP: () = {
static PTR_INT_CAST: () = {
let x = &0 as *const _ as usize;
//~^ ERROR could not evaluate static initializer
//~| unable to turn pointer into raw bytes
//~| "exposing pointers" needs an rfc before being allowed inside constants
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
let _v = x == x;
};

Expand Down
Loading