From 904923fa7a4635636b0bc5c33e9901773e7f6747 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Aug 2018 20:42:26 +0200 Subject: [PATCH 1/6] move some more helpers to rustc --- src/fn_call.rs | 8 +++--- src/helpers.rs | 45 -------------------------------- src/intrinsic.rs | 58 ++++++++++++++++-------------------------- src/lib.rs | 2 +- src/tls.rs | 8 +++--- tests/run-pass/char.rs | 4 +-- 6 files changed, 31 insertions(+), 94 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index a9791e2b7c..0e768fcccf 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -305,7 +305,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, ' }; self.write_scalar( - Scalar::from_i32(result), + Scalar::from_int(result, Size::from_bits(32)), dest, )?; } @@ -346,7 +346,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, ' let name = self.memory.read_c_str(name_ptr)?; match self.machine.env_vars.get(name) { Some(&var) => Scalar::Ptr(var), - None => Scalar::null(self.memory.pointer_size()), + None => Scalar::ptr_null(*self.tcx), } }; self.write_scalar(result, dest)?; @@ -446,7 +446,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, ' // Some things needed for sys::thread initialization to go through "signal" | "sigaction" | "sigaltstack" => { - self.write_scalar(Scalar::null(dest.layout.size), dest)?; + self.write_scalar(Scalar::from_int(0, dest.layout.size), dest)?; } "sysconf" => { @@ -729,6 +729,6 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, ' } fn write_null(&mut self, dest: PlaceTy<'tcx>) -> EvalResult<'tcx> { - self.write_scalar(Scalar::null(dest.layout.size), dest) + self.write_scalar(Scalar::from_int(0, dest.layout.size), dest) } } diff --git a/src/helpers.rs b/src/helpers.rs index 24285fa4a6..27b2109d18 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -1,17 +1,5 @@ -use rustc::ty::layout::Size; - use super::{Scalar, ScalarMaybeUndef, EvalResult}; -pub trait ScalarExt { - fn null(size: Size) -> Self; - fn from_i32(i: i32) -> Self; - fn from_uint(i: impl Into, ptr_size: Size) -> Self; - fn from_int(i: impl Into, ptr_size: Size) -> Self; - fn from_f32(f: f32) -> Self; - fn from_f64(f: f64) -> Self; - fn is_null(self) -> bool; -} - pub trait FalibleScalarExt { /// HACK: this function just extracts all bits if `defined != 0` /// Mainly used for args of C-functions and we should totally correctly fetch the size @@ -19,39 +7,6 @@ pub trait FalibleScalarExt { fn to_bytes(self) -> EvalResult<'static, u128>; } -impl ScalarExt for Scalar { - fn null(size: Size) -> Self { - Scalar::Bits { bits: 0, size: size.bytes() as u8 } - } - - fn from_i32(i: i32) -> Self { - Scalar::Bits { bits: i as u32 as u128, size: 4 } - } - - fn from_uint(i: impl Into, size: Size) -> Self { - Scalar::Bits { bits: i.into(), size: size.bytes() as u8 } - } - - fn from_int(i: impl Into, size: Size) -> Self { - Scalar::Bits { bits: i.into() as u128, size: size.bytes() as u8 } - } - - fn from_f32(f: f32) -> Self { - Scalar::Bits { bits: f.to_bits() as u128, size: 4 } - } - - fn from_f64(f: f64) -> Self { - Scalar::Bits { bits: f.to_bits() as u128, size: 8 } - } - - fn is_null(self) -> bool { - match self { - Scalar::Bits { bits, .. } => bits == 0, - Scalar::Ptr(_) => false - } - } -} - impl FalibleScalarExt for Scalar { fn to_bytes(self) -> EvalResult<'static, u128> { match self { diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 6847d8e546..695943d57b 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -2,12 +2,12 @@ use rustc::mir; use rustc::ty::layout::{self, LayoutOf, Size}; use rustc::ty; -use rustc::mir::interpret::{EvalResult, Scalar, ScalarMaybeUndef}; +use rustc::mir::interpret::{EvalResult, Scalar, ScalarMaybeUndef, PointerArithmetic}; use rustc_mir::interpret::{ PlaceTy, EvalContext, OpTy, Value }; -use super::{ScalarExt, FalibleScalarExt, OperatorEvalContextExt}; +use super::{FalibleScalarExt, OperatorEvalContextExt}; pub trait EvalContextExt<'tcx> { fn call_intrinsic( @@ -204,8 +204,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: "sinf32" | "fabsf32" | "cosf32" | "sqrtf32" | "expf32" | "exp2f32" | "logf32" | "log10f32" | "log2f32" | "floorf32" | "ceilf32" | "truncf32" => { - let f = self.read_scalar(args[0])?.to_bytes()?; - let f = f32::from_bits(f as u32); + let f = self.read_scalar(args[0])?.to_f32()?; let f = match intrinsic_name { "sinf32" => f.sin(), "fabsf32" => f.abs(), @@ -226,8 +225,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: "sinf64" | "fabsf64" | "cosf64" | "sqrtf64" | "expf64" | "exp2f64" | "logf64" | "log10f64" | "log2f64" | "floorf64" | "ceilf64" | "truncf64" => { - let f = self.read_scalar(args[0])?.to_bytes()?; - let f = f64::from_bits(f as u64); + let f = self.read_scalar(args[0])?.to_f64()?; let f = match intrinsic_name { "sinf64" => f.sin(), "fabsf64" => f.abs(), @@ -282,12 +280,12 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: if !dest.layout.is_zst() { // notzhing to do for ZST match dest.layout.abi { layout::Abi::Scalar(ref s) => { - let x = Scalar::null(s.value.size(&self)); + let x = Scalar::from_int(0, s.value.size(&self)); self.write_value(Value::Scalar(x.into()), dest)?; } layout::Abi::ScalarPair(ref s1, ref s2) => { - let x = Scalar::null(s1.value.size(&self)); - let y = Scalar::null(s2.value.size(&self)); + let x = Scalar::from_int(0, s1.value.size(&self)); + let y = Scalar::from_int(0, s2.value.size(&self)); self.write_value(Value::ScalarPair(x.into(), y.into()), dest)?; } _ => { @@ -304,7 +302,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: let ty = substs.type_at(0); let layout = self.layout_of(ty)?; let align = layout.align.pref(); - let ptr_size = self.memory.pointer_size(); + let ptr_size = self.pointer_size(); let align_val = Scalar::from_uint(align as u128, ptr_size); self.write_scalar(align_val, dest)?; } @@ -365,10 +363,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: } "powf32" => { - let f = self.read_scalar(args[0])?.to_bits(Size::from_bits(32))?; - let f = f32::from_bits(f as u32); - let f2 = self.read_scalar(args[1])?.to_bits(Size::from_bits(32))?; - let f2 = f32::from_bits(f2 as u32); + let f = self.read_scalar(args[0])?.to_f32()?; + let f2 = self.read_scalar(args[1])?.to_f32()?; self.write_scalar( Scalar::from_f32(f.powf(f2)), dest, @@ -376,10 +372,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: } "powf64" => { - let f = self.read_scalar(args[0])?.to_bits(Size::from_bits(64))?; - let f = f64::from_bits(f as u64); - let f2 = self.read_scalar(args[1])?.to_bits(Size::from_bits(64))?; - let f2 = f64::from_bits(f2 as u64); + let f = self.read_scalar(args[0])?.to_f64()?; + let f2 = self.read_scalar(args[1])?.to_f64()?; self.write_scalar( Scalar::from_f64(f.powf(f2)), dest, @@ -387,12 +381,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: } "fmaf32" => { - let a = self.read_scalar(args[0])?.to_bits(Size::from_bits(32))?; - let a = f32::from_bits(a as u32); - let b = self.read_scalar(args[1])?.to_bits(Size::from_bits(32))?; - let b = f32::from_bits(b as u32); - let c = self.read_scalar(args[2])?.to_bits(Size::from_bits(32))?; - let c = f32::from_bits(c as u32); + let a = self.read_scalar(args[0])?.to_f32()?; + let b = self.read_scalar(args[1])?.to_f32()?; + let c = self.read_scalar(args[2])?.to_f32()?; self.write_scalar( Scalar::from_f32(a * b + c), dest, @@ -400,12 +391,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: } "fmaf64" => { - let a = self.read_scalar(args[0])?.to_bits(Size::from_bits(64))?; - let a = f64::from_bits(a as u64); - let b = self.read_scalar(args[1])?.to_bits(Size::from_bits(64))?; - let b = f64::from_bits(b as u64); - let c = self.read_scalar(args[2])?.to_bits(Size::from_bits(64))?; - let c = f64::from_bits(c as u64); + let a = self.read_scalar(args[0])?.to_f64()?; + let b = self.read_scalar(args[1])?.to_f64()?; + let c = self.read_scalar(args[2])?.to_f64()?; self.write_scalar( Scalar::from_f64(a * b + c), dest, @@ -413,8 +401,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: } "powif32" => { - let f = self.read_scalar(args[0])?.to_bits(Size::from_bits(32))?; - let f = f32::from_bits(f as u32); + let f = self.read_scalar(args[0])?.to_f32()?; let i = self.read_scalar(args[1])?.to_i32()?; self.write_scalar( Scalar::from_f32(f.powi(i)), @@ -423,8 +410,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: } "powif64" => { - let f = self.read_scalar(args[0])?.to_bits(Size::from_bits(64))?; - let f = f64::from_bits(f as u64); + let f = self.read_scalar(args[0])?.to_f64()?; let i = self.read_scalar(args[1])?.to_i32()?; self.write_scalar( Scalar::from_f64(f.powi(i)), @@ -435,7 +421,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: "size_of_val" => { let mplace = self.ref_to_mplace(self.read_value(args[0])?)?; let (size, _) = self.size_and_align_of_mplace(mplace)?; - let ptr_size = self.memory.pointer_size(); + let ptr_size = self.pointer_size(); self.write_scalar( Scalar::from_uint(size.bytes() as u128, ptr_size), dest, @@ -446,7 +432,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: "align_of_val" => { let mplace = self.ref_to_mplace(self.read_value(args[0])?)?; let (_, align) = self.size_and_align_of_mplace(mplace)?; - let ptr_size = self.memory.pointer_size(); + let ptr_size = self.pointer_size(); self.write_scalar( Scalar::from_uint(align.abi(), ptr_size), dest, diff --git a/src/lib.rs b/src/lib.rs index 42dac6a28d..0ce510a659 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,7 +46,7 @@ use tls::EvalContextExt as TlsEvalContextExt; use memory::MemoryKind as MiriMemoryKind; use locks::LockInfo; use range_map::RangeMap; -use helpers::{ScalarExt, FalibleScalarExt}; +use helpers::FalibleScalarExt; pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>( tcx: TyCtxt<'a, 'tcx, 'tcx>, diff --git a/src/tls.rs b/src/tls.rs index 6cec39483c..bd0318a62e 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -1,6 +1,6 @@ use rustc::{ty, mir}; -use super::{TlsKey, TlsEntry, EvalResult, EvalErrorKind, Scalar, ScalarExt, Memory, Evaluator, +use super::{TlsKey, TlsEntry, EvalResult, EvalErrorKind, Scalar, Memory, Evaluator, Place, StackPopCleanup, EvalContext}; pub trait MemoryExt<'tcx> { @@ -22,11 +22,10 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> MemoryExt<'tcx> for Memory<'a, 'mir, 'tcx, Evalu fn create_tls_key(&mut self, dtor: Option>) -> TlsKey { let new_key = self.data.next_thread_local; self.data.next_thread_local += 1; - let ptr_size = self.pointer_size(); self.data.thread_local.insert( new_key, TlsEntry { - data: Scalar::null(ptr_size).into(), + data: Scalar::ptr_null(*self.tcx).into(), dtor, }, ); @@ -89,7 +88,6 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> MemoryExt<'tcx> for Memory<'a, 'mir, 'tcx, Evalu ) -> Option<(ty::Instance<'tcx>, Scalar, TlsKey)> { use std::collections::Bound::*; - let ptr_size = self.pointer_size(); let thread_local = &mut self.data.thread_local; let start = match key { Some(key) => Excluded(key), @@ -101,7 +99,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> MemoryExt<'tcx> for Memory<'a, 'mir, 'tcx, Evalu if !data.is_null() { if let Some(dtor) = dtor { let ret = Some((dtor, *data, key)); - *data = Scalar::null(ptr_size); + *data = Scalar::ptr_null(*self.tcx); return ret; } } diff --git a/tests/run-pass/char.rs b/tests/run-pass/char.rs index 505c09b0ad..5524f0ae7a 100644 --- a/tests/run-pass/char.rs +++ b/tests/run-pass/char.rs @@ -3,7 +3,5 @@ fn main() { assert_eq!(c, 'x'); assert!('a' < 'z'); assert!('1' < '9'); - assert_eq!(std::char::from_u32('x' as u32).unwrap(), 'x'); - // FIXME: - // assert_eq!(std::char::from_u32('x' as u32), Some('x')); + assert_eq!(std::char::from_u32('x' as u32), Some('x')); } From e239fcffc1a211ff3f2f7dce4d084615aaa73715 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 27 Aug 2018 17:08:42 +0200 Subject: [PATCH 2/6] new tests for new fn arg passing code --- tests/compile-fail/cast_fn_ptr.rs | 3 +-- tests/compile-fail/cast_fn_ptr2.rs | 3 +-- tests/compile-fail/cast_fn_ptr3.rs | 10 ++++++++++ tests/compile-fail/cast_fn_ptr4.rs | 9 +++++++++ tests/run-pass/non_capture_closure_to_fn_ptr.rs | 6 ++++++ 5 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 tests/compile-fail/cast_fn_ptr3.rs create mode 100644 tests/compile-fail/cast_fn_ptr4.rs diff --git a/tests/compile-fail/cast_fn_ptr.rs b/tests/compile-fail/cast_fn_ptr.rs index 0a8f5ef752..0add977bf9 100644 --- a/tests/compile-fail/cast_fn_ptr.rs +++ b/tests/compile-fail/cast_fn_ptr.rs @@ -5,6 +5,5 @@ fn main() { std::mem::transmute::(f) }; - g(42) //~ ERROR constant evaluation error - //~^ NOTE tried to call a function with sig fn() through a function pointer of type fn(i32) + g(42) //~ ERROR tried to call a function with incorrect number of arguments } diff --git a/tests/compile-fail/cast_fn_ptr2.rs b/tests/compile-fail/cast_fn_ptr2.rs index cb80521c60..5af527016f 100644 --- a/tests/compile-fail/cast_fn_ptr2.rs +++ b/tests/compile-fail/cast_fn_ptr2.rs @@ -5,6 +5,5 @@ fn main() { std::mem::transmute::(f) }; - g(42) //~ ERROR constant evaluation error - //~^ NOTE tried to call a function with sig fn((i32, i32)) through a function pointer of type fn(i32) + g(42) //~ ERROR tried to call a function with argument of type (i32, i32) passing data of type i32 } diff --git a/tests/compile-fail/cast_fn_ptr3.rs b/tests/compile-fail/cast_fn_ptr3.rs new file mode 100644 index 0000000000..29507e7c7c --- /dev/null +++ b/tests/compile-fail/cast_fn_ptr3.rs @@ -0,0 +1,10 @@ +fn main() { + fn f(_ : (i32,i32)) {} + + let g = unsafe { + std::mem::transmute::(f) + }; + + g() //~ ERROR tried to call a function with incorrect number of arguments +} + diff --git a/tests/compile-fail/cast_fn_ptr4.rs b/tests/compile-fail/cast_fn_ptr4.rs new file mode 100644 index 0000000000..f9a2cf9f69 --- /dev/null +++ b/tests/compile-fail/cast_fn_ptr4.rs @@ -0,0 +1,9 @@ +fn main() { + fn f(_ : *const [i32]) {} + + let g = unsafe { + std::mem::transmute::(f) + }; + + g(&42 as *const i32) //~ ERROR tried to call a function with argument of type *const [i32] passing data of type *const i32 +} diff --git a/tests/run-pass/non_capture_closure_to_fn_ptr.rs b/tests/run-pass/non_capture_closure_to_fn_ptr.rs index c9daff9c9f..d48c4df459 100644 --- a/tests/run-pass/non_capture_closure_to_fn_ptr.rs +++ b/tests/run-pass/non_capture_closure_to_fn_ptr.rs @@ -4,6 +4,9 @@ static FOO: fn() = || { assert_ne!(42, 43) }; #[allow(const_err)] static BAR: fn(i32, i32) = |a, b| { assert_ne!(a, b) }; +// use to first make the closure FnOnce() before making it fn() +fn magic(f: F) -> F { f } + fn main() { FOO(); BAR(44, 45); @@ -11,4 +14,7 @@ fn main() { unsafe { bar(46, 47) }; let boo: &Fn(i32, i32) = &BAR; boo(48, 49); + + let f = magic(||{}) as fn(); + f(); } From c44267960f4642a1875de9e39ef7287961c56d3a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 Aug 2018 18:13:58 +0200 Subject: [PATCH 3/6] ptr equality: only defined for ptrs in the same allocation and live ptrs --- src/intrinsic.rs | 19 ++- src/lib.rs | 4 +- src/operator.rs | 157 +++++++++++----------- tests/compile-fail/pointer_byte_read_1.rs | 3 +- tests/compile-fail/ptr_bitops.rs | 3 +- 5 files changed, 87 insertions(+), 99 deletions(-) diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 695943d57b..1b877256c3 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -116,11 +116,11 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: _ if intrinsic_name.starts_with("atomic_cxchg") => { let ptr = self.ref_to_mplace(self.read_value(args[0])?)?; - let expect_old = self.read_value(args[1])?; // read as value for the sake of `binary_op()` + let expect_old = self.read_value(args[1])?; // read as value for the sake of `binary_op_val()` let new = self.read_scalar(args[2])?; - let old = self.read_value(ptr.into())?; // read as value for the sake of `binary_op()` - // binary_op will bail if either of them is not a scalar - let (eq, _) = self.binary_op(mir::BinOp::Eq, old, expect_old)?; + let old = self.read_value(ptr.into())?; // read as value for the sake of `binary_op_val()` + // binary_op_val will bail if either of them is not a scalar + let (eq, _) = self.binary_op_val(mir::BinOp::Eq, old, expect_old)?; let res = Value::ScalarPair(old.to_scalar_or_undef(), eq.into()); self.write_value(res, dest)?; // old value is returned // update ptr depending on comparison @@ -167,8 +167,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: _ => bug!(), }; // FIXME: what do atomics do on overflow? - let (val, _) = self.binary_op(op, old, rhs)?; - self.write_scalar(val, ptr.into())?; + self.binop_ignore_overflow(op, old, rhs, ptr.into())?; } "breakpoint" => unimplemented!(), // halt miri @@ -255,8 +254,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: "frem_fast" => mir::BinOp::Rem, _ => bug!(), }; - let result = self.binary_op(op, a, b)?; - self.write_scalar(result.0, dest)?; + self.binop_ignore_overflow(op, a, b, dest)?; } "exact_div" => { @@ -265,11 +263,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: let a = self.read_value(args[0])?; let b = self.read_value(args[1])?; // check x % y != 0 - if !self.binary_op(mir::BinOp::Rem, a, b)?.0.is_null() { + if !self.binary_op_val(mir::BinOp::Rem, a, b)?.0.is_null() { return err!(ValidationFailure(format!("exact_div: {:?} cannot be divided by {:?}", a, b))); } - let result = self.binary_op(mir::BinOp::Div, a, b)?; - self.write_scalar(result.0, dest)?; + self.binop_ignore_overflow(mir::BinOp::Div, a, b, dest)?; }, "likely" | "unlikely" | "forget" => {} diff --git a/src/lib.rs b/src/lib.rs index 0ce510a659..b8d3c18c01 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -304,14 +304,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { ecx.call_intrinsic(instance, args, dest) } - fn try_ptr_op<'a>( + fn ptr_op<'a>( ecx: &rustc_mir::interpret::EvalContext<'a, 'mir, 'tcx, Self>, bin_op: mir::BinOp, left: Scalar, left_layout: TyLayout<'tcx>, right: Scalar, right_layout: TyLayout<'tcx>, - ) -> EvalResult<'tcx, Option<(Scalar, bool)>> { + ) -> EvalResult<'tcx, (Scalar, bool)> { ecx.ptr_op(bin_op, left, left_layout, right, right_layout) } diff --git a/src/operator.rs b/src/operator.rs index 03383ae61c..550e7014af 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -1,5 +1,4 @@ -use rustc::ty::{self, Ty}; -use rustc::ty::layout::{TyLayout, Primitive}; +use rustc::ty::{Ty, layout::TyLayout}; use rustc::mir; use super::*; @@ -12,7 +11,7 @@ pub trait EvalContextExt<'tcx> { left_layout: TyLayout<'tcx>, right: Scalar, right_layout: TyLayout<'tcx>, - ) -> EvalResult<'tcx, Option<(Scalar, bool)>>; + ) -> EvalResult<'tcx, (Scalar, bool)>; fn ptr_int_arithmetic( &self, @@ -22,6 +21,13 @@ pub trait EvalContextExt<'tcx> { signed: bool, ) -> EvalResult<'tcx, (Scalar, bool)>; + fn ptr_eq( + &self, + left: Scalar, + right: Scalar, + size: Size, + ) -> EvalResult<'tcx, bool>; + fn pointer_offset_inbounds( &self, ptr: Scalar, @@ -38,38 +44,14 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: left_layout: TyLayout<'tcx>, right: Scalar, right_layout: TyLayout<'tcx>, - ) -> EvalResult<'tcx, Option<(Scalar, bool)>> { + ) -> EvalResult<'tcx, (Scalar, bool)> { + use rustc::mir::BinOp::*; + trace!("ptr_op: {:?} {:?} {:?}", left, bin_op, right); + debug_assert!(left.is_ptr() || right.is_ptr() || bin_op == Offset); - use rustc::mir::BinOp::*; - use rustc::ty::layout::Integer::*; - let usize = Primitive::Int(match self.memory.pointer_size().bytes() { - 1 => I8, - 2 => I16, - 4 => I32, - 8 => I64, - 16 => I128, - _ => unreachable!(), - }, /*signed*/ false); - let isize = Primitive::Int(match self.memory.pointer_size().bytes() { - 1 => I8, - 2 => I16, - 4 => I32, - 8 => I64, - 16 => I128, - _ => unreachable!(), - }, /*signed*/ true); - let left_kind = match left_layout.abi { - ty::layout::Abi::Scalar(ref scalar) => scalar.value, - _ => Err(EvalErrorKind::TypeNotPrimitive(left_layout.ty))?, - }; - let right_kind = match right_layout.abi { - ty::layout::Abi::Scalar(ref scalar) => scalar.value, - _ => Err(EvalErrorKind::TypeNotPrimitive(right_layout.ty))?, - }; match bin_op { Offset => { - assert!(left_kind == Primitive::Pointer && right_kind == usize); let pointee_ty = left_layout.ty .builtin_deref(true) .expect("Offset called on non-ptr type") @@ -77,40 +59,19 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: let ptr = self.pointer_offset_inbounds( left, pointee_ty, - right.to_bits(self.memory.pointer_size())? as i64, + right.to_isize(self)?, )?; - Ok(Some((ptr, false))) + Ok((ptr, false)) } // These work on anything - Eq if left_kind == right_kind => { - let result = match (left, right) { - (Scalar::Bits { .. }, Scalar::Bits { .. }) => { - left.to_bits(left_layout.size)? == right.to_bits(right_layout.size)? - }, - // FIXME: Test if both allocations are still live *or* if they are in the same allocation? (same for Ne below) - (Scalar::Ptr(left), Scalar::Ptr(right)) => left == right, - // FIXME: We should probably error out when comparing anything but NULL with a pointer (same for Ne below) - _ => false, - }; - Ok(Some((Scalar::from_bool(result), false))) - } - Ne if left_kind == right_kind => { - let result = match (left, right) { - (Scalar::Bits { .. }, Scalar::Bits { .. }) => { - left.to_bits(left_layout.size)? != right.to_bits(right_layout.size)? - }, - (Scalar::Ptr(left), Scalar::Ptr(right)) => left != right, - _ => true, - }; - Ok(Some((Scalar::from_bool(result), false))) - } - // These need both pointers to be in the same allocation - Lt | Le | Gt | Ge | Sub - if left_kind == right_kind && - (left_kind == Primitive::Pointer || left_kind == usize || left_kind == isize) && - left.is_ptr() && right.is_ptr() => { - let left = left.to_ptr()?; - let right = right.to_ptr()?; + Eq => + Ok((Scalar::from_bool(self.ptr_eq(left, right, left_layout.size)?), false)), + Ne => + Ok((Scalar::from_bool(!self.ptr_eq(left, right, left_layout.size)?), false)), + // These need both to be pointer, and fail if they are not in the same location + Lt | Le | Gt | Ge | Sub if left.is_ptr() && right.is_ptr() => { + let left = left.to_ptr().expect("we checked is_ptr"); + let right = right.to_ptr().expect("we checked is_ptr"); if left.alloc_id == right.alloc_id { let res = match bin_op { Lt => left.offset < right.offset, @@ -118,51 +79,83 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: Gt => left.offset > right.offset, Ge => left.offset >= right.offset, Sub => { + // subtract the offsets let left_offset = Scalar::from_uint(left.offset.bytes(), self.memory.pointer_size()); let right_offset = Scalar::from_uint(right.offset.bytes(), self.memory.pointer_size()); let layout = self.layout_of(self.tcx.types.usize)?; return self.binary_op( Sub, - ValTy { value: Value::Scalar(left_offset.into()), layout }, - ValTy { value: Value::Scalar(right_offset.into()), layout }, - ).map(Some) + left_offset, layout, + right_offset, layout, + ) } _ => bug!("We already established it has to be one of these operators."), }; - Ok(Some((Scalar::from_bool(res), false))) + Ok((Scalar::from_bool(res), false)) } else { // Both are pointers, but from different allocations. err!(InvalidPointerMath) } } - // These work if the left operand is a pointer, the right an integer - Add | BitAnd | Sub | Rem - if left_kind == right_kind && (left_kind == usize || left_kind == isize) && - left.is_ptr() && right.is_bits() => { + // These work if the left operand is a pointer, and the right an integer + Add | BitAnd | Sub | Rem if left.is_ptr() && right.is_bits() => { // Cast to i128 is fine as we checked the kind to be ptr-sized self.ptr_int_arithmetic( bin_op, - left.to_ptr()?, - right.to_bits(self.memory.pointer_size())?, - left_kind == isize, - ).map(Some) + left.to_ptr().expect("we checked is_ptr"), + right.to_bits(self.memory.pointer_size()).expect("we checked is_bits"), + right_layout.abi.is_signed(), + ) } // Commutative operators also work if the integer is on the left - Add | BitAnd - if left_kind == right_kind && (left_kind == usize || left_kind == isize) && - left.is_bits() && right.is_ptr() => { + Add | BitAnd if left.is_bits() && right.is_ptr() => { // This is a commutative operation, just swap the operands self.ptr_int_arithmetic( bin_op, - right.to_ptr()?, - left.to_bits(self.memory.pointer_size())?, - left_kind == isize, - ).map(Some) + right.to_ptr().expect("we checked is_ptr"), + left.to_bits(self.memory.pointer_size()).expect("we checked is_bits"), + left_layout.abi.is_signed(), + ) } - _ => Ok(None), + // Nothing else works + _ => err!(InvalidPointerMath), } } + fn ptr_eq( + &self, + left: Scalar, + right: Scalar, + size: Size, + ) -> EvalResult<'tcx, bool> { + Ok(match (left, right) { + (Scalar::Bits { .. }, Scalar::Bits { .. }) => + left.to_bits(size)? == right.to_bits(size)?, + (Scalar::Ptr(left), Scalar::Ptr(right)) => { + // Comparison illegal if one of them is out-of-bounds, *unless* they + // are in the same allocation. + if left.alloc_id == right.alloc_id { + left.offset == right.offset + } else { + // This accepts one-past-the end. So technically there is still + // some non-determinism that we do not fully rule out when two + // allocations sit right next to each other. The C/C++ standards are + // somewhat fuzzy about this case, so I think for now this check is + // "good enough". + self.memory.check_bounds(left, false)?; + self.memory.check_bounds(right, false)?; + // Two live in-bounds pointers, we can compare across allocations + left == right + } + } + // Comparing ptr and integer -- we only allow compating with NULL + (Scalar::Ptr(_), Scalar::Bits { bits: 0, .. }) | + (Scalar::Bits { bits: 0, .. }, Scalar::Ptr(_)) => false, + // Nothing else is supported + _ => return err!(InvalidPointerMath), + }) + } + fn ptr_int_arithmetic( &self, bin_op: mir::BinOp, diff --git a/tests/compile-fail/pointer_byte_read_1.rs b/tests/compile-fail/pointer_byte_read_1.rs index 4cfdfb62e2..b25f09d485 100644 --- a/tests/compile-fail/pointer_byte_read_1.rs +++ b/tests/compile-fail/pointer_byte_read_1.rs @@ -3,6 +3,5 @@ fn main() { let y = &x; let z = &y as *const &i32 as *const usize; let ptr_bytes = unsafe { *z }; // the actual deref is fine, because we read the entire pointer at once - let _ = ptr_bytes / 432; //~ ERROR constant evaluation error - //~^ NOTE tried to access part of a pointer value as raw bytes + let _ = ptr_bytes / 432; //~ ERROR invalid arithmetic on pointers that would leak base addresses } diff --git a/tests/compile-fail/ptr_bitops.rs b/tests/compile-fail/ptr_bitops.rs index ecd47a186e..2706b0970d 100644 --- a/tests/compile-fail/ptr_bitops.rs +++ b/tests/compile-fail/ptr_bitops.rs @@ -2,7 +2,6 @@ fn main() { let bytes = [0i8, 1, 2, 3, 4, 5, 6, 7, 8, 9]; let one = bytes.as_ptr().wrapping_offset(1); let three = bytes.as_ptr().wrapping_offset(3); - let res = (one as usize) | (three as usize); //~ ERROR constant evaluation error - //~^ NOTE a raw memory access tried to access part of a pointer value as raw bytes + let res = (one as usize) | (three as usize); //~ ERROR invalid arithmetic on pointers that would leak base addresses println!("{}", res); } From 2a318264ea493c62b8ec2fe110654e3177b49939 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 30 Aug 2018 11:04:48 +0200 Subject: [PATCH 4/6] also allow comparing pointers with integers so big that they cannot be equal --- src/operator.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/operator.rs b/src/operator.rs index 550e7014af..576c4a271f 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -148,11 +148,30 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: left == right } } - // Comparing ptr and integer -- we only allow compating with NULL - (Scalar::Ptr(_), Scalar::Bits { bits: 0, .. }) | - (Scalar::Bits { bits: 0, .. }, Scalar::Ptr(_)) => false, - // Nothing else is supported - _ => return err!(InvalidPointerMath), + // Comparing ptr and integer -- we allow compating with NULL, and with addresses + // so close to the end of the `usize` range that they cannot overlap with an allocation + // of the given size. + (Scalar::Ptr(ptr), Scalar::Bits { bits, size }) | + (Scalar::Bits { bits, size }, Scalar::Ptr(ptr)) => { + assert_eq!(size as u64, self.pointer_size().bytes()); + if bits == 0 { + // Nothing equals 0 + false + } else { + // Compute the highest address at which this allocation could live + let alloc = self.memory.get(ptr.alloc_id)?; + let max_base_addr = + (1u128 << self.pointer_size().bits()) - alloc.bytes.len() as u128; + let max_addr = max_base_addr + ptr.offset.bytes() as u128; + if bits > max_addr { + // The integer is too big, this cannot possibly be equal + false + } else { + // TODO: We could also take alignment into account + return err!(InvalidPointerMath); + } + } + } }) } From 74c6a1aa49cadfc463f275a6b8d21ea5dd8d561b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 1 Sep 2018 10:33:53 +0200 Subject: [PATCH 5/6] bump toolchain --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index f54012d5e1..448c24468e 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1 +1 @@ -nightly-2018-08-30 +nightly-2018-09-01 From d3928f6356de808b1df402cc66e9e6d36a9dce8c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 1 Sep 2018 11:26:54 +0200 Subject: [PATCH 6/6] more permissive pointer comparison logic --- src/operator.rs | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/operator.rs b/src/operator.rs index 576c4a271f..4f697dbd5b 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -148,28 +148,21 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: left == right } } - // Comparing ptr and integer -- we allow compating with NULL, and with addresses - // so close to the end of the `usize` range that they cannot overlap with an allocation - // of the given size. + // Comparing ptr and integer (Scalar::Ptr(ptr), Scalar::Bits { bits, size }) | (Scalar::Bits { bits, size }, Scalar::Ptr(ptr)) => { assert_eq!(size as u64, self.pointer_size().bytes()); + if bits == 0 { - // Nothing equals 0 + // Nothing equals 0, not even dangling pointers. Ideally we would + // require them to be in-bounds of their (possilby dead) allocation, + // but with the allocation gonew e cannot check that. false } else { - // Compute the highest address at which this allocation could live - let alloc = self.memory.get(ptr.alloc_id)?; - let max_base_addr = - (1u128 << self.pointer_size().bits()) - alloc.bytes.len() as u128; - let max_addr = max_base_addr + ptr.offset.bytes() as u128; - if bits > max_addr { - // The integer is too big, this cannot possibly be equal - false - } else { - // TODO: We could also take alignment into account - return err!(InvalidPointerMath); - } + // Live pointers cannot equal an integer, but again do not + // allow comparing dead pointers. + self.memory.check_bounds(ptr, false)?; + false } } })