Skip to content

Commit

Permalink
Only lint non-promoted temporaries
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jun 26, 2023
1 parent 96c9004 commit 1f10fb5
Show file tree
Hide file tree
Showing 8 changed files with 184 additions and 117 deletions.
30 changes: 0 additions & 30 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ mod fn_to_numeric_cast_any;
mod fn_to_numeric_cast_with_truncation;
mod ptr_as_ptr;
mod ptr_cast_constness;
mod ptr_to_temporary;
mod unnecessary_cast;
mod utils;

Expand Down Expand Up @@ -658,33 +657,6 @@ declare_clippy_lint! {
"casting a known floating-point NaN into an integer"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for raw pointers that point to temporary values.
///
/// ### Why is this bad?
/// Usage of such a pointer can result in Undefined Behavior, as the pointer will stop pointing
/// to valid stack memory once the temporary is dropped.
///
/// ### Example
/// ```rust,ignore
/// const PS: [usize; 3] = [2usize, 3usize, 11usize];
/// let mut ps = vec![];
///
/// for p in PS {
/// ps.push(&p as *const usize);
/// }
///
/// for p in ps {
/// unsafe { p.read() }; // ⚠️
/// }
/// ```
#[clippy::version = "1.72.0"]
pub PTR_TO_TEMPORARY,
correctness,
"disallows obtaining a raw pointer to a temporary value"
}

pub struct Casts {
msrv: Msrv,
}
Expand Down Expand Up @@ -719,7 +691,6 @@ impl_lint_pass!(Casts => [
CAST_SLICE_FROM_RAW_PARTS,
AS_PTR_CAST_MUT,
CAST_NAN_TO_INT,
PTR_TO_TEMPORARY,
]);

impl<'tcx> LateLintPass<'tcx> for Casts {
Expand Down Expand Up @@ -765,7 +736,6 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
}

as_underscore::check(cx, expr, cast_to_hir);
ptr_to_temporary::check(cx, expr, cast_expr, cast_to_hir);

if self.msrv.meets(msrvs::BORROW_AS_PTR) {
borrow_as_ptr::check(cx, expr, cast_expr, cast_to_hir);
Expand Down
34 changes: 0 additions & 34 deletions clippy_lints/src/casts/ptr_to_temporary.rs

This file was deleted.

2 changes: 1 addition & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION_INFO,
crate::casts::PTR_AS_PTR_INFO,
crate::casts::PTR_CAST_CONSTNESS_INFO,
crate::casts::PTR_TO_TEMPORARY_INFO,
crate::casts::UNNECESSARY_CAST_INFO,
crate::checked_conversions::CHECKED_CONVERSIONS_INFO,
crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
Expand Down Expand Up @@ -532,6 +531,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::ptr::MUT_FROM_REF_INFO,
crate::ptr::PTR_ARG_INFO,
crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO,
crate::ptr_to_temporary::PTR_TO_TEMPORARY_INFO,
crate::pub_use::PUB_USE_INFO,
crate::question_mark::QUESTION_MARK_INFO,
crate::question_mark_used::QUESTION_MARK_USED_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ mod permissions_set_readonly_false;
mod precedence;
mod ptr;
mod ptr_offset_with_cast;
mod ptr_to_temporary;
mod pub_use;
mod question_mark;
mod question_mark_used;
Expand Down Expand Up @@ -1047,6 +1048,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let stack_size_threshold = conf.stack_size_threshold;
store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold)));
store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit));
store.register_late_pass(move |_| Box::new(ptr_to_temporary::PtrToTemporary));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
70 changes: 70 additions & 0 deletions clippy_lints/src/ptr_to_temporary.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use clippy_utils::{consts::is_promotable, diagnostics::span_lint_and_note, is_from_proc_macro};
use rustc_hir::{BorrowKind, Expr, ExprKind, ItemKind, OwnerNode};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// ### What it does
/// Checks for raw pointers pointing to temporary values that will **not** be promoted to a
/// constant through
/// [constant promotion](https://doc.rust-lang.org/stable/reference/destructors.html#constant-promotion).
///
/// ### Why is this bad?
/// Usage of such a pointer can result in Undefined Behavior, as the pointer will stop pointing
/// to valid stack memory once the temporary is dropped.
///
/// ### Example
/// ```rust,ignore
/// fn x() -> *const i32 {
/// let x = 0;
/// &x as *const i32
/// }
///
/// let x = x();
/// unsafe { *x }; // ⚠️
/// ```
#[clippy::version = "1.72.0"]
pub PTR_TO_TEMPORARY,
correctness,
"disallows returning a raw pointer to a temporary value"
}
declare_lint_pass!(PtrToTemporary => [PTR_TO_TEMPORARY]);

impl<'tcx> LateLintPass<'tcx> for PtrToTemporary {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if in_external_macro(cx.sess(), expr.span) {
return;
}

// Get the final return statement if this is a return statement, or don't lint
let expr = if let ExprKind::Ret(Some(expr)) = expr.kind {
expr
} else if let OwnerNode::Item(parent) = cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id))
&& let ItemKind::Fn(_, _, body) = parent.kind
&& let block = cx.tcx.hir().body(body).value
&& let ExprKind::Block(block, _) = block.kind
&& let Some(final_block_expr) = block.expr
&& final_block_expr.hir_id == expr.hir_id
{
expr
} else {
return;
};

