From f203e1ede5d46b0281b2b83250d761350e85deda Mon Sep 17 00:00:00 2001 From: Liam Germain Date: Tue, 24 Sep 2024 09:52:11 -0400 Subject: [PATCH 1/3] Fix false positive invalid null usage warning for `ptr::slice_from_raw_parts_*` functions --- clippy_lints/src/ptr.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 807636bb642..d523ec2edff 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -277,8 +277,6 @@ fn check_invalid_ptr_usage<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | sym::ptr_read_unaligned | sym::ptr_read_volatile | sym::ptr_replace - | sym::ptr_slice_from_raw_parts - | sym::ptr_slice_from_raw_parts_mut | sym::ptr_write | sym::ptr_write_bytes | sym::ptr_write_unaligned From 11363c7bf275a98d87cfa3ec3b0501deb6804147 Mon Sep 17 00:00:00 2001 From: ljgermain Date: Tue, 24 Sep 2024 11:22:26 -0400 Subject: [PATCH 2/3] Fix failing UI test --- tests/ui/invalid_null_ptr_usage.fixed | 5 ---- tests/ui/invalid_null_ptr_usage.rs | 5 ---- tests/ui/invalid_null_ptr_usage.stderr | 36 +++++++------------------- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/tests/ui/invalid_null_ptr_usage.fixed b/tests/ui/invalid_null_ptr_usage.fixed index eeddc2349a1..c8f1858b4f8 100644 --- a/tests/ui/invalid_null_ptr_usage.fixed +++ b/tests/ui/invalid_null_ptr_usage.fixed @@ -25,11 +25,6 @@ fn main() { let _a: A = std::ptr::replace(core::ptr::NonNull::dangling().as_ptr(), A); - let _slice: *const [usize] = std::ptr::slice_from_raw_parts(core::ptr::NonNull::dangling().as_ptr(), 0); - let _slice: *const [usize] = std::ptr::slice_from_raw_parts(core::ptr::NonNull::dangling().as_ptr(), 0); - - let _slice: *const [usize] = std::ptr::slice_from_raw_parts_mut(core::ptr::NonNull::dangling().as_ptr(), 0); - std::ptr::swap::(core::ptr::NonNull::dangling().as_ptr(), &mut A); std::ptr::swap::(&mut A, core::ptr::NonNull::dangling().as_ptr()); diff --git a/tests/ui/invalid_null_ptr_usage.rs b/tests/ui/invalid_null_ptr_usage.rs index 8569b774084..5f3fff8e90d 100644 --- a/tests/ui/invalid_null_ptr_usage.rs +++ b/tests/ui/invalid_null_ptr_usage.rs @@ -25,11 +25,6 @@ fn main() { let _a: A = std::ptr::replace(std::ptr::null_mut(), A); - let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null(), 0); - let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null_mut(), 0); - - let _slice: *const [usize] = std::ptr::slice_from_raw_parts_mut(std::ptr::null_mut(), 0); - std::ptr::swap::(std::ptr::null_mut(), &mut A); std::ptr::swap::(&mut A, std::ptr::null_mut()); diff --git a/tests/ui/invalid_null_ptr_usage.stderr b/tests/ui/invalid_null_ptr_usage.stderr index 54d79ba1aeb..ca7b8c133a5 100644 --- a/tests/ui/invalid_null_ptr_usage.stderr +++ b/tests/ui/invalid_null_ptr_usage.stderr @@ -85,70 +85,52 @@ LL | let _a: A = std::ptr::replace(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:28:69 - | -LL | let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null(), 0); - | ^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` - -error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:29:69 - | -LL | let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null_mut(), 0); - | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` - -error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:31:73 - | -LL | let _slice: *const [usize] = std::ptr::slice_from_raw_parts_mut(std::ptr::null_mut(), 0); - | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` - -error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:33:29 + --> tests/ui/invalid_null_ptr_usage.rs:28:29 | LL | std::ptr::swap::(std::ptr::null_mut(), &mut A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:34:37 + --> tests/ui/invalid_null_ptr_usage.rs:29:37 | LL | std::ptr::swap::(&mut A, std::ptr::null_mut()); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:36:44 + --> tests/ui/invalid_null_ptr_usage.rs:31:44 | LL | std::ptr::swap_nonoverlapping::(std::ptr::null_mut(), &mut A, 0); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:37:52 + --> tests/ui/invalid_null_ptr_usage.rs:32:52 | LL | std::ptr::swap_nonoverlapping::(&mut A, std::ptr::null_mut(), 0); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:39:25 + --> tests/ui/invalid_null_ptr_usage.rs:34:25 | LL | std::ptr::write(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:41:35 + --> tests/ui/invalid_null_ptr_usage.rs:36:35 | LL | std::ptr::write_unaligned(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:43:34 + --> tests/ui/invalid_null_ptr_usage.rs:38:34 | LL | std::ptr::write_volatile(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:45:40 + --> tests/ui/invalid_null_ptr_usage.rs:40:40 | LL | std::ptr::write_bytes::(std::ptr::null_mut(), 42, 0); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` -error: aborting due to 25 previous errors +error: aborting due to 22 previous errors From 31bbe2cee3acb720e4d60d5829db2c292a447c14 Mon Sep 17 00:00:00 2001 From: ljgermain Date: Tue, 24 Sep 2024 15:17:52 -0400 Subject: [PATCH 3/3] Apply review suggestions --- clippy_lints/src/ptr.rs | 6 ++++++ tests/ui/invalid_null_ptr_usage.fixed | 2 ++ tests/ui/invalid_null_ptr_usage.rs | 2 ++ tests/ui/invalid_null_ptr_usage.stderr | 16 ++++++++-------- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index d523ec2edff..bb8a9b6fca8 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -271,6 +271,12 @@ fn check_invalid_ptr_usage<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { && let Some(fun_def_id) = cx.qpath_res(qpath, fun.hir_id).opt_def_id() && let Some(name) = cx.tcx.get_diagnostic_name(fun_def_id) { + // TODO: `ptr_slice_from_raw_parts` and its mutable variant should probably still be linted + // conditionally based on how the return value is used, but not universally like the other + // functions since there are valid uses for null slice pointers. + // + // See: https://github.com/rust-lang/rust-clippy/pull/13452/files#r1773772034 + // `arg` positions where null would cause U.B. let arg_indices: &[_] = match name { sym::ptr_read diff --git a/tests/ui/invalid_null_ptr_usage.fixed b/tests/ui/invalid_null_ptr_usage.fixed index c8f1858b4f8..092e875a255 100644 --- a/tests/ui/invalid_null_ptr_usage.fixed +++ b/tests/ui/invalid_null_ptr_usage.fixed @@ -24,6 +24,8 @@ fn main() { let _a: A = std::ptr::read_volatile(core::ptr::NonNull::dangling().as_ptr()); let _a: A = std::ptr::replace(core::ptr::NonNull::dangling().as_ptr(), A); + let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null_mut(), 0); // shouldn't lint + let _slice: *const [usize] = std::ptr::slice_from_raw_parts_mut(std::ptr::null_mut(), 0); std::ptr::swap::(core::ptr::NonNull::dangling().as_ptr(), &mut A); std::ptr::swap::(&mut A, core::ptr::NonNull::dangling().as_ptr()); diff --git a/tests/ui/invalid_null_ptr_usage.rs b/tests/ui/invalid_null_ptr_usage.rs index 5f3fff8e90d..480b6642a3e 100644 --- a/tests/ui/invalid_null_ptr_usage.rs +++ b/tests/ui/invalid_null_ptr_usage.rs @@ -24,6 +24,8 @@ fn main() { let _a: A = std::ptr::read_volatile(std::ptr::null_mut()); let _a: A = std::ptr::replace(std::ptr::null_mut(), A); + let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null_mut(), 0); // shouldn't lint + let _slice: *const [usize] = std::ptr::slice_from_raw_parts_mut(std::ptr::null_mut(), 0); std::ptr::swap::(std::ptr::null_mut(), &mut A); std::ptr::swap::(&mut A, std::ptr::null_mut()); diff --git a/tests/ui/invalid_null_ptr_usage.stderr b/tests/ui/invalid_null_ptr_usage.stderr index ca7b8c133a5..a0be2c0ad75 100644 --- a/tests/ui/invalid_null_ptr_usage.stderr +++ b/tests/ui/invalid_null_ptr_usage.stderr @@ -85,49 +85,49 @@ LL | let _a: A = std::ptr::replace(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:28:29 + --> tests/ui/invalid_null_ptr_usage.rs:30:29 | LL | std::ptr::swap::(std::ptr::null_mut(), &mut A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:29:37 + --> tests/ui/invalid_null_ptr_usage.rs:31:37 | LL | std::ptr::swap::(&mut A, std::ptr::null_mut()); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:31:44 + --> tests/ui/invalid_null_ptr_usage.rs:33:44 | LL | std::ptr::swap_nonoverlapping::(std::ptr::null_mut(), &mut A, 0); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:32:52 + --> tests/ui/invalid_null_ptr_usage.rs:34:52 | LL | std::ptr::swap_nonoverlapping::(&mut A, std::ptr::null_mut(), 0); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:34:25 + --> tests/ui/invalid_null_ptr_usage.rs:36:25 | LL | std::ptr::write(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:36:35 + --> tests/ui/invalid_null_ptr_usage.rs:38:35 | LL | std::ptr::write_unaligned(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:38:34 + --> tests/ui/invalid_null_ptr_usage.rs:40:34 | LL | std::ptr::write_volatile(std::ptr::null_mut(), A); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()` error: pointer must be non-null - --> tests/ui/invalid_null_ptr_usage.rs:40:40 + --> tests/ui/invalid_null_ptr_usage.rs:42:40 | LL | std::ptr::write_bytes::(std::ptr::null_mut(), 42, 0); | ^^^^^^^^^^^^^^^^^^^^ help: change this to: `core::ptr::NonNull::dangling().as_ptr()`