Skip to content

Commit 1f10fb5

Browse files
committed
Only lint non-promoted temporaries
1 parent 96c9004 commit 1f10fb5

File tree

8 files changed

+184
-117
lines changed

8 files changed

+184
-117
lines changed

clippy_lints/src/casts/mod.rs

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ mod fn_to_numeric_cast_any;
1818
mod fn_to_numeric_cast_with_truncation;
1919
mod ptr_as_ptr;
2020
mod ptr_cast_constness;
21-
mod ptr_to_temporary;
2221
mod unnecessary_cast;
2322
mod utils;
2423

@@ -658,33 +657,6 @@ declare_clippy_lint! {
658657
"casting a known floating-point NaN into an integer"
659658
}
660659

661-
declare_clippy_lint! {
662-
/// ### What it does
663-
/// Checks for raw pointers that point to temporary values.
664-
///
665-
/// ### Why is this bad?
666-
/// Usage of such a pointer can result in Undefined Behavior, as the pointer will stop pointing
667-
/// to valid stack memory once the temporary is dropped.
668-
///
669-
/// ### Example
670-
/// ```rust,ignore
671-
/// const PS: [usize; 3] = [2usize, 3usize, 11usize];
672-
/// let mut ps = vec![];
673-
///
674-
/// for p in PS {
675-
/// ps.push(&p as *const usize);
676-
/// }
677-
///
678-
/// for p in ps {
679-
/// unsafe { p.read() }; // ⚠️
680-
/// }
681-
/// ```
682-
#[clippy::version = "1.72.0"]
683-
pub PTR_TO_TEMPORARY,
684-
correctness,
685-
"disallows obtaining a raw pointer to a temporary value"
686-
}
687-
688660
pub struct Casts {
689661
msrv: Msrv,
690662
}
@@ -719,7 +691,6 @@ impl_lint_pass!(Casts => [
719691
CAST_SLICE_FROM_RAW_PARTS,
720692
AS_PTR_CAST_MUT,
721693
CAST_NAN_TO_INT,
722-
PTR_TO_TEMPORARY,
723694
]);
724695

725696
impl<'tcx> LateLintPass<'tcx> for Casts {
@@ -765,7 +736,6 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
765736
}
766737

767738
as_underscore::check(cx, expr, cast_to_hir);
768-
ptr_to_temporary::check(cx, expr, cast_expr, cast_to_hir);
769739

770740
if self.msrv.meets(msrvs::BORROW_AS_PTR) {
771741
borrow_as_ptr::check(cx, expr, cast_expr, cast_to_hir);

clippy_lints/src/casts/ptr_to_temporary.rs

Lines changed: 0 additions & 34 deletions
This file was deleted.

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
9494
crate::casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION_INFO,
9595
crate::casts::PTR_AS_PTR_INFO,
9696
crate::casts::PTR_CAST_CONSTNESS_INFO,
97-
crate::casts::PTR_TO_TEMPORARY_INFO,
9897
crate::casts::UNNECESSARY_CAST_INFO,
9998
crate::checked_conversions::CHECKED_CONVERSIONS_INFO,
10099
crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
@@ -532,6 +531,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
532531
crate::ptr::MUT_FROM_REF_INFO,
533532
crate::ptr::PTR_ARG_INFO,
534533
crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
534+
crate::ptr_to_temporary::PTR_TO_TEMPORARY_INFO,
535535
crate::pub_use::PUB_USE_INFO,
536536
crate::question_mark::QUESTION_MARK_INFO,
537537
crate::question_mark_used::QUESTION_MARK_USED_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ mod permissions_set_readonly_false;
258258
mod precedence;
259259
mod ptr;
260260
mod ptr_offset_with_cast;
261+
mod ptr_to_temporary;
261262
mod pub_use;
262263
mod question_mark;
263264
mod question_mark_used;
@@ -1047,6 +1048,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10471048
let stack_size_threshold = conf.stack_size_threshold;
10481049
store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold)));
10491050
store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit));
1051+
store.register_late_pass(move |_| Box::new(ptr_to_temporary::PtrToTemporary));
10501052
// add lints here, do not remove this comment, it's used in `new_lint`
10511053
}
10521054

