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

ensure that integers cast to pointers will never point at a valid alloc, not even the zst alloc #81

Merged
merged 19 commits into from
Nov 17, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
5 changes: 1 addition & 4 deletions src/interpreter/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {

Bool | Char | U8 | U16 | U32 | U64 => self.cast_int(val.bits, ty, false),

FnPtr | Ptr => {
let ptr = val.expect_ptr("FnPtr- or Ptr-tagged PrimVal had no relocation");
self.cast_ptr(ptr, ty)
}
FnPtr | Ptr => self.cast_ptr(val.to_ptr(), ty),
}
}

Expand Down
40 changes: 17 additions & 23 deletions src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,33 +963,27 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
}

Deref => {
use interpreter::value::Value::*;
let val = self.eval_and_read_lvalue(&proj.base)?;

let val = match self.eval_and_read_lvalue(&proj.base)? {
ByRef(ptr) => self.read_value(ptr, base_ty)?,
v => v,
let pointee_type = match base_ty.sty {
ty::TyRawPtr(ty::TypeAndMut{ty, ..}) |
ty::TyRef(_, ty::TypeAndMut{ty, ..}) |
ty::TyBox(ty) => ty,
_ => bug!("can only deref pointer types"),
};

match val {
ByValPair(ptr, vtable)
if ptr.try_as_ptr().is_some() && vtable.try_as_ptr().is_some()
=> {
let ptr = ptr.try_as_ptr().unwrap();
let vtable = vtable.try_as_ptr().unwrap();
(ptr, LvalueExtra::Vtable(vtable))
}

ByValPair(ptr, n) if ptr.try_as_ptr().is_some() => {
let ptr = ptr.try_as_ptr().unwrap();
(ptr, LvalueExtra::Length(n.expect_uint("slice length")))
}
trace!("deref to {} on {:?}", pointee_type, val);

ByVal(ptr) if ptr.try_as_ptr().is_some() => {
let ptr = ptr.try_as_ptr().unwrap();
(ptr, LvalueExtra::None)
}

_ => bug!("can't deref non pointer types"),
match self.tcx.struct_tail(pointee_type).sty {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so much clearer. :D

ty::TyTrait(_) => {
let (ptr, vtable) = val.expect_ptr_vtable_pair(&self.memory)?;
(ptr, LvalueExtra::Vtable(vtable))
},
ty::TyStr | ty::TySlice(_) => {
let (ptr, len) = val.expect_slice(&self.memory)?;
(ptr, LvalueExtra::Length(len))
},
_ => (val.read_ptr(&self.memory)?, LvalueExtra::None),
}
}

Expand Down
14 changes: 8 additions & 6 deletions src/interpreter/terminator/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,17 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {

"drop_in_place" => {
let ty = substs.type_at(0);
trace!("drop in place on {}", ty);
let ptr_ty = self.tcx.mk_mut_ptr(ty);
let lvalue = match self.follow_by_ref_value(arg_vals[0], ptr_ty)? {
Value::ByRef(_) => bug!("follow_by_ref_value returned ByRef"),
Value::ByVal(ptr) => Lvalue::from_ptr(ptr.expect_ptr("drop_in_place first arg not a pointer")),
Value::ByVal(value) => Lvalue::from_ptr(value.to_ptr()),
Value::ByValPair(ptr, extra) => Lvalue::Ptr {
ptr: ptr.expect_ptr("drop_in_place first arg not a pointer"),
extra: match extra.try_as_ptr() {
Some(vtable) => LvalueExtra::Vtable(vtable),
None => LvalueExtra::Length(extra.expect_uint("either pointer or not, but not neither")),
ptr: ptr.to_ptr(),
extra: match self.tcx.struct_tail(ty).sty {
ty::TyTrait(_) => LvalueExtra::Vtable(extra.to_ptr()),
ty::TyStr | ty::TySlice(_) => LvalueExtra::Length(extra.try_as_uint()?),
_ => bug!("invalid fat pointer type: {}", ptr_ty),
},
},
};
Expand Down Expand Up @@ -440,7 +442,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
ty::TySlice(_) | ty::TyStr => {
let elem_ty = ty.sequence_element_type(self.tcx);
let elem_size = self.type_size(elem_ty).expect("slice element must be sized") as u64;
let len = value.expect_slice_len(&self.memory)?;
let (_, len) = value.expect_slice(&self.memory)?;
let align = self.type_align(elem_ty);
Ok((len * elem_size, align as u64))
}
Expand Down
14 changes: 7 additions & 7 deletions src/interpreter/terminator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
let func_ty = self.operand_ty(func);
match func_ty.sty {
ty::TyFnPtr(bare_fn_ty) => {
let fn_ptr = self.eval_operand_to_primval(func)?
.expect_fn_ptr("TyFnPtr callee did not evaluate to FnPtr");
let fn_ptr = self.eval_operand_to_primval(func)?.to_ptr();
let (def_id, substs, fn_ty) = self.memory.get_fn(fn_ptr.alloc_id)?;
if fn_ty != bare_fn_ty {
return Err(EvalError::FunctionPointerTyMismatch(fn_ty, bare_fn_ty));
Expand Down Expand Up @@ -542,14 +541,15 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
Value::ByRef(_) => bug!("follow_by_ref_value can't result in ByRef"),
Value::ByVal(ptr) => {
assert!(self.type_is_sized(contents_ty));
let contents_ptr = ptr.expect_ptr("value of Box type must be a pointer");
let contents_ptr = ptr.to_ptr();
self.drop(Lvalue::from_ptr(contents_ptr), contents_ty, drop)?;
},
Value::ByValPair(prim_ptr, extra) => {
let ptr = prim_ptr.expect_ptr("value of Box type must be a pointer");
let extra = match extra.try_as_ptr() {
Some(vtable) => LvalueExtra::Vtable(vtable),
None => LvalueExtra::Length(extra.expect_uint("slice length")),
let ptr = prim_ptr.to_ptr();
let extra = match self.tcx.struct_tail(contents_ty).sty {
ty::TyTrait(_) => LvalueExtra::Vtable(extra.to_ptr()),
ty::TyStr | ty::TySlice(_) => LvalueExtra::Length(extra.try_as_uint()?),
_ => bug!("invalid fat pointer type: {}", ty),
};
self.drop(
Lvalue::Ptr {
Expand Down
31 changes: 14 additions & 17 deletions src/interpreter/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ impl<'a, 'tcx: 'a> Value {
use self::Value::*;
match *self {
ByRef(ptr) => mem.read_ptr(ptr),

ByVal(ptr) | ByValPair(ptr, _) => {
Ok(ptr.try_as_ptr().expect("unimplemented: `read_ptr` on non-ptr primval"))
}
ByVal(ptr) | ByValPair(ptr, _) => Ok(ptr.to_ptr()),
}
}

Expand All @@ -35,29 +32,29 @@ impl<'a, 'tcx: 'a> Value {
) -> EvalResult<'tcx, (Pointer, Pointer)> {
use self::Value::*;
match *self {
ByRef(ptr) => {
let ptr = mem.read_ptr(ptr)?;
let vtable = mem.read_ptr(ptr.offset(mem.pointer_size() as isize))?;
ByRef(ref_ptr) => {
let ptr = mem.read_ptr(ref_ptr)?;
let vtable = mem.read_ptr(ref_ptr.offset(mem.pointer_size() as isize))?;
Ok((ptr, vtable))
}

ByValPair(ptr, vtable)
if ptr.try_as_ptr().is_some() && vtable.try_as_ptr().is_some()
=> {
let ptr = ptr.try_as_ptr().unwrap();
let vtable = vtable.try_as_ptr().unwrap();
Ok((ptr, vtable))
}
ByValPair(ptr, vtable) => Ok((ptr.to_ptr(), vtable.to_ptr())),

_ => bug!("expected ptr and vtable, got {:?}", self),
}
}

pub(super) fn expect_slice_len(&self, mem: &Memory<'a, 'tcx>) -> EvalResult<'tcx, u64> {
pub(super) fn expect_slice(&self, mem: &Memory<'a, 'tcx>) -> EvalResult<'tcx, (Pointer, u64)> {
use self::Value::*;
match *self {
ByRef(ptr) => mem.read_usize(ptr.offset(mem.pointer_size() as isize)),
ByValPair(_, val) if val.kind.is_int() => Ok(val.bits),
ByRef(ref_ptr) => {
let ptr = mem.read_ptr(ref_ptr)?;
let len = mem.read_usize(ref_ptr.offset(mem.pointer_size() as isize))?;
Ok((ptr, len))
},
ByValPair(ptr, val) => {
Ok((ptr.to_ptr(), val.try_as_uint()?))
},
_ => unimplemented!(),
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,8 +533,8 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
}

pub fn write_primval(&mut self, dest: Pointer, val: PrimVal) -> EvalResult<'tcx, ()> {
if let Some(ptr) = val.try_as_ptr() {
return self.write_ptr(dest, ptr);
if let Some(alloc_id) = val.relocation {
return self.write_ptr(dest, Pointer::new(alloc_id, val.bits as usize));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not your mistake, just making a note for posterity: this as usize is wrong when emulating a 64-bit target on a 32-bit host. I have a TODO on Pointer for changing the usize to u64.

}

use primval::PrimValKind::*;
Expand Down
50 changes: 21 additions & 29 deletions src/primval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,19 @@ impl PrimVal {
bits_to_f64(self.bits)
}

pub fn try_as_ptr(self) -> Option<Pointer> {
pub fn to_ptr(self) -> Pointer {
self.relocation.map(|alloc_id| {
Pointer::new(alloc_id, self.bits as usize)
})
}).unwrap_or_else(|| Pointer::from_int(self.bits as usize))
}

pub fn try_as_uint<'tcx>(self) -> EvalResult<'tcx, u64> {
self.to_ptr().to_int().map(|val| val as u64)
}

pub fn expect_uint(self, error_msg: &str) -> u64 {
if let Some(ptr) = self.try_as_ptr() {
return ptr.to_int().expect("non abstract ptr") as u64
if let Ok(int) = self.try_as_uint() {
return int;
}

use self::PrimValKind::*;
Expand All @@ -156,8 +160,8 @@ impl PrimVal {
}

pub fn expect_int(self, error_msg: &str) -> i64 {
if let Some(ptr) = self.try_as_ptr() {
return ptr.to_int().expect("non abstract ptr") as i64
if let Ok(int) = self.try_as_uint() {
return int as i64;
}

use self::PrimValKind::*;
Expand Down Expand Up @@ -188,15 +192,6 @@ impl PrimVal {
_ => bug!("{}", error_msg),
}
}

pub fn expect_ptr(self, error_msg: &str) -> Pointer {
self.try_as_ptr().expect(error_msg)
}

/// FIXME(solson): Refactored into a duplicate of `expect_ptr`. Investigate removal.
pub fn expect_fn_ptr(self, error_msg: &str) -> Pointer {
self.try_as_ptr().expect(error_msg)
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -277,19 +272,13 @@ pub fn binary_op<'tcx>(
use rustc::mir::BinOp::*;
use self::PrimValKind::*;

match (left.try_as_ptr(), right.try_as_ptr()) {
(Some(left_ptr), Some(right_ptr)) => {
if left_ptr.alloc_id != right_ptr.alloc_id {
return Ok((unrelated_ptr_ops(bin_op)?, false));
}

// If the pointers are into the same allocation, fall through to the more general match
// later, which will do comparisons on the `bits` fields, which are the pointer offsets
// in this case.
}

(None, None) => {}
_ => return Err(EvalError::ReadPointerAsBytes),
// If the pointers are into the same allocation, fall through to the more general match
// later, which will do comparisons on the `bits` fields, which are the pointer offsets
// in this case.
let left_ptr = left.to_ptr();
let right_ptr = right.to_ptr();
if left_ptr.alloc_id != right_ptr.alloc_id {
return Ok((unrelated_ptr_ops(bin_op, left_ptr, right_ptr)?, false));
}

let (l, r) = (left.bits, right.bits);
Expand Down Expand Up @@ -376,12 +365,15 @@ pub fn binary_op<'tcx>(
Ok((val, false))
}

fn unrelated_ptr_ops<'tcx>(bin_op: mir::BinOp) -> EvalResult<'tcx, PrimVal> {
fn unrelated_ptr_ops<'tcx>(bin_op: mir::BinOp, left: Pointer, right: Pointer) -> EvalResult<'tcx, PrimVal> {
use rustc::mir::BinOp::*;
match bin_op {
Eq => Ok(PrimVal::from_bool(false)),
Ne => Ok(PrimVal::from_bool(true)),
Lt | Le | Gt | Ge => Err(EvalError::InvalidPointerMath),
_ if left.to_int().is_ok() ^ right.to_int().is_ok() => {
Err(EvalError::ReadPointerAsBytes)
},
_ => bug!(),
}
}
Expand Down
7 changes: 7 additions & 0 deletions tests/compile-fail/cast_int_to_fn_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
let g = unsafe {
std::mem::transmute::<usize, fn(i32)>(42)
};

g(42) //~ ERROR tried to use an integer pointer or a dangling pointer as a function pointer
}