Auto merge of rust-lang#128985 - GrigorenkoPV:instantly-dangling-poin…

…ter, r=<try>

Lint against getting pointers from immediately dropped temporaries

Fixes rust-lang#123613

## Changes:
1. New lint: `dangling_pointers_from_temporaries`. Is a generalization of `temporary_cstring_as_ptr` for more types and more ways to get a temporary.
2. `temporary_cstring_as_ptr` is marked as renamed to `dangling_pointers_from_temporaries`.
3. `clippy::temporary_cstring_as_ptr` is marked as renamed to `dangling_pointers_from_temporaries`.
4. `core::cell::Cell` is now `rustc_diagnostic_item = "Cell"`

## TODO:
- [x] ~Add tests for different types~
- [x] ~Add tests for different ways of getting a temporary~
- [x] ~Regroup tests for this and `temporary_cstring_as_ptr`~
- [x] ~Fix a strange ICE when the lint is `allow`ed.~
- [x] ~Check what happens when you `break` with a bound variable from `loop`/`match`/`if`/`block`.~
- [x] ~Document the lint~
- [x] ~Fix tests rust-lang#128985 (comment)
- [x] ~Fix clippy~
- [x] ~Fix miri rust-lang#128985 (review)
- [ ] Crater run?

## Questions:
- [ ] Should we make something like `is_temporary_rvalue` (but not bogus) available in compiler?
- [x] ~Should `temporary_cstring_as_ptr` be deprecated? Across an edition boundary?~
- [ ] Instead of manually checking for a list of known methods, maybe add some sort of annotation to those methods in library and check for the presence of that annotation?
- [ ] Maybe even introduce some form of primitive lifetimes for pointers and check those in borrow-checker?

## Future improvements:
- [ ] Fix false negatives[^fn]
- [ ] Add suggestions rust-lang#128985 (comment)

## Known limitations:

### False negatives[^fn]:

[^fn]: lint **should** be emitted, but **is not**

- `temporary_unsafe_cell.get()`
- `temporary.field.as_ptr()`
- `temporary[index].as_ptr()`
- Converting `&temporary` to pointer with `as` or stuff like `ptr::from_ref`.
- The `&raw [mut] temporary`

### False positives[^fp]:

[^fp]: lint **should not** be emitted, but **is**

Both this lint and the already existing `temporary_cstring_as_ptr` will fire on code like this:
```rust
foo(CString::new("hello").unwrap().as_ptr())
```
even though using the resulting pointer inside of `foo` is completely fine (at least according to miri),
probably due to argument promotion logic.

compiler/rustc_lint/messages.ftl

@@ -209,12 +209,6 @@ lint_crate_name_in_cfg_attr_deprecated =
 lint_crate_type_in_cfg_attr_deprecated =
     `crate_type` within an `#![cfg_attr]` attribute is deprecated
 
-lint_cstring_ptr = getting the inner pointer of a temporary `CString`
-    .as_ptr_label = this pointer will be invalid
-    .unwrap_label = this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
-    .note = pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
-    .help = for more information, see https://doc.rust-lang.org/reference/destructors.html
-
 lint_custom_inner_attribute_unstable = custom inner attributes are unstable
 
 lint_default_hash_types = prefer `{$preferred}` over `{$used}`, it has better performance
@@ -411,6 +405,12 @@ lint_incomplete_include =
 
 lint_inner_macro_attribute_unstable = inner macro attributes are unstable
 
+lint_instantly_dangling = getting a pointer from a temporary `{$ty}` will result in a dangling pointer
+    .label_ptr = this pointer will immediately be invalid
+    .label_temporary = this `{$ty}` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
+    .note = pointers do not have a lifetime; when calling `{$callee}` the `{$ty}` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
+    .help = for more information, see https://doc.rust-lang.org/reference/destructors.html
+
 lint_invalid_asm_label_binary = avoid using labels containing only the digits `0` and `1` in inline assembly
     .label = use a different label that doesn't start with `0` or `1`
     .help = start numbering with `2` instead

compiler/rustc_lint/src/dangling.rs