clippy_lints/src/ptr_to_temporary.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
use clippy_utils::{consts::is_promotable, diagnostics::span_lint_and_note, is_from_proc_macro};
2+
use rustc_hir::{BorrowKind, Expr, ExprKind, ItemKind, OwnerNode};
3+
use rustc_lint::{LateContext, LateLintPass, LintContext};
4+
use rustc_middle::lint::in_external_macro;
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
7+
declare_clippy_lint! {
8+
/// ### What it does
9+
/// Checks for raw pointers pointing to temporary values that will **not** be promoted to a
10+
/// constant through
11+
/// [constant promotion](https://doc.rust-lang.org/stable/reference/destructors.html#constant-promotion).
12+
///
13+
/// ### Why is this bad?
14+
/// Usage of such a pointer can result in Undefined Behavior, as the pointer will stop pointing
15+
/// to valid stack memory once the temporary is dropped.
16+
///
17+
/// ### Example
18+
/// ```rust,ignore
19+
/// fn x() -> *const i32 {
20+
/// let x = 0;
21+
/// &x as *const i32
22+
/// }
23+
///
24+
/// let x = x();
25+
/// unsafe { *x }; // ⚠️
26+
/// ```
27+
#[clippy::version = "1.72.0"]
28+
pub PTR_TO_TEMPORARY,
29+
correctness,
30+
"disallows returning a raw pointer to a temporary value"
31+
}
32+
declare_lint_pass!(PtrToTemporary => [PTR_TO_TEMPORARY]);
33+
34+
impl<'tcx> LateLintPass<'tcx> for PtrToTemporary {
35+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
36+
if in_external_macro(cx.sess(), expr.span) {
37+
return;
38+
}
39+
40+
// Get the final return statement if this is a return statement, or don't lint
41+
let expr = if let ExprKind::Ret(Some(expr)) = expr.kind {
42+
expr
43+
} else if let OwnerNode::Item(parent) = cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id))
44+
&& let ItemKind::Fn(_, _, body) = parent.kind
45+
&& let block = cx.tcx.hir().body(body).value
46+
&& let ExprKind::Block(block, _) = block.kind
47+
&& let Some(final_block_expr) = block.expr
48+
&& final_block_expr.hir_id == expr.hir_id
49+
{
50+
expr
51+
} else {
52+
return;
53+
};
54+
55+
if let ExprKind::Cast(cast_expr, _) = expr.kind
56+
&& let ExprKind::AddrOf(BorrowKind::Ref, _, e) = cast_expr.kind
57+
&& !is_promotable(cx, e)
58+
&& !is_from_proc_macro(cx, expr)
59+
{
60+
span_lint_and_note(
61+
cx,
62+
PTR_TO_TEMPORARY,
63+
expr.span,
64+
"returning a raw pointer to a temporary value that cannot be promoted to a constant",
65+
None,
66+
"usage of this pointer by callers will cause Undefined Behavior",
67+
);
68+
}
69+
}
70+
}

clippy_utils/src/consts.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#![allow(clippy::float_cmp)]
22

33
use crate::source::{get_source_text, walk_span_to_context};
4-
use crate::{clip, is_direct_expn_of, sext, unsext};
4+
use crate::visitors::for_each_expr;
5+
use crate::{clip, is_ctor_or_promotable_const_function, is_direct_expn_of, path_res, sext, unsext};
56
use if_chain::if_chain;
67
use rustc_ast::ast::{self, LitFloatType, LitKind};
78
use rustc_data_structures::sync::Lrc;
@@ -19,6 +20,7 @@ use rustc_span::SyntaxContext;
1920
use std::cmp::Ordering::{self, Equal};
2021
use std::hash::{Hash, Hasher};
2122
use std::iter;
23+
use std::ops::ControlFlow;
2224

