Skip to content

Commit

Permalink
Auto merge of #11998 - cocodery:fix/issue11762, r=llogiq
Browse files Browse the repository at this point in the history 
Check whether out of bound when access a known length array with a constant index

fixes [Issue#11762](#11762)

Issue#11762 points that `Array references with known length are not flagged when indexed out of bounds`.

To fix this problem, it is needed to add check for `Expr::Index`. We expand this issue include reference and direct accessing a array.

When we access a array with a constant index `off`, and already know the length `size`, if `off >= size`, these code will throw an error, instead rustc's lint checking them or runtime panic happening.

changelog: [`out_of_bound_indexing`]: Add check for illegal accessing known length array with a constant index
  • Loading branch information
@bors
bors committed Dec 22, 2023
2 parents dc97526 + 18eb406 commit e0b25c5
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 46 deletions.
18 changes: 17 additions & 1 deletion clippy_lints/src/indexing_slicing.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,23 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
return;
}
// Index is a constant uint.
if constant(cx, cx.typeck_results(), index).is_some() {
if let Some(constant) = constant(cx, cx.typeck_results(), index) {
// only `usize` index is legal in rust array index
// leave other type to rustc
if let Constant::Int(off) = constant
&& let ty::Uint(utype) = cx.typeck_results().expr_ty(index).kind()
&& *utype == ty::UintTy::Usize
&& let ty::Array(_, s) = ty.kind()
&& let Some(size) = s.try_eval_target_usize(cx.tcx, cx.param_env)
{
// get constant offset and check whether it is in bounds
let off = usize::try_from(off).unwrap();
let size = usize::try_from(size).unwrap();

if off >= size {
span_lint(cx, OUT_OF_BOUNDS_INDEXING, expr.span, "index is out of bounds");
}
}
// Let rustc's `const_err` lint handle constant `usize` indexing on arrays.
return;
}
Expand Down
3 changes: 2 additions & 1 deletion tests/ui-toml/suppress_lint_in_const/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
unconditional_panic,
clippy::no_effect,
clippy::unnecessary_operation,
clippy::useless_vec
clippy::useless_vec,
clippy::out_of_bounds_indexing
)]

const ARR: [i32; 2] = [1, 2];
Expand Down
18 changes: 9 additions & 9 deletions tests/ui-toml/suppress_lint_in_const/test.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
error[E0080]: evaluation of `main::{constant#3}` failed
--> $DIR/test.rs:37:14
--> $DIR/test.rs:38:14
|
LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

note: erroneous constant encountered
--> $DIR/test.rs:37:5
--> $DIR/test.rs:38:5
|
LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
| ^^^^^^^^^^^^^^^^^^^^^^

error: indexing may panic
--> $DIR/test.rs:28:5
--> $DIR/test.rs:29:5
|
LL | x[index];
| ^^^^^^^^
Expand All @@ -21,47 +21,47 @@ LL | x[index];
= help: to override `-D warnings` add `#[allow(clippy::indexing_slicing)]`

error: indexing may panic
--> $DIR/test.rs:46:5
--> $DIR/test.rs:47:5
|
LL | v[0];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:47:5
--> $DIR/test.rs:48:5
|
LL | v[10];
| ^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:48:5
--> $DIR/test.rs:49:5
|
LL | v[1 << 3];
| ^^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:54:5
--> $DIR/test.rs:55:5
|
LL | v[N];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:55:5
--> $DIR/test.rs:56:5
|
LL | v[M];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error[E0080]: evaluation of constant value failed
--> $DIR/test.rs:15:24
--> $DIR/test.rs:16:24
|
LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/crashes/ice-5497.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// reduced from rustc issue-69020-assoc-const-arith-overflow.rs
#![allow(clippy::out_of_bounds_indexing)]

pub fn main() {}