@@ -0,0 +1,137 @@
+use rustc_hir::{Expr, ExprKind, LangItem};
+use rustc_middle::ty::{Ty, TyCtxt};
+use rustc_session::{declare_lint, declare_lint_pass};
+use rustc_span::symbol::sym;
+
+use crate::lints::InstantlyDangling;
+use crate::{LateContext, LateLintPass, LintContext};
+
+// FIXME: does not catch UnsafeCell::get
+// FIXME: does not catch getting a ref to a temporary and then converting it to a ptr
+declare_lint! {
+    /// The `dangling_pointers_from_temporaries` lint detects getting a pointer to data
+    /// of a temporary that will immediately get dropped.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// # #![allow(unused)]
+    /// # unsafe fn use_data(ptr: *const u8) {
+    /// #     dbg!(unsafe { ptr.read() });
+    /// # }
+    /// fn gather_and_use(bytes: impl Iterator<Item = u8>) {
+    ///     let x: *const u8 = bytes.collect::<Vec<u8>>().as_ptr();
+    ///     unsafe { use_data(x) }
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Getting a pointer from a temporary value will not prolong its lifetime,
+    /// which means that the value can be dropped and the allocation freed
+    /// while the pointer still exists, making the pointer dangling.
+    /// This is not an error (as far as the type system is concerned)
+    /// but probably is not what the user intended either.
+    ///
+    /// If you need stronger guarantees, consider using references instead,
+    /// as they are statically verified by the borrow-checker to never dangle.
+    pub DANGLING_POINTERS_FROM_TEMPORARIES,
+    Warn,
+    "detects getting a pointer from a temporary"
+}
+
+declare_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]);
+
+impl<'tcx> LateLintPass<'tcx> for DanglingPointers {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if let ExprKind::MethodCall(method, receiver, _args, _span) = expr.kind
+            && matches!(method.ident.name, sym::as_ptr | sym::as_mut_ptr)
+            && is_temporary_rvalue(receiver)
+            && let ty = cx.typeck_results().expr_ty(receiver)
+            && is_interesting(cx.tcx, ty)
+        {
+            cx.emit_span_lint(
+                DANGLING_POINTERS_FROM_TEMPORARIES,
+                method.ident.span,
+                InstantlyDangling {
+                    callee: method.ident.name,
+                    ty,
+                    ptr_span: method.ident.span,
+                    temporary_span: receiver.span,
+                },
+            )
+        }
+    }
+}
+
+fn is_temporary_rvalue(expr: &Expr<'_>) -> bool {
+    match expr.kind {
+        // Const is not temporary.
+        ExprKind::ConstBlock(..) | ExprKind::Repeat(..) | ExprKind::Lit(..) => false,
+
+        // This is literally lvalue.
+        ExprKind::Path(..) => false,
+
+        // Calls return rvalues.
+        ExprKind::Call(..) | ExprKind::MethodCall(..) | ExprKind::Binary(..) => true,
+
+        // Inner blocks are rvalues.
+        ExprKind::If(..) | ExprKind::Loop(..) | ExprKind::Match(..) | ExprKind::Block(..) => true,
+
+        // FIXME: these should probably recurse and typecheck along the way.
+        //        Some false negatives are possible for now.
+        ExprKind::Index(..) | ExprKind::Field(..) | ExprKind::Unary(..) => false,
+
+        ExprKind::Struct(..) => true,
+
+        // FIXME: this has false negatives, but I do not want to deal with 'static/const promotion just yet.
+        ExprKind::Array(..) => false,
+
+        // These typecheck to `!`
+        ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Become(..) => {
+            false
+        }
+
+        // These typecheck to `()`
+        ExprKind::Assign(..) | ExprKind::AssignOp(..) | ExprKind::Yield(..) => false,
+
+        // Compiler-magic macros
+        ExprKind::AddrOf(..) | ExprKind::OffsetOf(..) | ExprKind::InlineAsm(..) => false,
+
+        // We are not interested in these
+        ExprKind::Cast(..)
+        | ExprKind::Closure(..)
+        | ExprKind::Tup(..)
+        | ExprKind::DropTemps(..)
+        | ExprKind::Let(..) => false,
+
+        // Not applicable
+        ExprKind::Type(..) | ExprKind::Err(..) => false,
+    }
+}
+
+// Array, Vec, String, CString, MaybeUninit, Cell, Box<[_]>, Box<str>, Box<CStr>,
+// or any of the above in arbitrary many nested Box'es.
+fn is_interesting(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool {
+    if ty.is_array() {
+        true
+    } else if ty.is_box() {
+        let inner = ty.boxed_ty();
+        inner.is_slice()
+            || inner.is_str()
+            || inner.ty_adt_def().is_some_and(|def| tcx.is_lang_item(def.did(), LangItem::CStr))
+            || is_interesting(tcx, inner)
+    } else if let Some(def) = ty.ty_adt_def() {
+        for lang_item in [LangItem::String, LangItem::MaybeUninit] {
+            if tcx.is_lang_item(def.did(), lang_item) {
+                return true;
+            }
+        }
+        tcx.get_diagnostic_name(def.did())
+            .is_some_and(|name| matches!(name, sym::cstring_type | sym::Vec | sym::Cell))
+    } else {
+        false
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -45,6 +45,7 @@ mod async_closures;
 mod async_fn_in_trait;
 pub mod builtin;
 mod context;
+mod dangling;
 mod deref_into_dyn_supertrait;
 mod drop_forget_useless;
 mod early;
@@ -63,7 +64,6 @@ mod levels;
 mod lints;
 mod macro_expr_fragment_specifier_2024_migration;
 mod map_unit_fn;
-mod methods;
 mod multiple_supertrait_upcastable;
 mod non_ascii_idents;
 mod non_fmt_panic;
@@ -87,6 +87,7 @@ mod unused;
 use async_closures::AsyncClosureUsage;
 use async_fn_in_trait::AsyncFnInTrait;
 use builtin::*;
+use dangling::*;
 use deref_into_dyn_supertrait::*;
 use drop_forget_useless::*;
 use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
@@ -98,7 +99,6 @@ use invalid_from_utf8::*;
 use let_underscore::*;
 use macro_expr_fragment_specifier_2024_migration::*;
 use map_unit_fn::*;
-use methods::*;
 use multiple_supertrait_upcastable::*;
 use non_ascii_idents::*;
 use non_fmt_panic::NonPanicFmt;
@@ -225,7 +225,7 @@ late_lint_methods!(
             UngatedAsyncFnTrackCaller: UngatedAsyncFnTrackCaller,
             ShadowedIntoIter: ShadowedIntoIter,
             DropTraitConstraints: DropTraitConstraints,
-            TemporaryCStringAsPtr: TemporaryCStringAsPtr,
+            DanglingPointers: DanglingPointers,
             NonPanicFmt: NonPanicFmt,
             NoopMethodCall: NoopMethodCall,
             EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums,
@@ -346,6 +346,7 @@ fn register_builtins(store: &mut LintStore) {
     store.register_renamed("non_fmt_panic", "non_fmt_panics");
     store.register_renamed("unused_tuple_struct_fields", "dead_code");
     store.register_renamed("static_mut_ref", "static_mut_refs");
+    store.register_renamed("temporary_cstring_as_ptr", "dangling_pointers_from_temporaries");
 
     // These were moved to tool lints, but rustc still sees them when compiling normally, before
     // tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use

compiler/rustc_lint/src/lints.rs

@@ -1124,16 +1124,19 @@ pub struct IgnoredUnlessCrateSpecified<'a> {
     pub name: Symbol,
 }
 
-// methods.rs
+// dangling.rs
 #[derive(LintDiagnostic)]
-#[diag(lint_cstring_ptr)]
+#[diag(lint_instantly_dangling)]
 #[note]
 #[help]
-pub struct CStringPtr {
-    #[label(lint_as_ptr_label)]
-    pub as_ptr: Span,
-    #[label(lint_unwrap_label)]
-    pub unwrap: Span,
+// FIXME: use #[primary_span]
+pub struct InstantlyDangling<'tcx> {
+    pub callee: Symbol,
+    pub ty: Ty<'tcx>,
+    #[label(lint_label_ptr)]
+    pub ptr_span: Span,
+    #[label(lint_label_temporary)]
+    pub temporary_span: Span,
 }
 
 // multiple_supertrait_upcastable.rs

compiler/rustc_span/src/symbol.rs

@@ -170,6 +170,7 @@ symbols! {
         CallOnceFuture,
         CallRefFuture,
         Capture,
+        Cell,
         Center,
         Cleanup,
         Clone,
@@ -411,6 +412,7 @@ symbols! {
         arm,
         arm_target_feature,
         array,
+        as_mut_ptr,
         as_ptr,
         as_ref,
         as_str,

library/alloc/tests/boxed.rs

@@ -4,6 +4,7 @@ use core::mem::MaybeUninit;
 use core::ptr::NonNull;
 
 #[test]
+#[cfg_attr(not(bootstrap), expect(dangling_pointers_from_temporaries))]
 fn uninitialized_zero_size_box() {
     assert_eq!(
         &*Box::<()>::new_uninit() as *const _,

library/core/src/cell.rs

@@ -304,6 +304,7 @@ pub use once::OnceCell;
 /// ```
 ///
 /// See the [module-level documentation](self) for more.
+#[cfg_attr(not(test), rustc_diagnostic_item = "Cell")]
 #[stable(feature = "rust1", since = "1.0.0")]
 #[repr(transparent)]
 pub struct Cell<T: ?Sized> {

library/core/src/ffi/c_str.rs

@@ -463,7 +463,9 @@ impl CStr {
     /// behavior when `ptr` is used inside the `unsafe` block:
     ///
     /// ```no_run
-    /// # #![allow(unused_must_use)] #![allow(temporary_cstring_as_ptr)]
+    /// # #![allow(unused_must_use)]
+    /// # #![cfg_attr(bootstrap, expect(temporary_cstring_as_ptr))]
+    /// # #![cfg_attr(not(bootstrap), expect(dangling_pointers_from_temporaries))]
     /// use std::ffi::CString;
     ///
     /// // Do not do this:

src/tools/clippy/clippy_lints/src/deprecated_lints.rs

@@ -166,7 +166,7 @@ declare_with_version! { RENAMED(RENAMED_VERSION): &[(&str, &str)] = &[
     #[clippy::version = ""]
     ("clippy::positional_named_format_parameters", "named_arguments_used_positionally"),
     #[clippy::version = ""]
-    ("clippy::temporary_cstring_as_ptr", "temporary_cstring_as_ptr"),
+    ("clippy::temporary_cstring_as_ptr", "dangling_pointers_from_temporaries"),
     #[clippy::version = ""]
     ("clippy::undropped_manually_drops", "undropped_manually_drops"),
     #[clippy::version = ""]

src/tools/clippy/tests/ui/rename.fixed

@@ -54,7 +54,7 @@
 #![allow(enum_intrinsics_non_enums)]
 #![allow(non_fmt_panics)]
 #![allow(named_arguments_used_positionally)]
-#![allow(temporary_cstring_as_ptr)]
+#![allow(dangling_pointers_from_temporaries)]
 #![allow(undropped_manually_drops)]
 #![allow(unknown_lints)]
 #![allow(unused_labels)]
@@ -120,7 +120,7 @@
 #![warn(unexpected_cfgs)] //~ ERROR: lint `clippy::mismatched_target_os`
 #![warn(non_fmt_panics)] //~ ERROR: lint `clippy::panic_params`
 #![warn(named_arguments_used_positionally)] //~ ERROR: lint `clippy::positional_named_format_parameters`
-#![warn(temporary_cstring_as_ptr)] //~ ERROR: lint `clippy::temporary_cstring_as_ptr`
+#![warn(dangling_pointers_from_temporaries)] //~ ERROR: lint `clippy::temporary_cstring_as_ptr`
 #![warn(undropped_manually_drops)] //~ ERROR: lint `clippy::undropped_manually_drops`
 #![warn(unknown_lints)] //~ ERROR: lint `clippy::unknown_clippy_lints`
 #![warn(unused_labels)] //~ ERROR: lint `clippy::unused_label`