Skip to content

Commit

Permalink
Fix overflow detection in MIR evaluation
Browse files Browse the repository at this point in the history 
With a bit of higher-order macros everything sorts out well.

And also fix a discovered bug when comparing long strings.
  • Loading branch information
@ChayimFriedman2
ChayimFriedman2 committed Jan 2, 2025
1 parent 4500856 commit bb400ca
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 19 deletions.
162 changes: 143 additions & 19 deletions crates/hir-ty/src/mir/eval.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,9 @@ impl Evaluator<'_> {
}
lc = &lc[..self.ptr_size()];
rc = &rc[..self.ptr_size()];
ls
lc = self.read_memory(Address::from_bytes(lc)?, ls)?;
rc = self.read_memory(Address::from_bytes(rc)?, ls)?;
break 'binary_op Owned(vec![u8::from(lc == rc)]);
} else {
self.size_of_sized(&ty, locals, "operand of binary op")?
};
Expand Down Expand Up @@ -1340,18 +1342,8 @@ impl Evaluator<'_> {
}
} else {
let is_signed = matches!(ty.as_builtin(), Some(BuiltinType::Int(_)));
let l128 = i128::from_le_bytes(pad16(lc, is_signed));
let r128 = i128::from_le_bytes(pad16(rc, is_signed));
let check_overflow = |r: i128| {
// FIXME: this is not very correct, and only catches the basic cases.
let r = r.to_le_bytes();
for &k in &r[lc.len()..] {
if k != 0 && (k != 255 || !is_signed) {
return Err(MirEvalError::Panic(format!("Overflow in {op:?}")));
}
}
Ok(Owned(r[0..lc.len()].into()))
};
let l128 = IntValue::from_bytes(lc, is_signed);
let r128 = IntValue::from_bytes(rc, is_signed);
match op {
BinOp::Ge | BinOp::Gt | BinOp::Le | BinOp::Lt | BinOp::Eq | BinOp::Ne => {
let r = op.run_compare(l128, r128) as u8;
Expand All @@ -1366,25 +1358,31 @@ impl Evaluator<'_> {
| BinOp::Rem
| BinOp::Sub => {
let r = match op {
BinOp::Add => l128.overflowing_add(r128).0,
BinOp::Mul => l128.overflowing_mul(r128).0,
BinOp::Add => l128.checked_add(r128).ok_or_else(|| {
MirEvalError::Panic(format!("Overflow in {op:?}"))
})?,
BinOp::Mul => l128.checked_mul(r128).ok_or_else(|| {
MirEvalError::Panic(format!("Overflow in {op:?}"))
})?,
BinOp::Div => l128.checked_div(r128).ok_or_else(|| {
MirEvalError::Panic(format!("Overflow in {op:?}"))
})?,
BinOp::Rem => l128.checked_rem(r128).ok_or_else(|| {
MirEvalError::Panic(format!("Overflow in {op:?}"))
})?,
BinOp::Sub => l128.overflowing_sub(r128).0,
BinOp::Sub => l128.checked_sub(r128).ok_or_else(|| {
MirEvalError::Panic(format!("Overflow in {op:?}"))
})?,
BinOp::BitAnd => l128 & r128,
BinOp::BitOr => l128 | r128,
BinOp::BitXor => l128 ^ r128,
_ => unreachable!(),
};
check_overflow(r)?
Owned(r.to_bytes())
}
BinOp::Shl | BinOp::Shr => {
let r = 'b: {
if let Ok(shift_amount) = u32::try_from(r128) {
if let Some(shift_amount) = r128.as_u32() {
let r = match op {
BinOp::Shl => l128.checked_shl(shift_amount),
BinOp::Shr => l128.checked_shr(shift_amount),
Expand All @@ -1401,7 +1399,7 @@ impl Evaluator<'_> {
};
return Err(MirEvalError::Panic(format!("Overflow in {op:?}")));
};
Owned(r.to_le_bytes()[..lc.len()].to_vec())
Owned(r.to_bytes())
}
BinOp::Offset => not_supported!("offset binop"),
}
Expand Down Expand Up @@ -2974,3 +2972,129 @@ pub fn pad16(it: &[u8], is_signed: bool) -> [u8; 16] {
res[..it.len()].copy_from_slice(it);
res
}

macro_rules! for_each_int_type {
($call_macro:path, $args:tt) => {
$call_macro! {
$args
I8
U8
I16
U16
I32
U32
I64
U64
I128
U128
}
};
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
enum IntValue {
I8(i8),
U8(u8),
I16(i16),
U16(u16),
I32(i32),
U32(u32),
I64(i64),
U64(u64),
I128(i128),
U128(u128),
}

macro_rules! checked_int_op {
( [ $op:ident ] $( $int_ty:ident )+ ) => {
fn $op(self, other: Self) -> Option<Self> {
match (self, other) {
$( (Self::$int_ty(a), Self::$int_ty(b)) => a.$op(b).map(Self::$int_ty), )+
_ => panic!("incompatible integer types"),
}
}
};
}

macro_rules! int_bit_shifts {
( [ $op:ident ] $( $int_ty:ident )+ ) => {
fn $op(self, amount: u32) -> Option<Self> {
match self {
$( Self::$int_ty(this) => this.$op(amount).map(Self::$int_ty), )+
}
}
};
}

macro_rules! unchecked_int_op {
( [ $name:ident, $op:tt ] $( $int_ty:ident )+ ) => {
fn $name(self, other: Self) -> Self {
match (self, other) {
$( (Self::$int_ty(a), Self::$int_ty(b)) => Self::$int_ty(a $op b), )+
_ => panic!("incompatible integer types"),
}
}
};
}

impl IntValue {
fn from_bytes(bytes: &[u8], is_signed: bool) -> Self {
match (bytes.len(), is_signed) {
(1, false) => Self::U8(u8::from_le_bytes(bytes.try_into().unwrap())),
(1, true) => Self::I8(i8::from_le_bytes(bytes.try_into().unwrap())),
(2, false) => Self::U16(u16::from_le_bytes(bytes.try_into().unwrap())),
(2, true) => Self::I16(i16::from_le_bytes(bytes.try_into().unwrap())),
(4, false) => Self::U32(u32::from_le_bytes(bytes.try_into().unwrap())),
(4, true) => Self::I32(i32::from_le_bytes(bytes.try_into().unwrap())),
(8, false) => Self::U64(u64::from_le_bytes(bytes.try_into().unwrap())),
(8, true) => Self::I64(i64::from_le_bytes(bytes.try_into().unwrap())),
(16, false) => Self::U128(u128::from_le_bytes(bytes.try_into().unwrap())),
(16, true) => Self::I128(i128::from_le_bytes(bytes.try_into().unwrap())),
_ => panic!("invalid integer size"),
}
}

fn to_bytes(self) -> Vec<u8> {
macro_rules! m {
( [] $( $int_ty:ident )+ ) => {
match self {
$( Self::$int_ty(v) => v.to_le_bytes().to_vec() ),+
}
};
}
for_each_int_type! { m, [] }
}

fn as_u32(self) -> Option<u32> {
macro_rules! m {
( [] $( $int_ty:ident )+ ) => {
match self {
$( Self::$int_ty(v) => v.try_into().ok() ),+
}
};
}
for_each_int_type! { m, [] }
}

for_each_int_type!(checked_int_op, [checked_add]);
for_each_int_type!(checked_int_op, [checked_sub]);
for_each_int_type!(checked_int_op, [checked_div]);
for_each_int_type!(checked_int_op, [checked_rem]);
for_each_int_type!(checked_int_op, [checked_mul]);

for_each_int_type!(int_bit_shifts, [checked_shl]);
for_each_int_type!(int_bit_shifts, [checked_shr]);
}

impl std::ops::BitAnd for IntValue {
type Output = Self;
for_each_int_type!(unchecked_int_op, [bitand, &]);
}
impl std::ops::BitOr for IntValue {
type Output = Self;
for_each_int_type!(unchecked_int_op, [bitor, |]);
}
impl std::ops::BitXor for IntValue {
type Output = Self;
for_each_int_type!(unchecked_int_op, [bitxor, ^]);
}
29 changes: 29 additions & 0 deletions crates/hir-ty/src/mir/eval/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -879,3 +879,32 @@ fn main() {
"#,
);
}

#[test]
fn long_str_eq_same_prefix() {
check_pass_and_stdio(
r#"
//- minicore: slice, index, coerce_unsized
type pthread_key_t = u32;
type c_void = u8;
type c_int = i32;
extern "C" {
pub fn write(fd: i32, buf: *const u8, count: usize) -> usize;
}
fn main() {
// More than 16 bytes, the size of `i128`.
let long_str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab";
let output = match long_str {
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" => b"true" as &[u8],
_ => b"false",
};
write(1, &output[0], output.len());
}
"#,
"false",
"",
);
}
34 changes: 34 additions & 0 deletions crates/ide/src/hover/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10034,6 +10034,40 @@ fn bar() {
);
}

#[test]
fn i128_max() {
check(
r#"
//- /core.rs library crate:core
#![rustc_coherence_is_core]
impl u128 {
pub const MAX: Self = 340_282_366_920_938_463_463_374_607_431_768_211_455u128;
}
impl i128 {
pub const MAX: Self = (u128::MAX >> 1) as Self;
}
//- /foo.rs crate:foo deps:core
fn foo() {
let _ = i128::MAX$0;
}
"#,
expect![
r#"
*MAX*
```rust
core
```
```rust
pub const MAX: Self = 170141183460469231731687303715884105727 (0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
```
"#
],
);
}

#[test]
fn test_runnables_with_snapshot_tests() {
check_actions(
Expand Down

0 comments on commit bb400ca

Please sign in to comment.