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

interpret: ensure we check bool/char for validity when they are used in a cast #126265

Merged
merged 2 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 7 additions & 9 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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])?;
Expand Down Expand Up @@ -484,7 +484,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
ret_layout: TyAndLayout<'tcx>,
) -> InterpResult<'tcx, Scalar<M::Provenance>> {
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()),
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
29 changes: 15 additions & 14 deletions compiler/rustc_const_eval/src/interpret/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 5 additions & 10 deletions compiler/rustc_middle/src/ty/consts/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ impl ScalarInt {

#[inline]
pub fn try_from_uint(i: impl Into<u128>, size: Size) -> Option<Self> {
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.
Expand All @@ -223,20 +223,15 @@ impl ScalarInt {

#[inline]
pub fn try_from_int(i: impl Into<i128>, size: Size) -> Option<Self> {
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<i128>, 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)
}
Expand Down
21 changes: 21 additions & 0 deletions src/tools/miri/tests/fail/validity/invalid_char_cast.rs
Original file line number Diff line number Diff line change
@@ -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);
}
20 changes: 20 additions & 0 deletions src/tools/miri/tests/fail/validity/invalid_char_cast.stderr
Original file line number Diff line number Diff line change
@@ -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

26 changes: 26 additions & 0 deletions src/tools/miri/tests/fail/validity/invalid_char_match.rs
Original file line number Diff line number Diff line change
@@ -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);
}
23 changes: 23 additions & 0 deletions src/tools/miri/tests/fail/validity/invalid_char_match.stderr
Original file line number Diff line number Diff line change
@@ -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

15 changes: 15 additions & 0 deletions src/tools/miri/tests/fail/validity/invalid_enum_cast.rs
Original file line number Diff line number Diff line change
@@ -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);
}
20 changes: 20 additions & 0 deletions src/tools/miri/tests/fail/validity/invalid_enum_cast.stderr
Original file line number Diff line number Diff line change
@@ -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

11 changes: 11 additions & 0 deletions tests/ui/consts/const-eval/ub-invalid-values.rs
Original file line number Diff line number Diff line change
@@ -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() {}
20 changes: 20 additions & 0 deletions tests/ui/consts/const-eval/ub-invalid-values.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading