Add new lint to detect dangling pointer return from closure

CHANGELOG.md

@@ -5620,6 +5620,7 @@ Released 2018-09-13
 [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
 [`as_pointer_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_pointer_underscore
 [`as_ptr_cast_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut
+[`as_ptr_in_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_in_map
 [`as_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore
 [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
 [`assertions_on_result_states`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_result_states

clippy_lints/src/as_ptr_in_map.rs

@@ -0,0 +1,176 @@
+use std::ops::ControlFlow;
+
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::ty::is_copy;
+use clippy_utils::visitors::for_each_expr;
+use rustc_hir as hir;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::declare_lint_pass;
+use rustc_span::{Ident, Symbol};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for closure creating raw pointer from owned type and returning raw pointer
+    ///
+    /// ### Why is this bad?
+    /// It might create a dangling pointer becaused owned type in a closure are dropped after the call
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let v_opt = Some(vec![1]);
+    /// let _v_ptr = v_opt.map(|v| v.as_ptr());
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let v_opt = Some(vec![1]);
+    /// let _v_ptr = v_opt.as_ref().map(|v| v.as_ptr());
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub AS_PTR_IN_MAP,
+    nursery,
+    "check raw pointer inside closure from owned type"
+}
+declare_lint_pass!(AsPtrInMap => [AS_PTR_IN_MAP]);
+
+impl<'tcx> LateLintPass<'tcx> for AsPtrInMap {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
+        if let hir::ExprKind::Closure(hir::Closure { body, .. }) = expr.kind {
+            let typeck = cx.typeck_results();
+            let closure_output = if let ty::Closure(_, closure_subs) = typeck.expr_ty(expr).kind() {
+                let closure = closure_subs.as_closure();
+                let closure_sig = cx
+                    .tcx
+                    .signature_unclosure(closure.sig(), hir::Safety::Safe)
+                    .skip_binder();
+
+                closure_sig.output()
+            } else {
+                return;
+            };
+
+            // check if the return type of the closure contains a raw pointer
+            if ty_contains_raw_ptr(closure_output) {
+                let closure_body = cx.tcx.hir_body(*body);
+
+                let mut idents_needs_drop: Vec<_> = closure_body
+                    .params
+                    .iter()
+                    .flat_map(|param| get_ident_bindings(*param.pat))
+                    .filter(|(_, hir_id)| {
+                        let ty = typeck.node_type(*hir_id);
+                        !is_copy(cx, ty) && ty.needs_drop(cx.tcx, cx.typing_env())
+                    })
+                    .map(|(i, _)| (i, false))
+                    .collect();
+
+                // get usage of call that create raw pointer inside the closure
+                let _ = for_each_expr(cx, expr, |e| {
+                    if let Some(receiver) = is_creating_raw_ptr(*e) {
+                        for (ident, is_creating_raw_ptr) in &mut idents_needs_drop {
+                            if ident_eq(*ident, receiver) {
+                                *is_creating_raw_ptr = true;
+                            }
+                        }
+                    }
+                    if false {
+                        return ControlFlow::Break(());
+                    }
+                    ControlFlow::Continue(())
+                });
+
+                let indents_make_raw_pointer = idents_needs_drop
+                    .iter()
+                    .filter_map(|(ident, is_creating_raw_ptr)| is_creating_raw_ptr.then_some(ident))
+                    .collect::<Vec<_>>();
+
+                if !indents_make_raw_pointer.is_empty() {
+                    span_lint_and_then(
+                        cx,
+                        AS_PTR_IN_MAP,
+                        expr.span,
+                        "this closure might return a dangling pointer",
+                        |diag| {
+                            for ident in indents_make_raw_pointer {
+                                diag.span_note(
+                                    ident.span,
+                                    "this bindings is used to create a raw pointer that might be dangling",
+                                );
+                            }
+                        },
+                    );
+                }
+            }
+        }
+    }
+}
+
+fn ident_eq(name: Ident, path: hir::Expr<'_>) -> bool {
+    match path.kind {
+        hir::ExprKind::Path(hir::QPath::Resolved(None, path)) => {
+            path.segments.len() == 1 && path.segments[0].ident == name
+        },
+        hir::ExprKind::AddrOf(_, _, expr) => ident_eq(name, *expr),
+        _ => false,
+    }
+}
+
+fn ty_contains_raw_ptr(ty: ty::Ty<'_>) -> bool {
+    match ty.kind() {
+        #[allow(clippy::match_same_arms)]
+        ty::Adt(_, _) => false, // TODO: might contain raw pointer
+        ty::Array(ty, _) | ty::Slice(ty) => ty_contains_raw_ptr(*ty),
+        ty::RawPtr(_, _) => true,
+        ty::Tuple(ty_list) => ty_list.iter().any(|ty| ty_contains_raw_ptr(ty)),
+        _ => false,
+    }
+}
+
+fn get_ident_bindings(pat: hir::Pat<'_>) -> Vec<(Ident, hir::HirId)> {
+    match pat.kind {
+        hir::PatKind::Binding(hir::BindingMode::NONE | hir::BindingMode::MUT, hir_id, ident, _) => {
+            vec![(ident, hir_id)]
+        },
+        hir::PatKind::Struct(_, pat_fields, _) => pat_fields
+            .iter()
+            .flat_map(|pat_field| get_ident_bindings(*pat_field.pat))
+            .collect(),
+        hir::PatKind::TupleStruct(_, pats, _) => pats.iter().flat_map(|pat| get_ident_bindings(*pat)).collect(),
+        hir::PatKind::Tuple(pats, _) => pats.iter().flat_map(|pat| get_ident_bindings(*pat)).collect(),
+        hir::PatKind::Box(pat) => get_ident_bindings(*pat),
+        hir::PatKind::Slice(pats1, pat_opt, pats2) => pats1
+            .iter()
+            .flat_map(|pat| get_ident_bindings(*pat))
+            .chain(pat_opt.iter().flat_map(|pat| get_ident_bindings(**pat)))
+            .chain(pats2.iter().flat_map(|pat| get_ident_bindings(*pat)))
+            .collect(),
+        _ => vec![],
+    }
+    // });
+}
+
+fn is_creating_raw_ptr(expr: hir::Expr<'_>) -> Option<hir::Expr<'_>> {
+    match expr.kind {
+        hir::ExprKind::MethodCall(method, receiver, [], _) => {
+            if is_dangerous_ptr(method.ident.name) {
+                return Some(*receiver);
+            }
+        },
+        hir::ExprKind::Call(function, [arg]) => {
+            let hir::ExprKind::Path(hir::QPath::TypeRelative(_, path_segment)) = function.kind else {
+                return None;
+            };
+
+            if is_dangerous_ptr(path_segment.ident.name) {
+                return Some(*arg);
+            }
+        },
+        _ => (),
+    }
+
+    None
+}
+
+fn is_dangerous_ptr(s: Symbol) -> bool {
+    matches!(s.as_str(), "as_ptr" | "as_non_null" | "as_mut_ptr")
+}

clippy_lints/src/declared_lints.rs

@@ -9,6 +9,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::arbitrary_source_item_ordering::ARBITRARY_SOURCE_ITEM_ORDERING_INFO,
     crate::arc_with_non_send_sync::ARC_WITH_NON_SEND_SYNC_INFO,
     crate::as_conversions::AS_CONVERSIONS_INFO,
+    crate::as_ptr_in_map::AS_PTR_IN_MAP_INFO,
     crate::asm_syntax::INLINE_ASM_X86_ATT_SYNTAX_INFO,
     crate::asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX_INFO,
     crate::assertions_on_constants::ASSERTIONS_ON_CONSTANTS_INFO,

clippy_lints/src/lib.rs

@@ -76,6 +76,7 @@ mod approx_const;
 mod arbitrary_source_item_ordering;
 mod arc_with_non_send_sync;
 mod as_conversions;
+mod as_ptr_in_map;
 mod asm_syntax;
 mod assertions_on_constants;
 mod assertions_on_result_states;
@@ -830,5 +831,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
+    store.register_late_pass(|_| Box::new(as_ptr_in_map::AsPtrInMap));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

tests/ui/as_ptr_in_map.rs

@@ -0,0 +1,36 @@
+#![warn(clippy::as_ptr_in_map)]
+
+fn main() {
+    let v1_res = Ok(vec![1]);
+    let _v1_ptr: Result<_, ()> = v1_res.map(|v1| v1.as_ptr());
+    //~^ as_ptr_in_map
+
+    let v2_opt = Some((vec![2], 2));
+    let _v2_ptr = v2_opt.map(|(v2, _x)| {
+        v2.as_ptr();
+        2
+    });
+    // this is fine
+
+    let v3_opt = Some((vec![3], 3));
+    let _v3_ptr = v3_opt.map(|(v3, x)| {
+        //~^ as_ptr_in_map
+        let _a = x + 2;
+        let p = v3.as_ptr();
+        let _b = 6;
+        p
+    });
+
+    let v4_res = Ok(vec![4]);
+    let _v4_ptr: Result<_, &()> = v4_res.as_ref().map(|v4| v4.as_ptr());
+    // this is fine
+
+    let v5_opt = Some(vec![5]);
+    let _v5_ptr = v5_opt.map(|v5| std::vec::Vec::as_ptr(&v5));
+    //~^ as_ptr_in_map
+
+    let v6_res = Ok(vec![6]);
+    let v6_2_ptr = [6];
+    let _v6_ptr: Result<_, ()> = v6_res.map(|_v6| v6_2_ptr.as_ptr());
+    // this is fine
+}

tests/ui/as_ptr_in_map.stderr

@@ -0,0 +1,47 @@
+error: this closure might return a dangling pointer
+  --> tests/ui/as_ptr_in_map.rs:5:45
+   |
+LL |     let _v1_ptr: Result<_, ()> = v1_res.map(|v1| v1.as_ptr());
+   |                                             ^^^^^^^^^^^^^^^^
+   |
+note: this bindings is used to create a raw pointer that might be dangling
+  --> tests/ui/as_ptr_in_map.rs:5:46
+   |
+LL |     let _v1_ptr: Result<_, ()> = v1_res.map(|v1| v1.as_ptr());
+   |                                              ^^
+   = note: `-D clippy::as-ptr-in-map` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::as_ptr_in_map)]`
+
+error: this closure might return a dangling pointer
+  --> tests/ui/as_ptr_in_map.rs:16:30
+   |
+LL |       let _v3_ptr = v3_opt.map(|(v3, x)| {
+   |  ______________________________^
+LL | |
+LL | |         let _a = x + 2;
+LL | |         let p = v3.as_ptr();
+LL | |         let _b = 6;
+LL | |         p
+LL | |     });
+   | |_____^
+   |
+note: this bindings is used to create a raw pointer that might be dangling
+  --> tests/ui/as_ptr_in_map.rs:16:32
+   |
+LL |     let _v3_ptr = v3_opt.map(|(v3, x)| {
+   |                                ^^
+
+error: this closure might return a dangling pointer
+  --> tests/ui/as_ptr_in_map.rs:29:30
+   |
+LL |     let _v5_ptr = v5_opt.map(|v5| std::vec::Vec::as_ptr(&v5));
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: this bindings is used to create a raw pointer that might be dangling
+  --> tests/ui/as_ptr_in_map.rs:29:31
+   |
+LL |     let _v5_ptr = v5_opt.map(|v5| std::vec::Vec::as_ptr(&v5));
+   |                               ^^
+
+error: aborting due to 3 previous errors
+