Merge pull request #18819 from ChayimFriedman2/i128-max

fix: Fix overflow detection in MIR evaluation
This commit is contained in:
Lukas Wirth 2025-01-02 10:06:09 +00:00 committed by GitHub
commit a611d8eb4e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 206 additions and 19 deletions

View file

@ -1211,7 +1211,9 @@ impl Evaluator<'_> {
} }
lc = &lc[..self.ptr_size()]; lc = &lc[..self.ptr_size()];
rc = &rc[..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 { } else {
self.size_of_sized(&ty, locals, "operand of binary op")? self.size_of_sized(&ty, locals, "operand of binary op")?
}; };
@ -1340,18 +1342,8 @@ impl Evaluator<'_> {
} }
} else { } else {
let is_signed = matches!(ty.as_builtin(), Some(BuiltinType::Int(_))); let is_signed = matches!(ty.as_builtin(), Some(BuiltinType::Int(_)));
let l128 = i128::from_le_bytes(pad16(lc, is_signed)); let l128 = IntValue::from_bytes(lc, is_signed);
let r128 = i128::from_le_bytes(pad16(rc, is_signed)); let r128 = IntValue::from_bytes(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()))
};
match op { match op {
BinOp::Ge | BinOp::Gt | BinOp::Le | BinOp::Lt | BinOp::Eq | BinOp::Ne => { BinOp::Ge | BinOp::Gt | BinOp::Le | BinOp::Lt | BinOp::Eq | BinOp::Ne => {
let r = op.run_compare(l128, r128) as u8; let r = op.run_compare(l128, r128) as u8;
@ -1366,25 +1358,31 @@ impl Evaluator<'_> {
| BinOp::Rem | BinOp::Rem
| BinOp::Sub => { | BinOp::Sub => {
let r = match op { let r = match op {
BinOp::Add => l128.overflowing_add(r128).0, BinOp::Add => l128.checked_add(r128).ok_or_else(|| {
BinOp::Mul => l128.overflowing_mul(r128).0, 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(|| { BinOp::Div => l128.checked_div(r128).ok_or_else(|| {
MirEvalError::Panic(format!("Overflow in {op:?}")) MirEvalError::Panic(format!("Overflow in {op:?}"))
})?, })?,
BinOp::Rem => l128.checked_rem(r128).ok_or_else(|| { BinOp::Rem => l128.checked_rem(r128).ok_or_else(|| {
MirEvalError::Panic(format!("Overflow in {op:?}")) 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::BitAnd => l128 & r128,
BinOp::BitOr => l128 | r128, BinOp::BitOr => l128 | r128,
BinOp::BitXor => l128 ^ r128, BinOp::BitXor => l128 ^ r128,
_ => unreachable!(), _ => unreachable!(),
}; };
check_overflow(r)? Owned(r.to_bytes())
} }
BinOp::Shl | BinOp::Shr => { BinOp::Shl | BinOp::Shr => {
let r = 'b: { let r = 'b: {
if let Ok(shift_amount) = u32::try_from(r128) { if let Some(shift_amount) = r128.as_u32() {
let r = match op { let r = match op {
BinOp::Shl => l128.checked_shl(shift_amount), BinOp::Shl => l128.checked_shl(shift_amount),
BinOp::Shr => l128.checked_shr(shift_amount), BinOp::Shr => l128.checked_shr(shift_amount),
@ -1401,7 +1399,7 @@ impl Evaluator<'_> {
}; };
return Err(MirEvalError::Panic(format!("Overflow in {op:?}"))); 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"), 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[..it.len()].copy_from_slice(it);
res 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, ^]);
}

View file

@ -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",
"",
);
}

View file

@ -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] #[test]
fn test_runnables_with_snapshot_tests() { fn test_runnables_with_snapshot_tests() {
check_actions( check_actions(