From db44cae343fa306b6b2263355dc3f9a5f5db82b4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 11 Jun 2024 12:12:21 +0200 Subject: [PATCH 1/2] interpret: ensure we check bool/char for validity when they are used in a cast --- .../rustc_const_eval/src/interpret/cast.rs | 10 +++++-- .../src/interpret/intrinsics.rs | 16 +++++----- .../src/interpret/operator.rs | 29 ++++++++++--------- compiler/rustc_middle/src/ty/consts/int.rs | 15 ++++------ .../tests/fail/validity/invalid_char_cast.rs | 21 ++++++++++++++ .../fail/validity/invalid_char_cast.stderr | 20 +++++++++++++ .../tests/fail/validity/invalid_char_match.rs | 26 +++++++++++++++++ .../fail/validity/invalid_char_match.stderr | 23 +++++++++++++++ .../tests/fail/validity/invalid_enum_cast.rs | 15 ++++++++++ .../fail/validity/invalid_enum_cast.stderr | 20 +++++++++++++ 10 files changed, 159 insertions(+), 36 deletions(-) create mode 100644 src/tools/miri/tests/fail/validity/invalid_char_cast.rs create mode 100644 src/tools/miri/tests/fail/validity/invalid_char_cast.stderr create mode 100644 src/tools/miri/tests/fail/validity/invalid_char_match.rs create mode 100644 src/tools/miri/tests/fail/validity/invalid_char_match.stderr create mode 100644 src/tools/miri/tests/fail/validity/invalid_enum_cast.rs create mode 100644 src/tools/miri/tests/fail/validity/invalid_enum_cast.stderr diff --git a/compiler/rustc_const_eval/src/interpret/cast.rs b/compiler/rustc_const_eval/src/interpret/cast.rs index 19414c72c6a42..0a45bbb3edb2f 100644 --- a/compiler/rustc_const_eval/src/interpret/cast.rs +++ b/compiler/rustc_const_eval/src/interpret/cast.rs @@ -274,9 +274,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // 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); + let v = match src_layout.ty.kind() { + Uint(_) | RawPtr(..) | FnPtr(..) => scalar.to_uint(src_layout.size)?, + Int(_) => scalar.to_int(src_layout.size)? as u128, // we will cast back to `i128` below if the sign matters + Bool => scalar.to_bool()?.into(), + Char => scalar.to_char()?.into(), + _ => span_bug!(self.cur_span(), "invalid int-like cast from {}", src_layout.ty), + }; Ok(match *cast_ty.kind() { // int -> int diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index dac5c10addc28..7f16d441606e9 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -197,7 +197,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // rotate_right: (X << ((BW - S) % BW)) | (X >> (S % BW)) let layout_val = self.layout_of(instance_args.type_at(0))?; let val = self.read_scalar(&args[0])?; - let val_bits = val.to_bits(layout_val.size)?; + let val_bits = val.to_bits(layout_val.size)?; // sign is ignored here let layout_raw_shift = self.layout_of(self.tcx.types.u32)?; let raw_shift = self.read_scalar(&args[1])?; @@ -484,7 +484,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { ret_layout: TyAndLayout<'tcx>, ) -> InterpResult<'tcx, Scalar> { assert!(layout.ty.is_integral(), "invalid type for numeric intrinsic: {}", layout.ty); - let bits = val.to_bits(layout.size)?; + let bits = val.to_bits(layout.size)?; // these operations all ignore the sign let extra = 128 - u128::from(layout.size.bits()); let bits_out = match name { sym::ctpop => u128::from(bits.count_ones()), @@ -519,6 +519,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // `x % y != 0` or `y == 0` or `x == T::MIN && y == -1`. // First, check x % y != 0 (or if that computation overflows). let rem = self.binary_op(BinOp::Rem, a, b)?; + // sign does not matter for 0 test, so `to_bits` is fine if rem.to_scalar().to_bits(a.layout.size)? != 0 { throw_ub_custom!( fluent::const_eval_exact_div_has_remainder, @@ -545,22 +546,19 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { self.binary_op(mir_op.wrapping_to_overflowing().unwrap(), l, r)?.to_scalar_pair(); Ok(if overflowed.to_bool()? { let size = l.layout.size; - let num_bits = size.bits(); if l.layout.abi.is_signed() { // For signed ints the saturated value depends on the sign of the first // term since the sign of the second term can be inferred from this and // the fact that the operation has overflowed (if either is 0 no // overflow can occur) - let first_term: u128 = l.to_scalar().to_bits(l.layout.size)?; - let first_term_positive = first_term & (1 << (num_bits - 1)) == 0; - if first_term_positive { + let first_term: i128 = l.to_scalar().to_int(l.layout.size)?; + if first_term >= 0 { // Negative overflow not possible since the positive first term // can only increase an (in range) negative term for addition - // or corresponding negated positive term for subtraction + // or corresponding negated positive term for subtraction. Scalar::from_int(size.signed_int_max(), size) } else { - // Positive overflow not possible for similar reason - // max negative + // Positive overflow not possible for similar reason. Scalar::from_int(size.signed_int_min(), size) } } else { diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs index c821c98073d72..a6eef9f5662ca 100644 --- a/compiler/rustc_const_eval/src/interpret/operator.rs +++ b/compiler/rustc_const_eval/src/interpret/operator.rs @@ -437,23 +437,24 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { }; Ok(ImmTy::from_scalar(res, layout)) } - _ if layout.ty.is_integral() => { - let val = val.to_scalar(); - let val = val.to_bits(layout.size)?; + ty::Int(..) => { + let val = val.to_scalar().to_int(layout.size)?; let res = match un_op { - Not => self.truncate(!val, layout), // bitwise negation, then truncate - Neg => { - // arithmetic negation - assert!(layout.abi.is_signed()); - let val = self.sign_extend(val, layout) as i128; - let res = val.wrapping_neg(); - let res = res as u128; - // Truncate to target type. - self.truncate(res, layout) - } + Not => !val, + Neg => val.wrapping_neg(), _ => span_bug!(self.cur_span(), "Invalid integer op {:?}", un_op), }; - Ok(ImmTy::from_uint(res, layout)) + let res = ScalarInt::truncate_from_int(res, layout.size).0; + Ok(ImmTy::from_scalar(res.into(), layout)) + } + ty::Uint(..) => { + let val = val.to_scalar().to_uint(layout.size)?; + let res = match un_op { + Not => !val, + _ => span_bug!(self.cur_span(), "Invalid unsigned integer op {:?}", un_op), + }; + let res = ScalarInt::truncate_from_uint(res, layout.size).0; + Ok(ImmTy::from_scalar(res.into(), layout)) } ty::RawPtr(..) => { assert_eq!(un_op, PtrMetadata); diff --git a/compiler/rustc_middle/src/ty/consts/int.rs b/compiler/rustc_middle/src/ty/consts/int.rs index 52320dd141baf..13691b61941bd 100644 --- a/compiler/rustc_middle/src/ty/consts/int.rs +++ b/compiler/rustc_middle/src/ty/consts/int.rs @@ -209,8 +209,8 @@ impl ScalarInt { #[inline] pub fn try_from_uint(i: impl Into, size: Size) -> Option { - let data = i.into(); - if size.truncate(data) == data { Some(Self::raw(data, size)) } else { None } + let (r, overflow) = Self::truncate_from_uint(i, size); + if overflow { None } else { Some(r) } } /// Returns the truncated result, and whether truncation changed the value. @@ -223,20 +223,15 @@ impl ScalarInt { #[inline] pub fn try_from_int(i: impl Into, size: Size) -> Option { - let i = i.into(); - // `into` performed sign extension, we have to truncate - let truncated = size.truncate(i as u128); - if size.sign_extend(truncated) as i128 == i { - Some(Self::raw(truncated, size)) - } else { - None - } + let (r, overflow) = Self::truncate_from_int(i, size); + if overflow { None } else { Some(r) } } /// Returns the truncated result, and whether truncation changed the value. #[inline] pub fn truncate_from_int(i: impl Into, size: Size) -> (Self, bool) { let data = i.into(); + // `into` performed sign extension, we have to truncate let r = Self::raw(size.truncate(data as u128), size); (r, size.sign_extend(r.data) as i128 != data) } diff --git a/src/tools/miri/tests/fail/validity/invalid_char_cast.rs b/src/tools/miri/tests/fail/validity/invalid_char_cast.rs new file mode 100644 index 0000000000000..6a590dc7ba10b --- /dev/null +++ b/src/tools/miri/tests/fail/validity/invalid_char_cast.rs @@ -0,0 +1,21 @@ +// Make sure we find these even with many checks disabled. +//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation +#![feature(core_intrinsics)] +#![feature(custom_mir)] + +use std::intrinsics::mir::*; + +#[custom_mir(dialect = "runtime", phase = "optimized")] +fn cast(ptr: *const char) -> u32 { + mir! { + { + RET = *ptr as u32; //~ERROR: interpreting an invalid 32-bit value as a char + Return() + } + } +} + +pub fn main() { + let v = u32::MAX; + cast(&v as *const u32 as *const char); +} diff --git a/src/tools/miri/tests/fail/validity/invalid_char_cast.stderr b/src/tools/miri/tests/fail/validity/invalid_char_cast.stderr new file mode 100644 index 0000000000000..1b5c838cf463c --- /dev/null +++ b/src/tools/miri/tests/fail/validity/invalid_char_cast.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: interpreting an invalid 32-bit value as a char: $HEX + --> $DIR/invalid_char_cast.rs:LL:CC + | +LL | RET = *ptr as u32; + | ^^^^^^^^^^^^^^^^^ interpreting an invalid 32-bit value as a char: $HEX + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `cast` at $DIR/invalid_char_cast.rs:LL:CC +note: inside `main` + --> $DIR/invalid_char_cast.rs:LL:CC + | +LL | cast(&v as *const u32 as *const char); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/validity/invalid_char_match.rs b/src/tools/miri/tests/fail/validity/invalid_char_match.rs new file mode 100644 index 0000000000000..6c2e65b2bb744 --- /dev/null +++ b/src/tools/miri/tests/fail/validity/invalid_char_match.rs @@ -0,0 +1,26 @@ +// Make sure we find these even with many checks disabled. +//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation +#![feature(core_intrinsics)] +#![feature(custom_mir)] + +use std::intrinsics::mir::*; + +#[custom_mir(dialect = "runtime", phase = "optimized")] +fn switch_int(ptr: *const char) { + mir! { + { + match *ptr { //~ERROR: interpreting an invalid 32-bit value as a char + '0' => ret, + _ => ret, + } + } + ret = { + Return() + } + } +} + +pub fn main() { + let v = u32::MAX; + switch_int(&v as *const u32 as *const char); +} diff --git a/src/tools/miri/tests/fail/validity/invalid_char_match.stderr b/src/tools/miri/tests/fail/validity/invalid_char_match.stderr new file mode 100644 index 0000000000000..7706ed97316ce --- /dev/null +++ b/src/tools/miri/tests/fail/validity/invalid_char_match.stderr @@ -0,0 +1,23 @@ +error: Undefined Behavior: interpreting an invalid 32-bit value as a char: $HEX + --> $DIR/invalid_char_match.rs:LL:CC + | +LL | / match *ptr { +LL | | '0' => ret, +LL | | _ => ret, +LL | | } + | |_____________^ interpreting an invalid 32-bit value as a char: $HEX + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `switch_int` at $DIR/invalid_char_match.rs:LL:CC +note: inside `main` + --> $DIR/invalid_char_match.rs:LL:CC + | +LL | switch_int(&v as *const u32 as *const char); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/validity/invalid_enum_cast.rs b/src/tools/miri/tests/fail/validity/invalid_enum_cast.rs new file mode 100644 index 0000000000000..faf5fb699a6a1 --- /dev/null +++ b/src/tools/miri/tests/fail/validity/invalid_enum_cast.rs @@ -0,0 +1,15 @@ +// Make sure we find these even with many checks disabled. +//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation + +#[derive(Copy, Clone)] +#[allow(unused)] +enum E {A, B, C } + +fn cast(ptr: *const E) { unsafe { + let _val = *ptr as u32; //~ERROR: enum value has invalid tag +}} + +pub fn main() { + let v = u32::MAX; + cast(&v as *const u32 as *const E); +} diff --git a/src/tools/miri/tests/fail/validity/invalid_enum_cast.stderr b/src/tools/miri/tests/fail/validity/invalid_enum_cast.stderr new file mode 100644 index 0000000000000..d898887604581 --- /dev/null +++ b/src/tools/miri/tests/fail/validity/invalid_enum_cast.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: enum value has invalid tag: 0xff + --> $DIR/invalid_enum_cast.rs:LL:CC + | +LL | let _val = *ptr as u32; + | ^^^^^^^^^^^ enum value has invalid tag: 0xff + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `cast` at $DIR/invalid_enum_cast.rs:LL:CC +note: inside `main` + --> $DIR/invalid_enum_cast.rs:LL:CC + | +LL | cast(&v as *const u32 as *const E); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + From de4ac0c465facfdfd56ce08dc6bb5cfee901561c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 11 Jun 2024 13:28:36 +0200 Subject: [PATCH 2/2] add const eval bool-to-int cast test --- .../ui/consts/const-eval/ub-invalid-values.rs | 11 ++++++++++ .../const-eval/ub-invalid-values.stderr | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 tests/ui/consts/const-eval/ub-invalid-values.rs create mode 100644 tests/ui/consts/const-eval/ub-invalid-values.stderr diff --git a/tests/ui/consts/const-eval/ub-invalid-values.rs b/tests/ui/consts/const-eval/ub-invalid-values.rs new file mode 100644 index 0000000000000..1724a88dd3d9a --- /dev/null +++ b/tests/ui/consts/const-eval/ub-invalid-values.rs @@ -0,0 +1,11 @@ +const fn bool_cast(ptr: *const bool) { unsafe { + let _val = *ptr as u32; //~ERROR: evaluation of constant value failed + //~^ interpreting an invalid 8-bit value as a bool +}} + +const _: () = { + let v = 3_u8; + bool_cast(&v as *const u8 as *const bool); +}; + +fn main() {} diff --git a/tests/ui/consts/const-eval/ub-invalid-values.stderr b/tests/ui/consts/const-eval/ub-invalid-values.stderr new file mode 100644 index 0000000000000..edf72f731e5a0 --- /dev/null +++ b/tests/ui/consts/const-eval/ub-invalid-values.stderr @@ -0,0 +1,20 @@ +error[E0080]: evaluation of constant value failed + --> $DIR/ub-invalid-values.rs:2:16 + | +LL | let _val = *ptr as u32; + | ^^^^^^^^^^^ interpreting an invalid 8-bit value as a bool: 0x03 + | +note: inside `bool_cast` + --> $DIR/ub-invalid-values.rs:2:16 + | +LL | let _val = *ptr as u32; + | ^^^^^^^^^^^ +note: inside `_` + --> $DIR/ub-invalid-values.rs:8:5 + | +LL | bool_cast(&v as *const u8 as *const bool); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`.