if let ExprKind::Cast(cast_expr, _) = expr.kind
&& let ExprKind::AddrOf(BorrowKind::Ref, _, e) = cast_expr.kind
&& !is_promotable(cx, e)
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_note(
cx,
PTR_TO_TEMPORARY,
expr.span,
"returning a raw pointer to a temporary value that cannot be promoted to a constant",
None,
"usage of this pointer by callers will cause Undefined Behavior",
);
}
}
}
30 changes: 29 additions & 1 deletion clippy_utils/src/consts.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#![allow(clippy::float_cmp)]

use crate::source::{get_source_text, walk_span_to_context};
use crate::{clip, is_direct_expn_of, sext, unsext};
use crate::visitors::for_each_expr;
use crate::{clip, is_ctor_or_promotable_const_function, is_direct_expn_of, path_res, sext, unsext};
use if_chain::if_chain;
use rustc_ast::ast::{self, LitFloatType, LitKind};
use rustc_data_structures::sync::Lrc;
Expand All @@ -19,6 +20,7 @@ use rustc_span::SyntaxContext;
use std::cmp::Ordering::{self, Equal};
use std::hash::{Hash, Hasher};
use std::iter;
use std::ops::ControlFlow;

/// A `LitKind`-like enum to fold constant `Expr`s into.
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -708,3 +710,29 @@ pub fn miri_to_const<'tcx>(tcx: TyCtxt<'tcx>, result: mir::ConstantKind<'tcx>) -
_ => None,
}
}

/// Returns whether a certain `Expr` will be promoted to a constant.
pub fn is_promotable<'tcx>(cx: &LateContext<'tcx>, start: &'tcx Expr<'tcx>) -> bool {
let ty = cx.typeck_results().expr_ty(start);
for_each_expr(start, |expr| {
match expr.kind {
ExprKind::Binary(_, _, _) => ControlFlow::Break(()),
ExprKind::Unary(UnOp::Deref, e) if !matches!(path_res(cx, e), Res::Def(DefKind::Const, _)) => {
ControlFlow::Break(())
},
ExprKind::Call(_, _) if !is_ctor_or_promotable_const_function(cx, expr) => ControlFlow::Break(()),
ExprKind::MethodCall(..) if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
&& !cx.tcx.is_promotable_const_fn(def_id) =>
{
ControlFlow::Break(())
},
ExprKind::Path(qpath) if let Res::Local(_) = cx.qpath_res(&qpath, expr.hir_id) => {
ControlFlow::Break(())
},
_ => ControlFlow::Continue(()),
}
})
.is_none()
&& ty.is_freeze(cx.tcx, cx.param_env)
&& !ty.needs_drop(cx.tcx, cx.param_env)
}
81 changes: 56 additions & 25 deletions tests/ui/ptr_to_temporary.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,65 @@
#![allow(clippy::explicit_auto_deref, clippy::unnecessary_cast, clippy::useless_vec)]
//@aux-build:proc_macros.rs
#![allow(clippy::borrow_deref_ref, clippy::deref_addrof, clippy::identity_op, unused)]
#![warn(clippy::ptr_to_temporary)]
#![no_main]

use std::ptr::addr_of;
#[macro_use]
extern crate proc_macros;

fn a() -> i32 {
0
fn bad_1() -> *const i32 {
&(100 + *&0) as *const i32
}

struct Vec3 {
x: f32,
y: f32,
z: f32,
fn bad_2() -> *const i32 {
let a = 0i32;
&(*&a) as *const i32
}

fn main() {
let _p = &0 as *const i32;
let _p = &a() as *const i32;
let vec = vec![1];
let _p = &vec.len() as *const usize;
let x = &(1 + 2) as *const i32;
let x = &(x as *const i32) as *const *const i32;
fn bad_3() -> *const i32 {
let a = 0i32;
&a as *const i32
}

fn bad_4() -> *const i32 {
let mut a = 0i32;
&a as *const i32
}

fn bad_5() -> *const i32 {
const A: &i32 = &1i32;
let mut a = 0i32;

// Do not lint...
let ptr = &Vec3 { x: 1.0, y: 2.0, z: 3.0 };
let some_variable = 1i32;
let x = &(*ptr).x as *const f32;
let x = &(some_variable) as *const i32;
if true {
return &(*A) as *const i32;
}
&a as *const i32
}

fn fine_1() -> *const i32 {
&100 as *const i32
}

fn fine_2() -> *const i32 {
const A: &i32 = &0i32;
A as *const i32
}

fn fine_3() -> *const i32 {
const A: &i32 = &0i32;
&(*A) as *const i32
}

external! {
fn fine_external() -> *const i32 {
let a = 0i32;
&a as *const i32
}
}

// ...As `addr_of!` does not report anything out of the ordinary
let ptr = &Vec3 { x: 1.0, y: 2.0, z: 3.0 };
let some_variable = 1i32;
let x = addr_of!((*ptr).x) as *const f32;
let x = addr_of!(some_variable);
with_span! {
span
fn fine_proc_macro() -> *const i32 {
let a = 0i32;
&a as *const i32
}
}

0 comments on commit 1f10fb5

Please sign in to comment.