Skip to content

Commit bd33815

Browse files
authored
Merge pull request rust-lang#18819 from ChayimFriedman2/i128-max
fix: Fix overflow detection in MIR evaluation
2 parents ec46a15 + d93ab14 commit bd33815

File tree

3 files changed

+206
-19
lines changed

3 files changed

+206
-19
lines changed

src/tools/rust-analyzer/crates/hir-ty/src/mir/eval.rs

+143-19
Original file line numberDiff line numberDiff line change
@@ -1211,7 +1211,9 @@ impl Evaluator<'_> {
12111211
}
12121212
lc = &lc[..self.ptr_size()];
12131213
rc = &rc[..self.ptr_size()];
1214-
ls
1214+
lc = self.read_memory(Address::from_bytes(lc)?, ls)?;
1215+
rc = self.read_memory(Address::from_bytes(rc)?, ls)?;
1216+
break 'binary_op Owned(vec![u8::from(lc == rc)]);
12151217
} else {
12161218
self.size_of_sized(&ty, locals, "operand of binary op")?
12171219
};
@@ -1340,18 +1342,8 @@ impl Evaluator<'_> {
13401342
}
13411343
} else {
13421344
let is_signed = matches!(ty.as_builtin(), Some(BuiltinType::Int(_)));
1343-
let l128 = i128::from_le_bytes(pad16(lc, is_signed));
1344-
let r128 = i128::from_le_bytes(pad16(rc, is_signed));
1345-
let check_overflow = |r: i128| {
1346-
// FIXME: this is not very correct, and only catches the basic cases.
1347-
let r = r.to_le_bytes();
1348-
for &k in &r[lc.len()..] {
1349-
if k != 0 && (k != 255 || !is_signed) {
1350-
return Err(MirEvalError::Panic(format!("Overflow in {op:?}")));
1351-
}
1352-
}
1353-
Ok(Owned(r[0..lc.len()].into()))
1354-
};
1345+
let l128 = IntValue::from_bytes(lc, is_signed);
1346+
let r128 = IntValue::from_bytes(rc, is_signed);
13551347
match op {
13561348
BinOp::Ge | BinOp::Gt | BinOp::Le | BinOp::Lt | BinOp::Eq | BinOp::Ne => {
13571349
let r = op.run_compare(l128, r128) as u8;
@@ -1366,25 +1358,31 @@ impl Evaluator<'_> {
13661358
| BinOp::Rem
13671359
| BinOp::Sub => {
13681360
let r = match op {
1369-
BinOp::Add => l128.overflowing_add(r128).0,
1370-
BinOp::Mul => l128.overflowing_mul(r128).0,
1361+
BinOp::Add => l128.checked_add(r128).ok_or_else(|| {
1362+
MirEvalError::Panic(format!("Overflow in {op:?}"))
1363+
})?,
1364+
BinOp::Mul => l128.checked_mul(r128).ok_or_else(|| {
1365+
MirEvalError::Panic(format!("Overflow in {op:?}"))
1366+
})?,
13711367
BinOp::Div => l128.checked_div(r128).ok_or_else(|| {
13721368
MirEvalError::Panic(format!("Overflow in {op:?}"))
13731369
})?,
13741370
BinOp::Rem => l128.checked_rem(r128).ok_or_else(|| {
13751371
MirEvalError::Panic(format!("Overflow in {op:?}"))
13761372
})?,
1377-
BinOp::Sub => l128.overflowing_sub(r128).0,
1373+
BinOp::Sub => l128.checked_sub(r128).ok_or_else(|| {
1374+
MirEvalError::Panic(format!("Overflow in {op:?}"))
1375+
})?,
13781376
BinOp::BitAnd => l128 & r128,
13791377
BinOp::BitOr => l128 | r128,
13801378
BinOp::BitXor => l128 ^ r128,
13811379
_ => unreachable!(),
13821380
};
1383-
check_overflow(r)?
1381+
Owned(r.to_bytes())
13841382
}
13851383
BinOp::Shl | BinOp::Shr => {
13861384
let r = 'b: {
1387-
if let Ok(shift_amount) = u32::try_from(r128) {
1385+
if let Some(shift_amount) = r128.as_u32() {
13881386
let r = match op {
13891387
BinOp::Shl => l128.checked_shl(shift_amount),
13901388
BinOp::Shr => l128.checked_shr(shift_amount),
@@ -1401,7 +1399,7 @@ impl Evaluator<'_> {
14011399
};
14021400
return Err(MirEvalError::Panic(format!("Overflow in {op:?}")));
14031401
};
1404-
Owned(r.to_le_bytes()[..lc.len()].to_vec())
1402+
Owned(r.to_bytes())
14051403
}
14061404
BinOp::Offset => not_supported!("offset binop"),
14071405
}
@@ -2974,3 +2972,129 @@ pub fn pad16(it: &[u8], is_signed: bool) -> [u8; 16] {
29742972
res[..it.len()].copy_from_slice(it);
29752973
res
29762974
}
2975+
2976+
macro_rules! for_each_int_type {
2977+
($call_macro:path, $args:tt) => {
2978+
$call_macro! {
2979+
$args
2980+
I8
2981+
U8
2982+
I16
2983+
U16
2984+
I32
2985+
U32
2986+
I64
2987+
U64
2988+
I128
2989+
U128
2990+
}
2991+
};
2992+
}
2993+
2994+
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
2995+
enum IntValue {
2996+
I8(i8),
2997+
U8(u8),
2998+
I16(i16),
2999+
U16(u16),
3000+
I32(i32),
3001+
U32(u32),
3002+
I64(i64),
3003+
U64(u64),
3004+
I128(i128),
3005+
U128(u128),
3006+
}
3007+
3008+
macro_rules! checked_int_op {
3009+
( [ $op:ident ] $( $int_ty:ident )+ ) => {
3010+
fn $op(self, other: Self) -> Option<Self> {
3011+
match (self, other) {
3012+
$( (Self::$int_ty(a), Self::$int_ty(b)) => a.$op(b).map(Self::$int_ty), )+
3013+
_ => panic!("incompatible integer types"),
3014+
}
3015+
}
3016+
};
3017+
}
3018+
3019+
macro_rules! int_bit_shifts {
3020+
( [ $op:ident ] $( $int_ty:ident )+ ) => {
3021+
fn $op(self, amount: u32) -> Option<Self> {
3022+
match self {
3023+
$( Self::$int_ty(this) => this.$op(amount).map(Self::$int_ty), )+
3024+
}
3025+
}
3026+
};
3027+
}
3028+
3029+
macro_rules! unchecked_int_op {
3030+
( [ $name:ident, $op:tt ] $( $int_ty:ident )+ ) => {
3031+
fn $name(self, other: Self) -> Self {
3032+
match (self, other) {
3033+
$( (Self::$int_ty(a), Self::$int_ty(b)) => Self::$int_ty(a $op b), )+
3034+
_ => panic!("incompatible integer types"),
3035+
}
3036+
}
3037+
};
3038+
}
3039+
3040+
impl IntValue {
3041+
fn from_bytes(bytes: &[u8], is_signed: bool) -> Self {
3042+
match (bytes.len(), is_signed) {
3043+
(1, false) => Self::U8(u8::from_le_bytes(bytes.try_into().unwrap())),
3044+
(1, true) => Self::I8(i8::from_le_bytes(bytes.try_into().unwrap())),
3045+
(2, false) => Self::U16(u16::from_le_bytes(bytes.try_into().unwrap())),
3046+
(2, true) => Self::I16(i16::from_le_bytes(bytes.try_into().unwrap())),
3047+
(4, false) => Self::U32(u32::from_le_bytes(bytes.try_into().unwrap())),
3048+
(4, true) => Self::I32(i32::from_le_bytes(bytes.try_into().unwrap())),
3049+
(8, false) => Self::U64(u64::from_le_bytes(bytes.try_into().unwrap())),
3050+
(8, true) => Self::I64(i64::from_le_bytes(bytes.try_into().unwrap())),
3051+
(16, false) => Self::U128(u128::from_le_bytes(bytes.try_into().unwrap())),
3052+
(16, true) => Self::I128(i128::from_le_bytes(bytes.try_into().unwrap())),
3053+
_ => panic!("invalid integer size"),
3054+
}
3055+
}
3056+
3057+
fn to_bytes(self) -> Vec<u8> {
3058+
macro_rules! m {
3059+
( [] $( $int_ty:ident )+ ) => {
3060+
match self {
3061+
$( Self::$int_ty(v) => v.to_le_bytes().to_vec() ),+
3062+
}
3063+
};
3064+
}
3065+
for_each_int_type! { m, [] }
3066+
}
3067+
3068+
fn as_u32(self) -> Option<u32> {
3069+
macro_rules! m {
3070+
( [] $( $int_ty:ident )+ ) => {
3071+
match self {
3072+
$( Self::$int_ty(v) => v.try_into().ok() ),+
3073+
}
3074+
};
3075+
}
3076+
for_each_int_type! { m, [] }
3077+
}
3078+
3079+
for_each_int_type!(checked_int_op, [checked_add]);
3080+
for_each_int_type!(checked_int_op, [checked_sub]);
3081+
for_each_int_type!(checked_int_op, [checked_div]);
3082+
for_each_int_type!(checked_int_op, [checked_rem]);
3083+
for_each_int_type!(checked_int_op, [checked_mul]);
3084+
3085+
for_each_int_type!(int_bit_shifts, [checked_shl]);
3086+
for_each_int_type!(int_bit_shifts, [checked_shr]);
3087+
}
3088+
3089+
impl std::ops::BitAnd for IntValue {
3090+
type Output = Self;
3091+
for_each_int_type!(unchecked_int_op, [bitand, &]);
3092+
}
3093+
impl std::ops::BitOr for IntValue {
3094+
type Output = Self;
3095+
for_each_int_type!(unchecked_int_op, [bitor, |]);
3096+
}
3097+
impl std::ops::BitXor for IntValue {
3098+
type Output = Self;
3099+
for_each_int_type!(unchecked_int_op, [bitxor, ^]);
3100+
}

src/tools/rust-analyzer/crates/hir-ty/src/mir/eval/tests.rs

+29
Original file line numberDiff line numberDiff line change
@@ -879,3 +879,32 @@ fn main() {
879879
"#,
880880
);
881881
}
882+
883+
#[test]
884+
fn long_str_eq_same_prefix() {
885+
check_pass_and_stdio(
886+
r#"
887+
//- minicore: slice, index, coerce_unsized
888+
889+
type pthread_key_t = u32;
890+
type c_void = u8;
891+
type c_int = i32;
892+
893+
extern "C" {
894+
pub fn write(fd: i32, buf: *const u8, count: usize) -> usize;
895+
}
896+
897+
fn main() {
898+
// More than 16 bytes, the size of `i128`.
899+
let long_str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab";
900+
let output = match long_str {
901+
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" => b"true" as &[u8],
902+
_ => b"false",
903+
};
904+
write(1, &output[0], output.len());
905+
}
906+
"#,
907+
"false",
908+
"",
909+
);
910+
}

src/tools/rust-analyzer/crates/ide/src/hover/tests.rs

+34
Original file line numberDiff line numberDiff line change
@@ -10034,6 +10034,40 @@ fn bar() {
1003410034
);
1003510035
}
1003610036

10037+
#[test]
10038+
fn i128_max() {
10039+
check(
10040+
r#"
10041+
//- /core.rs library crate:core
10042+
#![rustc_coherence_is_core]
10043+
impl u128 {
10044+
pub const MAX: Self = 340_282_366_920_938_463_463_374_607_431_768_211_455u128;
10045+
}
10046+
impl i128 {
10047+
pub const MAX: Self = (u128::MAX >> 1) as Self;
10048+
}
10049+
10050+
//- /foo.rs crate:foo deps:core
10051+
fn foo() {
10052+
let _ = i128::MAX$0;
10053+
}
10054+
"#,
10055+
expect![
10056+
r#"
10057+
*MAX*
10058+
10059+
```rust
10060+
core
10061+
```
10062+
10063+
```rust
10064+
pub const MAX: Self = 170141183460469231731687303715884105727 (0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
10065+
```
10066+
"#
10067+
],
10068+
);
10069+
}
10070+
1003710071
#[test]
1003810072
fn test_runnables_with_snapshot_tests() {
1003910073
check_actions(

0 commit comments

Comments
 (0)