Skip to content

Commit 6dd5b0c

Browse files
committed
Taking a raw pointer on a union field is safe since Rust 1.92
1 parent d599529 commit 6dd5b0c

File tree

7 files changed

+307
-80
lines changed

7 files changed

+307
-80
lines changed

book/src/lint_configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
892892
* [`mem_replace_option_with_some`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_some)
893893
* [`mem_replace_with_default`](https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default)
894894
* [`missing_const_for_fn`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn)
895+
* [`multiple_unsafe_ops_per_block`](https://rust-lang.github.io/rust-clippy/master/index.html#multiple_unsafe_ops_per_block)
895896
* [`needless_borrow`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow)
896897
* [`non_std_lazy_statics`](https://rust-lang.github.io/rust-clippy/master/index.html#non_std_lazy_statics)
897898
* [`option_as_ref_deref`](https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref)

clippy_config/src/conf.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ define_Conf! {
781781
mem_replace_option_with_some,
782782
mem_replace_with_default,
783783
missing_const_for_fn,
784+
multiple_unsafe_ops_per_block,
784785
needless_borrow,
785786
non_std_lazy_statics,
786787
option_as_ref_deref,

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
725725
store.register_late_pass(move |_| Box::new(semicolon_block::SemicolonBlock::new(conf)));
726726
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
727727
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
728-
store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock));
728+
store.register_late_pass(move |_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock::new(conf)));
729729
store.register_late_pass(move |_| Box::new(extra_unused_type_parameters::ExtraUnusedTypeParameters::new(conf)));
730730
store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
731731
store.register_late_pass(|_| Box::new(collection_is_never_read::CollectionIsNeverRead));

clippy_lints/src/multiple_unsafe_ops_per_block.rs

Lines changed: 115 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
1-
use clippy_utils::desugar_await;
1+
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::visitors::{Descend, Visitable, for_each_expr};
4-
use core::ops::ControlFlow::Continue;
3+
use clippy_utils::msrvs::Msrv;
4+
use clippy_utils::{desugar_await, msrvs};
55
use hir::def::{DefKind, Res};
66
use hir::{BlockCheckMode, ExprKind, QPath, UnOp};
7-
use rustc_ast::Mutability;
7+
use rustc_ast::{BorrowKind, Mutability};
88
use rustc_hir as hir;
9+
use rustc_hir::intravisit::{Visitor, walk_body, walk_expr};
910
use rustc_lint::{LateContext, LateLintPass};
10-
use rustc_middle::ty;
11-
use rustc_session::declare_lint_pass;
11+
use rustc_middle::hir::nested_filter;
12+
use rustc_middle::ty::{self, TypeckResults};
13+
use rustc_session::impl_lint_pass;
1214
use rustc_span::{DesugaringKind, Span};
1315

1416
declare_clippy_lint! {
@@ -60,7 +62,18 @@ declare_clippy_lint! {
6062
restriction,
6163
"more than one unsafe operation per `unsafe` block"
6264
}
63-
declare_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]);
65+
66+
pub struct MultipleUnsafeOpsPerBlock {
67+
msrv: Msrv,
68+
}
69+
70+
impl_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]);
71+
72+
impl MultipleUnsafeOpsPerBlock {
73+
pub fn new(conf: &Conf) -> Self {
74+
Self { msrv: conf.msrv }
75+
}
76+
}
6477

6578
impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
6679
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
@@ -70,8 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
7083
{
7184
return;
7285
}
73-
let mut unsafe_ops = vec![];
74-
collect_unsafe_exprs(cx, block, &mut unsafe_ops);
86+
let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block, self.msrv);
7587
if unsafe_ops.len() > 1 {
7688
span_lint_and_then(
7789
cx,
@@ -91,25 +103,77 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
91103
}
92104
}
93105

