From d020196dc096d255caa72f04a6da5e8e41d2b717 Mon Sep 17 00:00:00 2001 From: cocodery Date: Fri, 22 Dec 2023 20:17:43 +0800 Subject: [PATCH 1/2] Add check for illegal accessing known length array with a constant index --- clippy_lints/src/indexing_slicing.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs index 0ae03d101ab5..391db0b0df72 100644 --- a/clippy_lints/src/indexing_slicing.rs +++ b/clippy_lints/src/indexing_slicing.rs @@ -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; } From 18eb406776a0d977c633283a15a970eaec64ddbb Mon Sep 17 00:00:00 2001 From: cocodery Date: Fri, 22 Dec 2023 20:31:48 +0800 Subject: [PATCH 2/2] Add test for indexing_slicing_index and modify related test --- tests/ui-toml/suppress_lint_in_const/test.rs | 3 +- .../suppress_lint_in_const/test.stderr | 18 +++--- tests/ui/crashes/ice-5497.rs | 2 + tests/ui/crashes/ice-5497.stderr | 2 +- tests/ui/get_unwrap.fixed | 3 +- tests/ui/get_unwrap.rs | 3 +- tests/ui/get_unwrap.stderr | 62 +++++++++---------- tests/ui/indexing_slicing_index.rs | 3 + tests/ui/indexing_slicing_index.stderr | 35 ++++++++++- 9 files changed, 86 insertions(+), 45 deletions(-) diff --git a/tests/ui-toml/suppress_lint_in_const/test.rs b/tests/ui-toml/suppress_lint_in_const/test.rs index 17c1b03d88c6..3edb3a10b769 100644 --- a/tests/ui-toml/suppress_lint_in_const/test.rs +++ b/tests/ui-toml/suppress_lint_in_const/test.rs @@ -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]; diff --git a/tests/ui-toml/suppress_lint_in_const/test.stderr b/tests/ui-toml/suppress_lint_in_const/test.stderr index f8ace7995939..84e7eff45573 100644 --- a/tests/ui-toml/suppress_lint_in_const/test.stderr +++ b/tests/ui-toml/suppress_lint_in_const/test.stderr @@ -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]; | ^^^^^^^^ @@ -21,7 +21,7 @@ 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]; | ^^^^ @@ -29,7 +29,7 @@ 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]; | ^^^^^ @@ -37,7 +37,7 @@ 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]; | ^^^^^^^^^ @@ -45,7 +45,7 @@ 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]; | ^^^^ @@ -53,7 +53,7 @@ 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]; | ^^^^ @@ -61,7 +61,7 @@ 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 diff --git a/tests/ui/crashes/ice-5497.rs b/tests/ui/crashes/ice-5497.rs index f77f691c1927..fe8d640c4707 100644 --- a/tests/ui/crashes/ice-5497.rs +++ b/tests/ui/crashes/ice-5497.rs @@ -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 { diff --git a/tests/ui/crashes/ice-5497.stderr b/tests/ui/crashes/ice-5497.stderr index ee69f3379b6a..3efaf05827e8 100644 --- a/tests/ui/crashes/ice-5497.stderr +++ b/tests/ui/crashes/ice-5497.stderr @@ -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 diff --git a/tests/ui/get_unwrap.fixed b/tests/ui/get_unwrap.fixed index d5a4309db594..62beb195939c 100644 --- a/tests/ui/get_unwrap.fixed +++ b/tests/ui/get_unwrap.fixed @@ -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)] diff --git a/tests/ui/get_unwrap.rs b/tests/ui/get_unwrap.rs index 5a9ad204f702..1e09ff5c67e5 100644 --- a/tests/ui/get_unwrap.rs +++ b/tests/ui/get_unwrap.rs @@ -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)] diff --git a/tests/ui/get_unwrap.stderr b/tests/ui/get_unwrap.stderr index 384860ea116e..700f3cfec4e0 100644 --- a/tests/ui/get_unwrap.stderr +++ b/tests/ui/get_unwrap.stderr @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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]` diff --git a/tests/ui/indexing_slicing_index.rs b/tests/ui/indexing_slicing_index.rs index f0da5dfc60bd..1ac0bb11014a 100644 --- a/tests/ui/indexing_slicing_index.rs +++ b/tests/ui/indexing_slicing_index.rs @@ -74,4 +74,7 @@ fn main() { //~^ ERROR: indexing may panic v[M]; //~^ ERROR: indexing may panic + + let slice = &x; + let _ = x[4]; } diff --git a/tests/ui/indexing_slicing_index.stderr b/tests/ui/indexing_slicing_index.stderr index 1c34875d2b8a..6d64fa1e6cf8 100644 --- a/tests/ui/indexing_slicing_index.stderr +++ b/tests/ui/indexing_slicing_index.stderr @@ -38,6 +38,21 @@ LL | x[index]; | = help: consider using `.get(n)` or `.get_mut(n)` instead +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:32:5 + | +LL | x[4]; + | ^^^^ + | + = note: `-D clippy::out-of-bounds-indexing` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::out_of_bounds_indexing)]` + +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:34:5 + | +LL | x[1 << 3]; + | ^^^^^^^^^ + error: indexing may panic --> $DIR/indexing_slicing_index.rs:45:14 | @@ -56,6 +71,12 @@ LL | const { &ARR[idx4()] }; = help: consider using `.get(n)` or `.get_mut(n)` instead = note: the suggestion might not be applicable in constant blocks +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:55:5 + | +LL | y[4]; + | ^^^^ + error: indexing may panic --> $DIR/indexing_slicing_index.rs:58:5 | @@ -80,6 +101,12 @@ LL | v[1 << 3]; | = help: consider using `.get(n)` or `.get_mut(n)` instead +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:70:5 + | +LL | x[N]; + | ^^^^ + error: indexing may panic --> $DIR/indexing_slicing_index.rs:73:5 | @@ -96,12 +123,18 @@ LL | v[M]; | = help: consider using `.get(n)` or `.get_mut(n)` instead +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:79:13 + | +LL | let _ = x[4]; + | ^^^^ + error[E0080]: evaluation of constant value failed --> $DIR/indexing_slicing_index.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 -error: aborting due to 12 previous errors +error: aborting due to 17 previous errors For more information about this error, try `rustc --explain E0080`.