Skip to content

Commit

Permalink
Lint against indexing and slicing
Browse files Browse the repository at this point in the history
This can be useful to prevent panics in a codebase. ATM it is a pedantic
lint, but in the future it should be added to the restricions group.
  • Loading branch information
aochagavia committed Mar 11, 2016
1 parent 84587d1 commit 87ef5f4
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 11 deletions.
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -11,7 +11,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
[Jump to link with clippy-service](#link-with-clippy-service)

##Lints
There are 133 lints included in this crate:
There are 134 lints included in this crate:

name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -58,6 +58,7 @@ name
[if_not_else](https://github.com/Manishearth/rust-clippy/wiki#if_not_else) | warn | finds if branches that could be swapped so no negation operation is necessary on the condition
[if_same_then_else](https://github.com/Manishearth/rust-clippy/wiki#if_same_then_else) | warn | if with the same *then* and *else* blocks
[ifs_same_cond](https://github.com/Manishearth/rust-clippy/wiki#ifs_same_cond) | warn | consecutive `ifs` with the same condition
[indexing_slicing](https://github.com/Manishearth/rust-clippy/wiki#indexing_slicing) | allow | indexing/slicing usage
[ineffective_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask) | warn | expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`
[inline_always](https://github.com/Manishearth/rust-clippy/wiki#inline_always) | warn | `#[inline(always)]` is a bad idea in most cases
[invalid_regex](https://github.com/Manishearth/rust-clippy/wiki#invalid_regex) | deny | finds invalid regular expressions in `Regex::new(_)` invocations
Expand Down
110 changes: 103 additions & 7 deletions src/array_indexing.rs
Expand Up @@ -3,7 +3,8 @@ use rustc::middle::const_eval::EvalHint::ExprTypeChecked;
use rustc::middle::const_eval::{eval_const_expr_partial, ConstVal};
use rustc::middle::ty::TyArray;
use rustc_front::hir::*;
use utils::span_lint;
use syntax::ast::RangeLimits;
use utils;

/// **What it does:** Check for out of bounds array indexing with a constant index.
///
Expand All @@ -17,35 +18,130 @@ use utils::span_lint;
/// let x = [1,2,3,4];
/// ...
/// x[9];
/// &x[2..9];
/// ```
declare_lint! {
pub OUT_OF_BOUNDS_INDEXING,
Deny,
"out of bound constant indexing"
}

/// **What it does:** Check for usage of indexing or slicing.
///
/// **Why is this bad?** Usually, this can be safely allowed. However,
/// in some domains such as kernel development, a panic can cause the
/// whole operating system to crash.
///
/// **Known problems:** Hopefully none.
///
/// **Example:**
///
/// ```
/// ...
/// x[2];
/// &x[0..2];
/// ```
declare_lint! {
pub INDEXING_SLICING,
Allow,
"indexing/slicing usage"
}

#[derive(Copy,Clone)]
pub struct ArrayIndexing;

impl LintPass for ArrayIndexing {
fn get_lints(&self) -> LintArray {
lint_array!(OUT_OF_BOUNDS_INDEXING)
lint_array!(INDEXING_SLICING, OUT_OF_BOUNDS_INDEXING)
}
}

impl LateLintPass for ArrayIndexing {
fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
if let ExprIndex(ref array, ref index) = e.node {
// Array with known size can be checked statically
let ty = cx.tcx.expr_ty(array);

if let TyArray(_, size) = ty.sty {
let index = eval_const_expr_partial(cx.tcx, &index, ExprTypeChecked, None);
if let Ok(ConstVal::Uint(index)) = index {
if size as u64 <= index {
span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "const index-expr is out of bounds");
let size = size as u64;

// Index is a constant uint
let const_index = eval_const_expr_partial(cx.tcx, &index, ExprTypeChecked, None);
if let Ok(ConstVal::Uint(const_index)) = const_index {
if size <= const_index {
utils::span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "const index is out of bounds");
utils::span_lint(cx, INDEXING_SLICING, e.span, "indexing may panic");
} else {
// Index is within bounds
return;
}
}

// Index is a constant range
if let Some(range) = utils::unsugar_range(index) {
let start = range.start.map(|start|
eval_const_expr_partial(cx.tcx, start, ExprTypeChecked, None));
let end = range.end.map(|end|
eval_const_expr_partial(cx.tcx, end, ExprTypeChecked, None));

if let Some((start, end)) = to_const_range(start, end, range.limits, size) {
if start >= size && end >= size {
utils::span_lint(cx,
OUT_OF_BOUNDS_INDEXING,
e.span,
"range is out of bounds");
utils::span_lint(cx,
INDEXING_SLICING,
e.span,
"slicing may panic");
} else {
// Range is within bounds
return;
}
}
}
}

if let Some(range) = utils::unsugar_range(index) {
// Full ranges are always valid
if range.start.is_none() && range.end.is_none() {
return;
}

// Impossible to know if indexing or slicing is correct
utils::span_lint(cx, INDEXING_SLICING, e.span, "slicing may panic");
} else {
utils::span_lint(cx, INDEXING_SLICING, e.span, "indexing may panic");
}
}
}
}

/// Returns an option containing a tuple with the start and end (exclusive) of the range
///
/// Note: we assume the start and the end of the range are unsigned, since array slicing
/// works only on usize
fn to_const_range<T>(start: Option<Result<ConstVal, T>>,
end: Option<Result<ConstVal, T>>,
limits: RangeLimits,
array_size: u64)
-> Option<(u64, u64)> {
let start = match start {
Some(Ok(ConstVal::Uint(x))) => x,
Some(_) => return None,
None => 0,
};

let end = match end {
Some(Ok(ConstVal::Uint(x))) => {
if limits == RangeLimits::Closed {
x
} else {
x - 1
}
}
Some(_) => return None,
None => array_size - 1,
};

Some((start, end))
}
1 change: 1 addition & 0 deletions src/lib.rs
Expand Up @@ -181,6 +181,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box new_without_default::NewWithoutDefault);

reg.register_lint_group("clippy_pedantic", vec![
array_indexing::INDEXING_SLICING,
enum_glob_use::ENUM_GLOB_USE,
matches::SINGLE_MATCH_ELSE,
methods::OPTION_UNWRAP_USED,
Expand Down
25 changes: 22 additions & 3 deletions tests/compile-fail/array_indexing.rs
@@ -1,13 +1,32 @@
#![feature(plugin)]
#![feature(inclusive_range_syntax, plugin)]
#![plugin(clippy)]

#![deny(indexing_slicing)]
#![deny(out_of_bounds_indexing)]
#![allow(no_effect)]

fn main() {
let x = [1,2,3,4];
x[0];
x[3];
x[4]; //~ERROR: const index-expr is out of bounds
x[1 << 3]; //~ERROR: const index-expr is out of bounds
x[4]; //~ERROR: indexing may panic
//~^ ERROR: const index is out of bounds
x[1 << 3]; //~ERROR: indexing may panic
//~^ ERROR: const index is out of bounds
&x[1..5]; //~ERROR: slicing may panic
//~^ ERROR: range is out of bounds
&x[0..3];
&x[0...4]; //~ERROR: slicing may panic
//~^ ERROR: range is out of bounds
&x[..];
&x[1..];
&x[..4];
&x[..5]; //~ERROR: slicing may panic
//~^ ERROR: range is out of bounds

let y = &x;
y[0]; //~ERROR: indexing may panic
&y[1..2]; //~ERROR: slicing may panic
&y[..];
&y[0...4]; //~ERROR: slicing may panic
}

0 comments on commit 87ef5f4

Please sign in to comment.