2325
/// A `LitKind`-like enum to fold constant `Expr`s into.
2426
#[derive(Debug, Clone)]
@@ -708,3 +710,29 @@ pub fn miri_to_const<'tcx>(tcx: TyCtxt<'tcx>, result: mir::ConstantKind<'tcx>) -
708710
_ => None,
709711
}
710712
}
713+
714+
/// Returns whether a certain `Expr` will be promoted to a constant.
715+
pub fn is_promotable<'tcx>(cx: &LateContext<'tcx>, start: &'tcx Expr<'tcx>) -> bool {
716+
let ty = cx.typeck_results().expr_ty(start);
717+
for_each_expr(start, |expr| {
718+
match expr.kind {
719+
ExprKind::Binary(_, _, _) => ControlFlow::Break(()),
720+
ExprKind::Unary(UnOp::Deref, e) if !matches!(path_res(cx, e), Res::Def(DefKind::Const, _)) => {
721+
ControlFlow::Break(())
722+
},
723+
ExprKind::Call(_, _) if !is_ctor_or_promotable_const_function(cx, expr) => ControlFlow::Break(()),
724+
ExprKind::MethodCall(..) if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
725+
&& !cx.tcx.is_promotable_const_fn(def_id) =>
726+
{
727+
ControlFlow::Break(())
728+
},
729+
ExprKind::Path(qpath) if let Res::Local(_) = cx.qpath_res(&qpath, expr.hir_id) => {
730+
ControlFlow::Break(())
731+
},
732+
_ => ControlFlow::Continue(()),
733+
}
734+
})
735+
.is_none()
736+
&& ty.is_freeze(cx.tcx, cx.param_env)
737+
&& !ty.needs_drop(cx.tcx, cx.param_env)
738+
}

tests/ui/ptr_to_temporary.rs

Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,65 @@
1-
#![allow(clippy::explicit_auto_deref, clippy::unnecessary_cast, clippy::useless_vec)]
1+
//@aux-build:proc_macros.rs
2+
#![allow(clippy::borrow_deref_ref, clippy::deref_addrof, clippy::identity_op, unused)]
3+
#![warn(clippy::ptr_to_temporary)]
4+
#![no_main]
25

3-
use std::ptr::addr_of;
6+
#[macro_use]
7+
extern crate proc_macros;
48

5-
fn a() -> i32 {
6-
0
9+
fn bad_1() -> *const i32 {
10+
&(100 + *&0) as *const i32
711
}
812

9-
struct Vec3 {
10-
x: f32,
11-
y: f32,
12-
z: f32,
13+
fn bad_2() -> *const i32 {
14+
let a = 0i32;
15+
&(*&a) as *const i32
1316
}
1417

15-
fn main() {
16-
let _p = &0 as *const i32;
17-
let _p = &a() as *const i32;
18-
let vec = vec![1];
19-
let _p = &vec.len() as *const usize;
20-
let x = &(1 + 2) as *const i32;
21-
let x = &(x as *const i32) as *const *const i32;
18+
fn bad_3() -> *const i32 {
19+
let a = 0i32;
20+
&a as *const i32
21+
}
22+
23+
fn bad_4() -> *const i32 {
24+
let mut a = 0i32;
25+
&a as *const i32
26+
}
27+
28+
fn bad_5() -> *const i32 {
29+
const A: &i32 = &1i32;
30+
let mut a = 0i32;
2231

23-
// Do not lint...
24-
let ptr = &Vec3 { x: 1.0, y: 2.0, z: 3.0 };
25-
let some_variable = 1i32;
26-
let x = &(*ptr).x as *const f32;
27-
let x = &(some_variable) as *const i32;
32+
if true {
33+
return &(*A) as *const i32;
34+
}
35+
&a as *const i32
36+
}
37+
38+
fn fine_1() -> *const i32 {
39+
&100 as *const i32
40+
}
41+
42+
fn fine_2() -> *const i32 {
43+
const A: &i32 = &0i32;
44+
A as *const i32
45+
}
46+
47+
fn fine_3() -> *const i32 {
48+
const A: &i32 = &0i32;
49+
&(*A) as *const i32
50+
}
51+
52+
external! {
53+
fn fine_external() -> *const i32 {
54+
let a = 0i32;
55+
&a as *const i32
56+
}
57+
}
2858

29-
// ...As `addr_of!` does not report anything out of the ordinary
30-
let ptr = &Vec3 { x: 1.0, y: 2.0, z: 3.0 };
31-
let some_variable = 1i32;
32-
let x = addr_of!((*ptr).x) as *const f32;
33-
let x = addr_of!(some_variable);
59+
with_span! {
60+
span
61+
fn fine_proc_macro() -> *const i32 {
62+
let a = 0i32;
63+
&a as *const i32
64+
}
3465
}

0 commit comments

Comments
 (0)