Skip to content

Commit 96519fa

Browse files
boxdotboxdot
committed
Add invalid null pointer usage lint.
1 parent 86fb0e8 commit 96519fa

File tree

8 files changed

+343
-8
lines changed

8 files changed

+343
-8
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,6 +2266,7 @@ Released 2018-09-13
22662266
[`into_iter_on_array`]: https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_on_array
22672267
[`into_iter_on_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#into_iter_on_ref
22682268
[`invalid_atomic_ordering`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_atomic_ordering
2269+
[`invalid_null_ptr_usage`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_null_ptr_usage
22692270
[`invalid_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_ref
22702271
[`invalid_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_regex
22712272
[`invalid_upcast_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_upcast_comparisons

‎clippy_lints/src/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
901901
&pattern_type_mismatch::PATTERN_TYPE_MISMATCH,
902902
&precedence::PRECEDENCE,
903903
&ptr::CMP_NULL,
904+
&ptr::INVALID_NULL_PTR_USAGE,
904905
&ptr::MUT_FROM_REF,
905906
&ptr::PTR_ARG,
906907
&ptr_eq::PTR_EQ,
@@ -1669,6 +1670,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16691670
LintId::of(&partialeq_ne_impl::PARTIALEQ_NE_IMPL),
16701671
LintId::of(&precedence::PRECEDENCE),
16711672
LintId::of(&ptr::CMP_NULL),
1673+
LintId::of(&ptr::INVALID_NULL_PTR_USAGE),
16721674
LintId::of(&ptr::MUT_FROM_REF),
16731675
LintId::of(&ptr::PTR_ARG),
16741676
LintId::of(&ptr_eq::PTR_EQ),
@@ -2007,6 +2009,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
20072009
LintId::of(&non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
20082010
LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
20092011
LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP),
2012+
LintId::of(&ptr::INVALID_NULL_PTR_USAGE),
20102013
LintId::of(&ptr::MUT_FROM_REF),
20112014
LintId::of(&ranges::REVERSED_EMPTY_RANGES),
20122015
LintId::of(&regex::INVALID_REGEX),

‎clippy_lints/src/ptr.rs‎

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_the
44
use clippy_utils::ptr::get_spans;
55
use clippy_utils::source::snippet_opt;
66
use clippy_utils::ty::{is_type_diagnostic_item, match_type, walk_ptrs_hir_ty};
7-
use clippy_utils::{is_allowed, match_qpath, paths};
7+
use clippy_utils::{is_allowed, match_function_call, match_qpath, paths};
88
use if_chain::if_chain;
99
use rustc_errors::Applicability;
1010
use rustc_hir::{
@@ -94,7 +94,7 @@ declare_clippy_lint! {
9494
/// ```
9595
pub CMP_NULL,
9696
style,
97-
"comparing a pointer to a null pointer, suggesting to use `.is_null()` instead."
97+
"comparing a pointer to a null pointer, suggesting to use `.is_null()` instead"
9898
}
9999

100100
declare_clippy_lint! {
@@ -119,7 +119,28 @@ declare_clippy_lint! {
119119
"fns that create mutable refs from immutable ref args"
120120
}
121121

122-
declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF]);
122+
declare_clippy_lint! {
123+
/// **What it does:** This lint checks for invalid usages of `ptr::null`.
124+
///
125+
/// **Why is this bad?** This causes undefined behavior.
126+
///
127+
/// **Known problems:** None.
128+
///
129+
/// **Example:**
130+
/// ```ignore
131+
/// // Bad. Undefined behavior
132+
/// unsafe { std::slice::from_raw_parts(ptr::null(), 0); }
133+
/// ```
134+
///
135+
/// // Good
136+
/// unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), 0); }
137+
/// ```
138+
pub INVALID_NULL_PTR_USAGE,
139+
correctness,
140+
"invalid usage of a null pointer, suggesting `NonNull::dangling()` instead"
141+
}
142+
143+
declare_lint_pass!(Ptr => [PTR_ARG, CMP_NULL, MUT_FROM_REF, INVALID_NULL_PTR_USAGE]);
123144

124145
impl<'tcx> LateLintPass<'tcx> for Ptr {
125146
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
@@ -161,10 +182,55 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
161182
"comparing with null is better expressed by the `.is_null()` method",
162183
);
163184
}
185+
} else {
186+
check_invalid_ptr_usage(cx, expr);
164187
}
165188
}
166189
}
167190