94-
fn collect_unsafe_exprs<'tcx>(
95-
cx: &LateContext<'tcx>,
96-
node: impl Visitable<'tcx>,
97-
unsafe_ops: &mut Vec<(&'static str, Span)>,
98-
) {
99-
for_each_expr(cx, node, |expr| {
106+
#[derive(Clone, Copy)]
107+
enum UnderRawPtr {
108+
/// The expression is not located under a raw pointer
109+
No,
110+
/// The expression is located under a raw pointer, MSRV yet unknown
111+
Yes,
112+
/// The expression is located under a raw pointer and MSRV has been determined.
113+
/// `true` means that taking a raw pointer to a union field is a safe operation.
114+
WithSafeMsrv(bool),
115+
}
116+
117+
struct UnsafeExprCollector<'cx, 'tcx> {
118+
cx: &'cx LateContext<'tcx>,
119+
typeck_results: &'tcx TypeckResults<'tcx>,
120+
msrv: Msrv,
121+
unsafe_ops: Vec<(&'static str, Span)>,
122+
under_raw_ptr: UnderRawPtr,
123+
}
124+
125+
impl<'cx, 'tcx> UnsafeExprCollector<'cx, 'tcx> {
126+
fn collect_unsafe_exprs(
127+
cx: &'cx LateContext<'tcx>,
128+
block: &'tcx hir::Block<'tcx>,
129+
msrv: Msrv,
130+
) -> Vec<(&'static str, Span)> {
131+
let mut collector = Self {
132+
cx,
133+
typeck_results: cx.typeck_results(),
134+
msrv,
135+
unsafe_ops: vec![],
136+
under_raw_ptr: UnderRawPtr::No,
137+
};
138+
collector.visit_block(block);
139+
collector.unsafe_ops
140+
}
141+
}
142+
143+
impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'_, 'tcx> {
144+
type NestedFilter = nested_filter::OnlyBodies;
145+
146+
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
147+
// `self.under_raw_ptr` is preventively reset, while the current value is
148+
// preserved in `under_raw_ptr`.
149+
let under_raw_ptr = self.under_raw_ptr;
150+
self.under_raw_ptr = UnderRawPtr::No;
151+
100152
match expr.kind {
101153
// The `await` itself will desugar to two unsafe calls, but we should ignore those.
102154
// Instead, check the expression that is `await`ed
103155
_ if let Some(e) = desugar_await(expr) => {
104-
collect_unsafe_exprs(cx, e, unsafe_ops);
105-
return Continue(Descend::No);
156+
return self.visit_expr(e);
106157
},
107158

108-
ExprKind::InlineAsm(_) => unsafe_ops.push(("inline assembly used here", expr.span)),
159+
ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)),
160+
161+
ExprKind::AddrOf(BorrowKind::Raw, _, _) => {
162+
self.under_raw_ptr = UnderRawPtr::Yes;
163+
},
109164

110165
ExprKind::Field(e, _) => {
111-
if cx.typeck_results().expr_ty(e).is_union() {
112-
unsafe_ops.push(("union field access occurs here", expr.span));
166+
if self.typeck_results.expr_ty(e).is_union() {
167+
// Restore `self.under_raw_pointer` and determine safety of taking a raw pointer to
168+
// a union field if this is not known already.
169+
self.under_raw_ptr = if matches!(under_raw_ptr, UnderRawPtr::Yes) {
170+
UnderRawPtr::WithSafeMsrv(self.msrv.meets(self.cx, msrvs::SAFE_RAW_PTR_TO_UNION_FIELD))
171+
} else {
172+
under_raw_ptr
173+
};
174+
if matches!(self.under_raw_ptr, UnderRawPtr::No | UnderRawPtr::WithSafeMsrv(false)) {
175+
self.unsafe_ops.push(("union field access occurs here", expr.span));
176+
}
113177
}
114178
},
115179

@@ -127,32 +191,32 @@ fn collect_unsafe_exprs<'tcx>(
127191
..
128192
},
129193
)) => {
130-
unsafe_ops.push(("access of a mutable static occurs here", expr.span));
194+
self.unsafe_ops
195+
.push(("access of a mutable static occurs here", expr.span));
131196
},
132197

133-
ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty_adjusted(e).is_raw_ptr() => {
134-
unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
198+
ExprKind::Unary(UnOp::Deref, e) if self.typeck_results.expr_ty_adjusted(e).is_raw_ptr() => {
199+
self.unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
135200
},
136201

137202
ExprKind::Call(path_expr, _) => {
138-
let sig = match *cx.typeck_results().expr_ty(path_expr).kind() {
139-
ty::FnDef(id, _) => cx.tcx.fn_sig(id).skip_binder(),
140-
ty::FnPtr(sig_tys, hdr) => sig_tys.with(hdr),
141-
_ => return Continue(Descend::Yes),
203+
let opt_sig = match *self.typeck_results.expr_ty(path_expr).kind() {
204+
ty::FnDef(id, _) => Some(self.cx.tcx.fn_sig(id).skip_binder()),
205+
ty::FnPtr(sig_tys, hdr) => Some(sig_tys.with(hdr)),
206+
_ => None,
142207
};
143-
if sig.safety().is_unsafe() {
144-
unsafe_ops.push(("unsafe function call occurs here", expr.span));
208+
if opt_sig.is_some_and(|sig| sig.safety().is_unsafe()) {
209+
self.unsafe_ops.push(("unsafe function call occurs here", expr.span));
145210
}
146211
},
147212

148213
ExprKind::MethodCall(..) => {
149-
if let Some(sig) = cx
150-
.typeck_results()
214+
let opt_sig = self
215+
.typeck_results
151216
.type_dependent_def_id(expr.hir_id)
152-
.map(|def_id| cx.tcx.fn_sig(def_id))
153-
&& sig.skip_binder().safety().is_unsafe()
154-
{
155-
unsafe_ops.push(("unsafe method call occurs here", expr.span));
217+
.map(|def_id| self.cx.tcx.fn_sig(def_id));
218+
if opt_sig.is_some_and(|sig| sig.skip_binder().safety().is_unsafe()) {
219+
self.unsafe_ops.push(("unsafe method call occurs here", expr.span));
156220
}
157221
},
158222

@@ -173,15 +237,26 @@ fn collect_unsafe_exprs<'tcx>(
173237
}
174238
))
175239
) {
176-
unsafe_ops.push(("modification of a mutable static occurs here", expr.span));
177-
collect_unsafe_exprs(cx, rhs, unsafe_ops);
178-
return Continue(Descend::No);
240+
self.unsafe_ops
241+
.push(("modification of a mutable static occurs here", expr.span));
242+
return self.visit_expr(rhs);
179243
}
180244
},
181245

182246
_ => {},
183247
}
184248

