Skip to content

Commit 1c7348e

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#94512 - RalfJung:sdiv-ub, r=oli-obk
Miri/CTFE: properly treat overflow in (signed) division/rem as UB To my surprise, it looks like LLVM treats overflow of signed div/rem as UB. From what I can tell, MIR `Div`/`Rem` directly lowers to the corresponding LLVM operation, so to make that correct we also have to consider these overflows UB in the CTFE/Miri interpreter engine. r? ``````@oli-obk``````
2 parents a41efcc + 6739299 commit 1c7348e

File tree

10 files changed

+93
-76
lines changed

10 files changed

+93
-76
lines changed

Diff for: ‎compiler/rustc_const_eval/src/interpret/intrinsics.rs

+3-9
Original file line numberDiff line numberDiff line change
@@ -500,15 +500,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
500500
// `x % y != 0` or `y == 0` or `x == T::MIN && y == -1`.
501501
// First, check x % y != 0 (or if that computation overflows).
502502
let (res, overflow, _ty) = self.overflowing_binary_op(BinOp::Rem, &a, &b)?;
503-
if overflow || res.assert_bits(a.layout.size) != 0 {
504-
// Then, check if `b` is -1, which is the "MIN / -1" case.
505-
let minus1 = Scalar::from_int(-1, dest.layout.size);
506-
let b_scalar = b.to_scalar().unwrap();
507-
if b_scalar == minus1 {
508-
throw_ub_format!("exact_div: result of dividing MIN by -1 cannot be represented")
509-
} else {
510-
throw_ub_format!("exact_div: {} cannot be divided by {} without remainder", a, b,)
511-
}
503+
assert!(!overflow); // All overflow is UB, so this should never return on overflow.
504+
if res.assert_bits(a.layout.size) != 0 {
505+
throw_ub_format!("exact_div: {} cannot be divided by {} without remainder", a, b)
512506
}
513507
// `Rem` says this is all right, so we can let `Div` do its job.
514508
self.binop_ignore_overflow(BinOp::Div, &a, &b, dest)

Diff for: ‎compiler/rustc_const_eval/src/interpret/operator.rs

+11-7
Original file line numberDiff line numberDiff line change
@@ -196,16 +196,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
196196
_ => None,
197197
};
198198
if let Some(op) = op {
199+
let l = self.sign_extend(l, left_layout) as i128;
199200
let r = self.sign_extend(r, right_layout) as i128;
200-
// We need a special check for overflowing remainder:
201-
// "int_min % -1" overflows and returns 0, but after casting things to a larger int
202-
// type it does *not* overflow nor give an unrepresentable result!
203-
if bin_op == Rem {
204-
if r == -1 && l == (1 << (size.bits() - 1)) {
205-
return Ok((Scalar::from_int(0, size), true, left_layout.ty));
201+
202+
// We need a special check for overflowing Rem and Div since they are *UB*
203+
// on overflow, which can happen with "int_min $OP -1".
204+
if matches!(bin_op, Rem | Div) {
205+
if l == size.signed_int_min() && r == -1 {
206+
if bin_op == Rem {
207+
throw_ub!(RemainderOverflow)
208+
} else {
209+
throw_ub!(DivisionOverflow)
210+
}
206211
}
207212
}
208-
let l = self.sign_extend(l, left_layout) as i128;
209213

210214
let (result, oflo) = op(l, r);
211215
// This may be out-of-bounds for the result type, so we have to truncate ourselves.

Diff for: ‎compiler/rustc_middle/src/mir/interpret/error.rs

+6
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,10 @@ pub enum UndefinedBehaviorInfo<'tcx> {
233233
DivisionByZero,
234234
/// Something was "remainded" by 0 (x % 0).
235235
RemainderByZero,
236+
/// Signed division overflowed (INT_MIN / -1).
237+
DivisionOverflow,
238+
/// Signed remainder overflowed (INT_MIN % -1).
239+
RemainderOverflow,
236240
/// Overflowing inbounds pointer arithmetic.
237241
PointerArithOverflow,
238242
/// Invalid metadata in a wide pointer (using `str` to avoid allocations).
@@ -310,6 +314,8 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
310314
}
311315
DivisionByZero => write!(f, "dividing by zero"),
312316
RemainderByZero => write!(f, "calculating the remainder with a divisor of zero"),
317+
DivisionOverflow => write!(f, "overflow in signed division (dividing MIN by -1)"),
318+
RemainderOverflow => write!(f, "overflow in signed remainder (dividing MIN by -1)"),
313319
PointerArithOverflow => write!(f, "overflowing in-bounds pointer arithmetic"),
314320
InvalidMeta(msg) => write!(f, "invalid metadata in wide pointer: {}", msg),
315321
InvalidVtableDropFn(sig) => write!(

Diff for: ‎compiler/rustc_mir_transform/src/const_prop.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -1196,12 +1196,21 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
11961196
AssertKind::RemainderByZero(op) => {
11971197
Some(AssertKind::RemainderByZero(eval_to_int(op)))
11981198
}
1199+
AssertKind::Overflow(bin_op @ (BinOp::Div | BinOp::Rem), op1, op2) => {
1200+
// Division overflow is *UB* in the MIR, and different than the
1201+
// other overflow checks.
1202+
Some(AssertKind::Overflow(
1203+
*bin_op,
1204+
eval_to_int(op1),
1205+
eval_to_int(op2),
1206+
))
1207+
}
11991208
AssertKind::BoundsCheck { ref len, ref index } => {
12001209
let len = eval_to_int(len);
12011210
let index = eval_to_int(index);
12021211
Some(AssertKind::BoundsCheck { len, index })
12031212
}
1204-
// Overflow is are already covered by checks on the binary operators.
1213+
// Remaining overflow errors are already covered by checks on the binary operators.
12051214
AssertKind::Overflow(..) | AssertKind::OverflowNeg(_) => None,
12061215
// Need proper const propagator for these.
12071216
_ => None,

Diff for: ‎library/core/tests/num/wrapping.rs

+10
Original file line numberDiff line numberDiff line change
@@ -308,3 +308,13 @@ fn wrapping_int_api() {
308308
}
309309
}
310310
}
311+
312+
#[test]
313+
fn wrapping_const() {
314+
// Specifically the wrapping behavior of division and remainder is subtle,
315+
// see https://github.com/rust-lang/rust/pull/94512.
316+
const _: () = {
317+
assert!(i32::MIN.wrapping_div(-1) == i32::MIN);
318+
assert!(i32::MIN.wrapping_rem(-1) == 0);
319+
};
320+
}

Diff for: ‎src/test/ui/consts/const-int-unchecked.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ error[E0080]: evaluation of constant value failed
266266
--> $DIR/const-int-unchecked.rs:134:25
267267
|
268268
LL | const _: i32 = unsafe { std::intrinsics::unchecked_div(i32::MIN, -1) };
269-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_div`
269+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow in signed division (dividing MIN by -1)
270270

271271
error[E0080]: evaluation of constant value failed
272272
--> $DIR/const-int-unchecked.rs:137:25
@@ -278,7 +278,7 @@ error[E0080]: evaluation of constant value failed
278278
--> $DIR/const-int-unchecked.rs:139:25
279279
|
280280
LL | const _: i32 = unsafe { std::intrinsics::unchecked_rem(i32::MIN, -1) };
281-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow executing `unchecked_rem`
281+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflow in signed remainder (dividing MIN by -1)
282282

283283
error[E0080]: evaluation of constant value failed
284284
--> $DIR/const-int-unchecked.rs:144:25

Diff for: ‎src/test/ui/numbers-arithmetic/issue-8460-const.noopt.stderr

+13-15
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,36 @@
1-
error: this arithmetic operation will overflow
1+
error: this operation will panic at runtime
22
--> $DIR/issue-8460-const.rs:13:36
33
|
44
LL | assert!(thread::spawn(move|| { isize::MIN / -1; }).join().is_err());
55
| ^^^^^^^^^^^^^^^ attempt to compute `isize::MIN / -1_isize`, which would overflow
66
|
7-
= note: `#[deny(arithmetic_overflow)]` on by default
7+
= note: `#[deny(unconditional_panic)]` on by default
88

9-
error: this arithmetic operation will overflow
9+
error: this operation will panic at runtime
1010
--> $DIR/issue-8460-const.rs:15:36
1111
|
1212
LL | assert!(thread::spawn(move|| { i8::MIN / -1; }).join().is_err());
1313
| ^^^^^^^^^^^^ attempt to compute `i8::MIN / -1_i8`, which would overflow
1414

15-
error: this arithmetic operation will overflow
15+
error: this operation will panic at runtime
1616
--> $DIR/issue-8460-const.rs:17:36
1717
|
1818
LL | assert!(thread::spawn(move|| { i16::MIN / -1; }).join().is_err());
1919
| ^^^^^^^^^^^^^ attempt to compute `i16::MIN / -1_i16`, which would overflow
2020

21-
error: this arithmetic operation will overflow
21+
error: this operation will panic at runtime
2222
--> $DIR/issue-8460-const.rs:19:36
2323
|
2424
LL | assert!(thread::spawn(move|| { i32::MIN / -1; }).join().is_err());
2525
| ^^^^^^^^^^^^^ attempt to compute `i32::MIN / -1_i32`, which would overflow
2626

27-
error: this arithmetic operation will overflow
27+
error: this operation will panic at runtime
2828
--> $DIR/issue-8460-const.rs:21:36
2929
|
3030
LL | assert!(thread::spawn(move|| { i64::MIN / -1; }).join().is_err());
3131
| ^^^^^^^^^^^^^ attempt to compute `i64::MIN / -1_i64`, which would overflow
3232

33-
error: this arithmetic operation will overflow
33+
error: this operation will panic at runtime
3434
--> $DIR/issue-8460-const.rs:23:36
3535
|
3636
LL | assert!(thread::spawn(move|| { i128::MIN / -1; }).join().is_err());
@@ -41,8 +41,6 @@ error: this operation will panic at runtime
4141
|
4242
LL | assert!(thread::spawn(move|| { 1isize / 0; }).join().is_err());
4343
| ^^^^^^^^^^ attempt to divide `1_isize` by zero
44-
|
45-
= note: `#[deny(unconditional_panic)]` on by default
4644

4745
error: this operation will panic at runtime
4846
--> $DIR/issue-8460-const.rs:27:36
@@ -74,37 +72,37 @@ error: this operation will panic at runtime
7472
LL | assert!(thread::spawn(move|| { 1i128 / 0; }).join().is_err());
7573
| ^^^^^^^^^ attempt to divide `1_i128` by zero
7674

77-
error: this arithmetic operation will overflow
75+
error: this operation will panic at runtime
7876
--> $DIR/issue-8460-const.rs:37:36
7977
|
8078
LL | assert!(thread::spawn(move|| { isize::MIN % -1; }).join().is_err());
8179
| ^^^^^^^^^^^^^^^ attempt to compute the remainder of `isize::MIN % -1_isize`, which would overflow
8280

83-
error: this arithmetic operation will overflow
81+
error: this operation will panic at runtime
8482
--> $DIR/issue-8460-const.rs:39:36
8583
|
8684
LL | assert!(thread::spawn(move|| { i8::MIN % -1; }).join().is_err());
8785
| ^^^^^^^^^^^^ attempt to compute the remainder of `i8::MIN % -1_i8`, which would overflow
8886

89-
error: this arithmetic operation will overflow
87+
error: this operation will panic at runtime
9088
--> $DIR/issue-8460-const.rs:41:36
9189
|
9290
LL | assert!(thread::spawn(move|| { i16::MIN % -1; }).join().is_err());
9391
| ^^^^^^^^^^^^^ attempt to compute the remainder of `i16::MIN % -1_i16`, which would overflow
9492

95-
error: this arithmetic operation will overflow
93+
error: this operation will panic at runtime
9694
--> $DIR/issue-8460-const.rs:43:36
9795
|
9896
LL | assert!(thread::spawn(move|| { i32::MIN % -1; }).join().is_err());
9997
| ^^^^^^^^^^^^^ attempt to compute the remainder of `i32::MIN % -1_i32`, which would overflow
10098

101-
error: this arithmetic operation will overflow
99+
error: this operation will panic at runtime
102100
--> $DIR/issue-8460-const.rs:45:36
103101
|
104102
LL | assert!(thread::spawn(move|| { i64::MIN % -1; }).join().is_err());
105103
| ^^^^^^^^^^^^^ attempt to compute the remainder of `i64::MIN % -1_i64`, which would overflow
106104

107-
error: this arithmetic operation will overflow
105+
error: this operation will panic at runtime
108106
--> $DIR/issue-8460-const.rs:47:36
109107
|
110108
LL | assert!(thread::spawn(move|| { i128::MIN % -1; }).join().is_err());

Diff for: ‎src/test/ui/numbers-arithmetic/issue-8460-const.opt.stderr

+13-15
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,36 @@
1-
error: this arithmetic operation will overflow
1+
error: this operation will panic at runtime
22
--> $DIR/issue-8460-const.rs:13:36
33
|
44
LL | assert!(thread::spawn(move|| { isize::MIN / -1; }).join().is_err());
55
| ^^^^^^^^^^^^^^^ attempt to compute `isize::MIN / -1_isize`, which would overflow
66
|
7-
= note: `#[deny(arithmetic_overflow)]` on by default
7+
= note: `#[deny(unconditional_panic)]` on by default
88

9-
error: this arithmetic operation will overflow
9+
error: this operation will panic at runtime
1010
--> $DIR/issue-8460-const.rs:15:36
1111
|
1212
LL | assert!(thread::spawn(move|| { i8::MIN / -1; }).join().is_err());
1313
| ^^^^^^^^^^^^ attempt to compute `i8::MIN / -1_i8`, which would overflow
1414

15-
error: this arithmetic operation will overflow
15+
error: this operation will panic at runtime
1616
--> $DIR/issue-8460-const.rs:17:36
1717
|
1818
LL | assert!(thread::spawn(move|| { i16::MIN / -1; }).join().is_err());
1919
| ^^^^^^^^^^^^^ attempt to compute `i16::MIN / -1_i16`, which would overflow
2020

21-
error: this arithmetic operation will overflow
21+
error: this operation will panic at runtime
2222
--> $DIR/issue-8460-const.rs:19:36
2323
|
2424
LL | assert!(thread::spawn(move|| { i32::MIN / -1; }).join().is_err());
2525
| ^^^^^^^^^^^^^ attempt to compute `i32::MIN / -1_i32`, which would overflow
2626

27-
error: this arithmetic operation will overflow
27+
error: this operation will panic at runtime
2828
--> $DIR/issue-8460-const.rs:21:36
2929
|
3030
LL | assert!(thread::spawn(move|| { i64::MIN / -1; }).join().is_err());
3131
| ^^^^^^^^^^^^^ attempt to compute `i64::MIN / -1_i64`, which would overflow
3232

33-
error: this arithmetic operation will overflow
33+
error: this operation will panic at runtime
3434
--> $DIR/issue-8460-const.rs:23:36
3535
|
3636
LL | assert!(thread::spawn(move|| { i128::MIN / -1; }).join().is_err());
@@ -41,8 +41,6 @@ error: this operation will panic at runtime
4141
|
4242
LL | assert!(thread::spawn(move|| { 1isize / 0; }).join().is_err());
4343
| ^^^^^^^^^^ attempt to divide `1_isize` by zero
44-
|
45-
= note: `#[deny(unconditional_panic)]` on by default
4644

4745
error: this operation will panic at runtime
4846
--> $DIR/issue-8460-const.rs:27:36
@@ -74,37 +72,37 @@ error: this operation will panic at runtime
7472
LL | assert!(thread::spawn(move|| { 1i128 / 0; }).join().is_err());
7573
| ^^^^^^^^^ attempt to divide `1_i128` by zero
7674

77-
error: this arithmetic operation will overflow
75+
error: this operation will panic at runtime
7876
--> $DIR/issue-8460-const.rs:37:36
7977
|
8078
LL | assert!(thread::spawn(move|| { isize::MIN % -1; }).join().is_err());
8179
| ^^^^^^^^^^^^^^^ attempt to compute the remainder of `isize::MIN % -1_isize`, which would overflow
8280

83-
error: this arithmetic operation will overflow
81+
error: this operation will panic at runtime
8482
--> $DIR/issue-8460-const.rs:39:36
8583
|
8684
LL | assert!(thread::spawn(move|| { i8::MIN % -1; }).join().is_err());
8785
| ^^^^^^^^^^^^ attempt to compute the remainder of `i8::MIN % -1_i8`, which would overflow
8886

89-
error: this arithmetic operation will overflow
87+
error: this operation will panic at runtime
9088
--> $DIR/issue-8460-const.rs:41:36
9189
|
9290
LL | assert!(thread::spawn(move|| { i16::MIN % -1; }).join().is_err());
9391
| ^^^^^^^^^^^^^ attempt to compute the remainder of `i16::MIN % -1_i16`, which would overflow
9492

95-
error: this arithmetic operation will overflow
93+
error: this operation will panic at runtime
9694
--> $DIR/issue-8460-const.rs:43:36
9795
|
9896
LL | assert!(thread::spawn(move|| { i32::MIN % -1; }).join().is_err());
9997
| ^^^^^^^^^^^^^ attempt to compute the remainder of `i32::MIN % -1_i32`, which would overflow
10098

101-
error: this arithmetic operation will overflow
99+
error: this operation will panic at runtime
102100
--> $DIR/issue-8460-const.rs:45:36
103101
|
104102
LL | assert!(thread::spawn(move|| { i64::MIN % -1; }).join().is_err());
105103
| ^^^^^^^^^^^^^ attempt to compute the remainder of `i64::MIN % -1_i64`, which would overflow
106104

107-
error: this arithmetic operation will overflow
105+
error: this operation will panic at runtime
108106
--> $DIR/issue-8460-const.rs:47:36
109107
|
110108
LL | assert!(thread::spawn(move|| { i128::MIN % -1; }).join().is_err());

0 commit comments

Comments
 (0)