pub trait Foo {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-5497.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this operation will panic at runtime
--> $DIR/ice-5497.rs:9:22
--> $DIR/ice-5497.rs:11:22
|
LL | const OOB: i32 = [1][1] + T::OOB;
| ^^^^^^ index out of bounds: the length is 1 but the index is 1
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/get_unwrap.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
unused_mut,
clippy::from_iter_instead_of_collect,
clippy::get_first,
clippy::useless_vec
clippy::useless_vec,
clippy::out_of_bounds_indexing
)]
#![warn(clippy::unwrap_used)]
#![deny(clippy::get_unwrap)]
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/get_unwrap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
unused_mut,
clippy::from_iter_instead_of_collect,
clippy::get_first,
clippy::useless_vec
clippy::useless_vec,
clippy::out_of_bounds_indexing
)]
#![warn(clippy::unwrap_used)]
#![deny(clippy::get_unwrap)]
Expand Down
62 changes: 31 additions & 31 deletions tests/ui/get_unwrap.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:36:17
--> $DIR/get_unwrap.rs:37:17
|
LL | let _ = boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&boxed_slice[1]`
|
note: the lint level is defined here
--> $DIR/get_unwrap.rs:8:9
--> $DIR/get_unwrap.rs:9:9
|
LL | #![deny(clippy::get_unwrap)]
| ^^^^^^^^^^^^^^^^^^

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:36:17
--> $DIR/get_unwrap.rs:37:17
|
LL | let _ = boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -22,13 +22,13 @@ LL | let _ = boxed_slice.get(1).unwrap();
= help: to override `-D warnings` add `#[allow(clippy::unwrap_used)]`

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:37:17
--> $DIR/get_unwrap.rs:38:17
|
LL | let _ = some_slice.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_slice[0]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:37:17
--> $DIR/get_unwrap.rs:38:17
|
LL | let _ = some_slice.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -37,13 +37,13 @@ LL | let _ = some_slice.get(0).unwrap();
= help: consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:38:17
--> $DIR/get_unwrap.rs:39:17
|
LL | let _ = some_vec.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_vec[0]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:38:17
--> $DIR/get_unwrap.rs:39:17
|
LL | let _ = some_vec.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -52,13 +52,13 @@ LL | let _ = some_vec.get(0).unwrap();
= help: consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a VecDeque. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:39:17
--> $DIR/get_unwrap.rs:40:17
|
LL | let _ = some_vecdeque.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_vecdeque[0]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:39:17
--> $DIR/get_unwrap.rs:40:17
|
LL | let _ = some_vecdeque.get(0).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -67,13 +67,13 @@ LL | let _ = some_vecdeque.get(0).unwrap();
= help: consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a HashMap. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:40:17
--> $DIR/get_unwrap.rs:41:17
|
LL | let _ = some_hashmap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_hashmap[&1]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:40:17
--> $DIR/get_unwrap.rs:41:17
|
LL | let _ = some_hashmap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -82,13 +82,13 @@ LL | let _ = some_hashmap.get(&1).unwrap();
= help: consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:41:17
--> $DIR/get_unwrap.rs:42:17
|
LL | let _ = some_btreemap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_btreemap[&1]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:41:17
--> $DIR/get_unwrap.rs:42:17
|
LL | let _ = some_btreemap.get(&1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -97,13 +97,13 @@ LL | let _ = some_btreemap.get(&1).unwrap();
= help: consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:45:21
--> $DIR/get_unwrap.rs:46:21
|
LL | let _: u8 = *boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `boxed_slice[1]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:45:22
--> $DIR/get_unwrap.rs:46:22
|
LL | let _: u8 = *boxed_slice.get(1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -112,13 +112,13 @@ LL | let _: u8 = *boxed_slice.get(1).unwrap();
= help: consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:50:9
--> $DIR/get_unwrap.rs:51:9
|
LL | *boxed_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `boxed_slice[0]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:50:10
--> $DIR/get_unwrap.rs:51:10
|
LL | *boxed_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -127,13 +127,13 @@ LL | *boxed_slice.get_mut(0).unwrap() = 1;
= help: consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:51:9
--> $DIR/get_unwrap.rs:52:9
|
LL | *some_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_slice[0]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:51:10
--> $DIR/get_unwrap.rs:52:10
|
LL | *some_slice.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -142,13 +142,13 @@ LL | *some_slice.get_mut(0).unwrap() = 1;
= help: consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:52:9
--> $DIR/get_unwrap.rs:53:9
|
LL | *some_vec.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[0]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:52:10
--> $DIR/get_unwrap.rs:53:10
|
LL | *some_vec.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -157,13 +157,13 @@ LL | *some_vec.get_mut(0).unwrap() = 1;
= help: consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:53:9
--> $DIR/get_unwrap.rs:54:9
|
LL | *some_vecdeque.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vecdeque[0]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:53:10
--> $DIR/get_unwrap.rs:54:10
|
LL | *some_vecdeque.get_mut(0).unwrap() = 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -172,13 +172,13 @@ LL | *some_vecdeque.get_mut(0).unwrap() = 1;
= help: consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:65:17
--> $DIR/get_unwrap.rs:66:17
|
LL | let _ = some_vec.get(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[0..1]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:65:17
--> $DIR/get_unwrap.rs:66:17
|
LL | let _ = some_vec.get(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -187,13 +187,13 @@ LL | let _ = some_vec.get(0..1).unwrap().to_vec();
= help: consider using `expect()` to provide a better panic message

error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:66:17
--> $DIR/get_unwrap.rs:67:17
|
LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[0..1]`

error: used `unwrap()` on an `Option` value
--> $DIR/get_unwrap.rs:66:17
--> $DIR/get_unwrap.rs:67:17
|
LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -202,25 +202,25 @@ LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec();
= help: consider using `expect()` to provide a better panic message

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:76:24
--> $DIR/get_unwrap.rs:77:24
|
LL | let _x: &i32 = f.get(1 + 2).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `&f[1 + 2]`

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:79:18
--> $DIR/get_unwrap.rs:80:18
|
LL | let _x = f.get(1 + 2).unwrap().to_string();
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `f[1 + 2]`

error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:82:18
--> $DIR/get_unwrap.rs:83:18
|
LL | let _x = f.get(1 + 2).unwrap().abs();
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `f[1 + 2]`

error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise
--> $DIR/get_unwrap.rs:99:33
--> $DIR/get_unwrap.rs:100:33
|
LL | let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut rest[linidx(j, k) - linidx(i, k) - 1]`
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/indexing_slicing_index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,7 @@ fn main() {
//~^ ERROR: indexing may panic
v[M];
//~^ ERROR: indexing may panic

let slice = &x;
let _ = x[4];
}
Loading

0 comments on commit e0b25c5

Please sign in to comment.