Skip to content

Commit aa0eb7f

Browse files
committed
Auto merge of #10206 - Niki4tap:one_unsafe_op_per_block, r=Alexendoo
Add `multiple_unsafe_ops_per_block` lint Adds a lint, which restricts an `unsafe` block to only one unsafe operation. Closes #10064 --- changelog: New lint: [`multiple_unsafe_ops_per_block`] [#10206](#10206) <!-- changelog_checked -->
2 parents 36e3e26 + 875e36f commit aa0eb7f

6 files changed

+428
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4383,6 +4383,7 @@ Released 2018-09-13
43834383
[`multi_assignments`]: https://rust-lang.github.io/rust-clippy/master/index.html#multi_assignments
43844384
[`multiple_crate_versions`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions
43854385
[`multiple_inherent_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl
4386+
[`multiple_unsafe_ops_per_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_unsafe_ops_per_block
43864387
[`must_use_candidate`]: https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate
43874388
[`must_use_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#must_use_unit
43884389
[`mut_from_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_from_ref

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
422422
crate::module_style::MOD_MODULE_FILES_INFO,
423423
crate::module_style::SELF_NAMED_MODULE_FILES_INFO,
424424
crate::multi_assignments::MULTI_ASSIGNMENTS_INFO,
425+
crate::multiple_unsafe_ops_per_block::MULTIPLE_UNSAFE_OPS_PER_BLOCK_INFO,
425426
crate::mut_key::MUTABLE_KEY_TYPE_INFO,
426427
crate::mut_mut::MUT_MUT_INFO,
427428
crate::mut_reference::UNNECESSARY_MUT_PASSED_INFO,

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ mod missing_trait_methods;
198198
mod mixed_read_write_in_expression;
199199
mod module_style;
200200
mod multi_assignments;
201+
mod multiple_unsafe_ops_per_block;
201202
mod mut_key;
202203
mod mut_mut;
203204
mod mut_reference;
@@ -908,6 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
908909
store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck));
909910
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
910911
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
912+
store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock));
911913
// add lints here, do not remove this comment, it's used in `new_lint`
912914
}
913915

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
use clippy_utils::{
2+
diagnostics::span_lint_and_then,
3+
visitors::{for_each_expr_with_closures, Descend, Visitable},
4+
};
5+
use core::ops::ControlFlow::Continue;
6+
use hir::{
7+
def::{DefKind, Res},
8+
BlockCheckMode, ExprKind, QPath, UnOp, Unsafety,
9+
};
10+
use rustc_ast::Mutability;
11+
use rustc_hir as hir;
12+
use rustc_lint::{LateContext, LateLintPass};
13+
use rustc_session::{declare_lint_pass, declare_tool_lint};
14+
use rustc_span::Span;
15+
16+
declare_clippy_lint! {
17+
/// ### What it does
18+
/// Checks for `unsafe` blocks that contain more than one unsafe operation.
19+
///
20+
/// ### Why is this bad?
21+
/// Combined with `undocumented_unsafe_blocks`,
22+
/// this lint ensures that each unsafe operation must be independently justified.
23+
/// Combined with `unused_unsafe`, this lint also ensures
24+
/// elimination of unnecessary unsafe blocks through refactoring.
25+
///
26+
/// ### Example
27+
/// ```rust
28+
/// /// Reads a `char` from the given pointer.
29+
/// ///
30+
/// /// # Safety
31+
/// ///
32+
/// /// `ptr` must point to four consecutive, initialized bytes which
33+
/// /// form a valid `char` when interpreted in the native byte order.
34+
/// fn read_char(ptr: *const u8) -> char {
35+
/// // SAFETY: The caller has guaranteed that the value pointed
36+
/// // to by `bytes` is a valid `char`.
37+
/// unsafe { char::from_u32_unchecked(*ptr.cast::<u32>()) }
38+
/// }
39+
/// ```
40+
/// Use instead:
41+
/// ```rust
42+
/// /// Reads a `char` from the given pointer.
43+
/// ///
44+
/// /// # Safety
45+
/// ///
46+
/// /// - `ptr` must be 4-byte aligned, point to four consecutive
47+
/// /// initialized bytes, and be valid for reads of 4 bytes.
48+
/// /// - The bytes pointed to by `ptr` must represent a valid
49+
/// /// `char` when interpreted in the native byte order.
50+
/// fn read_char(ptr: *const u8) -> char {
51+
/// // SAFETY: `ptr` is 4-byte aligned, points to four consecutive
52+
/// // initialized bytes, and is valid for reads of 4 bytes.
53+
/// let int_value = unsafe { *ptr.cast::<u32>() };
54+
///
55+
/// // SAFETY: The caller has guaranteed that the four bytes
56+
/// // pointed to by `bytes` represent a valid `char`.
57+
/// unsafe { char::from_u32_unchecked(int_value) }
58+
/// }
59+
/// ```
60+
#[clippy::version = "1.68.0"]
61+
pub MULTIPLE_UNSAFE_OPS_PER_BLOCK,
62+
restriction,
63+
"more than one unsafe operation per `unsafe` block"
64+
}
65+
declare_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]);
66+
67+
impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
68+
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
69+
if !matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) {
70+
return;
71+
}
72+
let mut unsafe_ops = vec![];
73+
collect_unsafe_exprs(cx, block, &mut unsafe_ops);
74+
if unsafe_ops.len() > 1 {
75+
span_lint_and_then(
76+
cx,
77+
MULTIPLE_UNSAFE_OPS_PER_BLOCK,
78+
block.span,
79+
&format!(
80+
"this `unsafe` block contains {} unsafe operations, expected only one",
81+
unsafe_ops.len()
82+
),
83+
|diag| {
84+
for (msg, span) in unsafe_ops {
85+
diag.span_note(span, msg);
86+
}
87+
},
88+
);
89+
}
90+
}
91+
}
92+
93+
fn collect_unsafe_exprs<'tcx>(
94+
cx: &LateContext<'tcx>,
95+
node: impl Visitable<'tcx>,
96+
unsafe_ops: &mut Vec<(&'static str, Span)>,
97+
) {
98+
for_each_expr_with_closures(cx, node, |expr| {
99+
match expr.kind {
100+
ExprKind::InlineAsm(_) => unsafe_ops.push(("inline assembly used here", expr.span)),
101+
102+
ExprKind::Field(e, _) => {
103+
if cx.typeck_results().expr_ty(e).is_union() {
104+
unsafe_ops.push(("union field access occurs here", expr.span));
105+
}
106+
},
107+
108+
ExprKind::Path(QPath::Resolved(
109+
_,
110+
hir::Path {
111+
res: Res::Def(DefKind::Static(Mutability::Mut), _),
112+
..
113+
},
114+
)) => {
115+
unsafe_ops.push(("access of a mutable static occurs here", expr.span));
116+
},
117+
118+
ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty_adjusted(e).is_unsafe_ptr() => {
119+
unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
120+
},
121+
122+
ExprKind::Call(path_expr, _) => match path_expr.kind {
123+
ExprKind::Path(QPath::Resolved(
124+
_,
125+
hir::Path {
126+
res: Res::Def(kind, def_id),
127+
..
128+
},
129+
)) if kind.is_fn_like() => {
130+
let sig = cx.tcx.bound_fn_sig(*def_id);
131+
if sig.0.unsafety() == Unsafety::Unsafe {
132+
unsafe_ops.push(("unsafe function call occurs here", expr.span));
133+
}
134+
},
135+
136+
ExprKind::Path(QPath::TypeRelative(..)) => {
137+
if let Some(sig) = cx
138+
.typeck_results()
139+
.type_dependent_def_id(path_expr.hir_id)
140+
.map(|def_id| cx.tcx.bound_fn_sig(def_id))
141+
{
142+
if sig.0.unsafety() == Unsafety::Unsafe {
143+
unsafe_ops.push(("unsafe function call occurs here", expr.span));
144+
}
145+
}
146+
},
147+
148+
_ => {},
149+
},
150+
151+
ExprKind::MethodCall(..) => {
152+
if let Some(sig) = cx
153+
.typeck_results()
154+
.type_dependent_def_id(expr.hir_id)
155+
.map(|def_id| cx.tcx.bound_fn_sig(def_id))
156+
{
157+
if sig.0.unsafety() == Unsafety::Unsafe {
158+
unsafe_ops.push(("unsafe method call occurs here", expr.span));
159+
}
160+
}
161+
},
162+
163+
ExprKind::AssignOp(_, lhs, rhs) | ExprKind::Assign(lhs, rhs, _) => {
164+
if matches!(
165+
lhs.kind,
166+
ExprKind::Path(QPath::Resolved(
167+
_,
168+
hir::Path {
169+
res: Res::Def(DefKind::Static(Mutability::Mut), _),
170+
..
171+
}
172+
))
173+
) {
174+
unsafe_ops.push(("modification of a mutable static occurs here", expr.span));
175+
collect_unsafe_exprs(cx, rhs, unsafe_ops);
176+
return Continue(Descend::No);
177+
}
178+
},
179+
180+
_ => {},
181+
};
182+
183+
Continue::<(), _>(Descend::Yes)
184+
});
185+
}
+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
#![allow(unused)]
2+
#![allow(deref_nullptr)]
3+
#![allow(clippy::unnecessary_operation)]
4+
#![allow(clippy::drop_copy)]
5+
#![warn(clippy::multiple_unsafe_ops_per_block)]
6+
7+
use core::arch::asm;
8+
9+
fn raw_ptr() -> *const () {
10+
core::ptr::null()
11+
}
12+
13+
unsafe fn not_very_safe() {}
14+
15+
struct Sample;
16+
17+
impl Sample {
18+
unsafe fn not_very_safe(&self) {}
19+
}
20+
21+
#[allow(non_upper_case_globals)]
22+
const sample: Sample = Sample;
23+
24+
union U {
25+
i: i32,
26+
u: u32,
27+
}
28+
29+
static mut STATIC: i32 = 0;
30+
31+
fn test1() {
32+
unsafe {
33+
STATIC += 1;
34+
not_very_safe();
35+
}
36+
}
37+
38+
fn test2() {
39+
let u = U { i: 0 };
40+
41+
unsafe {
42+
drop(u.u);
43+
*raw_ptr();
44+
}
45+
}
46+
47+
fn test3() {
48+
unsafe {
49+
asm!("nop");
50+
sample.not_very_safe();
51+
STATIC = 0;
52+
}
53+
}
54+
55+
fn test_all() {
56+
let u = U { i: 0 };
57+
unsafe {
58+
drop(u.u);
59+
drop(STATIC);
60+
sample.not_very_safe();
61+
not_very_safe();
62+
*raw_ptr();
63+
asm!("nop");
64+
}
65+
}
66+
67+
// no lint
68+
fn correct1() {
69+
unsafe {
70+
STATIC += 1;
71+
}
72+
}
73+
74+
// no lint
75+
fn correct2() {
76+
unsafe {
77+
STATIC += 1;
78+
}
79+
80+
unsafe {
81+
*raw_ptr();
82+
}
83+
}
84+
85+
// no lint
86+
fn correct3() {
87+
let u = U { u: 0 };
88+
89+
unsafe {
90+
not_very_safe();
91+
}
92+
93+
unsafe {
94+
drop(u.i);
95+
}
96+
}
97+
98+
// tests from the issue (https://github.com/rust-lang/rust-clippy/issues/10064)
99+
100+
unsafe fn read_char_bad(ptr: *const u8) -> char {
101+
unsafe { char::from_u32_unchecked(*ptr.cast::<u32>()) }
102+
}
103+
104+
// no lint
105+
unsafe fn read_char_good(ptr: *const u8) -> char {
106+
let int_value = unsafe { *ptr.cast::<u32>() };
107+
unsafe { core::char::from_u32_unchecked(int_value) }
108+
}
109+
110+
fn main() {}

0 commit comments

Comments
 (0)