Skip to content

Commit

Permalink
integer_arithmetic: check all/only overflowing ops
Browse files Browse the repository at this point in the history
Make the `integer_arithmetic` lint detect all the operations that are
defined as being capable of overflow in the Rust Reference.

Changes:

* Disallow bit shifting (`<<`, `>>`). They overflow if the RHS exceeds
  the bit width.
* Allow unsigned division and remainder (they cannot overflow).
  • Loading branch information
michaelsproul committed Apr 7, 2020
1 parent 7907abe commit 69f554d
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 20 deletions.
22 changes: 16 additions & 6 deletions clippy_lints/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ use crate::consts::constant_simple;
use crate::utils::span_lint;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;

declare_clippy_lint! {
/// **What it does:** Checks for plain integer arithmetic.
/// **What it does:** Checks for integer arithmetic operations that could overflow.
///
/// **Why is this bad?** This is only checked against overflow in debug builds.
/// In some applications one wants explicitly checked, wrapping or saturating
/// Specifically, checks for any operators (`+`, `-`, `*`, `<<`, etc) which are capable
/// of overflowing according to the [Rust
/// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow).
/// No bounds analysis or sophisticated reasoning is attempted.
///
/// **Why is this bad?** Integer overflow will trigger a panic in debug builds or will wrap in
/// release mode. In some applications one wants explicitly checked, wrapping or saturating
/// arithmetic.
///
/// **Known problems:** None.
Expand All @@ -21,7 +27,7 @@ declare_clippy_lint! {
/// ```
pub INTEGER_ARITHMETIC,
restriction,
"any integer arithmetic statement"
"any integer arithmetic expression which could overflow"
}

declare_clippy_lint! {
Expand Down Expand Up @@ -71,8 +77,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
| hir::BinOpKind::BitAnd
| hir::BinOpKind::BitOr
| hir::BinOpKind::BitXor
| hir::BinOpKind::Shl
| hir::BinOpKind::Shr
| hir::BinOpKind::Eq
| hir::BinOpKind::Lt
| hir::BinOpKind::Le
Expand All @@ -84,6 +88,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {

let (l_ty, r_ty) = (cx.tables.expr_ty(l), cx.tables.expr_ty(r));
if l_ty.peel_refs().is_integral() && r_ty.peel_refs().is_integral() {
// Allow unsigned integer division and modulo (it can't overflow)
if let ty::Uint(_) = l_ty.kind {
if op.node == hir::BinOpKind::Div || op.node == hir::BinOpKind::Rem {
return;
}
}
span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
self.expr_span = Some(expr.span);
} else if l_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() {
Expand Down
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
Lint {
name: "integer_arithmetic",
group: "restriction",
desc: "any integer arithmetic statement",
desc: "any integer arithmetic expression which could overflow",
deprecation: None,
module: "arithmetic",
},
Expand Down
46 changes: 40 additions & 6 deletions tests/ui/integer_arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ fn main() {
i / 2; // no error, this is part of the expression in the preceding line
i - 2 + 2 - i;
-i;
i >> 1;
i << 1;

// no error, overflows are checked by `overflowing_literals`
-1;
Expand All @@ -26,18 +28,16 @@ fn main() {
i & 1; // no wrapping
i | 1;
i ^ 1;
i >> 1;
i << 1;

i += 1;
i -= 1;
i *= 2;
i /= 2;
i %= 2;

// no errors
i <<= 3;
i >>= 2;

// no errors
i |= 1;
i &= 1;
i ^= i;
Expand Down Expand Up @@ -73,8 +73,6 @@ fn main() {
1 + 1
};
}


}

// warn on references as well! (#5328)
Expand All @@ -84,6 +82,42 @@ pub fn int_arith_ref() {
&3 + &1;
}

pub fn unsigned() {
let mut i = 1000u64;

// should error
i + 2;
i - 2;
i * 2;
i << 2;
i >> i;

// should error
i += 2;
i -= 2;
i *= i;
i <<= 2;
i >>= 2;

// should not error
i | 3;
i & 2;
i ^ 4;
i / 2;
i % 5;

// should not error
i |= 1;
i &= 1;
i ^= i;
i /= i;
i %= 6;

// should not error
!i;
!(!i);
}

pub fn foo(x: &i32) -> i32 {
let a = 5;
a + x
Expand Down
98 changes: 91 additions & 7 deletions tests/ui/integer_arithmetic.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@ error: integer arithmetic detected
LL | -i;
| ^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:21:5
|
LL | i >> 1;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:22:5
|
LL | i << 1;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:32:5
|
Expand Down Expand Up @@ -62,46 +74,118 @@ LL | i %= 2;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:82:5
--> $DIR/integer_arithmetic.rs:37:5
|
LL | i <<= 3;
| ^^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:38:5
|
LL | i >>= 2;
| ^^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:80:5
|
LL | 3 + &1;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:83:5
--> $DIR/integer_arithmetic.rs:81:5
|
LL | &3 + 1;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:84:5
--> $DIR/integer_arithmetic.rs:82:5
|
LL | &3 + &1;
| ^^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:89:5
|
LL | a + x
LL | i + 2;
| ^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:90:5
|
LL | i - 2;
| ^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:91:5
|
LL | i * 2;
| ^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:92:5
|
LL | i << 2;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:93:5
|
LL | i >> i;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:96:5
|
LL | i += 2;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:97:5
|
LL | i -= 2;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:98:5
|
LL | i *= i;
| ^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:99:5
|
LL | i <<= 2;
| ^^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:100:5
|
LL | i >>= 2;
| ^^^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:123:5
|
LL | a + x
| ^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:127:5
|
LL | x + y
| ^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:97:5
--> $DIR/integer_arithmetic.rs:131:5
|
LL | x + y
| ^^^^^

error: integer arithmetic detected
--> $DIR/integer_arithmetic.rs:101:5
--> $DIR/integer_arithmetic.rs:135:5
|
LL | (&x + &y)
| ^^^^^^^^^

error: aborting due to 17 previous errors
error: aborting due to 31 previous errors

0 comments on commit 69f554d

Please sign in to comment.