Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #18819 from ChayimFriedman2/i128-max
fix: Fix overflow detection in MIR evaluation
Lukas Wirth 2025-01-02
parent 4500856 · parent bb400ca · commit a611d8e
-rw-r--r--crates/hir-ty/src/mir/eval.rs162
-rw-r--r--crates/hir-ty/src/mir/eval/tests.rs29
-rw-r--r--crates/ide/src/hover/tests.rs34
3 files changed, 206 insertions, 19 deletions
diff --git a/crates/hir-ty/src/mir/eval.rs b/crates/hir-ty/src/mir/eval.rs
index e3072d6ee7..632f117e02 100644
--- a/crates/hir-ty/src/mir/eval.rs
+++ b/crates/hir-ty/src/mir/eval.rs
@@ -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")?
};
@@ -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;
@@ -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),
@@ -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"),
}
@@ -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, ^]);
+}
diff --git a/crates/hir-ty/src/mir/eval/tests.rs b/crates/hir-ty/src/mir/eval/tests.rs
index 30d1137373..ce43e90df7 100644
--- a/crates/hir-ty/src/mir/eval/tests.rs
+++ b/crates/hir-ty/src/mir/eval/tests.rs
@@ -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",
+ "",
+ );
+}
diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs
index 039d380925..8ea305ed8e 100644
--- a/crates/ide/src/hover/tests.rs
+++ b/crates/ide/src/hover/tests.rs
@@ -10035,6 +10035,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(
r#"