From 467c58ea544b8296cecefa65d51ee4e61614697a Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 04:05:17 +0300 Subject: [PATCH 01/26] Add `temporary_as_ptr` lint, reuse it for `temporary_cstring_as_ptr` --- compiler/rustc_lint/messages.ftl | 12 +-- compiler/rustc_lint/src/lints.rs | 12 ++- compiler/rustc_lint/src/methods.rs | 146 +++++++++++++++++++++++------ compiler/rustc_span/src/symbol.rs | 1 + 4 files changed, 131 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 759320b9eb65f..f22010b059f55 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/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 @@ -773,6 +767,12 @@ lint_suspicious_double_ref_deref = lint_tail_expr_drop_order = these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021 .label = these values have significant drop implementation and will observe changes in drop order under Edition 2024 +lint_temporary_as_ptr = getting the inner pointer of a temporary `{$ty}` + .as_ptr_label = this pointer will be invalid + .temporary_label = 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 `{$method}` 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_trailing_semi_macro = trailing semicolon in macro used in expression position .note1 = macro invocations at the end of a block are treated as expressions .note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}` diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 9050f36acba7b..0d2bbe78c1e9c 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1138,14 +1138,16 @@ pub(crate) struct IgnoredUnlessCrateSpecified<'a> { // methods.rs #[derive(LintDiagnostic)] -#[diag(lint_cstring_ptr)] +#[diag(lint_temporary_as_ptr)] #[note] #[help] -pub(crate) struct CStringPtr { +pub(crate) struct TemporaryAsPtr { + pub method: Symbol, + pub ty: String, #[label(lint_as_ptr_label)] - pub as_ptr: Span, - #[label(lint_unwrap_label)] - pub unwrap: Span, + pub as_ptr_span: Span, + #[label(lint_temporary_label)] + pub temporary_span: Span, } // multiple_supertrait_upcastable.rs diff --git a/compiler/rustc_lint/src/methods.rs b/compiler/rustc_lint/src/methods.rs index dff72bb622f23..3e5c94b14a75c 100644 --- a/compiler/rustc_lint/src/methods.rs +++ b/compiler/rustc_lint/src/methods.rs @@ -1,10 +1,9 @@ -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, LangItem}; use rustc_middle::ty; use rustc_session::{declare_lint, declare_lint_pass}; -use rustc_span::symbol::sym; -use rustc_span::Span; +use rustc_span::symbol::{sym, Ident}; -use crate::lints::CStringPtr; +use crate::lints::TemporaryAsPtr; use crate::{LateContext, LateLintPass, LintContext}; declare_lint! { @@ -33,38 +32,127 @@ declare_lint! { "detects getting the inner pointer of a temporary `CString`" } -declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR]); +declare_lint! { + /// TODO + pub TEMPORARY_AS_PTR, + Warn, + "TODO" +} + +declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR, TEMPORARY_AS_PTR]); impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::MethodCall(as_ptr_path, as_ptr_receiver, ..) = expr.kind - && as_ptr_path.ident.name == sym::as_ptr - && let ExprKind::MethodCall(unwrap_path, unwrap_receiver, ..) = as_ptr_receiver.kind - && (unwrap_path.ident.name == sym::unwrap || unwrap_path.ident.name == sym::expect) - { - lint_cstring_as_ptr(cx, as_ptr_path.ident.span, unwrap_receiver, as_ptr_receiver); + // We have a method call. + let ExprKind::MethodCall(method, receiver, _args, _span) = expr.kind else { + return; + }; + let Ident { name: method_name, span: method_span } = method.ident; + tracing::debug!(?method); + + // The method is `.as_ptr()` or `.as_mut_ptr`. + if method_name != sym::as_ptr && method_name != sym::as_mut_ptr { + return; + } + + // It is called on a temporary rvalue. + let is_temp = is_temporary_rvalue(receiver); + tracing::debug!(?receiver, ?is_temp); + if !is_temp { + return; } + + // The temporary value's type is array, box, Vec, String, or CString + let ty = cx.typeck_results().expr_ty(receiver); + tracing::debug!(?ty); + + // None => not a container + // Some(true) => CString + // Some(false) => String, Vec, box, array + let lint_is_cstring = match ty.kind() { + ty::Array(_, _) => Some(false), + ty::Adt(def, _) if def.is_box() => Some(false), + ty::Adt(def, _) if cx.tcx.lang_items().get(LangItem::String) == Some(def.did()) => { + Some(false) + } + ty::Adt(def, _) => match cx.tcx.get_diagnostic_name(def.did()) { + Some(sym::Vec) => Some(false), + Some(sym::cstring_type) => Some(true), + _ => None, + }, + _ => None, + }; + tracing::debug!(?lint_is_cstring); + let Some(is_cstring) = lint_is_cstring else { + return; + }; + + let span = method.ident.span; + let decorator = TemporaryAsPtr { + method: method_name, + ty: ty.to_string(), + as_ptr_span: method_span, + temporary_span: receiver.span, + }; + + if is_cstring { + cx.emit_span_lint(TEMPORARY_CSTRING_AS_PTR, span, decorator); + } else { + cx.emit_span_lint(TEMPORARY_AS_PTR, span, decorator); + }; } } -fn lint_cstring_as_ptr( - cx: &LateContext<'_>, - as_ptr_span: Span, - source: &rustc_hir::Expr<'_>, - unwrap: &rustc_hir::Expr<'_>, -) { - let source_type = cx.typeck_results().expr_ty(source); - if let ty::Adt(def, args) = source_type.kind() { - if cx.tcx.is_diagnostic_item(sym::Result, def.did()) { - if let ty::Adt(adt, _) = args.type_at(0).kind() { - if cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did()) { - cx.emit_span_lint( - TEMPORARY_CSTRING_AS_PTR, - as_ptr_span, - CStringPtr { as_ptr: as_ptr_span, unwrap: unwrap.span }, - ); - } - } +fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { + match expr.kind { + // We are not interested in these + ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) => false, + + // Const is not temporary. + ExprKind::ConstBlock(_) => false, + + // This is literally lvalue. + ExprKind::Path(_) => false, + + // Calls return rvalues. + ExprKind::Call(_, _) + | ExprKind::MethodCall(_, _, _, _) + | ExprKind::Index(_, _, _) + | ExprKind::Binary(_, _, _) => true, + + // TODO: Check if x: &String, *(x).as_ptr() gets triggered + ExprKind::Unary(_, _) => true, + + // Inner blocks are rvalues. + ExprKind::If(_, _, _) + | ExprKind::Loop(_, _, _, _) + | ExprKind::Match(_, _, _) + | ExprKind::Block(_, _) => true, + + ExprKind::Field(parent, _) => is_temporary_rvalue(parent), + + // FIXME: some of these get promoted to const/'static ? + ExprKind::Struct(_, _, _) + | ExprKind::Array(_) + | ExprKind::Repeat(_, _) + | ExprKind::Lit(_) => true, + + // These typecheck to `!` + ExprKind::Break(_, _) | ExprKind::Continue(_) | ExprKind::Ret(_) | ExprKind::Become(_) => { + false } + + // These typecheck to `()` + ExprKind::Assign(_, _, _) | ExprKind::AssignOp(_, _, _) => false, + + // Not applicable + ExprKind::Type(_, _) | ExprKind::Err(_) | ExprKind::Let(_) => false, + + // These are compiler-magic macros + ExprKind::AddrOf(_, _, _) | ExprKind::OffsetOf(_, _) | ExprKind::InlineAsm(_) => false, + + // TODO: WTF are these + ExprKind::DropTemps(_) => todo!(), + ExprKind::Yield(_, _) => todo!(), } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 418d1078900ac..1abaadaf941cb 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -413,6 +413,7 @@ symbols! { arm, arm_target_feature, array, + as_mut_ptr, as_ptr, as_ref, as_str, From 05bfa20873ab5a53e7239cf454baef4938f3fa89 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 15:11:26 +0300 Subject: [PATCH 02/26] temporary_as_ptr: fixes 1. Fix `is_temporary_rvalue` for: - Unary operations (such as dereference) - `yield` - Literals, array literals, repeat array literals - `DropTemps` 2. Fix typecheck to account for `T` in `Box` --- compiler/rustc_lint/src/methods.rs | 77 ++++++++++++++++-------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_lint/src/methods.rs b/compiler/rustc_lint/src/methods.rs index 3e5c94b14a75c..bf66cfb39f1ca 100644 --- a/compiler/rustc_lint/src/methods.rs +++ b/compiler/rustc_lint/src/methods.rs @@ -1,5 +1,5 @@ use rustc_hir::{Expr, ExprKind, LangItem}; -use rustc_middle::ty; +use rustc_middle::ty::Ty; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::symbol::{sym, Ident}; @@ -36,7 +36,7 @@ declare_lint! { /// TODO pub TEMPORARY_AS_PTR, Warn, - "TODO" + "detects getting the inner pointer of a temporary container" } declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR, TEMPORARY_AS_PTR]); @@ -57,7 +57,7 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr { // It is called on a temporary rvalue. let is_temp = is_temporary_rvalue(receiver); - tracing::debug!(?receiver, ?is_temp); + tracing::debug!(receiver = ?receiver.kind, ?is_temp); if !is_temp { return; } @@ -65,25 +65,7 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr { // The temporary value's type is array, box, Vec, String, or CString let ty = cx.typeck_results().expr_ty(receiver); tracing::debug!(?ty); - - // None => not a container - // Some(true) => CString - // Some(false) => String, Vec, box, array - let lint_is_cstring = match ty.kind() { - ty::Array(_, _) => Some(false), - ty::Adt(def, _) if def.is_box() => Some(false), - ty::Adt(def, _) if cx.tcx.lang_items().get(LangItem::String) == Some(def.did()) => { - Some(false) - } - ty::Adt(def, _) => match cx.tcx.get_diagnostic_name(def.did()) { - Some(sym::Vec) => Some(false), - Some(sym::cstring_type) => Some(true), - _ => None, - }, - _ => None, - }; - tracing::debug!(?lint_is_cstring); - let Some(is_cstring) = lint_is_cstring else { + let Some(is_cstring) = as_container(cx, ty) else { return; }; @@ -106,10 +88,10 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr { fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { match expr.kind { // We are not interested in these - ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) => false, + ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) | ExprKind::Lit(_) => false, // Const is not temporary. - ExprKind::ConstBlock(_) => false, + ExprKind::ConstBlock(_) | ExprKind::Repeat(_, _) => false, // This is literally lvalue. ExprKind::Path(_) => false, @@ -120,8 +102,8 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { | ExprKind::Index(_, _, _) | ExprKind::Binary(_, _, _) => true, - // TODO: Check if x: &String, *(x).as_ptr() gets triggered - ExprKind::Unary(_, _) => true, + // This is likely a dereference. + ExprKind::Unary(_, _) => false, // Inner blocks are rvalues. ExprKind::If(_, _, _) @@ -129,13 +111,12 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { | ExprKind::Match(_, _, _) | ExprKind::Block(_, _) => true, + ExprKind::DropTemps(inner) => is_temporary_rvalue(inner), ExprKind::Field(parent, _) => is_temporary_rvalue(parent), - // FIXME: some of these get promoted to const/'static ? - ExprKind::Struct(_, _, _) - | ExprKind::Array(_) - | ExprKind::Repeat(_, _) - | ExprKind::Lit(_) => true, + ExprKind::Struct(_, _, _) => true, + // False negatives are possible, but arrays get promoted to 'static way too often. + ExprKind::Array(_) => false, // These typecheck to `!` ExprKind::Break(_, _) | ExprKind::Continue(_) | ExprKind::Ret(_) | ExprKind::Become(_) => { @@ -143,16 +124,42 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { } // These typecheck to `()` - ExprKind::Assign(_, _, _) | ExprKind::AssignOp(_, _, _) => false, + ExprKind::Assign(_, _, _) | ExprKind::AssignOp(_, _, _) | ExprKind::Yield(_, _) => false, // Not applicable ExprKind::Type(_, _) | ExprKind::Err(_) | ExprKind::Let(_) => false, // These are compiler-magic macros ExprKind::AddrOf(_, _, _) | ExprKind::OffsetOf(_, _) | ExprKind::InlineAsm(_) => false, + } +} - // TODO: WTF are these - ExprKind::DropTemps(_) => todo!(), - ExprKind::Yield(_, _) => todo!(), +// None => not a container +// Some(true) => CString +// Some(false) => String, Vec, box, array +fn as_container(cx: &LateContext<'_>, ty: Ty<'_>) -> Option { + if ty.is_array() { + Some(false) + } else if let Some(inner) = ty.boxed_ty() { + // We only care about Box<[..]>, Box, Box, + // or Box iff T is another type we care about + if inner.is_slice() + || inner.is_str() + || inner.ty_adt_def().is_some_and(|def| cx.tcx.is_lang_item(def.did(), LangItem::CStr)) + || as_container(cx, inner).is_some() + { + Some(false) + } else { + None + } + } else if let Some(def) = ty.ty_adt_def() { + match cx.tcx.get_diagnostic_name(def.did()) { + Some(sym::cstring_type) => Some(true), + Some(sym::Vec) => Some(false), + _ if cx.tcx.is_lang_item(def.did(), LangItem::String) => Some(false), + _ => None, + } + } else { + None } } From ac2ab666667a2ba5fe9c6f6a18b2a4b7f6021db3 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 16:20:41 +0300 Subject: [PATCH 03/26] `temporary_as_ptr`: also fire on `Cell` & `MaybeUninit` --- compiler/rustc_lint/src/methods.rs | 10 +++++++--- compiler/rustc_span/src/symbol.rs | 1 + library/core/src/cell.rs | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_lint/src/methods.rs b/compiler/rustc_lint/src/methods.rs index bf66cfb39f1ca..c118996950c15 100644 --- a/compiler/rustc_lint/src/methods.rs +++ b/compiler/rustc_lint/src/methods.rs @@ -136,7 +136,7 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { // None => not a container // Some(true) => CString -// Some(false) => String, Vec, box, array +// Some(false) => String, Vec, box, array, MaybeUninit, Cell fn as_container(cx: &LateContext<'_>, ty: Ty<'_>) -> Option { if ty.is_array() { Some(false) @@ -153,10 +153,14 @@ fn as_container(cx: &LateContext<'_>, ty: Ty<'_>) -> Option { None } } else if let Some(def) = ty.ty_adt_def() { + for lang_item in [LangItem::String, LangItem::MaybeUninit] { + if cx.tcx.is_lang_item(def.did(), lang_item) { + return Some(false); + } + } match cx.tcx.get_diagnostic_name(def.did()) { Some(sym::cstring_type) => Some(true), - Some(sym::Vec) => Some(false), - _ if cx.tcx.is_lang_item(def.did(), LangItem::String) => Some(false), + Some(sym::Vec | sym::Cell) => Some(false), _ => None, } } else { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 1abaadaf941cb..8308414d4d34f 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -170,6 +170,7 @@ symbols! { CallOnceFuture, CallRefFuture, Capture, + Cell, Center, Cleanup, Clone, diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 5dd9721d3fee8..871fb0bce9351 100644 --- a/library/core/src/cell.rs +++ b/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)] #[cfg_attr(not(bootstrap), rustc_pub_transparent)] From a10fae1a1640e00e4ecb8b6a96b82a6d179b4917 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 16:42:58 +0300 Subject: [PATCH 04/26] `temporary_as_ptr`: rename to `instantly_dangling_pointer` --- compiler/rustc_lint/messages.ftl | 12 ++++++------ compiler/rustc_lint/src/lib.rs | 2 +- compiler/rustc_lint/src/lints.rs | 8 ++++---- compiler/rustc_lint/src/methods.rs | 19 ++++++++++--------- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index f22010b059f55..6bd71cbfe09ad 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -414,6 +414,12 @@ lint_incomplete_include = lint_inner_macro_attribute_unstable = inner macro attributes are unstable +lint_instantly_dangling = this pointer will immediately dangle + .ptr_label = this pointer will be invalid + .temporary_label = 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 `{$method}` 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 @@ -767,12 +773,6 @@ lint_suspicious_double_ref_deref = lint_tail_expr_drop_order = these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021 .label = these values have significant drop implementation and will observe changes in drop order under Edition 2024 -lint_temporary_as_ptr = getting the inner pointer of a temporary `{$ty}` - .as_ptr_label = this pointer will be invalid - .temporary_label = 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 `{$method}` 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_trailing_semi_macro = trailing semicolon in macro used in expression position .note1 = macro invocations at the end of a block are treated as expressions .note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}` diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index a9627e9b78907..7c9efe847f44c 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -227,7 +227,7 @@ late_lint_methods!( UngatedAsyncFnTrackCaller: UngatedAsyncFnTrackCaller, ShadowedIntoIter: ShadowedIntoIter, DropTraitConstraints: DropTraitConstraints, - TemporaryCStringAsPtr: TemporaryCStringAsPtr, + InstantlyDanglingPtr: InstantlyDanglingPtr, NonPanicFmt: NonPanicFmt, NoopMethodCall: NoopMethodCall, EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 0d2bbe78c1e9c..b0011e69a3093 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1138,14 +1138,14 @@ pub(crate) struct IgnoredUnlessCrateSpecified<'a> { // methods.rs #[derive(LintDiagnostic)] -#[diag(lint_temporary_as_ptr)] +#[diag(lint_instantly_dangling)] #[note] #[help] -pub(crate) struct TemporaryAsPtr { +pub(crate) struct InstantlyDangling { pub method: Symbol, pub ty: String, - #[label(lint_as_ptr_label)] - pub as_ptr_span: Span, + #[label(lint_ptr_label)] + pub ptr_span: Span, #[label(lint_temporary_label)] pub temporary_span: Span, } diff --git a/compiler/rustc_lint/src/methods.rs b/compiler/rustc_lint/src/methods.rs index c118996950c15..785c7452adf15 100644 --- a/compiler/rustc_lint/src/methods.rs +++ b/compiler/rustc_lint/src/methods.rs @@ -3,7 +3,7 @@ use rustc_middle::ty::Ty; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::symbol::{sym, Ident}; -use crate::lints::TemporaryAsPtr; +use crate::lints::InstantlyDangling; use crate::{LateContext, LateLintPass, LintContext}; declare_lint! { @@ -32,16 +32,17 @@ declare_lint! { "detects getting the inner pointer of a temporary `CString`" } +// 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! { - /// TODO - pub TEMPORARY_AS_PTR, + pub INSTANTLY_DANGLING_POINTER, Warn, - "detects getting the inner pointer of a temporary container" + "detects getting a pointer that will immediately dangle" } -declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR, TEMPORARY_AS_PTR]); +declare_lint_pass!(InstantlyDanglingPtr => [TEMPORARY_CSTRING_AS_PTR, INSTANTLY_DANGLING_POINTER]); -impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr { +impl<'tcx> LateLintPass<'tcx> for InstantlyDanglingPtr { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // We have a method call. let ExprKind::MethodCall(method, receiver, _args, _span) = expr.kind else { @@ -70,17 +71,17 @@ impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr { }; let span = method.ident.span; - let decorator = TemporaryAsPtr { + let decorator = InstantlyDangling { method: method_name, ty: ty.to_string(), - as_ptr_span: method_span, + ptr_span: method_span, temporary_span: receiver.span, }; if is_cstring { cx.emit_span_lint(TEMPORARY_CSTRING_AS_PTR, span, decorator); } else { - cx.emit_span_lint(TEMPORARY_AS_PTR, span, decorator); + cx.emit_span_lint(INSTANTLY_DANGLING_POINTER, span, decorator); }; } } From b32139d621439d2f30dcf8fe5d138b3cf176ce6e Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 17:05:18 +0300 Subject: [PATCH 05/26] mv compiler/rustc_lint/src/{methods,dangling}.rs --- compiler/rustc_lint/src/{methods.rs => dangling.rs} | 0 compiler/rustc_lint/src/lib.rs | 4 ++-- compiler/rustc_lint/src/lints.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename compiler/rustc_lint/src/{methods.rs => dangling.rs} (100%) diff --git a/compiler/rustc_lint/src/methods.rs b/compiler/rustc_lint/src/dangling.rs similarity index 100% rename from compiler/rustc_lint/src/methods.rs rename to compiler/rustc_lint/src/dangling.rs diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 7c9efe847f44c..6a27d2b883fa8 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -47,6 +47,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; @@ -65,7 +66,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; @@ -89,6 +89,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; @@ -100,7 +101,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; diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index b0011e69a3093..26fe3ff0fa210 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1136,7 +1136,7 @@ pub(crate) struct IgnoredUnlessCrateSpecified<'a> { pub name: Symbol, } -// methods.rs +// dangling.rs #[derive(LintDiagnostic)] #[diag(lint_instantly_dangling)] #[note] From b3d7e94ca30f6130004a7f58f1e375cb66be1eb2 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 21:00:29 +0300 Subject: [PATCH 06/26] `instantly_dangling_pointer`: fix the treatment of [] indexing --- compiler/rustc_lint/src/dangling.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 785c7452adf15..0925600a1b0e5 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -98,13 +98,10 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { ExprKind::Path(_) => false, // Calls return rvalues. - ExprKind::Call(_, _) - | ExprKind::MethodCall(_, _, _, _) - | ExprKind::Index(_, _, _) - | ExprKind::Binary(_, _, _) => true, + ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _, _) | ExprKind::Binary(_, _, _) => true, - // This is likely a dereference. - ExprKind::Unary(_, _) => false, + // Produces lvalue. + ExprKind::Unary(_, _) | ExprKind::Index(_, _, _) => false, // Inner blocks are rvalues. ExprKind::If(_, _, _) From 88fd6fb62582e2abb96e9e2ce798fe103be2eb4b Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 21:04:48 +0300 Subject: [PATCH 07/26] `instantly_dangling_pointer`: remove tracing & bless tests --- compiler/rustc_lint/src/dangling.rs | 6 +----- tests/ui/consts/zst_no_llvm_alloc.rs | 7 +++++-- tests/ui/lint/lint-temporary-cstring-as-param.rs | 2 +- tests/ui/lint/lint-temporary-cstring-as-param.stderr | 2 +- tests/ui/lint/lint-temporary-cstring-as-ptr.rs | 4 ++-- tests/ui/lint/lint-temporary-cstring-as-ptr.stderr | 4 ++-- 6 files changed, 12 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 0925600a1b0e5..1aec3cd079896 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -49,7 +49,6 @@ impl<'tcx> LateLintPass<'tcx> for InstantlyDanglingPtr { return; }; let Ident { name: method_name, span: method_span } = method.ident; - tracing::debug!(?method); // The method is `.as_ptr()` or `.as_mut_ptr`. if method_name != sym::as_ptr && method_name != sym::as_mut_ptr { @@ -57,15 +56,12 @@ impl<'tcx> LateLintPass<'tcx> for InstantlyDanglingPtr { } // It is called on a temporary rvalue. - let is_temp = is_temporary_rvalue(receiver); - tracing::debug!(receiver = ?receiver.kind, ?is_temp); - if !is_temp { + if !is_temporary_rvalue(receiver) { return; } // The temporary value's type is array, box, Vec, String, or CString let ty = cx.typeck_results().expr_ty(receiver); - tracing::debug!(?ty); let Some(is_cstring) = as_container(cx, ty) else { return; }; diff --git a/tests/ui/consts/zst_no_llvm_alloc.rs b/tests/ui/consts/zst_no_llvm_alloc.rs index 48ef11e2b58cf..4c244f78ef203 100644 --- a/tests/ui/consts/zst_no_llvm_alloc.rs +++ b/tests/ui/consts/zst_no_llvm_alloc.rs @@ -17,8 +17,11 @@ fn main() { // The exact addresses returned by these library functions are not necessarily stable guarantees // but for now we assert that we're still matching. - assert_eq!(>::new().as_ptr(), <&[i32]>::default().as_ptr()); - assert_eq!(>::default().as_ptr(), (&[]).as_ptr()); + #[expect(instantly_dangling_pointer)] + { + assert_eq!(>::new().as_ptr(), <&[i32]>::default().as_ptr()); + assert_eq!(>::default().as_ptr(), (&[]).as_ptr()); + }; // statics must have a unique address (see https://github.com/rust-lang/rust/issues/18297, not // clear whether this is a stable guarantee) diff --git a/tests/ui/lint/lint-temporary-cstring-as-param.rs b/tests/ui/lint/lint-temporary-cstring-as-param.rs index 9f5805367e43d..91584fd403dfc 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-param.rs +++ b/tests/ui/lint/lint-temporary-cstring-as-param.rs @@ -7,5 +7,5 @@ fn some_function(data: *const c_char) {} fn main() { some_function(CString::new("").unwrap().as_ptr()); - //~^ ERROR getting the inner pointer of a temporary `CString` + //~^ ERROR this pointer will immediately dangle } diff --git a/tests/ui/lint/lint-temporary-cstring-as-param.stderr b/tests/ui/lint/lint-temporary-cstring-as-param.stderr index 7aa21f2560c2d..694e71f0d5769 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-param.stderr +++ b/tests/ui/lint/lint-temporary-cstring-as-param.stderr @@ -1,4 +1,4 @@ -error: getting the inner pointer of a temporary `CString` +error: this pointer will immediately dangle --> $DIR/lint-temporary-cstring-as-param.rs:9:45 | LL | some_function(CString::new("").unwrap().as_ptr()); diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.rs b/tests/ui/lint/lint-temporary-cstring-as-ptr.rs index fab792f128411..b326da6a737cf 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-ptr.rs +++ b/tests/ui/lint/lint-temporary-cstring-as-ptr.rs @@ -6,12 +6,12 @@ use std::ffi::CString; macro_rules! mymacro { () => { let s = CString::new("some text").unwrap().as_ptr(); - //~^ ERROR getting the inner pointer of a temporary `CString` + //~^ ERROR this pointer will immediately dangle } } fn main() { let s = CString::new("some text").unwrap().as_ptr(); - //~^ ERROR getting the inner pointer of a temporary `CString` + //~^ ERROR this pointer will immediately dangle mymacro!(); } diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr b/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr index 4e5c8aa069364..69c99063d01bd 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr +++ b/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr @@ -1,4 +1,4 @@ -error: getting the inner pointer of a temporary `CString` +error: this pointer will immediately dangle --> $DIR/lint-temporary-cstring-as-ptr.rs:14:48 | LL | let s = CString::new("some text").unwrap().as_ptr(); @@ -14,7 +14,7 @@ note: the lint level is defined here LL | #![deny(temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: getting the inner pointer of a temporary `CString` +error: this pointer will immediately dangle --> $DIR/lint-temporary-cstring-as-ptr.rs:8:52 | LL | let s = CString::new("some text").unwrap().as_ptr(); From 35b25434c6c16a22f415949bca3f56d615e44e50 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 22:12:13 +0300 Subject: [PATCH 08/26] `instantly_dangling_pointer`: change wording a bit --- compiler/rustc_lint/messages.ftl | 8 ++++---- compiler/rustc_lint/src/dangling.rs | 13 ++++++++----- compiler/rustc_lint/src/lib.rs | 2 +- compiler/rustc_lint/src/lints.rs | 6 +++--- tests/ui/lint/lint-temporary-cstring-as-param.rs | 2 +- .../ui/lint/lint-temporary-cstring-as-param.stderr | 4 ++-- tests/ui/lint/lint-temporary-cstring-as-ptr.rs | 4 ++-- tests/ui/lint/lint-temporary-cstring-as-ptr.stderr | 8 ++++---- 8 files changed, 25 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 6bd71cbfe09ad..df407b3ff5162 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -414,10 +414,10 @@ lint_incomplete_include = lint_inner_macro_attribute_unstable = inner macro attributes are unstable -lint_instantly_dangling = this pointer will immediately dangle - .ptr_label = this pointer will be invalid - .temporary_label = 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 `{$method}` the `{$ty}` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned +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 diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 1aec3cd079896..a75dcd07b4beb 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -35,14 +35,15 @@ declare_lint! { // 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! { + /// TODO pub INSTANTLY_DANGLING_POINTER, Warn, "detects getting a pointer that will immediately dangle" } -declare_lint_pass!(InstantlyDanglingPtr => [TEMPORARY_CSTRING_AS_PTR, INSTANTLY_DANGLING_POINTER]); +declare_lint_pass!(DanglingPointers => [TEMPORARY_CSTRING_AS_PTR, INSTANTLY_DANGLING_POINTER]); -impl<'tcx> LateLintPass<'tcx> for InstantlyDanglingPtr { +impl<'tcx> LateLintPass<'tcx> for DanglingPointers { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // We have a method call. let ExprKind::MethodCall(method, receiver, _args, _span) = expr.kind else { @@ -66,9 +67,9 @@ impl<'tcx> LateLintPass<'tcx> for InstantlyDanglingPtr { return; }; - let span = method.ident.span; + let span = method_span; let decorator = InstantlyDangling { - method: method_name, + callee: method_name, ty: ty.to_string(), ptr_span: method_span, temporary_span: receiver.span, @@ -85,7 +86,7 @@ impl<'tcx> LateLintPass<'tcx> for InstantlyDanglingPtr { fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { match expr.kind { // We are not interested in these - ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) | ExprKind::Lit(_) => false, + ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) => false, // Const is not temporary. ExprKind::ConstBlock(_) | ExprKind::Repeat(_, _) => false, @@ -109,6 +110,8 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { ExprKind::Field(parent, _) => is_temporary_rvalue(parent), ExprKind::Struct(_, _, _) => true, + // These are 'static + ExprKind::Lit(_) => false, // False negatives are possible, but arrays get promoted to 'static way too often. ExprKind::Array(_) => false, diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 6a27d2b883fa8..fe68ece144a47 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -227,7 +227,7 @@ late_lint_methods!( UngatedAsyncFnTrackCaller: UngatedAsyncFnTrackCaller, ShadowedIntoIter: ShadowedIntoIter, DropTraitConstraints: DropTraitConstraints, - InstantlyDanglingPtr: InstantlyDanglingPtr, + DanglingPointers: DanglingPointers, NonPanicFmt: NonPanicFmt, NoopMethodCall: NoopMethodCall, EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 26fe3ff0fa210..ba02b414ad1a8 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1142,11 +1142,11 @@ pub(crate) struct IgnoredUnlessCrateSpecified<'a> { #[note] #[help] pub(crate) struct InstantlyDangling { - pub method: Symbol, + pub callee: Symbol, pub ty: String, - #[label(lint_ptr_label)] + #[label(lint_label_ptr)] pub ptr_span: Span, - #[label(lint_temporary_label)] + #[label(lint_label_temporary)] pub temporary_span: Span, } diff --git a/tests/ui/lint/lint-temporary-cstring-as-param.rs b/tests/ui/lint/lint-temporary-cstring-as-param.rs index 91584fd403dfc..07d7d52a11d92 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-param.rs +++ b/tests/ui/lint/lint-temporary-cstring-as-param.rs @@ -7,5 +7,5 @@ fn some_function(data: *const c_char) {} fn main() { some_function(CString::new("").unwrap().as_ptr()); - //~^ ERROR this pointer will immediately dangle + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer } diff --git a/tests/ui/lint/lint-temporary-cstring-as-param.stderr b/tests/ui/lint/lint-temporary-cstring-as-param.stderr index 694e71f0d5769..028f5c7d38b0f 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-param.stderr +++ b/tests/ui/lint/lint-temporary-cstring-as-param.stderr @@ -1,8 +1,8 @@ -error: this pointer will immediately dangle +error: getting a pointer from a temporary `CString` will result in a dangling pointer --> $DIR/lint-temporary-cstring-as-param.rs:9:45 | LL | some_function(CString::new("").unwrap().as_ptr()); - | ------------------------- ^^^^^^ this pointer will be invalid + | ------------------------- ^^^^^^ this pointer will immediately be invalid | | | this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime | diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.rs b/tests/ui/lint/lint-temporary-cstring-as-ptr.rs index b326da6a737cf..6993cfc5501d2 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-ptr.rs +++ b/tests/ui/lint/lint-temporary-cstring-as-ptr.rs @@ -6,12 +6,12 @@ use std::ffi::CString; macro_rules! mymacro { () => { let s = CString::new("some text").unwrap().as_ptr(); - //~^ ERROR this pointer will immediately dangle + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer } } fn main() { let s = CString::new("some text").unwrap().as_ptr(); - //~^ ERROR this pointer will immediately dangle + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer mymacro!(); } diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr b/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr index 69c99063d01bd..8086442a752cf 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr +++ b/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr @@ -1,8 +1,8 @@ -error: this pointer will immediately dangle +error: getting a pointer from a temporary `CString` will result in a dangling pointer --> $DIR/lint-temporary-cstring-as-ptr.rs:14:48 | LL | let s = CString::new("some text").unwrap().as_ptr(); - | ---------------------------------- ^^^^^^ this pointer will be invalid + | ---------------------------------- ^^^^^^ this pointer will immediately be invalid | | | this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime | @@ -14,11 +14,11 @@ note: the lint level is defined here LL | #![deny(temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: this pointer will immediately dangle +error: getting a pointer from a temporary `CString` will result in a dangling pointer --> $DIR/lint-temporary-cstring-as-ptr.rs:8:52 | LL | let s = CString::new("some text").unwrap().as_ptr(); - | ---------------------------------- ^^^^^^ this pointer will be invalid + | ---------------------------------- ^^^^^^ this pointer will immediately be invalid | | | this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime ... From 2627c76858bed2039f622c68af664dbfa4d14bbf Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 22:55:44 +0300 Subject: [PATCH 09/26] `instantly_dangling_pointer`: add some FIXMEs --- compiler/rustc_lint/src/dangling.rs | 2 +- compiler/rustc_lint/src/lints.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index a75dcd07b4beb..d3b083b94f3c7 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -112,7 +112,7 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { ExprKind::Struct(_, _, _) => true, // These are 'static ExprKind::Lit(_) => false, - // False negatives are possible, but arrays get promoted to 'static way too often. + // FIXME: False negatives are possible, but arrays get promoted to 'static way too often. ExprKind::Array(_) => false, // These typecheck to `!` diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index ba02b414ad1a8..383154f4a12c8 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1141,6 +1141,7 @@ pub(crate) struct IgnoredUnlessCrateSpecified<'a> { #[diag(lint_instantly_dangling)] #[note] #[help] +// FIXME: use #[primary_span] pub(crate) struct InstantlyDangling { pub callee: Symbol, pub ty: String, From 410a41636631feba42322372178ca476af5b6c54 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sun, 11 Aug 2024 23:06:36 +0300 Subject: [PATCH 10/26] Restore the original `temporary_cstring_as_ptr` behavior --- compiler/rustc_lint/src/dangling.rs | 113 +++++++++++++++------------- 1 file changed, 59 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index d3b083b94f3c7..c064251d69038 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -1,7 +1,7 @@ use rustc_hir::{Expr, ExprKind, LangItem}; -use rustc_middle::ty::Ty; +use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint, declare_lint_pass}; -use rustc_span::symbol::{sym, Ident}; +use rustc_span::symbol::sym; use crate::lints::InstantlyDangling; use crate::{LateContext, LateLintPass, LintContext}; @@ -45,42 +45,57 @@ declare_lint_pass!(DanglingPointers => [TEMPORARY_CSTRING_AS_PTR, INSTANTLY_DANG impl<'tcx> LateLintPass<'tcx> for DanglingPointers { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - // We have a method call. - let ExprKind::MethodCall(method, receiver, _args, _span) = expr.kind else { - return; - }; - let Ident { name: method_name, span: method_span } = method.ident; - - // The method is `.as_ptr()` or `.as_mut_ptr`. - if method_name != sym::as_ptr && method_name != sym::as_mut_ptr { - return; + if let ExprKind::MethodCall(as_ptr_path, as_ptr_receiver, ..) = expr.kind + && as_ptr_path.ident.name == sym::as_ptr + && let ExprKind::MethodCall(unwrap_path, unwrap_receiver, ..) = as_ptr_receiver.kind + && (unwrap_path.ident.name == sym::unwrap || unwrap_path.ident.name == sym::expect) + && lint_cstring_as_ptr(cx, unwrap_receiver) + { + cx.emit_span_lint( + TEMPORARY_CSTRING_AS_PTR, + as_ptr_path.ident.span, + InstantlyDangling { + callee: as_ptr_path.ident.name, + ty: "CString".into(), + ptr_span: as_ptr_path.ident.span, + temporary_span: as_ptr_receiver.span, + }, + ); + return; // One lint is enough } - // It is called on a temporary rvalue. - if !is_temporary_rvalue(receiver) { - return; + 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, ty) + { + cx.emit_span_lint( + INSTANTLY_DANGLING_POINTER, + method.ident.span, + InstantlyDangling { + callee: method.ident.name, + ty: ty.to_string(), + ptr_span: method.ident.span, + temporary_span: receiver.span, + }, + ) } + } +} - // The temporary value's type is array, box, Vec, String, or CString - let ty = cx.typeck_results().expr_ty(receiver); - let Some(is_cstring) = as_container(cx, ty) else { - return; - }; - - let span = method_span; - let decorator = InstantlyDangling { - callee: method_name, - ty: ty.to_string(), - ptr_span: method_span, - temporary_span: receiver.span, - }; - - if is_cstring { - cx.emit_span_lint(TEMPORARY_CSTRING_AS_PTR, span, decorator); - } else { - cx.emit_span_lint(INSTANTLY_DANGLING_POINTER, span, decorator); - }; +fn lint_cstring_as_ptr(cx: &LateContext<'_>, source: &rustc_hir::Expr<'_>) -> bool { + let source_type = cx.typeck_results().expr_ty(source); + if let ty::Adt(def, args) = source_type.kind() { + if cx.tcx.is_diagnostic_item(sym::Result, def.did()) { + if let ty::Adt(adt, _) = args.type_at(0).kind() { + if cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did()) { + return true; + } + } + } } + false } fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { @@ -131,36 +146,26 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { } } -// None => not a container -// Some(true) => CString -// Some(false) => String, Vec, box, array, MaybeUninit, Cell -fn as_container(cx: &LateContext<'_>, ty: Ty<'_>) -> Option { +// Array, Vec, String, CString, MaybeUninit, Cell, Box<[_]>, Box, Box, +// or any of the above in arbitrary many nested Box'es. +fn is_interesting(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { if ty.is_array() { - Some(false) + true } else if let Some(inner) = ty.boxed_ty() { - // We only care about Box<[..]>, Box, Box, - // or Box iff T is another type we care about - if inner.is_slice() + inner.is_slice() || inner.is_str() || inner.ty_adt_def().is_some_and(|def| cx.tcx.is_lang_item(def.did(), LangItem::CStr)) - || as_container(cx, inner).is_some() - { - Some(false) - } else { - None - } + || is_interesting(cx, inner) } else if let Some(def) = ty.ty_adt_def() { for lang_item in [LangItem::String, LangItem::MaybeUninit] { if cx.tcx.is_lang_item(def.did(), lang_item) { - return Some(false); + return true; } } - match cx.tcx.get_diagnostic_name(def.did()) { - Some(sym::cstring_type) => Some(true), - Some(sym::Vec | sym::Cell) => Some(false), - _ => None, - } + cx.tcx + .get_diagnostic_name(def.did()) + .is_some_and(|name| matches!(name, sym::cstring_type | sym::Vec | sym::Cell)) } else { - None + false } } From 2343e1089b63054f75a32af0b55e8a27b27f91b0 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 12 Aug 2024 00:35:16 +0300 Subject: [PATCH 11/26] `instantly_dangling_pointer`, `temporary_cstring_as_ptr`: add and move tests --- .../cstring-as-param.rs} | 0 .../cstring-as-param.stderr} | 4 +- .../cstring-as-ptr.rs} | 0 .../cstring-as-ptr.stderr} | 6 +- .../instantly-dangling-pointer-issue123613.rs | 13 ++ ...tantly-dangling-pointer-issue123613.stderr | 29 +++ .../instantly-dangling-pointer-methods.rs | 6 + .../instantly-dangling-pointer-methods.stderr | 29 +++ .../instantly-dangling-pointer-types.rs | 37 ++++ .../instantly-dangling-pointer-types.stderr | 172 ++++++++++++++++++ 10 files changed, 291 insertions(+), 5 deletions(-) rename tests/ui/lint/{lint-temporary-cstring-as-param.rs => dangling-ptr/cstring-as-param.rs} (100%) rename tests/ui/lint/{lint-temporary-cstring-as-param.stderr => dangling-ptr/cstring-as-param.stderr} (89%) rename tests/ui/lint/{lint-temporary-cstring-as-ptr.rs => dangling-ptr/cstring-as-ptr.rs} (100%) rename tests/ui/lint/{lint-temporary-cstring-as-ptr.stderr => dangling-ptr/cstring-as-ptr.stderr} (92%) create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.rs create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.stderr create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.stderr create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.stderr diff --git a/tests/ui/lint/lint-temporary-cstring-as-param.rs b/tests/ui/lint/dangling-ptr/cstring-as-param.rs similarity index 100% rename from tests/ui/lint/lint-temporary-cstring-as-param.rs rename to tests/ui/lint/dangling-ptr/cstring-as-param.rs diff --git a/tests/ui/lint/lint-temporary-cstring-as-param.stderr b/tests/ui/lint/dangling-ptr/cstring-as-param.stderr similarity index 89% rename from tests/ui/lint/lint-temporary-cstring-as-param.stderr rename to tests/ui/lint/dangling-ptr/cstring-as-param.stderr index 028f5c7d38b0f..7a13c23735e4b 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-param.stderr +++ b/tests/ui/lint/dangling-ptr/cstring-as-param.stderr @@ -1,5 +1,5 @@ error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/lint-temporary-cstring-as-param.rs:9:45 + --> $DIR/cstring-as-param.rs:9:45 | LL | some_function(CString::new("").unwrap().as_ptr()); | ------------------------- ^^^^^^ this pointer will immediately be invalid @@ -9,7 +9,7 @@ LL | some_function(CString::new("").unwrap().as_ptr()); = 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 note: the lint level is defined here - --> $DIR/lint-temporary-cstring-as-param.rs:1:9 + --> $DIR/cstring-as-param.rs:1:9 | LL | #![deny(temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.rs b/tests/ui/lint/dangling-ptr/cstring-as-ptr.rs similarity index 100% rename from tests/ui/lint/lint-temporary-cstring-as-ptr.rs rename to tests/ui/lint/dangling-ptr/cstring-as-ptr.rs diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr b/tests/ui/lint/dangling-ptr/cstring-as-ptr.stderr similarity index 92% rename from tests/ui/lint/lint-temporary-cstring-as-ptr.stderr rename to tests/ui/lint/dangling-ptr/cstring-as-ptr.stderr index 8086442a752cf..240774fb38283 100644 --- a/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr +++ b/tests/ui/lint/dangling-ptr/cstring-as-ptr.stderr @@ -1,5 +1,5 @@ error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/lint-temporary-cstring-as-ptr.rs:14:48 + --> $DIR/cstring-as-ptr.rs:14:48 | LL | let s = CString::new("some text").unwrap().as_ptr(); | ---------------------------------- ^^^^^^ this pointer will immediately be invalid @@ -9,13 +9,13 @@ LL | let s = CString::new("some text").unwrap().as_ptr(); = 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 note: the lint level is defined here - --> $DIR/lint-temporary-cstring-as-ptr.rs:2:9 + --> $DIR/cstring-as-ptr.rs:2:9 | LL | #![deny(temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/lint-temporary-cstring-as-ptr.rs:8:52 + --> $DIR/cstring-as-ptr.rs:8:52 | LL | let s = CString::new("some text").unwrap().as_ptr(); | ---------------------------------- ^^^^^^ this pointer will immediately be invalid diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.rs b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.rs new file mode 100644 index 0000000000000..9ac72e2d70570 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.rs @@ -0,0 +1,13 @@ +#![deny(instantly_dangling_pointer)] + +const MAX_PATH: usize = 260; +fn main() { + let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); + //~^ ERROR [instantly_dangling_pointer] + let str2 = String::from("TotototototototototototototototototoT").as_ptr(); + //~^ ERROR [instantly_dangling_pointer] + unsafe { + std::ptr::copy_nonoverlapping(str2, str1, 30); + println!("{:?}", String::from_raw_parts(str1, 30, 30)); + } +} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.stderr b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.stderr new file mode 100644 index 0000000000000..3fb6f62892004 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.stderr @@ -0,0 +1,29 @@ +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-issue123613.rs:5:48 + | +LL | let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); + | ------------------------------- ^^^^^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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_mut_ptr` the `String` 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 +note: the lint level is defined here + --> $DIR/instantly-dangling-pointer-issue123613.rs:1:9 + | +LL | #![deny(instantly_dangling_pointer)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-issue123613.rs:7:70 + | +LL | let str2 = String::from("TotototototototototototototototototoT").as_ptr(); + | ----------------------------------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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 `String` 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 + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs new file mode 100644 index 0000000000000..12e80a13d3f86 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs @@ -0,0 +1,6 @@ +#![deny(instantly_dangling_pointer)] + +fn main() { + vec![0u8].as_ptr(); //~ ERROR [instantly_dangling_pointer] + vec![0u8].as_mut_ptr(); //~ ERROR [instantly_dangling_pointer] +} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.stderr b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.stderr new file mode 100644 index 0000000000000..e1f451d7ccf69 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.stderr @@ -0,0 +1,29 @@ +error: getting a pointer from a temporary `Vec` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-methods.rs:4:15 + | +LL | vec![0u8].as_ptr(); + | --------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Vec` 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 `Vec` 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 +note: the lint level is defined here + --> $DIR/instantly-dangling-pointer-methods.rs:1:9 + | +LL | #![deny(instantly_dangling_pointer)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: getting a pointer from a temporary `Vec` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-methods.rs:5:15 + | +LL | vec![0u8].as_mut_ptr(); + | --------- ^^^^^^^^^^ this pointer will immediately be invalid + | | + | this `Vec` 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_mut_ptr` the `Vec` 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 + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs new file mode 100644 index 0000000000000..67d9845a0d9c8 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs @@ -0,0 +1,37 @@ +#![deny(instantly_dangling_pointer)] + +use std::cell::Cell; +use std::ffi::{CStr, CString}; +use std::mem::MaybeUninit; + +struct AsPtrFake; + +impl AsPtrFake { + fn as_ptr(&self) -> *const () { + std::ptr::null() + } +} + +fn declval() -> T { + loop {} +} + +fn main() { + declval::().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::<[u8; 10]>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>>>>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] + declval::>().as_ptr(); + declval::().as_ptr(); +} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.stderr b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.stderr new file mode 100644 index 0000000000000..1040bef5dac98 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.stderr @@ -0,0 +1,172 @@ +error: getting a pointer from a temporary `CString` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:20:26 + | +LL | declval::().as_ptr(); + | -------------------- ^^^^^^ this pointer will immediately be invalid + | | + | 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 +note: the lint level is defined here + --> $DIR/instantly-dangling-pointer-types.rs:1:9 + | +LL | #![deny(instantly_dangling_pointer)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:21:25 + | +LL | declval::().as_ptr(); + | ------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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 `String` 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 + +error: getting a pointer from a temporary `Vec` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:22:26 + | +LL | declval::>().as_ptr(); + | -------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Vec` 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 `Vec` 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 + +error: getting a pointer from a temporary `Box` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:23:31 + | +LL | declval::>().as_ptr(); + | ------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Box` 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 `Box` 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 + +error: getting a pointer from a temporary `Box<[u8]>` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:24:28 + | +LL | declval::>().as_ptr(); + | ---------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Box<[u8]>` 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 `Box<[u8]>` 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 + +error: getting a pointer from a temporary `Box` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:25:27 + | +LL | declval::>().as_ptr(); + | --------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Box` 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 `Box` 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 + +error: getting a pointer from a temporary `Box` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:26:28 + | +LL | declval::>().as_ptr(); + | ---------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Box` 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 `Box` 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 + +error: getting a pointer from a temporary `[u8; 10]` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:27:27 + | +LL | declval::<[u8; 10]>().as_ptr(); + | --------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `[u8; 10]` 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 `[u8; 10]` 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 + +error: getting a pointer from a temporary `Box<[u8; 10]>` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:28:32 + | +LL | declval::>().as_ptr(); + | -------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Box<[u8; 10]>` 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 `Box<[u8; 10]>` 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 + +error: getting a pointer from a temporary `Box>` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:29:31 + | +LL | declval::>>().as_ptr(); + | ------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Box>` 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 `Box>` 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 + +error: getting a pointer from a temporary `Box` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:30:30 + | +LL | declval::>().as_ptr(); + | ------------------------ ^^^^^^ this pointer will immediately be invalid + | | + | this `Box` 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 `Box` 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 + +error: getting a pointer from a temporary `Box>>>` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:31:43 + | +LL | declval::>>>>().as_ptr(); + | ------------------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Box>>>` 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 `Box>>>` 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 + +error: getting a pointer from a temporary `Cell` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:32:27 + | +LL | declval::>().as_ptr(); + | --------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Cell` 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 `Cell` 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 + +error: getting a pointer from a temporary `MaybeUninit` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:33:34 + | +LL | declval::>().as_ptr(); + | ---------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `MaybeUninit` 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 `MaybeUninit` 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 + +error: getting a pointer from a temporary `Vec` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-types.rs:34:33 + | +LL | declval::>().as_ptr(); + | --------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Vec` 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 `Vec` 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 + +error: aborting due to 15 previous errors + From dca5211f4f08600efb787d2ea895762589b01ed9 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 12 Aug 2024 01:17:39 +0300 Subject: [PATCH 12/26] `instantly_dangling_pointer`: test for temporaries & fix some false positives --- compiler/rustc_lint/src/dangling.rs | 51 ++++--- .../instantly-dangling-pointer-temporaries.rs | 129 ++++++++++++++++++ ...tantly-dangling-pointer-temporaries.stderr | 99 ++++++++++++++ 3 files changed, 253 insertions(+), 26 deletions(-) create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.rs create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.stderr diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index c064251d69038..1ed28efdf0bc0 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -100,49 +100,48 @@ fn lint_cstring_as_ptr(cx: &LateContext<'_>, source: &rustc_hir::Expr<'_>) -> bo fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { match expr.kind { - // We are not interested in these - ExprKind::Cast(_, _) | ExprKind::Closure(_) | ExprKind::Tup(_) => false, - // Const is not temporary. - ExprKind::ConstBlock(_) | ExprKind::Repeat(_, _) => false, + ExprKind::ConstBlock(..) | ExprKind::Repeat(..) | ExprKind::Lit(..) => false, // This is literally lvalue. - ExprKind::Path(_) => false, + ExprKind::Path(..) => false, // Calls return rvalues. - ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _, _) | ExprKind::Binary(_, _, _) => true, - - // Produces lvalue. - ExprKind::Unary(_, _) | ExprKind::Index(_, _, _) => false, + ExprKind::Call(..) | ExprKind::MethodCall(..) | ExprKind::Binary(..) => true, + // FIXME: this is probably wrong. // Inner blocks are rvalues. - ExprKind::If(_, _, _) - | ExprKind::Loop(_, _, _, _) - | ExprKind::Match(_, _, _) - | ExprKind::Block(_, _) => true, + 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::DropTemps(inner) => is_temporary_rvalue(inner), - ExprKind::Field(parent, _) => is_temporary_rvalue(parent), + ExprKind::Struct(..) => true, - ExprKind::Struct(_, _, _) => true, - // These are 'static - ExprKind::Lit(_) => false, - // FIXME: False negatives are possible, but arrays get promoted to 'static way too often. - ExprKind::Array(_) => false, + // 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(_) => { + ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) | ExprKind::Become(..) => { false } // These typecheck to `()` - ExprKind::Assign(_, _, _) | ExprKind::AssignOp(_, _, _) | ExprKind::Yield(_, _) => false, + ExprKind::Assign(..) | ExprKind::AssignOp(..) | ExprKind::Yield(..) => false, - // Not applicable - ExprKind::Type(_, _) | ExprKind::Err(_) | ExprKind::Let(_) => 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, - // These are compiler-magic macros - ExprKind::AddrOf(_, _, _) | ExprKind::OffsetOf(_, _) | ExprKind::InlineAsm(_) => false, + // Not applicable + ExprKind::Type(..) | ExprKind::Err(..) => false, } } diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.rs b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.rs new file mode 100644 index 0000000000000..42ce20142cf68 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.rs @@ -0,0 +1,129 @@ +#![allow(unused)] +#![deny(instantly_dangling_pointer)] + +fn string() -> String { + "hello".into() +} + +struct Wrapper(String); + +fn main() { + // ConstBlock + const { String::new() }.as_ptr(); + + // Array + { + [string()].as_ptr(); // False negative + [true].as_ptr(); + } + + // Call + string().as_ptr(); //~ ERROR [instantly_dangling_pointer] + + // MethodCall + "hello".to_string().as_ptr(); //~ ERROR [instantly_dangling_pointer] + + // Tup + // impossible + + // Binary + (string() + "hello").as_ptr(); //~ ERROR [instantly_dangling_pointer] + + // Path + { + let x = string(); + x.as_ptr(); + } + + // Unary + { + let x = string(); + let x: &String = &x; + (*x).as_ptr(); + (&[0u8]).as_ptr(); + (&string()).as_ptr(); // False negative + (*&string()).as_ptr(); // False negative + } + + // Lit + "hello".as_ptr(); + + // Cast + // impossible + + // Type + // impossible + + // DropTemps + // impossible + + // Let + // impossible + + // If + { + (if true { String::new() } else { "hello".into() }).as_ptr(); + //~^ ERROR [instantly_dangling_pointer] + } + + // Loop + { + (loop { + break String::new(); + }) + .as_ptr(); //~ ERROR [instantly_dangling_pointer] + } + + // Match + { + match string() { + s => s, + } + .as_ptr(); //~ ERROR [instantly_dangling_pointer] + } + + // Closure + // impossible + + // Block + { string() }.as_ptr(); //~ ERROR [instantly_dangling_pointer] + + // Assign, AssignOp + // impossible + + // Field + { + Wrapper(string()).0.as_ptr(); // False negative + let x = Wrapper(string()); + x.0.as_ptr(); + } + + // Index + { + vec![string()][0].as_ptr(); // False negative + let x = vec![string()]; + x[0].as_ptr(); + } + + // AddrOf, InlineAsm, OffsetOf + // impossible + + // Break, Continue, Ret + // are ! + + // Become, Yield + // unstable, are ! + + // Repeat + [0u8; 100].as_ptr(); + [const { String::new() }; 100].as_ptr(); + + // Struct + // Cannot test this without access to private fields of the linted types. + + // Err + // impossible + + // Macro + vec![0u8].as_ptr(); //~ ERROR [instantly_dangling_pointer] +} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.stderr b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.stderr new file mode 100644 index 0000000000000..547735b65dd7b --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.stderr @@ -0,0 +1,99 @@ +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:21:14 + | +LL | string().as_ptr(); + | -------- ^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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 `String` 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 +note: the lint level is defined here + --> $DIR/instantly-dangling-pointer-temporaries.rs:2:9 + | +LL | #![deny(instantly_dangling_pointer)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:24:25 + | +LL | "hello".to_string().as_ptr(); + | ------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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 `String` 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 + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:30:26 + | +LL | (string() + "hello").as_ptr(); + | -------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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 `String` 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 + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:65:61 + | +LL | (if true { String::new() } else { "hello".into() }).as_ptr(); + | --------------------------------------------------- ^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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 `String` 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 + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:74:10 + | +LL | / (loop { +LL | | break String::new(); +LL | | }) + | |__________- this `String` is deallocated at the end of the statement, bind it to a variable to extend its lifetime +LL | .as_ptr(); + | ^^^^^^ this pointer will immediately be invalid + | + = note: pointers do not have a lifetime; when calling `as_ptr` the `String` 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 + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:82:10 + | +LL | / match string() { +LL | | s => s, +LL | | } + | |_________- this `String` is deallocated at the end of the statement, bind it to a variable to extend its lifetime +LL | .as_ptr(); + | ^^^^^^ this pointer will immediately be invalid + | + = note: pointers do not have a lifetime; when calling `as_ptr` the `String` 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 + +error: getting a pointer from a temporary `String` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:89:18 + | +LL | { string() }.as_ptr(); + | ------------ ^^^^^^ this pointer will immediately be invalid + | | + | this `String` 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 `String` 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 + +error: getting a pointer from a temporary `Vec` will result in a dangling pointer + --> $DIR/instantly-dangling-pointer-temporaries.rs:128:15 + | +LL | vec![0u8].as_ptr(); + | --------- ^^^^^^ this pointer will immediately be invalid + | | + | this `Vec` 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 `Vec` 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 + +error: aborting due to 8 previous errors + From 9bbd689ee70beca8a87f1ddd573c7eb0c8f422a7 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 12 Aug 2024 01:46:21 +0300 Subject: [PATCH 13/26] `instantly_dangling_pointer`: fix ICE Turns out you cannot format Ty's you won't emit in unsuppressed diagnostics --- compiler/rustc_lint/src/dangling.rs | 25 ++++++------------- compiler/rustc_lint/src/lints.rs | 4 +-- .../instantly-dangling-pointer-allow.rs | 9 +++++++ 3 files changed, 19 insertions(+), 19 deletions(-) create mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-allow.rs diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 1ed28efdf0bc0..6576127be75e2 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -49,14 +49,19 @@ impl<'tcx> LateLintPass<'tcx> for DanglingPointers { && as_ptr_path.ident.name == sym::as_ptr && let ExprKind::MethodCall(unwrap_path, unwrap_receiver, ..) = as_ptr_receiver.kind && (unwrap_path.ident.name == sym::unwrap || unwrap_path.ident.name == sym::expect) - && lint_cstring_as_ptr(cx, unwrap_receiver) + && let source_type = cx.typeck_results().expr_ty(unwrap_receiver) + && let ty::Adt(def, args) = source_type.kind() + && cx.tcx.is_diagnostic_item(sym::Result, def.did()) + && let ty = args.type_at(0) + && let ty::Adt(adt, _) = ty.kind() + && cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did()) { cx.emit_span_lint( TEMPORARY_CSTRING_AS_PTR, as_ptr_path.ident.span, InstantlyDangling { callee: as_ptr_path.ident.name, - ty: "CString".into(), + ty, ptr_span: as_ptr_path.ident.span, temporary_span: as_ptr_receiver.span, }, @@ -75,7 +80,7 @@ impl<'tcx> LateLintPass<'tcx> for DanglingPointers { method.ident.span, InstantlyDangling { callee: method.ident.name, - ty: ty.to_string(), + ty, ptr_span: method.ident.span, temporary_span: receiver.span, }, @@ -84,20 +89,6 @@ impl<'tcx> LateLintPass<'tcx> for DanglingPointers { } } -fn lint_cstring_as_ptr(cx: &LateContext<'_>, source: &rustc_hir::Expr<'_>) -> bool { - let source_type = cx.typeck_results().expr_ty(source); - if let ty::Adt(def, args) = source_type.kind() { - if cx.tcx.is_diagnostic_item(sym::Result, def.did()) { - if let ty::Adt(adt, _) = args.type_at(0).kind() { - if cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did()) { - return true; - } - } - } - } - false -} - fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { match expr.kind { // Const is not temporary. diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 383154f4a12c8..cd1d1d3a70ec6 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1142,9 +1142,9 @@ pub(crate) struct IgnoredUnlessCrateSpecified<'a> { #[note] #[help] // FIXME: use #[primary_span] -pub(crate) struct InstantlyDangling { +pub(crate) struct InstantlyDangling<'tcx> { pub callee: Symbol, - pub ty: String, + pub ty: Ty<'tcx>, #[label(lint_label_ptr)] pub ptr_span: Span, #[label(lint_label_temporary)] diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-allow.rs b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-allow.rs new file mode 100644 index 0000000000000..bc4e675cfcba9 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-allow.rs @@ -0,0 +1,9 @@ +//@ check-pass + +// This should not ICE. + +#![allow(instantly_dangling_pointer)] + +fn main() { + dbg!(String::new().as_ptr()); +} From 63909f7cf2b8fc44b246d38c3463577d85ab7849 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 12 Aug 2024 22:02:15 +0300 Subject: [PATCH 14/26] `instantly_dangling_pointer`: add documentation --- compiler/rustc_lint/src/dangling.rs | 38 +++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 6576127be75e2..11726820fb937 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -35,7 +35,42 @@ declare_lint! { // 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! { - /// TODO + /// The `instantly_dangling_pointer` 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) { + /// let x: *const u8 = bytes.collect::>().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. + /// + /// Note: This lint does **not** get triggered by methods & functions + /// that intentionally produce dangling pointers, such as: + /// + /// - `core::ptr::dangling` & `core::ptr::dangling_mut` + /// - `core::ptr::NonNull::dangling` + /// - `std::alloc::Layout::dangling` + /// pub INSTANTLY_DANGLING_POINTER, Warn, "detects getting a pointer that will immediately dangle" @@ -100,7 +135,6 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { // Calls return rvalues. ExprKind::Call(..) | ExprKind::MethodCall(..) | ExprKind::Binary(..) => true, - // FIXME: this is probably wrong. // Inner blocks are rvalues. ExprKind::If(..) | ExprKind::Loop(..) | ExprKind::Match(..) | ExprKind::Block(..) => true, From a1a55503651046877fad382416beb5e635e9724d Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 12 Aug 2024 22:41:55 +0300 Subject: [PATCH 15/26] alloc: `#[allow(instantly_dangling_pointer)]` for a test about dangling pointers --- library/alloc/tests/boxed.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/alloc/tests/boxed.rs b/library/alloc/tests/boxed.rs index bfc31a626fadd..298a80ca81361 100644 --- a/library/alloc/tests/boxed.rs +++ b/library/alloc/tests/boxed.rs @@ -4,6 +4,9 @@ use core::mem::MaybeUninit; use core::ptr::NonNull; #[test] +// FIXME(GrigorenkoPV) +#[allow(unknown_lints, reason = "`instantly_dangling_pointer` does not exist at stage 0 yet")] +#[allow(instantly_dangling_pointer)] fn uninitialized_zero_size_box() { assert_eq!( &*Box::<()>::new_uninit() as *const _, From 66c4e00cce19146444f2f746b662c4e6eb943c4a Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 19 Aug 2024 22:23:28 +0300 Subject: [PATCH 16/26] `instantly_dangling_pointer`: rename to `dangling_pointers_from_temporaries` --- compiler/rustc_lint/src/dangling.rs | 18 +++------ library/alloc/tests/boxed.rs | 7 +++- tests/ui/consts/zst_no_llvm_alloc.rs | 2 +- ...tly-dangling-pointer-allow.rs => allow.rs} | 2 +- .../instantly-dangling-pointer-methods.rs | 6 --- .../instantly-dangling-pointer-types.rs | 37 ------------------- ...-pointer-issue123613.rs => issue123613.rs} | 6 +-- ...-issue123613.stderr => issue123613.stderr} | 10 ++--- tests/ui/lint/dangling-ptr/methods.rs | 6 +++ ...-pointer-methods.stderr => methods.stderr} | 10 ++--- ...-pointer-temporaries.rs => temporaries.rs} | 18 ++++----- ...-temporaries.stderr => temporaries.stderr} | 22 +++++------ tests/ui/lint/dangling-ptr/types.rs | 37 +++++++++++++++++++ ...ling-pointer-types.stderr => types.stderr} | 36 +++++++++--------- 14 files changed, 106 insertions(+), 111 deletions(-) rename tests/ui/lint/dangling-ptr/{instantly-dangling-pointer-allow.rs => allow.rs} (66%) delete mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs delete mode 100644 tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs rename tests/ui/lint/dangling-ptr/{instantly-dangling-pointer-issue123613.rs => issue123613.rs} (68%) rename tests/ui/lint/dangling-ptr/{instantly-dangling-pointer-issue123613.stderr => issue123613.stderr} (85%) create mode 100644 tests/ui/lint/dangling-ptr/methods.rs rename tests/ui/lint/dangling-ptr/{instantly-dangling-pointer-methods.stderr => methods.stderr} (84%) rename tests/ui/lint/dangling-ptr/{instantly-dangling-pointer-temporaries.rs => temporaries.rs} (74%) rename tests/ui/lint/dangling-ptr/{instantly-dangling-pointer-temporaries.stderr => temporaries.stderr} (89%) create mode 100644 tests/ui/lint/dangling-ptr/types.rs rename tests/ui/lint/dangling-ptr/{instantly-dangling-pointer-types.stderr => types.stderr} (91%) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 11726820fb937..0bfb639ae0a08 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -35,7 +35,7 @@ declare_lint! { // 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 `instantly_dangling_pointer` lint detects getting a pointer to data + /// The `dangling_pointers_from_temporaries` lint detects getting a pointer to data /// of a temporary that will immediately get dropped. /// /// ### Example @@ -63,20 +63,12 @@ declare_lint! { /// /// If you need stronger guarantees, consider using references instead, /// as they are statically verified by the borrow-checker to never dangle. - /// - /// Note: This lint does **not** get triggered by methods & functions - /// that intentionally produce dangling pointers, such as: - /// - /// - `core::ptr::dangling` & `core::ptr::dangling_mut` - /// - `core::ptr::NonNull::dangling` - /// - `std::alloc::Layout::dangling` - /// - pub INSTANTLY_DANGLING_POINTER, + pub DANGLING_POINTERS_FROM_TEMPORARIES, Warn, - "detects getting a pointer that will immediately dangle" + "detects getting a pointer from a temporary" } -declare_lint_pass!(DanglingPointers => [TEMPORARY_CSTRING_AS_PTR, INSTANTLY_DANGLING_POINTER]); +declare_lint_pass!(DanglingPointers => [TEMPORARY_CSTRING_AS_PTR, DANGLING_POINTERS_FROM_TEMPORARIES]); impl<'tcx> LateLintPass<'tcx> for DanglingPointers { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { @@ -111,7 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for DanglingPointers { && is_interesting(cx, ty) { cx.emit_span_lint( - INSTANTLY_DANGLING_POINTER, + DANGLING_POINTERS_FROM_TEMPORARIES, method.ident.span, InstantlyDangling { callee: method.ident.name, diff --git a/library/alloc/tests/boxed.rs b/library/alloc/tests/boxed.rs index 298a80ca81361..3df00864bfa69 100644 --- a/library/alloc/tests/boxed.rs +++ b/library/alloc/tests/boxed.rs @@ -5,8 +5,11 @@ use core::ptr::NonNull; #[test] // FIXME(GrigorenkoPV) -#[allow(unknown_lints, reason = "`instantly_dangling_pointer` does not exist at stage 0 yet")] -#[allow(instantly_dangling_pointer)] +#[allow( + unknown_lints, + reason = "`dangling_pointers_from_temporaries` does not exist at stage 0 yet" +)] +#[allow(dangling_pointers_from_temporaries)] fn uninitialized_zero_size_box() { assert_eq!( &*Box::<()>::new_uninit() as *const _, diff --git a/tests/ui/consts/zst_no_llvm_alloc.rs b/tests/ui/consts/zst_no_llvm_alloc.rs index 4c244f78ef203..d29f64b4729db 100644 --- a/tests/ui/consts/zst_no_llvm_alloc.rs +++ b/tests/ui/consts/zst_no_llvm_alloc.rs @@ -17,7 +17,7 @@ fn main() { // The exact addresses returned by these library functions are not necessarily stable guarantees // but for now we assert that we're still matching. - #[expect(instantly_dangling_pointer)] + #[expect(dangling_pointers_from_temporaries)] { assert_eq!(>::new().as_ptr(), <&[i32]>::default().as_ptr()); assert_eq!(>::default().as_ptr(), (&[]).as_ptr()); diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-allow.rs b/tests/ui/lint/dangling-ptr/allow.rs similarity index 66% rename from tests/ui/lint/dangling-ptr/instantly-dangling-pointer-allow.rs rename to tests/ui/lint/dangling-ptr/allow.rs index bc4e675cfcba9..8566191987ff5 100644 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-allow.rs +++ b/tests/ui/lint/dangling-ptr/allow.rs @@ -2,7 +2,7 @@ // This should not ICE. -#![allow(instantly_dangling_pointer)] +#![allow(dangling_pointers_from_temporaries)] fn main() { dbg!(String::new().as_ptr()); diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs deleted file mode 100644 index 12e80a13d3f86..0000000000000 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.rs +++ /dev/null @@ -1,6 +0,0 @@ -#![deny(instantly_dangling_pointer)] - -fn main() { - vec![0u8].as_ptr(); //~ ERROR [instantly_dangling_pointer] - vec![0u8].as_mut_ptr(); //~ ERROR [instantly_dangling_pointer] -} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs b/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs deleted file mode 100644 index 67d9845a0d9c8..0000000000000 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.rs +++ /dev/null @@ -1,37 +0,0 @@ -#![deny(instantly_dangling_pointer)] - -use std::cell::Cell; -use std::ffi::{CStr, CString}; -use std::mem::MaybeUninit; - -struct AsPtrFake; - -impl AsPtrFake { - fn as_ptr(&self) -> *const () { - std::ptr::null() - } -} - -fn declval() -> T { - loop {} -} - -fn main() { - declval::().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::<[u8; 10]>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>>>>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); //~ ERROR [instantly_dangling_pointer] - declval::>().as_ptr(); - declval::().as_ptr(); -} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.rs b/tests/ui/lint/dangling-ptr/issue123613.rs similarity index 68% rename from tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.rs rename to tests/ui/lint/dangling-ptr/issue123613.rs index 9ac72e2d70570..717f3d4614fc4 100644 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.rs +++ b/tests/ui/lint/dangling-ptr/issue123613.rs @@ -1,11 +1,11 @@ -#![deny(instantly_dangling_pointer)] +#![deny(dangling_pointers_from_temporaries)] const MAX_PATH: usize = 260; fn main() { let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); - //~^ ERROR [instantly_dangling_pointer] + //~^ ERROR [dangling_pointers_from_temporaries] let str2 = String::from("TotototototototototototototototototoT").as_ptr(); - //~^ ERROR [instantly_dangling_pointer] + //~^ ERROR [dangling_pointers_from_temporaries] unsafe { std::ptr::copy_nonoverlapping(str2, str1, 30); println!("{:?}", String::from_raw_parts(str1, 30, 30)); diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.stderr b/tests/ui/lint/dangling-ptr/issue123613.stderr similarity index 85% rename from tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.stderr rename to tests/ui/lint/dangling-ptr/issue123613.stderr index 3fb6f62892004..88666d934e543 100644 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-issue123613.stderr +++ b/tests/ui/lint/dangling-ptr/issue123613.stderr @@ -1,5 +1,5 @@ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-issue123613.rs:5:48 + --> $DIR/issue123613.rs:5:48 | LL | let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); | ------------------------------- ^^^^^^^^^^ this pointer will immediately be invalid @@ -9,13 +9,13 @@ LL | let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); = note: pointers do not have a lifetime; when calling `as_mut_ptr` the `String` 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 note: the lint level is defined here - --> $DIR/instantly-dangling-pointer-issue123613.rs:1:9 + --> $DIR/issue123613.rs:1:9 | -LL | #![deny(instantly_dangling_pointer)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(dangling_pointers_from_temporaries)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-issue123613.rs:7:70 + --> $DIR/issue123613.rs:7:70 | LL | let str2 = String::from("TotototototototototototototototototoT").as_ptr(); | ----------------------------------------------------- ^^^^^^ this pointer will immediately be invalid diff --git a/tests/ui/lint/dangling-ptr/methods.rs b/tests/ui/lint/dangling-ptr/methods.rs new file mode 100644 index 0000000000000..1959aed70a519 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/methods.rs @@ -0,0 +1,6 @@ +#![deny(dangling_pointers_from_temporaries)] + +fn main() { + vec![0u8].as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + vec![0u8].as_mut_ptr(); //~ ERROR [dangling_pointers_from_temporaries] +} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.stderr b/tests/ui/lint/dangling-ptr/methods.stderr similarity index 84% rename from tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.stderr rename to tests/ui/lint/dangling-ptr/methods.stderr index e1f451d7ccf69..4369a8a470a6f 100644 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-methods.stderr +++ b/tests/ui/lint/dangling-ptr/methods.stderr @@ -1,5 +1,5 @@ error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-methods.rs:4:15 + --> $DIR/methods.rs:4:15 | LL | vec![0u8].as_ptr(); | --------- ^^^^^^ this pointer will immediately be invalid @@ -9,13 +9,13 @@ LL | vec![0u8].as_ptr(); = note: pointers do not have a lifetime; when calling `as_ptr` the `Vec` 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 note: the lint level is defined here - --> $DIR/instantly-dangling-pointer-methods.rs:1:9 + --> $DIR/methods.rs:1:9 | -LL | #![deny(instantly_dangling_pointer)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(dangling_pointers_from_temporaries)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-methods.rs:5:15 + --> $DIR/methods.rs:5:15 | LL | vec![0u8].as_mut_ptr(); | --------- ^^^^^^^^^^ this pointer will immediately be invalid diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.rs b/tests/ui/lint/dangling-ptr/temporaries.rs similarity index 74% rename from tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.rs rename to tests/ui/lint/dangling-ptr/temporaries.rs index 42ce20142cf68..2802aff5dc67e 100644 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.rs +++ b/tests/ui/lint/dangling-ptr/temporaries.rs @@ -1,5 +1,5 @@ #![allow(unused)] -#![deny(instantly_dangling_pointer)] +#![deny(dangling_pointers_from_temporaries)] fn string() -> String { "hello".into() @@ -18,16 +18,16 @@ fn main() { } // Call - string().as_ptr(); //~ ERROR [instantly_dangling_pointer] + string().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] // MethodCall - "hello".to_string().as_ptr(); //~ ERROR [instantly_dangling_pointer] + "hello".to_string().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] // Tup // impossible // Binary - (string() + "hello").as_ptr(); //~ ERROR [instantly_dangling_pointer] + (string() + "hello").as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] // Path { @@ -63,7 +63,7 @@ fn main() { // If { (if true { String::new() } else { "hello".into() }).as_ptr(); - //~^ ERROR [instantly_dangling_pointer] + //~^ ERROR [dangling_pointers_from_temporaries] } // Loop @@ -71,7 +71,7 @@ fn main() { (loop { break String::new(); }) - .as_ptr(); //~ ERROR [instantly_dangling_pointer] + .as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] } // Match @@ -79,14 +79,14 @@ fn main() { match string() { s => s, } - .as_ptr(); //~ ERROR [instantly_dangling_pointer] + .as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] } // Closure // impossible // Block - { string() }.as_ptr(); //~ ERROR [instantly_dangling_pointer] + { string() }.as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] // Assign, AssignOp // impossible @@ -125,5 +125,5 @@ fn main() { // impossible // Macro - vec![0u8].as_ptr(); //~ ERROR [instantly_dangling_pointer] + vec![0u8].as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] } diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.stderr b/tests/ui/lint/dangling-ptr/temporaries.stderr similarity index 89% rename from tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.stderr rename to tests/ui/lint/dangling-ptr/temporaries.stderr index 547735b65dd7b..b205ed96799b8 100644 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-temporaries.stderr +++ b/tests/ui/lint/dangling-ptr/temporaries.stderr @@ -1,5 +1,5 @@ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:21:14 + --> $DIR/temporaries.rs:21:14 | LL | string().as_ptr(); | -------- ^^^^^^ this pointer will immediately be invalid @@ -9,13 +9,13 @@ LL | string().as_ptr(); = note: pointers do not have a lifetime; when calling `as_ptr` the `String` 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 note: the lint level is defined here - --> $DIR/instantly-dangling-pointer-temporaries.rs:2:9 + --> $DIR/temporaries.rs:2:9 | -LL | #![deny(instantly_dangling_pointer)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(dangling_pointers_from_temporaries)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:24:25 + --> $DIR/temporaries.rs:24:25 | LL | "hello".to_string().as_ptr(); | ------------------- ^^^^^^ this pointer will immediately be invalid @@ -26,7 +26,7 @@ LL | "hello".to_string().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:30:26 + --> $DIR/temporaries.rs:30:26 | LL | (string() + "hello").as_ptr(); | -------------------- ^^^^^^ this pointer will immediately be invalid @@ -37,7 +37,7 @@ LL | (string() + "hello").as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:65:61 + --> $DIR/temporaries.rs:65:61 | LL | (if true { String::new() } else { "hello".into() }).as_ptr(); | --------------------------------------------------- ^^^^^^ this pointer will immediately be invalid @@ -48,7 +48,7 @@ LL | (if true { String::new() } else { "hello".into() }).as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:74:10 + --> $DIR/temporaries.rs:74:10 | LL | / (loop { LL | | break String::new(); @@ -61,7 +61,7 @@ LL | .as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:82:10 + --> $DIR/temporaries.rs:82:10 | LL | / match string() { LL | | s => s, @@ -74,7 +74,7 @@ LL | .as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:89:18 + --> $DIR/temporaries.rs:89:18 | LL | { string() }.as_ptr(); | ------------ ^^^^^^ this pointer will immediately be invalid @@ -85,7 +85,7 @@ LL | { string() }.as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-temporaries.rs:128:15 + --> $DIR/temporaries.rs:128:15 | LL | vec![0u8].as_ptr(); | --------- ^^^^^^ this pointer will immediately be invalid diff --git a/tests/ui/lint/dangling-ptr/types.rs b/tests/ui/lint/dangling-ptr/types.rs new file mode 100644 index 0000000000000..978191c49b234 --- /dev/null +++ b/tests/ui/lint/dangling-ptr/types.rs @@ -0,0 +1,37 @@ +#![deny(dangling_pointers_from_temporaries)] + +use std::cell::Cell; +use std::ffi::{CStr, CString}; +use std::mem::MaybeUninit; + +struct AsPtrFake; + +impl AsPtrFake { + fn as_ptr(&self) -> *const () { + std::ptr::null() + } +} + +fn declval() -> T { + loop {} +} + +fn main() { + declval::().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::<[u8; 10]>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>>>>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::>().as_ptr(); + declval::().as_ptr(); +} diff --git a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.stderr b/tests/ui/lint/dangling-ptr/types.stderr similarity index 91% rename from tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.stderr rename to tests/ui/lint/dangling-ptr/types.stderr index 1040bef5dac98..ced8780e4eedd 100644 --- a/tests/ui/lint/dangling-ptr/instantly-dangling-pointer-types.stderr +++ b/tests/ui/lint/dangling-ptr/types.stderr @@ -1,5 +1,5 @@ error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:20:26 + --> $DIR/types.rs:20:26 | LL | declval::().as_ptr(); | -------------------- ^^^^^^ this pointer will immediately be invalid @@ -9,13 +9,13 @@ LL | declval::().as_ptr(); = 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 note: the lint level is defined here - --> $DIR/instantly-dangling-pointer-types.rs:1:9 + --> $DIR/types.rs:1:9 | -LL | #![deny(instantly_dangling_pointer)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(dangling_pointers_from_temporaries)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:21:25 + --> $DIR/types.rs:21:25 | LL | declval::().as_ptr(); | ------------------- ^^^^^^ this pointer will immediately be invalid @@ -26,7 +26,7 @@ LL | declval::().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:22:26 + --> $DIR/types.rs:22:26 | LL | declval::>().as_ptr(); | -------------------- ^^^^^^ this pointer will immediately be invalid @@ -37,7 +37,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:23:31 + --> $DIR/types.rs:23:31 | LL | declval::>().as_ptr(); | ------------------------- ^^^^^^ this pointer will immediately be invalid @@ -48,7 +48,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box<[u8]>` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:24:28 + --> $DIR/types.rs:24:28 | LL | declval::>().as_ptr(); | ---------------------- ^^^^^^ this pointer will immediately be invalid @@ -59,7 +59,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:25:27 + --> $DIR/types.rs:25:27 | LL | declval::>().as_ptr(); | --------------------- ^^^^^^ this pointer will immediately be invalid @@ -70,7 +70,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:26:28 + --> $DIR/types.rs:26:28 | LL | declval::>().as_ptr(); | ---------------------- ^^^^^^ this pointer will immediately be invalid @@ -81,7 +81,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `[u8; 10]` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:27:27 + --> $DIR/types.rs:27:27 | LL | declval::<[u8; 10]>().as_ptr(); | --------------------- ^^^^^^ this pointer will immediately be invalid @@ -92,7 +92,7 @@ LL | declval::<[u8; 10]>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box<[u8; 10]>` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:28:32 + --> $DIR/types.rs:28:32 | LL | declval::>().as_ptr(); | -------------------------- ^^^^^^ this pointer will immediately be invalid @@ -103,7 +103,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box>` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:29:31 + --> $DIR/types.rs:29:31 | LL | declval::>>().as_ptr(); | ------------------------- ^^^^^^ this pointer will immediately be invalid @@ -114,7 +114,7 @@ LL | declval::>>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:30:30 + --> $DIR/types.rs:30:30 | LL | declval::>().as_ptr(); | ------------------------ ^^^^^^ this pointer will immediately be invalid @@ -125,7 +125,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box>>>` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:31:43 + --> $DIR/types.rs:31:43 | LL | declval::>>>>().as_ptr(); | ------------------------------------- ^^^^^^ this pointer will immediately be invalid @@ -136,7 +136,7 @@ LL | declval::>>>>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Cell` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:32:27 + --> $DIR/types.rs:32:27 | LL | declval::>().as_ptr(); | --------------------- ^^^^^^ this pointer will immediately be invalid @@ -147,7 +147,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `MaybeUninit` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:33:34 + --> $DIR/types.rs:33:34 | LL | declval::>().as_ptr(); | ---------------------------- ^^^^^^ this pointer will immediately be invalid @@ -158,7 +158,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/instantly-dangling-pointer-types.rs:34:33 + --> $DIR/types.rs:34:33 | LL | declval::>().as_ptr(); | --------------------------- ^^^^^^ this pointer will immediately be invalid From e3df230846b0cf8e9205aa5c3b0e16aa26deea77 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 19 Aug 2024 22:35:07 +0300 Subject: [PATCH 17/26] pass less context around --- compiler/rustc_lint/src/dangling.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 0bfb639ae0a08..d352944e0a3c4 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -1,5 +1,5 @@ use rustc_hir::{Expr, ExprKind, LangItem}; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::symbol::sym; @@ -100,7 +100,7 @@ impl<'tcx> LateLintPass<'tcx> for DanglingPointers { && 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, ty) + && is_interesting(cx.tcx, ty) { cx.emit_span_lint( DANGLING_POINTERS_FROM_TEMPORARIES, @@ -164,22 +164,21 @@ fn is_temporary_rvalue(expr: &Expr<'_>) -> bool { // Array, Vec, String, CString, MaybeUninit, Cell, Box<[_]>, Box, Box, // or any of the above in arbitrary many nested Box'es. -fn is_interesting(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { +fn is_interesting(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool { if ty.is_array() { true } else if let Some(inner) = ty.boxed_ty() { inner.is_slice() || inner.is_str() - || inner.ty_adt_def().is_some_and(|def| cx.tcx.is_lang_item(def.did(), LangItem::CStr)) - || is_interesting(cx, inner) + || 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 cx.tcx.is_lang_item(def.did(), lang_item) { + if tcx.is_lang_item(def.did(), lang_item) { return true; } } - cx.tcx - .get_diagnostic_name(def.did()) + tcx.get_diagnostic_name(def.did()) .is_some_and(|name| matches!(name, sym::cstring_type | sym::Vec | sym::Cell)) } else { false From ca9657423852efe21c419b0b4a42d8e07054805e Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 19 Aug 2024 22:40:52 +0300 Subject: [PATCH 18/26] replace `temporary_cstring_as_ptr` with `dangling_pointers_from_temporaries` --- compiler/rustc_lint/src/dangling.rs | 54 +------------------ compiler/rustc_lint/src/lib.rs | 1 + .../allow.rs | 0 .../cstring-as-param.rs | 1 + .../cstring-as-param.stderr | 12 ++++- .../cstring-as-ptr.rs | 1 + .../cstring-as-ptr.stderr | 14 +++-- .../issue123613.rs | 0 .../issue123613.stderr | 0 .../methods.rs | 0 .../methods.stderr | 0 .../temporaries.rs | 0 .../temporaries.stderr | 0 .../types.rs | 0 .../types.stderr | 0 15 files changed, 26 insertions(+), 57 deletions(-) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/allow.rs (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/cstring-as-param.rs (89%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/cstring-as-param.stderr (65%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/cstring-as-ptr.rs (94%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/cstring-as-ptr.stderr (78%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/issue123613.rs (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/issue123613.stderr (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/methods.rs (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/methods.stderr (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/temporaries.rs (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/temporaries.stderr (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/types.rs (100%) rename tests/ui/lint/{dangling-ptr => dangling-pointers-from-temporaries}/types.stderr (100%) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index d352944e0a3c4..c8bd2690d7c43 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -1,37 +1,11 @@ use rustc_hir::{Expr, ExprKind, LangItem}; -use rustc_middle::ty::{self, Ty, TyCtxt}; +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}; -declare_lint! { - /// The `temporary_cstring_as_ptr` lint detects getting the inner pointer of - /// a temporary `CString`. - /// - /// ### Example - /// - /// ```rust - /// # #![allow(unused)] - /// # use std::ffi::CString; - /// let c_str = CString::new("foo").unwrap().as_ptr(); - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// The inner pointer of a `CString` lives only as long as the `CString` it - /// points to. Getting the inner pointer of a *temporary* `CString` allows the `CString` - /// to be dropped at the end of the statement, as it is not being referenced as far as the - /// typesystem is concerned. This means outside of the statement the pointer will point to - /// freed memory, which causes undefined behavior if the pointer is later dereferenced. - pub TEMPORARY_CSTRING_AS_PTR, - Warn, - "detects getting the inner pointer of a temporary `CString`" -} - // 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! { @@ -68,34 +42,10 @@ declare_lint! { "detects getting a pointer from a temporary" } -declare_lint_pass!(DanglingPointers => [TEMPORARY_CSTRING_AS_PTR, DANGLING_POINTERS_FROM_TEMPORARIES]); +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(as_ptr_path, as_ptr_receiver, ..) = expr.kind - && as_ptr_path.ident.name == sym::as_ptr - && let ExprKind::MethodCall(unwrap_path, unwrap_receiver, ..) = as_ptr_receiver.kind - && (unwrap_path.ident.name == sym::unwrap || unwrap_path.ident.name == sym::expect) - && let source_type = cx.typeck_results().expr_ty(unwrap_receiver) - && let ty::Adt(def, args) = source_type.kind() - && cx.tcx.is_diagnostic_item(sym::Result, def.did()) - && let ty = args.type_at(0) - && let ty::Adt(adt, _) = ty.kind() - && cx.tcx.is_diagnostic_item(sym::cstring_type, adt.did()) - { - cx.emit_span_lint( - TEMPORARY_CSTRING_AS_PTR, - as_ptr_path.ident.span, - InstantlyDangling { - callee: as_ptr_path.ident.name, - ty, - ptr_span: as_ptr_path.ident.span, - temporary_span: as_ptr_receiver.span, - }, - ); - return; // One lint is enough - } - 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) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index fe68ece144a47..1998890548620 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -348,6 +348,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 diff --git a/tests/ui/lint/dangling-ptr/allow.rs b/tests/ui/lint/dangling-pointers-from-temporaries/allow.rs similarity index 100% rename from tests/ui/lint/dangling-ptr/allow.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/allow.rs diff --git a/tests/ui/lint/dangling-ptr/cstring-as-param.rs b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs similarity index 89% rename from tests/ui/lint/dangling-ptr/cstring-as-param.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs index 07d7d52a11d92..559e2b47a29e9 100644 --- a/tests/ui/lint/dangling-ptr/cstring-as-param.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs @@ -1,4 +1,5 @@ #![deny(temporary_cstring_as_ptr)] +//~^ [renamed_and_removed_lints] use std::ffi::CString; use std::os::raw::c_char; diff --git a/tests/ui/lint/dangling-ptr/cstring-as-param.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.stderr similarity index 65% rename from tests/ui/lint/dangling-ptr/cstring-as-param.stderr rename to tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.stderr index 7a13c23735e4b..c28cd82ed289c 100644 --- a/tests/ui/lint/dangling-ptr/cstring-as-param.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.stderr @@ -1,5 +1,13 @@ +warning: lint `temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` + --> $DIR/cstring-as-param.rs:1:9 + | +LL | #![deny(temporary_cstring_as_ptr)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `dangling_pointers_from_temporaries` + | + = note: `#[warn(renamed_and_removed_lints)]` on by default + error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/cstring-as-param.rs:9:45 + --> $DIR/cstring-as-param.rs:10:45 | LL | some_function(CString::new("").unwrap().as_ptr()); | ------------------------- ^^^^^^ this pointer will immediately be invalid @@ -14,5 +22,5 @@ note: the lint level is defined here LL | #![deny(temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 1 previous error +error: aborting due to 1 previous error; 1 warning emitted diff --git a/tests/ui/lint/dangling-ptr/cstring-as-ptr.rs b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.rs similarity index 94% rename from tests/ui/lint/dangling-ptr/cstring-as-ptr.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.rs index 6993cfc5501d2..4e6868b6d5309 100644 --- a/tests/ui/lint/dangling-ptr/cstring-as-ptr.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.rs @@ -1,5 +1,6 @@ // this program is not technically incorrect, but is an obscure enough style to be worth linting #![deny(temporary_cstring_as_ptr)] +//~^ [renamed_and_removed_lints] use std::ffi::CString; diff --git a/tests/ui/lint/dangling-ptr/cstring-as-ptr.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.stderr similarity index 78% rename from tests/ui/lint/dangling-ptr/cstring-as-ptr.stderr rename to tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.stderr index 240774fb38283..9d31a50839eab 100644 --- a/tests/ui/lint/dangling-ptr/cstring-as-ptr.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.stderr @@ -1,5 +1,13 @@ +warning: lint `temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` + --> $DIR/cstring-as-ptr.rs:2:9 + | +LL | #![deny(temporary_cstring_as_ptr)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `dangling_pointers_from_temporaries` + | + = note: `#[warn(renamed_and_removed_lints)]` on by default + error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/cstring-as-ptr.rs:14:48 + --> $DIR/cstring-as-ptr.rs:15:48 | LL | let s = CString::new("some text").unwrap().as_ptr(); | ---------------------------------- ^^^^^^ this pointer will immediately be invalid @@ -15,7 +23,7 @@ LL | #![deny(temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/cstring-as-ptr.rs:8:52 + --> $DIR/cstring-as-ptr.rs:9:52 | LL | let s = CString::new("some text").unwrap().as_ptr(); | ---------------------------------- ^^^^^^ this pointer will immediately be invalid @@ -29,5 +37,5 @@ LL | mymacro!(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html = note: this error originates in the macro `mymacro` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 2 previous errors +error: aborting due to 2 previous errors; 1 warning emitted diff --git a/tests/ui/lint/dangling-ptr/issue123613.rs b/tests/ui/lint/dangling-pointers-from-temporaries/issue123613.rs similarity index 100% rename from tests/ui/lint/dangling-ptr/issue123613.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/issue123613.rs diff --git a/tests/ui/lint/dangling-ptr/issue123613.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/issue123613.stderr similarity index 100% rename from tests/ui/lint/dangling-ptr/issue123613.stderr rename to tests/ui/lint/dangling-pointers-from-temporaries/issue123613.stderr diff --git a/tests/ui/lint/dangling-ptr/methods.rs b/tests/ui/lint/dangling-pointers-from-temporaries/methods.rs similarity index 100% rename from tests/ui/lint/dangling-ptr/methods.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/methods.rs diff --git a/tests/ui/lint/dangling-ptr/methods.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/methods.stderr similarity index 100% rename from tests/ui/lint/dangling-ptr/methods.stderr rename to tests/ui/lint/dangling-pointers-from-temporaries/methods.stderr diff --git a/tests/ui/lint/dangling-ptr/temporaries.rs b/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.rs similarity index 100% rename from tests/ui/lint/dangling-ptr/temporaries.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/temporaries.rs diff --git a/tests/ui/lint/dangling-ptr/temporaries.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.stderr similarity index 100% rename from tests/ui/lint/dangling-ptr/temporaries.stderr rename to tests/ui/lint/dangling-pointers-from-temporaries/temporaries.stderr diff --git a/tests/ui/lint/dangling-ptr/types.rs b/tests/ui/lint/dangling-pointers-from-temporaries/types.rs similarity index 100% rename from tests/ui/lint/dangling-ptr/types.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/types.rs diff --git a/tests/ui/lint/dangling-ptr/types.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/types.stderr similarity index 100% rename from tests/ui/lint/dangling-ptr/types.stderr rename to tests/ui/lint/dangling-pointers-from-temporaries/types.stderr From 3254f0c8b951c2cf8af6661e13fc03711aead8f4 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Mon, 19 Aug 2024 22:51:59 +0300 Subject: [PATCH 19/26] tidy --- .../{issue123613.rs => example-from-issue123613.rs} | 0 .../{issue123613.stderr => example-from-issue123613.stderr} | 6 +++--- 2 files changed, 3 insertions(+), 3 deletions(-) rename tests/ui/lint/dangling-pointers-from-temporaries/{issue123613.rs => example-from-issue123613.rs} (100%) rename tests/ui/lint/dangling-pointers-from-temporaries/{issue123613.stderr => example-from-issue123613.stderr} (92%) diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/issue123613.rs b/tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.rs similarity index 100% rename from tests/ui/lint/dangling-pointers-from-temporaries/issue123613.rs rename to tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.rs diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/issue123613.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.stderr similarity index 92% rename from tests/ui/lint/dangling-pointers-from-temporaries/issue123613.stderr rename to tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.stderr index 88666d934e543..de66565674c0a 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/issue123613.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.stderr @@ -1,5 +1,5 @@ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/issue123613.rs:5:48 + --> $DIR/example-from-issue123613.rs:5:48 | LL | let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); | ------------------------------- ^^^^^^^^^^ this pointer will immediately be invalid @@ -9,13 +9,13 @@ LL | let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); = note: pointers do not have a lifetime; when calling `as_mut_ptr` the `String` 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 note: the lint level is defined here - --> $DIR/issue123613.rs:1:9 + --> $DIR/example-from-issue123613.rs:1:9 | LL | #![deny(dangling_pointers_from_temporaries)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/issue123613.rs:7:70 + --> $DIR/example-from-issue123613.rs:7:70 | LL | let str2 = String::from("TotototototototototototototototototoT").as_ptr(); | ----------------------------------------------------- ^^^^^^ this pointer will immediately be invalid From 9f1e2326253dc79044584c54328ee1d0e1173694 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Wed, 21 Aug 2024 01:23:51 +0300 Subject: [PATCH 20/26] Bless clippy --- .../clippy_lints/src/deprecated_lints.rs | 2 +- src/tools/clippy/tests/ui/rename.fixed | 4 ++-- src/tools/clippy/tests/ui/rename.stderr | 18 ++++++++++++------ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs index 0066ed643251a..77dbe9b78a182 100644 --- a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs +++ b/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 = ""] diff --git a/src/tools/clippy/tests/ui/rename.fixed b/src/tools/clippy/tests/ui/rename.fixed index b810fd8224f63..0d6e07aa546cf 100644 --- a/src/tools/clippy/tests/ui/rename.fixed +++ b/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` diff --git a/src/tools/clippy/tests/ui/rename.stderr b/src/tools/clippy/tests/ui/rename.stderr index 46d9f0fac59f9..b906079d7df3e 100644 --- a/src/tools/clippy/tests/ui/rename.stderr +++ b/src/tools/clippy/tests/ui/rename.stderr @@ -1,11 +1,17 @@ +error: lint `temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` + --> tests/ui/rename.rs:57:10 + | +LL | #![allow(temporary_cstring_as_ptr)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `dangling_pointers_from_temporaries` + | + = note: `-D renamed-and-removed-lints` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(renamed_and_removed_lints)]` + error: lint `clippy::almost_complete_letter_range` has been renamed to `clippy::almost_complete_range` --> tests/ui/rename.rs:63:9 | LL | #![warn(clippy::almost_complete_letter_range)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::almost_complete_range` - | - = note: `-D renamed-and-removed-lints` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(renamed_and_removed_lints)]` error: lint `clippy::blacklisted_name` has been renamed to `clippy::disallowed_names` --> tests/ui/rename.rs:64:9 @@ -361,11 +367,11 @@ error: lint `clippy::positional_named_format_parameters` has been renamed to `na LL | #![warn(clippy::positional_named_format_parameters)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `named_arguments_used_positionally` -error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr` +error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` --> tests/ui/rename.rs:123:9 | LL | #![warn(clippy::temporary_cstring_as_ptr)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `dangling_pointers_from_temporaries` error: lint `clippy::undropped_manually_drops` has been renamed to `undropped_manually_drops` --> tests/ui/rename.rs:124:9 @@ -397,5 +403,5 @@ error: lint `clippy::reverse_range_loop` has been renamed to `clippy::reversed_e LL | #![warn(clippy::reverse_range_loop)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::reversed_empty_ranges` -error: aborting due to 66 previous errors +error: aborting due to 67 previous errors From 00ef4132ede6c10580a5c1dbc96eb46efd82509d Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Wed, 21 Aug 2024 18:38:47 +0300 Subject: [PATCH 21/26] fix a doctest in core --- library/core/src/ffi/c_str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 7808d42ab5de4..2abcd392fbff9 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -464,7 +464,7 @@ 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)] #![allow(dangling_pointers_from_temporaries)] /// use std::ffi::CString; /// /// // Do not do this: From 0078bb6f1785cef4f0ca01eb56a8ba13cec6d8d9 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Wed, 21 Aug 2024 19:09:38 +0300 Subject: [PATCH 22/26] `dangling_pointers_from_temporaries`: don't use [lint_name] syntax in tests --- .../cstring-as-param.rs | 2 +- .../cstring-as-ptr.rs | 2 +- .../example-from-issue123613.rs | 4 +- .../methods.rs | 6 ++- .../methods.stderr | 2 +- .../temporaries.rs | 23 ++++++---- .../temporaries.stderr | 14 +++--- .../types.rs | 45 ++++++++++++------- .../types.stderr | 28 ++++++------ 9 files changed, 75 insertions(+), 51 deletions(-) diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs index 559e2b47a29e9..843c898ce823a 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs @@ -1,5 +1,5 @@ #![deny(temporary_cstring_as_ptr)] -//~^ [renamed_and_removed_lints] +//~^ WARNING lint `temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` use std::ffi::CString; use std::os::raw::c_char; diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.rs b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.rs index 4e6868b6d5309..659d4e11b85f2 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-ptr.rs @@ -1,6 +1,6 @@ // this program is not technically incorrect, but is an obscure enough style to be worth linting #![deny(temporary_cstring_as_ptr)] -//~^ [renamed_and_removed_lints] +//~^ WARNING lint `temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` use std::ffi::CString; diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.rs b/tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.rs index 717f3d4614fc4..457b5fb4ee743 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/example-from-issue123613.rs @@ -3,9 +3,9 @@ const MAX_PATH: usize = 260; fn main() { let str1 = String::with_capacity(MAX_PATH).as_mut_ptr(); - //~^ ERROR [dangling_pointers_from_temporaries] + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer let str2 = String::from("TotototototototototototototototototoT").as_ptr(); - //~^ ERROR [dangling_pointers_from_temporaries] + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer unsafe { std::ptr::copy_nonoverlapping(str2, str1, 30); println!("{:?}", String::from_raw_parts(str1, 30, 30)); diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/methods.rs b/tests/ui/lint/dangling-pointers-from-temporaries/methods.rs index 1959aed70a519..6287b4317d525 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/methods.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/methods.rs @@ -1,6 +1,8 @@ #![deny(dangling_pointers_from_temporaries)] fn main() { - vec![0u8].as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - vec![0u8].as_mut_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + vec![0u8].as_ptr(); + //~^ ERROR getting a pointer from a temporary `Vec` will result in a dangling pointer + vec![0u8].as_mut_ptr(); + //~^ ERROR getting a pointer from a temporary `Vec` will result in a dangling pointer } diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/methods.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/methods.stderr index 4369a8a470a6f..245204325b837 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/methods.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/methods.stderr @@ -15,7 +15,7 @@ LL | #![deny(dangling_pointers_from_temporaries)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/methods.rs:5:15 + --> $DIR/methods.rs:6:15 | LL | vec![0u8].as_mut_ptr(); | --------- ^^^^^^^^^^ this pointer will immediately be invalid diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.rs b/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.rs index 2802aff5dc67e..3bfb9c69a319d 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.rs @@ -18,16 +18,19 @@ fn main() { } // Call - string().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + string().as_ptr(); + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer // MethodCall - "hello".to_string().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + "hello".to_string().as_ptr(); + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer // Tup // impossible // Binary - (string() + "hello").as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + (string() + "hello").as_ptr(); + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer // Path { @@ -63,7 +66,7 @@ fn main() { // If { (if true { String::new() } else { "hello".into() }).as_ptr(); - //~^ ERROR [dangling_pointers_from_temporaries] + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer } // Loop @@ -71,7 +74,8 @@ fn main() { (loop { break String::new(); }) - .as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + .as_ptr(); + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer } // Match @@ -79,14 +83,16 @@ fn main() { match string() { s => s, } - .as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + .as_ptr(); + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer } // Closure // impossible // Block - { string() }.as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + { string() }.as_ptr(); + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer // Assign, AssignOp // impossible @@ -125,5 +131,6 @@ fn main() { // impossible // Macro - vec![0u8].as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + vec![0u8].as_ptr(); + //~^ ERROR getting a pointer from a temporary `Vec` will result in a dangling pointer } diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.stderr index b205ed96799b8..d46b2375fdf73 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/temporaries.stderr @@ -15,7 +15,7 @@ LL | #![deny(dangling_pointers_from_temporaries)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/temporaries.rs:24:25 + --> $DIR/temporaries.rs:25:25 | LL | "hello".to_string().as_ptr(); | ------------------- ^^^^^^ this pointer will immediately be invalid @@ -26,7 +26,7 @@ LL | "hello".to_string().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/temporaries.rs:30:26 + --> $DIR/temporaries.rs:32:26 | LL | (string() + "hello").as_ptr(); | -------------------- ^^^^^^ this pointer will immediately be invalid @@ -37,7 +37,7 @@ LL | (string() + "hello").as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/temporaries.rs:65:61 + --> $DIR/temporaries.rs:68:61 | LL | (if true { String::new() } else { "hello".into() }).as_ptr(); | --------------------------------------------------- ^^^^^^ this pointer will immediately be invalid @@ -48,7 +48,7 @@ LL | (if true { String::new() } else { "hello".into() }).as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/temporaries.rs:74:10 + --> $DIR/temporaries.rs:77:10 | LL | / (loop { LL | | break String::new(); @@ -61,7 +61,7 @@ LL | .as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/temporaries.rs:82:10 + --> $DIR/temporaries.rs:86:10 | LL | / match string() { LL | | s => s, @@ -74,7 +74,7 @@ LL | .as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/temporaries.rs:89:18 + --> $DIR/temporaries.rs:94:18 | LL | { string() }.as_ptr(); | ------------ ^^^^^^ this pointer will immediately be invalid @@ -85,7 +85,7 @@ LL | { string() }.as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/temporaries.rs:128:15 + --> $DIR/temporaries.rs:134:15 | LL | vec![0u8].as_ptr(); | --------- ^^^^^^ this pointer will immediately be invalid diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/types.rs b/tests/ui/lint/dangling-pointers-from-temporaries/types.rs index 978191c49b234..80cb7d056489c 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/types.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/types.rs @@ -17,21 +17,36 @@ fn declval() -> T { } fn main() { - declval::().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::<[u8; 10]>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>>>>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] - declval::>().as_ptr(); //~ ERROR [dangling_pointers_from_temporaries] + declval::().as_ptr(); + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer + declval::().as_ptr(); + //~^ ERROR getting a pointer from a temporary `String` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Vec` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box<[u8]>` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box` will result in a dangling pointer + declval::<[u8; 10]>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `[u8; 10]` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box<[u8; 10]>` will result in a dangling pointer + declval::>>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box>` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box` will result in a dangling pointer + declval::>>>>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Box>>>` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Cell` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `MaybeUninit` will result in a dangling pointer + declval::>().as_ptr(); + //~^ ERROR getting a pointer from a temporary `Vec` will result in a dangling pointer declval::>().as_ptr(); declval::().as_ptr(); } diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/types.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/types.stderr index ced8780e4eedd..cb9c9702420b8 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/types.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/types.stderr @@ -15,7 +15,7 @@ LL | #![deny(dangling_pointers_from_temporaries)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `String` will result in a dangling pointer - --> $DIR/types.rs:21:25 + --> $DIR/types.rs:22:25 | LL | declval::().as_ptr(); | ------------------- ^^^^^^ this pointer will immediately be invalid @@ -26,7 +26,7 @@ LL | declval::().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/types.rs:22:26 + --> $DIR/types.rs:24:26 | LL | declval::>().as_ptr(); | -------------------- ^^^^^^ this pointer will immediately be invalid @@ -37,7 +37,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/types.rs:23:31 + --> $DIR/types.rs:26:31 | LL | declval::>().as_ptr(); | ------------------------- ^^^^^^ this pointer will immediately be invalid @@ -48,7 +48,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box<[u8]>` will result in a dangling pointer - --> $DIR/types.rs:24:28 + --> $DIR/types.rs:28:28 | LL | declval::>().as_ptr(); | ---------------------- ^^^^^^ this pointer will immediately be invalid @@ -59,7 +59,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/types.rs:25:27 + --> $DIR/types.rs:30:27 | LL | declval::>().as_ptr(); | --------------------- ^^^^^^ this pointer will immediately be invalid @@ -70,7 +70,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/types.rs:26:28 + --> $DIR/types.rs:32:28 | LL | declval::>().as_ptr(); | ---------------------- ^^^^^^ this pointer will immediately be invalid @@ -81,7 +81,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `[u8; 10]` will result in a dangling pointer - --> $DIR/types.rs:27:27 + --> $DIR/types.rs:34:27 | LL | declval::<[u8; 10]>().as_ptr(); | --------------------- ^^^^^^ this pointer will immediately be invalid @@ -92,7 +92,7 @@ LL | declval::<[u8; 10]>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box<[u8; 10]>` will result in a dangling pointer - --> $DIR/types.rs:28:32 + --> $DIR/types.rs:36:32 | LL | declval::>().as_ptr(); | -------------------------- ^^^^^^ this pointer will immediately be invalid @@ -103,7 +103,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box>` will result in a dangling pointer - --> $DIR/types.rs:29:31 + --> $DIR/types.rs:38:31 | LL | declval::>>().as_ptr(); | ------------------------- ^^^^^^ this pointer will immediately be invalid @@ -114,7 +114,7 @@ LL | declval::>>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box` will result in a dangling pointer - --> $DIR/types.rs:30:30 + --> $DIR/types.rs:40:30 | LL | declval::>().as_ptr(); | ------------------------ ^^^^^^ this pointer will immediately be invalid @@ -125,7 +125,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Box>>>` will result in a dangling pointer - --> $DIR/types.rs:31:43 + --> $DIR/types.rs:42:43 | LL | declval::>>>>().as_ptr(); | ------------------------------------- ^^^^^^ this pointer will immediately be invalid @@ -136,7 +136,7 @@ LL | declval::>>>>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Cell` will result in a dangling pointer - --> $DIR/types.rs:32:27 + --> $DIR/types.rs:44:27 | LL | declval::>().as_ptr(); | --------------------- ^^^^^^ this pointer will immediately be invalid @@ -147,7 +147,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `MaybeUninit` will result in a dangling pointer - --> $DIR/types.rs:33:34 + --> $DIR/types.rs:46:34 | LL | declval::>().as_ptr(); | ---------------------------- ^^^^^^ this pointer will immediately be invalid @@ -158,7 +158,7 @@ LL | declval::>().as_ptr(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `Vec` will result in a dangling pointer - --> $DIR/types.rs:34:33 + --> $DIR/types.rs:48:33 | LL | declval::>().as_ptr(); | --------------------------- ^^^^^^ this pointer will immediately be invalid From 6ee354459ee3f21a5f29bf1026165223942fcda1 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Wed, 21 Aug 2024 23:14:44 +0300 Subject: [PATCH 23/26] Oh, yeah, #[cfg(bootstrap)] exists --- library/alloc/tests/boxed.rs | 7 +------ library/core/src/ffi/c_str.rs | 4 +++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/library/alloc/tests/boxed.rs b/library/alloc/tests/boxed.rs index 3df00864bfa69..544f60da587c8 100644 --- a/library/alloc/tests/boxed.rs +++ b/library/alloc/tests/boxed.rs @@ -4,12 +4,7 @@ use core::mem::MaybeUninit; use core::ptr::NonNull; #[test] -// FIXME(GrigorenkoPV) -#[allow( - unknown_lints, - reason = "`dangling_pointers_from_temporaries` does not exist at stage 0 yet" -)] -#[allow(dangling_pointers_from_temporaries)] +#[cfg_attr(not(bootstrap), expect(dangling_pointers_from_temporaries))] fn uninitialized_zero_size_box() { assert_eq!( &*Box::<()>::new_uninit() as *const _, diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 2abcd392fbff9..2eef9e587f379 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -464,7 +464,9 @@ impl CStr { /// behavior when `ptr` is used inside the `unsafe` block: /// /// ```no_run - /// # #![allow(unused_must_use)] #![allow(dangling_pointers_from_temporaries)] + /// # #![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: From 8a640ce55ea73396dd1417b35de1ce48f730e897 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Thu, 5 Sep 2024 03:34:09 +0300 Subject: [PATCH 24/26] `dangling_pointers_from_temporaries`: fix false positives The solution is hacky and itroduces more false negatives. Also added some docs and tests. --- compiler/rustc_lint/src/dangling.rs | 113 ++++++++++++++++-- compiler/rustc_lint/src/lib.rs | 2 +- tests/ui/consts/zst_no_llvm_alloc.rs | 2 +- .../calls.rs | 45 +++++++ .../calls.stderr | 40 +++++++ .../cstring-as-param.rs | 3 +- .../cstring-as-param.stderr | 20 +--- 7 files changed, 196 insertions(+), 29 deletions(-) create mode 100644 tests/ui/lint/dangling-pointers-from-temporaries/calls.rs create mode 100644 tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index c8bd2690d7c43..2d110fb70501c 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -1,13 +1,11 @@ -use rustc_hir::{Expr, ExprKind, LangItem}; +use rustc_hir::{Expr, ExprKind, HirId, LangItem}; use rustc_middle::ty::{Ty, TyCtxt}; -use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_session::{declare_lint, impl_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. @@ -16,9 +14,7 @@ declare_lint! { /// /// ```rust /// # #![allow(unused)] - /// # unsafe fn use_data(ptr: *const u8) { - /// # dbg!(unsafe { ptr.read() }); - /// # } + /// # unsafe fn use_data(ptr: *const u8) { } /// fn gather_and_use(bytes: impl Iterator) { /// let x: *const u8 = bytes.collect::>().as_ptr(); /// unsafe { use_data(x) } @@ -42,10 +38,111 @@ declare_lint! { "detects getting a pointer from a temporary" } -declare_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]); +#[derive(Copy, Clone, Default)] +pub(crate) struct DanglingPointers { + /// HACK: trying to deal with argument lifetime extension. + /// + /// This produces a dangling pointer: + /// ```ignore (example) + /// let ptr = CString::new("hello").unwrap().as_ptr(); + /// foo(ptr) + /// ``` + /// + /// But this does not: + /// ```ignore (example) + /// foo(CString::new("hello").unwrap().as_ptr()) + /// ``` + /// + /// So we have to deal with this situation somehow. + /// + /// If we were a visitor, we could just not visit the arguments. + /// + /// Or, maybe visit them while maintaining a flag that would indicate + /// when the lifetime extension can occur, because code like this + /// ```ignore (example) + /// foo({ let ptr = CString::new("hello").unwrap().as_ptr(); ptr }) + /// ``` + /// still produces a dangling pointer. + /// + /// But we are not a visitor. We are a LateLintPass, + /// and we get called on every expression there is. + /// + /// The best we can do is to keep track of when we enter + /// a function/method call and stop linting until we exit it. + /// + /// Sadly, we cannot update this *after* we have visited the LHS of a call, + /// but *before* we visit the arguments. + /// So we update this even before visiting the LHS, + /// which means we don't get to walk it properly. Sad. + token: Option, +} +impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]); + +/// FIXME: false negatives (i.e. the lint is not emitted when it should be) +/// 1. Method calls that are not checked for: +/// - [`temporary_unsafe_cell.get()`][`core::cell::UnsafeCell::get()`] +/// - [`temporary_sync_unsafe_cell.get()`][`core::cell::SyncUnsafeCell::get()`] +/// 2. Ways to get a temporary that are not recognized: +/// - `owning_temporary.field` +/// - `owning_temporary[index]` +/// 3. No checks for ref-to-ptr conversions: +/// - `&raw [mut] temporary` +/// - `&temporary as *(const|mut) _` +/// - `ptr::from_ref(&temporary)` and friends +/// 4. Aggressive mitigation for lifetime extension of function/method call arguments \ +/// (see the comment on [`DanglingPointers::token`]): +/// - **Function/method call arguments are completely skipped.** \ +/// Technically this means we overlook things like +/// ```ignore (example) +/// foo({ let x = get_cstring(); x.as_ptr() }, 1) +/// ``` +/// but who in their right mind would write something like that? +/// - **A function call's callee / a method call's receiver is not visited fully.** \ +/// We descent into it recursively, but we stop at the first expression that is +/// neither [`ExprKind::Call`] nor [`ExprKind::MethodCall`]. +/// This means +/// ```ignore (example) +/// CString::new("Hello").unwrap().as_ptr().cast() +/// ``` +/// will get linted, but +/// ```ignore (example) +/// { CString::new("Hello").unwrap().as_ptr() }.cast() +/// ``` +/// will not, because we stop at the block and do not descend any further. impl<'tcx> LateLintPass<'tcx> for DanglingPointers { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if self.token.is_some() { + tracing::debug!(skip = ?cx.sess().source_map().span_to_snippet(expr.span)); + return; + } + + if let ExprKind::Call(lhs, _args) | ExprKind::MethodCall(_, lhs, _args, _) = expr.kind { + tracing::trace!(enter = ?cx.sess().source_map().span_to_snippet(expr.span)); + + // Peel any nested calls from the LHS. + self.check_expr(cx, lhs); + self.check_expr_post(cx, lhs); + + debug_assert_eq!(self.token, None); + self.token = Some(expr.hir_id); + + tracing::debug!(set = ?cx.sess().source_map().span_to_snippet(expr.span)); + } + self.lint_expr(cx, expr); + } + + fn check_expr_post(&mut self, _cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if self.token == Some(expr.hir_id) { + self.token = None; + + tracing::debug!(unset = ?_cx.sess().source_map().span_to_snippet(expr.span)); + } + } +} + +impl DanglingPointers { + fn lint_expr(&self, cx: &LateContext<'_>, expr: &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) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 1998890548620..2291aec35d08e 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -227,7 +227,7 @@ late_lint_methods!( UngatedAsyncFnTrackCaller: UngatedAsyncFnTrackCaller, ShadowedIntoIter: ShadowedIntoIter, DropTraitConstraints: DropTraitConstraints, - DanglingPointers: DanglingPointers, + DanglingPointers: DanglingPointers::default(), NonPanicFmt: NonPanicFmt, NoopMethodCall: NoopMethodCall, EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums, diff --git a/tests/ui/consts/zst_no_llvm_alloc.rs b/tests/ui/consts/zst_no_llvm_alloc.rs index d29f64b4729db..1e92e3bbd4c1b 100644 --- a/tests/ui/consts/zst_no_llvm_alloc.rs +++ b/tests/ui/consts/zst_no_llvm_alloc.rs @@ -17,7 +17,7 @@ fn main() { // The exact addresses returned by these library functions are not necessarily stable guarantees // but for now we assert that we're still matching. - #[expect(dangling_pointers_from_temporaries)] + #[allow(dangling_pointers_from_temporaries)] { assert_eq!(>::new().as_ptr(), <&[i32]>::default().as_ptr()); assert_eq!(>::default().as_ptr(), (&[]).as_ptr()); diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs b/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs new file mode 100644 index 0000000000000..b5a5421f18af4 --- /dev/null +++ b/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs @@ -0,0 +1,45 @@ +#![deny(dangling_pointers_from_temporaries)] + +use std::ffi::{c_char, CString}; + +fn cstring() -> CString { + CString::new("hello").unwrap() +} + +fn consume(ptr: *const c_char) { + let c = unsafe { ptr.read() }; + dbg!(c); +} + +// None of these should trigger the lint. +fn ok() { + consume(cstring().as_ptr()); + consume({ cstring() }.as_ptr()); + consume({ cstring().as_ptr() }); + consume(cstring().as_ptr().cast()); + consume({ cstring() }.as_ptr().cast()); + consume({ cstring().as_ptr() }.cast()); +} + +// All of these should trigger the lint. +fn not_ok() { + let ptr = cstring().as_ptr(); + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer + consume(ptr); + consume({ + let ptr = cstring().as_ptr(); + //^ FIXME: should error + ptr + }); + let _ptr: *const u8 = cstring().as_ptr().cast(); + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer + let _ptr: *const u8 = { cstring() }.as_ptr().cast(); + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer + let _ptr: *const u8 = { cstring().as_ptr() }.cast(); + //^ FIXME: should error +} + +fn main() { + ok(); + not_ok(); +} diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr new file mode 100644 index 0000000000000..5b2a741118074 --- /dev/null +++ b/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr @@ -0,0 +1,40 @@ +error: getting a pointer from a temporary `CString` will result in a dangling pointer + --> $DIR/calls.rs:26:25 + | +LL | let ptr = cstring().as_ptr(); + | --------- ^^^^^^ this pointer will immediately be invalid + | | + | 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 +note: the lint level is defined here + --> $DIR/calls.rs:1:9 + | +LL | #![deny(dangling_pointers_from_temporaries)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: getting a pointer from a temporary `CString` will result in a dangling pointer + --> $DIR/calls.rs:34:37 + | +LL | let _ptr: *const u8 = cstring().as_ptr().cast(); + | --------- ^^^^^^ this pointer will immediately be invalid + | | + | 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 + +error: getting a pointer from a temporary `CString` will result in a dangling pointer + --> $DIR/calls.rs:36:41 + | +LL | let _ptr: *const u8 = { cstring() }.as_ptr().cast(); + | ------------- ^^^^^^ this pointer will immediately be invalid + | | + | 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 + +error: aborting due to 3 previous errors + diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs index 843c898ce823a..fb6ed3632720e 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.rs @@ -1,3 +1,5 @@ +//@ check-pass + #![deny(temporary_cstring_as_ptr)] //~^ WARNING lint `temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` @@ -8,5 +10,4 @@ fn some_function(data: *const c_char) {} fn main() { some_function(CString::new("").unwrap().as_ptr()); - //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer } diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.stderr index c28cd82ed289c..dd54b4971dd87 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/cstring-as-param.stderr @@ -1,26 +1,10 @@ warning: lint `temporary_cstring_as_ptr` has been renamed to `dangling_pointers_from_temporaries` - --> $DIR/cstring-as-param.rs:1:9 + --> $DIR/cstring-as-param.rs:3:9 | LL | #![deny(temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `dangling_pointers_from_temporaries` | = note: `#[warn(renamed_and_removed_lints)]` on by default -error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/cstring-as-param.rs:10:45 - | -LL | some_function(CString::new("").unwrap().as_ptr()); - | ------------------------- ^^^^^^ this pointer will immediately be invalid - | | - | 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 -note: the lint level is defined here - --> $DIR/cstring-as-param.rs:1:9 - | -LL | #![deny(temporary_cstring_as_ptr)] - | ^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error; 1 warning emitted +warning: 1 warning emitted From d9dd850f6e5f0821f054a43a3e5c1d0d5d1d3303 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Thu, 5 Sep 2024 13:25:42 +0300 Subject: [PATCH 25/26] `dangling_pointers_from_temporaries`: properly walk LHS of calls --- compiler/rustc_lint/src/dangling.rs | 102 ++++++++---------- compiler/rustc_lint/src/lib.rs | 1 + .../calls.rs | 15 ++- .../calls.stderr | 27 +++-- 4 files changed, 75 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 2d110fb70501c..9ab0f511477db 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -38,7 +38,13 @@ declare_lint! { "detects getting a pointer from a temporary" } -#[derive(Copy, Clone, Default)] +#[derive(Clone, Debug, PartialEq, Eq)] +enum CallPart { + LHS { lhs: HirId, call: HirId }, + Arguments { call: HirId }, +} + +#[derive(Clone, Default)] pub(crate) struct DanglingPointers { /// HACK: trying to deal with argument lifetime extension. /// @@ -67,14 +73,9 @@ pub(crate) struct DanglingPointers { /// But we are not a visitor. We are a LateLintPass, /// and we get called on every expression there is. /// - /// The best we can do is to keep track of when we enter - /// a function/method call and stop linting until we exit it. - /// - /// Sadly, we cannot update this *after* we have visited the LHS of a call, - /// but *before* we visit the arguments. - /// So we update this even before visiting the LHS, - /// which means we don't get to walk it properly. Sad. - token: Option, + /// So let's try to maintain this context stack explicitly + /// instead of as a part of the call stack. + nested_calls: Vec, } impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]); @@ -98,68 +99,53 @@ impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]); /// foo({ let x = get_cstring(); x.as_ptr() }, 1) /// ``` /// but who in their right mind would write something like that? -/// - **A function call's callee / a method call's receiver is not visited fully.** \ -/// We descent into it recursively, but we stop at the first expression that is -/// neither [`ExprKind::Call`] nor [`ExprKind::MethodCall`]. -/// This means -/// ```ignore (example) -/// CString::new("Hello").unwrap().as_ptr().cast() -/// ``` -/// will get linted, but -/// ```ignore (example) -/// { CString::new("Hello").unwrap().as_ptr() }.cast() -/// ``` -/// will not, because we stop at the block and do not descend any further. impl<'tcx> LateLintPass<'tcx> for DanglingPointers { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if self.token.is_some() { + if let Some(CallPart::Arguments { .. }) = self.nested_calls.last() { tracing::debug!(skip = ?cx.sess().source_map().span_to_snippet(expr.span)); return; } - if let ExprKind::Call(lhs, _args) | ExprKind::MethodCall(_, lhs, _args, _) = expr.kind { - tracing::trace!(enter = ?cx.sess().source_map().span_to_snippet(expr.span)); + lint_expr(cx, expr); - // Peel any nested calls from the LHS. - self.check_expr(cx, lhs); - self.check_expr_post(cx, lhs); - - debug_assert_eq!(self.token, None); - self.token = Some(expr.hir_id); - - tracing::debug!(set = ?cx.sess().source_map().span_to_snippet(expr.span)); + match expr.kind { + ExprKind::Call(lhs, _args) | ExprKind::MethodCall(_, lhs, _args, _) => { + self.nested_calls.push(CallPart::LHS { lhs: lhs.hir_id, call: expr.hir_id }) + } + _ => {} } - self.lint_expr(cx, expr); } - fn check_expr_post(&mut self, _cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if self.token == Some(expr.hir_id) { - self.token = None; - - tracing::debug!(unset = ?_cx.sess().source_map().span_to_snippet(expr.span)); - } + fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + self.nested_calls.pop_if(|pos| match pos { + &mut CallPart::LHS { lhs, call } => { + if expr.hir_id == lhs { + *pos = CallPart::Arguments { call }; + }; + false + } + &mut CallPart::Arguments { call } => expr.hir_id == call, + }); } } -impl DanglingPointers { - fn lint_expr(&self, cx: &LateContext<'_>, expr: &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 lint_expr(cx: &LateContext<'_>, expr: &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, + }, + ) } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 2291aec35d08e..73d9122de2ffe 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -40,6 +40,7 @@ #![feature(rustc_attrs)] #![feature(rustdoc_internals)] #![feature(trait_upcasting)] +#![feature(vec_pop_if)] #![warn(unreachable_pub)] // tidy-alphabetical-end diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs b/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs index b5a5421f18af4..48cb371d617d6 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs @@ -23,20 +23,27 @@ fn ok() { // All of these should trigger the lint. fn not_ok() { - let ptr = cstring().as_ptr(); - //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer - consume(ptr); + { + let ptr = cstring().as_ptr(); + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer + consume(ptr); + } consume({ let ptr = cstring().as_ptr(); //^ FIXME: should error ptr }); + consume({ + let s = cstring(); + s.as_ptr() + //^ FIXME: should error + }); let _ptr: *const u8 = cstring().as_ptr().cast(); //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer let _ptr: *const u8 = { cstring() }.as_ptr().cast(); //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer let _ptr: *const u8 = { cstring().as_ptr() }.cast(); - //^ FIXME: should error + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer } fn main() { diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr index 5b2a741118074..8e5fcae9c31a4 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr @@ -1,10 +1,10 @@ error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/calls.rs:26:25 + --> $DIR/calls.rs:27:29 | -LL | let ptr = cstring().as_ptr(); - | --------- ^^^^^^ this pointer will immediately be invalid - | | - | this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime +LL | let ptr = cstring().as_ptr(); + | --------- ^^^^^^ this pointer will immediately be invalid + | | + | 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 @@ -15,7 +15,7 @@ LL | #![deny(dangling_pointers_from_temporaries)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/calls.rs:34:37 + --> $DIR/calls.rs:41:37 | LL | let _ptr: *const u8 = cstring().as_ptr().cast(); | --------- ^^^^^^ this pointer will immediately be invalid @@ -26,7 +26,7 @@ LL | let _ptr: *const u8 = cstring().as_ptr().cast(); = help: for more information, see https://doc.rust-lang.org/reference/destructors.html error: getting a pointer from a temporary `CString` will result in a dangling pointer - --> $DIR/calls.rs:36:41 + --> $DIR/calls.rs:43:41 | LL | let _ptr: *const u8 = { cstring() }.as_ptr().cast(); | ------------- ^^^^^^ this pointer will immediately be invalid @@ -36,5 +36,16 @@ LL | let _ptr: *const u8 = { cstring() }.as_ptr().cast(); = 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 -error: aborting due to 3 previous errors +error: getting a pointer from a temporary `CString` will result in a dangling pointer + --> $DIR/calls.rs:45:39 + | +LL | let _ptr: *const u8 = { cstring().as_ptr() }.cast(); + | --------- ^^^^^^ this pointer will immediately be invalid + | | + | 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 + +error: aborting due to 4 previous errors From 76c60f305ab81c72e60c9c44836a02c0c41d38cc Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Thu, 5 Sep 2024 16:38:00 +0300 Subject: [PATCH 26/26] `dangling_pointers_from_temporaries`: deal with argument lifetime externsion --- compiler/rustc_lint/src/dangling.rs | 81 +++++++++++-------- .../calls.rs | 2 +- .../calls.stderr | 13 ++- 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_lint/src/dangling.rs b/compiler/rustc_lint/src/dangling.rs index 9ab0f511477db..1439635c27955 100644 --- a/compiler/rustc_lint/src/dangling.rs +++ b/compiler/rustc_lint/src/dangling.rs @@ -1,4 +1,4 @@ -use rustc_hir::{Expr, ExprKind, HirId, LangItem}; +use rustc_hir::{Block, Expr, ExprKind, HirId, LangItem}; use rustc_middle::ty::{Ty, TyCtxt}; use rustc_session::{declare_lint, impl_lint_pass}; use rustc_span::symbol::sym; @@ -39,14 +39,21 @@ declare_lint! { } #[derive(Clone, Debug, PartialEq, Eq)] -enum CallPart { - LHS { lhs: HirId, call: HirId }, - Arguments { call: HirId }, +enum LifetimeExtension { + /// Lifetime extension has not kicked in yet, but it will soon. + /// Example: walking LHS of a function/method call. + EnableLater { after_exit: HirId, until_exit: HirId }, + /// Lifetime extension is currently active. + /// Example: walking a function/method call's arguments. + Enable { until_exit: HirId }, + /// Temporary disable lifetime extension. + /// Example: statements of a block that is a function/method call's argument. + Disable { until_exit: HirId }, } #[derive(Clone, Default)] pub(crate) struct DanglingPointers { - /// HACK: trying to deal with argument lifetime extension. + /// Trying to deal with argument lifetime extension. /// /// This produces a dangling pointer: /// ```ignore (example) @@ -59,23 +66,26 @@ pub(crate) struct DanglingPointers { /// foo(CString::new("hello").unwrap().as_ptr()) /// ``` /// - /// So we have to deal with this situation somehow. - /// - /// If we were a visitor, we could just not visit the arguments. - /// - /// Or, maybe visit them while maintaining a flag that would indicate - /// when the lifetime extension can occur, because code like this + /// But this does: /// ```ignore (example) /// foo({ let ptr = CString::new("hello").unwrap().as_ptr(); ptr }) /// ``` - /// still produces a dangling pointer. /// - /// But we are not a visitor. We are a LateLintPass, - /// and we get called on every expression there is. + /// We have to deal with this situation somehow. + /// + /// If we were a visitor, we could just keep track of + /// when we enter and exit places where lifetime extension kicks in + /// during visiting/walking and update a boolean flag accordingly. /// + /// But we are not a visitor. We are a LateLintPass. + /// We are not the one who does the visiting & walking + /// and can maintain this state directly in the call stack. + /// But we do get called on every expression there is, + /// both when entering it and exiting from it + /// during our depth-first walk of the tree. /// So let's try to maintain this context stack explicitly /// instead of as a part of the call stack. - nested_calls: Vec, + nested_calls: Vec, } impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]); @@ -91,40 +101,41 @@ impl_lint_pass!(DanglingPointers => [DANGLING_POINTERS_FROM_TEMPORARIES]); /// - `&raw [mut] temporary` /// - `&temporary as *(const|mut) _` /// - `ptr::from_ref(&temporary)` and friends -/// 4. Aggressive mitigation for lifetime extension of function/method call arguments \ -/// (see the comment on [`DanglingPointers::token`]): -/// - **Function/method call arguments are completely skipped.** \ -/// Technically this means we overlook things like -/// ```ignore (example) -/// foo({ let x = get_cstring(); x.as_ptr() }, 1) -/// ``` -/// but who in their right mind would write something like that? impl<'tcx> LateLintPass<'tcx> for DanglingPointers { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(CallPart::Arguments { .. }) = self.nested_calls.last() { - tracing::debug!(skip = ?cx.sess().source_map().span_to_snippet(expr.span)); - return; + if let Some(LifetimeExtension::Enable { .. }) = self.nested_calls.last() { + match expr.kind { + ExprKind::Block(Block { stmts: [.., last_stmt], .. }, _) => self + .nested_calls + .push(LifetimeExtension::Disable { until_exit: last_stmt.hir_id }), + _ => { + tracing::debug!(skip = ?cx.sess().source_map().span_to_snippet(expr.span)); + return; + } + } } lint_expr(cx, expr); - match expr.kind { - ExprKind::Call(lhs, _args) | ExprKind::MethodCall(_, lhs, _args, _) => { - self.nested_calls.push(CallPart::LHS { lhs: lhs.hir_id, call: expr.hir_id }) - } - _ => {} + if let ExprKind::Call(lhs, _args) | ExprKind::MethodCall(_, lhs, _args, _) = expr.kind { + self.nested_calls.push(LifetimeExtension::EnableLater { + after_exit: lhs.hir_id, + until_exit: expr.hir_id, + }) } } fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { self.nested_calls.pop_if(|pos| match pos { - &mut CallPart::LHS { lhs, call } => { - if expr.hir_id == lhs { - *pos = CallPart::Arguments { call }; + LifetimeExtension::Enable { until_exit } + | LifetimeExtension::Disable { until_exit } => expr.hir_id == *until_exit, + + &mut LifetimeExtension::EnableLater { after_exit, until_exit } => { + if expr.hir_id == after_exit { + *pos = LifetimeExtension::Enable { until_exit }; }; false } - &mut CallPart::Arguments { call } => expr.hir_id == call, }); } } diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs b/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs index 48cb371d617d6..e42dc287fe5ff 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs +++ b/tests/ui/lint/dangling-pointers-from-temporaries/calls.rs @@ -30,7 +30,7 @@ fn not_ok() { } consume({ let ptr = cstring().as_ptr(); - //^ FIXME: should error + //~^ ERROR getting a pointer from a temporary `CString` will result in a dangling pointer ptr }); consume({ diff --git a/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr b/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr index 8e5fcae9c31a4..a3fe2ee02cd79 100644 --- a/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr +++ b/tests/ui/lint/dangling-pointers-from-temporaries/calls.stderr @@ -14,6 +14,17 @@ note: the lint level is defined here LL | #![deny(dangling_pointers_from_temporaries)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: getting a pointer from a temporary `CString` will result in a dangling pointer + --> $DIR/calls.rs:32:29 + | +LL | let ptr = cstring().as_ptr(); + | --------- ^^^^^^ this pointer will immediately be invalid + | | + | 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 + error: getting a pointer from a temporary `CString` will result in a dangling pointer --> $DIR/calls.rs:41:37 | @@ -47,5 +58,5 @@ LL | let _ptr: *const u8 = { cstring().as_ptr() }.cast(); = 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 -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors