From 2ea6663440b1a26396caf328aa36497224799abf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Sep 2017 11:39:51 +0200 Subject: [PATCH 1/3] validation: check that int, float etc. are not undef --- src/librustc_mir/interpret/eval_context.rs | 2 ++ src/librustc_mir/interpret/validation.rs | 29 ++++++++++++++++--- .../undefined_byte_read.rs | 3 ++ tests/compile-fail/invalid_bool.rs | 4 +-- tests/compile-fail/match_char.rs | 4 +-- tests/compile-fail/reference_to_packed.rs | 3 ++ tests/compile-fail/transmute_fat.rs | 2 ++ tests/run-pass/move-undef-primval.rs | 3 ++ 8 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 780155214a09..8fb63b3cb2ca 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -1483,6 +1483,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { Value::ByRef { .. } => bug!("follow_by_ref_value can't result in `ByRef`"), Value::ByVal(primval) => { + // TODO: Do we really want insta-UB here? self.ensure_valid_value(primval, ty)?; Ok(primval) } @@ -1817,6 +1818,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { let val = match val { PrimVal::Bytes(0) => false, PrimVal::Bytes(1) => true, + // TODO: This seems a little overeager, should reading at bool type already be UB? _ => return err!(InvalidBool), }; PrimVal::from_bool(val) diff --git a/src/librustc_mir/interpret/validation.rs b/src/librustc_mir/interpret/validation.rs index 23bb0d8dbb78..af245a0b2a64 100644 --- a/src/librustc_mir/interpret/validation.rs +++ b/src/librustc_mir/interpret/validation.rs @@ -492,12 +492,29 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { let res = do catch { match query.ty.sty { TyInt(_) | TyUint(_) | TyRawPtr(_) => { - // TODO: Make sure these are not undef. - // We could do a bounds-check and other sanity checks on the lvalue, but it would be a bug in miri for this to ever fail. + if mode.acquiring() { + // Make sure there is no undef + let val = self.read_lvalue(query.lval.1)?; + // This is essentially value_to_primval with support for fat pointers + let has_undef = match self.follow_by_ref_value(val, query.ty)? { + Value::ByRef { .. } => bug!("follow_by_ref_value can't result in `ByRef`"), + Value::ByVal(primval) => primval.is_undef(), + Value::ByValPair(primval1, primval2) => + primval1.is_undef() || primval2.is_undef() + }; + if has_undef { + return err!(ReadUndefBytes); + } + } Ok(()) } - TyBool | TyFloat(_) | TyChar | TyStr => { - // TODO: Check if these are valid bool/float/codepoint/UTF-8, respectively (and in particular, not undef). + TyBool | TyFloat(_) | TyChar => { + if mode.acquiring() { + let val = self.read_lvalue(query.lval.1)?; + let val = self.value_to_primval(ValTy { value: val, ty: query.ty })?; + let _val = val.to_bytes()?; + // TODO: Check if these are valid bool/float/codepoint/UTF-8 + } Ok(()) } TyNever => err!(ValidationFailure(format!("The empty type is never valid."))), @@ -542,6 +559,10 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { } // Compound types + TyStr => { + // TODO: Validate strings + Ok(()) + } TySlice(elem_ty) => { let len = match query.lval.1 { Lvalue::Ptr { extra: LvalueExtra::Length(len), .. } => len, diff --git a/tests/compile-fail-fullmir/undefined_byte_read.rs b/tests/compile-fail-fullmir/undefined_byte_read.rs index f8b6f7f4aec1..99404b7d5f3f 100644 --- a/tests/compile-fail-fullmir/undefined_byte_read.rs +++ b/tests/compile-fail-fullmir/undefined_byte_read.rs @@ -1,3 +1,6 @@ +// This should fail even without validation +// compile-flags: -Zmir-emit-validate=0 + fn main() { let v: Vec = Vec::with_capacity(10); let undef = unsafe { *v.get_unchecked(5) }; diff --git a/tests/compile-fail/invalid_bool.rs b/tests/compile-fail/invalid_bool.rs index 9de2630797ec..c30c9b439a46 100644 --- a/tests/compile-fail/invalid_bool.rs +++ b/tests/compile-fail/invalid_bool.rs @@ -1,4 +1,4 @@ fn main() { - let b = unsafe { std::mem::transmute::(2) }; - if b { unreachable!() } else { unreachable!() } //~ ERROR: invalid boolean value read + let b = unsafe { std::mem::transmute::(2) }; //~ ERROR: invalid boolean value read + if b { unreachable!() } else { unreachable!() } } diff --git a/tests/compile-fail/match_char.rs b/tests/compile-fail/match_char.rs index a91c7fef6aa1..4fee6e692bad 100644 --- a/tests/compile-fail/match_char.rs +++ b/tests/compile-fail/match_char.rs @@ -1,7 +1,7 @@ fn main() { assert!(std::char::from_u32(-1_i32 as u32).is_none()); - match unsafe { std::mem::transmute::(-1) } { - 'a' => {}, //~ERROR tried to interpret an invalid 32-bit value as a char: 4294967295 + match unsafe { std::mem::transmute::(-1) } { //~ERROR tried to interpret an invalid 32-bit value as a char: 4294967295 + 'a' => {}, 'b' => {}, _ => {}, } diff --git a/tests/compile-fail/reference_to_packed.rs b/tests/compile-fail/reference_to_packed.rs index 5ca733a64df2..cc927f879504 100644 --- a/tests/compile-fail/reference_to_packed.rs +++ b/tests/compile-fail/reference_to_packed.rs @@ -1,3 +1,6 @@ +// This should fail even without validation +// compile-flags: -Zmir-emit-validate=0 + #![allow(dead_code, unused_variables)] #[repr(packed)] diff --git a/tests/compile-fail/transmute_fat.rs b/tests/compile-fail/transmute_fat.rs index 6b9e6f876481..7d5d95a1dc6d 100644 --- a/tests/compile-fail/transmute_fat.rs +++ b/tests/compile-fail/transmute_fat.rs @@ -1,3 +1,5 @@ +// This should fail even without validation +// compile-flags: -Zmir-emit-validate=0 #![feature(i128_type)] fn main() { diff --git a/tests/run-pass/move-undef-primval.rs b/tests/run-pass/move-undef-primval.rs index 73c33943a63a..2c18c2d3687a 100644 --- a/tests/run-pass/move-undef-primval.rs +++ b/tests/run-pass/move-undef-primval.rs @@ -1,3 +1,6 @@ +// Moving around undef is not allowed by validation +// compile-flags: -Zmir-emit-validate=0 + struct Foo { _inner: i32, } From 8509dbbafe1a08ea5cee367bb1599e0bb09a2a2d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Sep 2017 12:08:26 +0200 Subject: [PATCH 2/3] validation: allow undef integers and raw pointers, as a crude work-around --- src/librustc_mir/interpret/validation.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/interpret/validation.rs b/src/librustc_mir/interpret/validation.rs index af245a0b2a64..9be9341ee239 100644 --- a/src/librustc_mir/interpret/validation.rs +++ b/src/librustc_mir/interpret/validation.rs @@ -493,18 +493,11 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { match query.ty.sty { TyInt(_) | TyUint(_) | TyRawPtr(_) => { if mode.acquiring() { - // Make sure there is no undef + // Make sure we can read this. let val = self.read_lvalue(query.lval.1)?; - // This is essentially value_to_primval with support for fat pointers - let has_undef = match self.follow_by_ref_value(val, query.ty)? { - Value::ByRef { .. } => bug!("follow_by_ref_value can't result in `ByRef`"), - Value::ByVal(primval) => primval.is_undef(), - Value::ByValPair(primval1, primval2) => - primval1.is_undef() || primval2.is_undef() - }; - if has_undef { - return err!(ReadUndefBytes); - } + self.follow_by_ref_value(val, query.ty)?; + // FIXME: It would be great to rule out Undef here, but that doesn't actually work. + // Passing around undef data is a thing that e.g. Vec::extend_with does. } Ok(()) } @@ -512,7 +505,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { if mode.acquiring() { let val = self.read_lvalue(query.lval.1)?; let val = self.value_to_primval(ValTy { value: val, ty: query.ty })?; - let _val = val.to_bytes()?; + val.to_bytes()?; // TODO: Check if these are valid bool/float/codepoint/UTF-8 } Ok(()) From bc240ff606586bae4e1496ec3954c2f3a2b27c76 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Sep 2017 12:11:48 +0200 Subject: [PATCH 3/3] add an undef validation test --- tests/compile-fail/validation_undef.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/compile-fail/validation_undef.rs diff --git a/tests/compile-fail/validation_undef.rs b/tests/compile-fail/validation_undef.rs new file mode 100644 index 000000000000..b889b1ea5317 --- /dev/null +++ b/tests/compile-fail/validation_undef.rs @@ -0,0 +1,14 @@ +#![allow(unused_variables)] +// error-pattern: attempted to read undefined bytes + +mod safe { + use std::mem; + + pub(crate) fn make_float() -> f32 { + unsafe { mem::uninitialized() } + } +} + +fn main() { + let _x = safe::make_float(); +}