Skip to content

Commit

Permalink
OUT_OF_BOUNDS_INDEXING fix #3102 false negative
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshMcguigan committed Oct 13, 2018
1 parent 601cc9d commit 0f3345e
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 23 deletions.
70 changes: 48 additions & 22 deletions clippy_lints/src/indexing_slicing.rs
Expand Up @@ -111,17 +111,43 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
// Ranged indexes, i.e. &x[n..m], &x[n..], &x[..n] and &x[..]
if let ty::Array(_, s) = ty.sty {
let size: u128 = s.assert_usize(cx.tcx).unwrap().into();
// Index is a constant range.
if let Some((start, end)) = to_const_range(cx, range, size) {
if start > size || end > size {
utils::span_lint(
cx,
OUT_OF_BOUNDS_INDEXING,
expr.span,
"range is out of bounds",
);
}
return;

match to_const_range(cx, range, size) {
(None, None) => {},
(Some(start), None) => {
if start > size {
utils::span_lint(
cx,
OUT_OF_BOUNDS_INDEXING,
expr.span,
"range is out of bounds",
);
return;
}
},
(None, Some(end)) => {
if end > size {
utils::span_lint(
cx,
OUT_OF_BOUNDS_INDEXING,
expr.span,
"range is out of bounds",
);
return;
}
},
(Some(start), Some(end)) => {
if start > size || end > size {
utils::span_lint(
cx,
OUT_OF_BOUNDS_INDEXING,
expr.span,
"range is out of bounds",
);
}
// early return because both start and end are constant
return;
},
}
}

Expand Down Expand Up @@ -161,34 +187,34 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IndexingSlicing {
}
}

/// Returns an option containing a tuple with the start and end (exclusive) of
/// the range.
/// Returns a tuple of options with the start and end (exclusive) values of
/// the range. If the start or end is not constant, None is returned.
fn to_const_range<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
range: Range<'_>,
array_size: u128,
) -> Option<(u128, u128)> {
) -> (Option<u128>, Option<u128>) {
let s = range
.start
.map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c));
let start = match s {
Some(Some(Constant::Int(x))) => x,
Some(_) => return None,
None => 0,
Some(Some(Constant::Int(x))) => Some(x),
Some(_) => None,
None => Some(0),
};

let e = range
.end
.map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c));
let end = match e {
Some(Some(Constant::Int(x))) => if range.limits == RangeLimits::Closed {
x + 1
Some(x + 1)
} else {
x
Some(x)
},
Some(_) => return None,
None => array_size,
Some(_) => None,
None => Some(array_size),
};

Some((start, end))
(start, end)
}
5 changes: 5 additions & 0 deletions tests/ui/indexing_slicing.rs
Expand Up @@ -91,4 +91,9 @@ fn main() {
x[M]; // Ok, should not produce stderr.
v[N];
v[M];

// issue 3102
let num = 1;
&x[num..10]; // should trigger out of bounds error
&x[10..num]; // should trigger out of bounds error
}
14 changes: 13 additions & 1 deletion tests/ui/indexing_slicing.stderr
Expand Up @@ -267,5 +267,17 @@ error: indexing may panic.
|
= help: Consider using `.get(n)` or `.get_mut(n)` instead

error: aborting due to 37 previous errors
error: range is out of bounds
--> $DIR/indexing_slicing.rs:97:6
|
97 | &x[num..10]; // should trigger out of bounds error
| ^^^^^^^^^^

error: range is out of bounds
--> $DIR/indexing_slicing.rs:98:6
|
98 | &x[10..num]; // should trigger out of bounds error
| ^^^^^^^^^^

error: aborting due to 39 previous errors

0 comments on commit 0f3345e

Please sign in to comment.