185-
Continue::<(), _>(Descend::Yes)
186-
});
249+
walk_expr(self, expr);
250+
}
251+
252+
fn visit_body(&mut self, body: &hir::Body<'tcx>) {
253+
let saved_typeck_results = self.typeck_results;
254+
self.typeck_results = self.cx.tcx.typeck_body(body.id());
255+
walk_body(self, body);
256+
self.typeck_results = saved_typeck_results;
257+
}
258+
259+
fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
260+
self.cx.tcx
261+
}
187262
}

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ macro_rules! msrv_aliases {
2323

2424
// names may refer to stabilized feature flags or library items
2525
msrv_aliases! {
26+
1,92,0 { SAFE_RAW_PTR_TO_UNION_FIELD }
2627
1,88,0 { LET_CHAINS }
2728
1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST }
2829
1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL }

tests/ui/multiple_unsafe_ops_per_block.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//@needs-asm-support
22
//@aux-build:proc_macros.rs
3+
#![feature(stmt_expr_attributes)]
34
#![expect(
45
dropping_copy_types,
56
clippy::unnecessary_operation,
@@ -209,4 +210,69 @@ async fn issue13879() {
209210
}
210211
}
211212

213+
fn issue16076() {
214+
#[derive(Clone, Copy)]
215+
union U {
216+
i: u32,
217+
f: f32,
218+
}
219+
220+
let u = U { i: 0 };
221+
222+
// Taking a raw pointer to a place is safe since Rust 1.92
223+
#[clippy::msrv = "1.92"]
224+
unsafe {
225+
_ = &raw const u.i;
226+
_ = &raw const u.i;
227+
}
228+
229+
// However it was not the case before Rust 1.92
230+
#[clippy::msrv = "1.91"]
231+
unsafe {
232+
//~^ multiple_unsafe_ops_per_block
233+
_ = &raw const u.i;
234+
_ = &raw const u.i;
235+
}
236+
237+
// Taking a reference to a union field is not safe
238+
unsafe {
239+
//~^ multiple_unsafe_ops_per_block
240+
_ = &u.i;
241+
_ = &u.i;
242+
}
243+
244+
// Check that we still check and lint the prefix of the raw pointer to a field access
245+
#[expect(clippy::deref_addrof)]
246+
unsafe {
247+
//~^ multiple_unsafe_ops_per_block
248+
_ = &raw const (*&raw const u).i;
249+
_ = &raw const (*&raw const u).i;
250+
}
251+
252+
union V {
253+
u: U,
254+
}
255+
256+
// Taking a raw pointer to a union field of an union field (etc.) is safe
257+
let v = V { u };
258+
unsafe {
259+
_ = &raw const v.u.i;
260+
_ = &raw const v.u.i;
261+
}
262+
}
263+
264+
fn check_closures() {
265+
unsafe fn apply(f: impl Fn()) {
266+
todo!()
267+
}
268+
unsafe fn f(_x: i32) {
269+
todo!()
270+
}
271+
272+
unsafe {
273+
//~^ multiple_unsafe_ops_per_block
274+
apply(|| f(0));
275+
}
276+
}
277+
212278
fn main() {}

0 commit comments

Comments
 (0)