191+
fn check_invalid_ptr_usage<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<()> {
192+
// fn_name, arg_idx
193+
const INVALID_NULL_PTR_USAGE_TABLE: [(&[&str], usize); 20] = [
194+
(&paths::SLICE_FROM_RAW_PARTS, 0),
195+
(&paths::SLICE_FROM_RAW_PARTS_MUT, 0),
196+
(&paths::PTR_COPY, 0),
197+
(&paths::PTR_COPY, 1),
198+
(&paths::PTR_COPY_NONOVERLAPPING, 0),
199+
(&paths::PTR_COPY_NONOVERLAPPING, 1),
200+
(&paths::PTR_READ, 0),
201+
(&paths::PTR_READ_UNALIGNED, 0),
202+
(&paths::PTR_READ_VOLATILE, 0),
203+
(&paths::PTR_REPLACE, 0),
204+
(&paths::PTR_SLICE_FROM_RAW_PARTS, 0),
205+
(&paths::PTR_SLICE_FROM_RAW_PARTS_MUT, 0),
206+
(&paths::PTR_SWAP, 0),
207+
(&paths::PTR_SWAP, 1),
208+
(&paths::PTR_SWAP_NONOVERLAPPING, 0),
209+
(&paths::PTR_SWAP_NONOVERLAPPING, 1),
210+
(&paths::PTR_WRITE, 0),
211+
(&paths::PTR_WRITE_UNALIGNED, 0),
212+
(&paths::PTR_WRITE_VOLATILE, 0),
213+
(&paths::PTR_WRITE_BYTES, 0),
214+
];
215+
216+
let arg = INVALID_NULL_PTR_USAGE_TABLE.iter().find_map(|(fn_name, arg_idx)| {
217+
let args = match_function_call(cx, expr, fn_name)?;
218+
args.get(*arg_idx).filter(|arg| is_null_path(arg))
219+
})?;
220+
221+
span_lint_and_sugg(
222+
cx,
223+
INVALID_NULL_PTR_USAGE,
224+
arg.span,
225+
"pointer must be non-null",
226+
"change this to",
227+
"core::ptr::NonNull::dangling().as_ptr()".to_string(),
228+
Applicability::MachineApplicable,
229+
);
230+
231+
Some(())
232+
}
233+
168234
#[allow(clippy::too_many_lines)]
169235
fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id: Option<BodyId>) {
170236
let fn_def_id = cx.tcx.hir().local_def_id(fn_id);
@@ -349,7 +415,10 @@ fn is_null_path(expr: &Expr<'_>) -> bool {
349415
if let ExprKind::Call(ref pathexp, ref args) = expr.kind {
350416
if args.is_empty() {
351417
if let ExprKind::Path(ref path) = pathexp.kind {
352-
return match_qpath(path, &paths::PTR_NULL) || match_qpath(path, &paths::PTR_NULL_MUT);
418+
return match_qpath(path, &paths::PTR_NULL)
419+
|| match_qpath(path, &paths::PTR_NULL_MUT)
420+
|| match_qpath(path, &paths::STD_PTR_NULL)
421+
|| match_qpath(path, &paths::STD_PTR_NULL_MUT);
353422
}
354423
}
355424
}

‎clippy_lints/src/size_of_in_element_count.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, inverted: bool)
6565

6666
fn get_pointee_ty_and_count_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<(Ty<'tcx>, &'tcx Expr<'tcx>)> {
6767
const FUNCTIONS: [&[&str]; 8] = [
68-
&paths::COPY_NONOVERLAPPING,
69-
&paths::COPY,
68+
&paths::PTR_COPY_NONOVERLAPPING,
69+
&paths::PTR_COPY,
7070
&paths::WRITE_BYTES,
7171
&paths::PTR_SWAP_NONOVERLAPPING,
7272
&paths::PTR_SLICE_FROM_RAW_PARTS,

‎clippy_utils/src/paths.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ pub const BTREESET: [&str; 5] = ["alloc", "collections", "btree", "set", "BTreeS
1818
pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"];
1919
pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"];
2020
pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"];
21-
pub const COPY: [&str; 4] = ["core", "intrinsics", "", "copy_nonoverlapping"];
22-
pub const COPY_NONOVERLAPPING: [&str; 4] = ["core", "intrinsics", "", "copy"];
2321
pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"];
2422
pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"];
2523
pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
@@ -101,12 +99,23 @@ pub const PERMISSIONS_FROM_MODE: [&str; 7] = ["std", "sys", "unix", "ext", "fs",
10199
pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"];
102100
pub const POLL_PENDING: [&str; 5] = ["core", "task", "poll", "Poll", "Pending"];
103101
pub const POLL_READY: [&str; 5] = ["core", "task", "poll", "Poll", "Ready"];
102+
pub const PTR_COPY: [&str; 4] = ["core", "intrinsics", "", "copy"];
103+
pub const PTR_COPY_NONOVERLAPPING: [&str; 4] = ["core", "intrinsics", "", "copy_nonoverlapping"];
104104
pub const PTR_EQ: [&str; 3] = ["core", "ptr", "eq"];
105105
pub const PTR_NULL: [&str; 3] = ["core", "ptr", "null"];
106106
pub const PTR_NULL_MUT: [&str; 3] = ["core", "ptr", "null_mut"];
107107
pub const PTR_SLICE_FROM_RAW_PARTS: [&str; 3] = ["core", "ptr", "slice_from_raw_parts"];
108108
pub const PTR_SLICE_FROM_RAW_PARTS_MUT: [&str; 3] = ["core", "ptr", "slice_from_raw_parts_mut"];
109109
pub const PTR_SWAP_NONOVERLAPPING: [&str; 3] = ["core", "ptr", "swap_nonoverlapping"];
110+
pub const PTR_READ: [&str; 3] = ["core", "ptr", "read"];
111+
pub const PTR_READ_UNALIGNED: [&str; 3] = ["core", "ptr", "read_unaligned"];
112+
pub const PTR_READ_VOLATILE: [&str; 3] = ["core", "ptr", "read_volatile"];
113+
pub const PTR_REPLACE: [&str; 3] = ["core", "ptr", "replace"];
114+
pub const PTR_SWAP: [&str; 3] = ["core", "ptr", "swap"];
115+
pub const PTR_WRITE: [&str; 3] = ["core", "ptr", "write"];
116+
pub const PTR_WRITE_BYTES: [&str; 3] = ["core", "intrinsics", "write_bytes"];
117+
pub const PTR_WRITE_UNALIGNED: [&str; 3] = ["core", "ptr", "write_unaligned"];
118+
pub const PTR_WRITE_VOLATILE: [&str; 3] = ["core", "ptr", "write_volatile"];
110119
pub const PUSH_STR: [&str; 4] = ["alloc", "string", "String", "push_str"];
111120
pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"];
112121
pub const RC_PTR_EQ: [&str; 4] = ["alloc", "rc", "Rc", "ptr_eq"];
@@ -136,6 +145,7 @@ pub const STD_CONVERT_IDENTITY: [&str; 3] = ["std", "convert", "identity"];
136145
pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"];
137146
pub const STD_MEM_TRANSMUTE: [&str; 3] = ["std", "mem", "transmute"];
138147
pub const STD_PTR_NULL: [&str; 3] = ["std", "ptr", "null"];
148+
pub const STD_PTR_NULL_MUT: [&str; 3] = ["std", "ptr", "null_mut"];
139149
pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"];
140150
pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"];
141151
pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "<impl str>", "ends_with"];

‎tests/ui/invalid_null_ptr_usage.fixed‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// run-rustfix
2+
3+
fn main() {
4+
unsafe {
5+
let _slice: &[usize] = std::slice::from_raw_parts(core::ptr::NonNull::dangling().as_ptr(), 0);
6+
let _slice: &[usize] = std::slice::from_raw_parts(core::ptr::NonNull::dangling().as_ptr(), 0);
7+
8+
let _slice: &[usize] = std::slice::from_raw_parts_mut(core::ptr::NonNull::dangling().as_ptr(), 0);
9+
10+
std::ptr::copy::<usize>(core::ptr::NonNull::dangling().as_ptr(), std::ptr::NonNull::dangling().as_ptr(), 0);
11+
std::ptr::copy::<usize>(std::ptr::NonNull::dangling().as_ptr(), core::ptr::NonNull::dangling().as_ptr(), 0);
12+
13+
std::ptr::copy_nonoverlapping::<usize>(core::ptr::NonNull::dangling().as_ptr(), std::ptr::NonNull::dangling().as_ptr(), 0);
14+
std::ptr::copy_nonoverlapping::<usize>(std::ptr::NonNull::dangling().as_ptr(), core::ptr::NonNull::dangling().as_ptr(), 0);
15+
16+
struct A; // zero sized struct
17+
assert_eq!(std::mem::size_of::<A>(), 0);
18+
19+
let _a: A = std::ptr::read(core::ptr::NonNull::dangling().as_ptr());
20+
let _a: A = std::ptr::read(core::ptr::NonNull::dangling().as_ptr());
21+
22+
let _a: A = std::ptr::read_unaligned(core::ptr::NonNull::dangling().as_ptr());
23+
let _a: A = std::ptr::read_unaligned(core::ptr::NonNull::dangling().as_ptr());
24+
25+
let _a: A = std::ptr::read_volatile(core::ptr::NonNull::dangling().as_ptr());
26+
let _a: A = std::ptr::read_volatile(core::ptr::NonNull::dangling().as_ptr());
27+
28+
let _a: A = std::ptr::replace(core::ptr::NonNull::dangling().as_ptr(), A);
29+
30+
let _slice: *const [usize] = std::ptr::slice_from_raw_parts(core::ptr::NonNull::dangling().as_ptr(), 0);
31+
let _slice: *const [usize] = std::ptr::slice_from_raw_parts(core::ptr::NonNull::dangling().as_ptr(), 0);
32+
33+
let _slice: *const [usize] = std::ptr::slice_from_raw_parts_mut(core::ptr::NonNull::dangling().as_ptr(), 0);
34+
35+
std::ptr::swap::<A>(core::ptr::NonNull::dangling().as_ptr(), &mut A);
36+
std::ptr::swap::<A>(&mut A, core::ptr::NonNull::dangling().as_ptr());
37+
38+
std::ptr::swap_nonoverlapping::<A>(core::ptr::NonNull::dangling().as_ptr(), &mut A, 0);
39+
std::ptr::swap_nonoverlapping::<A>(&mut A, core::ptr::NonNull::dangling().as_ptr(), 0);
40+
41+
std::ptr::write(core::ptr::NonNull::dangling().as_ptr(), A);
42+
43+
std::ptr::write_unaligned(core::ptr::NonNull::dangling().as_ptr(), A);
44+
45+
std::ptr::write_volatile(core::ptr::NonNull::dangling().as_ptr(), A);
46+
47+
std::ptr::write_bytes::<usize>(core::ptr::NonNull::dangling().as_ptr(), 42, 0);
48+
}
49+
}

‎tests/ui/invalid_null_ptr_usage.rs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// run-rustfix
2+
3+
fn main() {
4+
unsafe {
5+
let _slice: &[usize] = std::slice::from_raw_parts(std::ptr::null(), 0);
6+
let _slice: &[usize] = std::slice::from_raw_parts(std::ptr::null_mut(), 0);
7+
8+
let _slice: &[usize] = std::slice::from_raw_parts_mut(std::ptr::null_mut(), 0);
9+
10+
std::ptr::copy::<usize>(std::ptr::null(), std::ptr::NonNull::dangling().as_ptr(), 0);
11+
std::ptr::copy::<usize>(std::ptr::NonNull::dangling().as_ptr(), std::ptr::null_mut(), 0);
12+
13+
std::ptr::copy_nonoverlapping::<usize>(std::ptr::null(), std::ptr::NonNull::dangling().as_ptr(), 0);
14+
std::ptr::copy_nonoverlapping::<usize>(std::ptr::NonNull::dangling().as_ptr(), std::ptr::null_mut(), 0);
15+
16+
struct A; // zero sized struct
17+
assert_eq!(std::mem::size_of::<A>(), 0);
18+
19+
let _a: A = std::ptr::read(std::ptr::null());
20+
let _a: A = std::ptr::read(std::ptr::null_mut());
21+
22+
let _a: A = std::ptr::read_unaligned(std::ptr::null());
23+
let _a: A = std::ptr::read_unaligned(std::ptr::null_mut());
24+
25+
let _a: A = std::ptr::read_volatile(std::ptr::null());
26+
let _a: A = std::ptr::read_volatile(std::ptr::null_mut());
27+
28+
let _a: A = std::ptr::replace(std::ptr::null_mut(), A);
29+
30+
let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null(), 0);
31+
let _slice: *const [usize] = std::ptr::slice_from_raw_parts(std::ptr::null_mut(), 0);
32+
33+
let _slice: *const [usize] = std::ptr::slice_from_raw_parts_mut(std::ptr::null_mut(), 0);
34+
35+
std::ptr::swap::<A>(std::ptr::null_mut(), &mut A);
36+
std::ptr::swap::<A>(&mut A, std::ptr::null_mut());
37+
38+
std::ptr::swap_nonoverlapping::<A>(std::ptr::null_mut(), &mut A, 0);
39+
std::ptr::swap_nonoverlapping::<A>(&mut A, std::ptr::null_mut(), 0);
40+
41+
std::ptr::write(std::ptr::null_mut(), A);
42+
43+
std::ptr::write_unaligned(std::ptr::null_mut(), A);
44+
45+
std::ptr::write_volatile(std::ptr::null_mut(), A);
46+
47+
std::ptr::write_bytes::<usize>(std::ptr::null_mut(), 42, 0);
48+
}
49+
}

0 commit comments

Comments
 (0)