Skip to content

Commit

Permalink
Auto merge of #95826 - carbotaniuman:miri-permissive-provenance, r=Ra…
Browse files Browse the repository at this point in the history
…lfJung

Initial work on Miri permissive-exposed-provenance

Rustc portion of the changes for portions of a permissive ptr-to-int model for Miri. The main changes here are changing `ptr_get_alloc` and `get_alloc_id` to return an Option, and also making ptr-to-int casts have an expose side effect.
  • Loading branch information
bors committed May 14, 2022
2 parents 17180f4 + bd5fce6 commit 8019fa0
Show file tree
Hide file tree
Showing 12 changed files with 142 additions and 45 deletions.
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>>;

// 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()`.
///
/// When this fails, that means the pointer does not point to a live allocation.
fn ptr_get_alloc(
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))
}

#[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 @@ -1142,7 +1144,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 @@ -1182,10 +1184,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 @@ -741,17 +741,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
let _v = x == x;
};

Expand Down
Loading

0 comments on commit 8019fa0

Please sign in to comment.