From 345d6b816b8b131bc592f2aa532e1b3aeef23d0b Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 14 Jul 2023 21:54:27 +0200 Subject: [PATCH 1/5] Revert "Temporarily switch invalid_reference_casting lint to allow-by-default" This reverts commit f25ad54a4d0febbcb2b7e951835228b7b2320b49. --- compiler/rustc_lint/src/reference_casting.rs | 3 +-- .../ui/const-generics/issues/issue-100313.rs | 1 + .../const-generics/issues/issue-100313.stderr | 12 +++++++-- tests/ui/lint/reference_casting.rs | 1 - tests/ui/lint/reference_casting.stderr | 26 ++++++++----------- 5 files changed, 23 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_lint/src/reference_casting.rs b/compiler/rustc_lint/src/reference_casting.rs index d343aaf35d5bb..1a172dd694251 100644 --- a/compiler/rustc_lint/src/reference_casting.rs +++ b/compiler/rustc_lint/src/reference_casting.rs @@ -12,7 +12,6 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail - /// # #![deny(invalid_reference_casting)] /// fn x(r: &i32) { /// unsafe { /// *(r as *const i32 as *mut i32) += 1; @@ -30,7 +29,7 @@ declare_lint! { /// `UnsafeCell` is the only way to obtain aliasable data that is considered /// mutable. INVALID_REFERENCE_CASTING, - Allow, + Deny, "casts of `&T` to `&mut T` without interior mutability" } diff --git a/tests/ui/const-generics/issues/issue-100313.rs b/tests/ui/const-generics/issues/issue-100313.rs index 4e9d3626aa89c..9a9d4721c8494 100644 --- a/tests/ui/const-generics/issues/issue-100313.rs +++ b/tests/ui/const-generics/issues/issue-100313.rs @@ -9,6 +9,7 @@ impl T { unsafe { *(B as *const bool as *mut bool) = false; //~^ ERROR evaluation of constant value failed [E0080] + //~| ERROR casting `&T` to `&mut T` is undefined behavior } } } diff --git a/tests/ui/const-generics/issues/issue-100313.stderr b/tests/ui/const-generics/issues/issue-100313.stderr index d4b486376cac8..17e4850bd12a4 100644 --- a/tests/ui/const-generics/issues/issue-100313.stderr +++ b/tests/ui/const-generics/issues/issue-100313.stderr @@ -1,3 +1,11 @@ +error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` + --> $DIR/issue-100313.rs:10:13 + | +LL | *(B as *const bool as *mut bool) = false; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[deny(invalid_reference_casting)]` on by default + error[E0080]: evaluation of constant value failed --> $DIR/issue-100313.rs:10:13 | @@ -10,11 +18,11 @@ note: inside `T::<&true>::set_false` LL | *(B as *const bool as *mut bool) = false; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: inside `_` - --> $DIR/issue-100313.rs:18:5 + --> $DIR/issue-100313.rs:19:5 | LL | x.set_false(); | ^^^^^^^^^^^^^ -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/lint/reference_casting.rs b/tests/ui/lint/reference_casting.rs index 9963820499edd..745d707014366 100644 --- a/tests/ui/lint/reference_casting.rs +++ b/tests/ui/lint/reference_casting.rs @@ -1,7 +1,6 @@ // check-fail #![feature(ptr_from_ref)] -#![deny(invalid_reference_casting)] extern "C" { // N.B., mutability can be easily incorrect in FFI calls -- as diff --git a/tests/ui/lint/reference_casting.stderr b/tests/ui/lint/reference_casting.stderr index d5b9bbef6439f..d1dd1b32ff44d 100644 --- a/tests/ui/lint/reference_casting.stderr +++ b/tests/ui/lint/reference_casting.stderr @@ -1,65 +1,61 @@ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:20:9 + --> $DIR/reference_casting.rs:19:9 | LL | (*(a as *const _ as *mut String)).push_str(" world"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: the lint level is defined here - --> $DIR/reference_casting.rs:4:9 - | -LL | #![deny(invalid_reference_casting)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `#[deny(invalid_reference_casting)]` on by default error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:22:9 + --> $DIR/reference_casting.rs:21:9 | LL | *(a as *const _ as *mut _) = String::from("Replaced"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:24:9 + --> $DIR/reference_casting.rs:23:9 | LL | *(a as *const _ as *mut String) += " world"; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:26:25 + --> $DIR/reference_casting.rs:25:25 | LL | let _num = &mut *(num as *const i32 as *mut i32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:28:25 + --> $DIR/reference_casting.rs:27:25 | LL | let _num = &mut *(num as *const i32).cast_mut(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:30:20 + --> $DIR/reference_casting.rs:29:20 | LL | let _num = *{ num as *const i32 }.cast_mut(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:32:9 + --> $DIR/reference_casting.rs:31:9 | LL | *std::ptr::from_ref(num).cast_mut() += 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:34:9 + --> $DIR/reference_casting.rs:33:9 | LL | *std::ptr::from_ref({ num }).cast_mut() += 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:36:9 + --> $DIR/reference_casting.rs:35:9 | LL | *{ std::ptr::from_ref(num) }.cast_mut() += 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:38:9 + --> $DIR/reference_casting.rs:37:9 | LL | *(std::ptr::from_ref({ num }) as *mut i32) += 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From f3dafe91ff753e3a801aa336d41be9eca75925bc Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 14 Jul 2023 22:10:14 +0200 Subject: [PATCH 2/5] Add support for deferred casting for the invalid_reference_casting lint --- compiler/rustc_lint/messages.ftl | 1 + compiler/rustc_lint/src/lib.rs | 2 +- compiler/rustc_lint/src/lints.rs | 5 +- compiler/rustc_lint/src/reference_casting.rs | 91 +++++++++++++------- tests/ui/lint/reference_casting.rs | 3 + tests/ui/lint/reference_casting.stderr | 10 ++- 6 files changed, 80 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 16e17fc9d6a5c..95f0b6519620c 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -319,6 +319,7 @@ lint_invalid_nan_comparisons_eq_ne = incorrect NaN comparison, NaN cannot be dir lint_invalid_nan_comparisons_lt_le_gt_ge = incorrect NaN comparison, NaN is not orderable lint_invalid_reference_casting = casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` + .label = casting happend here lint_lintpass_by_hand = implementing `LintPass` by hand .help = try using `declare_lint_pass!` or `impl_lint_pass!` instead diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 53089294fe2e6..96fd3ccf77444 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -218,7 +218,7 @@ late_lint_methods!( BoxPointers: BoxPointers, PathStatements: PathStatements, LetUnderscore: LetUnderscore, - InvalidReferenceCasting: InvalidReferenceCasting, + InvalidReferenceCasting: InvalidReferenceCasting::default(), // Depends on referenced function signatures in expressions UnusedResults: UnusedResults, NonUpperCaseGlobals: NonUpperCaseGlobals, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 968172693a93b..6d774070ec433 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -746,7 +746,10 @@ pub enum InvalidFromUtf8Diag { // reference_casting.rs #[derive(LintDiagnostic)] #[diag(lint_invalid_reference_casting)] -pub struct InvalidReferenceCastingDiag; +pub struct InvalidReferenceCastingDiag { + #[label] + pub orig_cast: Option, +} // hidden_unicode_codepoints.rs #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint/src/reference_casting.rs b/compiler/rustc_lint/src/reference_casting.rs index 1a172dd694251..e6cab4cebe739 100644 --- a/compiler/rustc_lint/src/reference_casting.rs +++ b/compiler/rustc_lint/src/reference_casting.rs @@ -1,7 +1,8 @@ use rustc_ast::Mutability; -use rustc_hir::{Expr, ExprKind, MutTy, TyKind, UnOp}; -use rustc_middle::ty; -use rustc_span::sym; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, QPath, StmtKind, UnOp}; +use rustc_middle::ty::{self, TypeAndMut}; +use rustc_span::{sym, Span}; use crate::{lints::InvalidReferenceCastingDiag, LateContext, LateLintPass, LintContext}; @@ -33,42 +34,74 @@ declare_lint! { "casts of `&T` to `&mut T` without interior mutability" } -declare_lint_pass!(InvalidReferenceCasting => [INVALID_REFERENCE_CASTING]); +#[derive(Default)] +pub struct InvalidReferenceCasting { + casted: FxHashMap, +} + +impl_lint_pass!(InvalidReferenceCasting => [INVALID_REFERENCE_CASTING]); impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - let ExprKind::Unary(UnOp::Deref, e) = &expr.kind else { + fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx rustc_hir::Stmt<'tcx>) { + let StmtKind::Local(local) = stmt.kind else { return; }; - - let e = e.peel_blocks(); - let e = if let ExprKind::Cast(e, t) = e.kind - && let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind { - e - } else if let ExprKind::MethodCall(_, expr, [], _) = e.kind - && let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) - && cx.tcx.is_diagnostic_item(sym::ptr_cast_mut, def_id) { - expr - } else { + let Local { init: Some(init), els: None, .. } = local else { return; }; - let e = e.peel_blocks(); - let e = if let ExprKind::Cast(e, t) = e.kind - && let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind { - e - } else if let ExprKind::Call(path, [arg]) = e.kind - && let ExprKind::Path(ref qpath) = path.kind - && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() - && cx.tcx.is_diagnostic_item(sym::ptr_from_ref, def_id) { - arg - } else { + if is_cast_from_const_to_mut(cx, init) { + self.casted.insert(local.pat.hir_id, init.span); + } + } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + let ExprKind::Unary(UnOp::Deref, e) = &expr.kind else { return; }; - let e = e.peel_blocks(); - if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind() { - cx.emit_spanned_lint(INVALID_REFERENCE_CASTING, expr.span, InvalidReferenceCastingDiag); + if is_cast_from_const_to_mut(cx, e) { + cx.emit_spanned_lint(INVALID_REFERENCE_CASTING, expr.span, InvalidReferenceCastingDiag { orig_cast: None }); + } else if let ExprKind::Path(QPath::Resolved(_, path)) = e.kind + && let Res::Local(hir_id) = &path.res + && let Some(orig_cast) = self.casted.get(hir_id) { + cx.emit_spanned_lint(INVALID_REFERENCE_CASTING, expr.span, InvalidReferenceCastingDiag { orig_cast: Some(*orig_cast) }); } } } + +fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool { + let e = e.peel_blocks(); + + // as *mut ... + let e = if let ExprKind::Cast(e, t) = e.kind + && let ty::RawPtr(TypeAndMut { mutbl: Mutability::Mut, .. }) = cx.typeck_results().node_type(t.hir_id).kind() { + e + // .cast_mut() + } else if let ExprKind::MethodCall(_, expr, [], _) = e.kind + && let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) + && cx.tcx.is_diagnostic_item(sym::ptr_cast_mut, def_id) { + expr + } else { + return false; + }; + + let e = e.peel_blocks(); + + // as *const ... + let e = if let ExprKind::Cast(e, t) = e.kind + && let ty::RawPtr(TypeAndMut { mutbl: Mutability::Not, .. }) = cx.typeck_results().node_type(t.hir_id).kind() { + e + // ptr::from_ref() + } else if let ExprKind::Call(path, [arg]) = e.kind + && let ExprKind::Path(ref qpath) = path.kind + && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() + && cx.tcx.is_diagnostic_item(sym::ptr_from_ref, def_id) { + arg + } else { + return false; + }; + + let e = e.peel_blocks(); + matches!(cx.typeck_results().node_type(e.hir_id).kind(), ty::Ref(..)) +} diff --git a/tests/ui/lint/reference_casting.rs b/tests/ui/lint/reference_casting.rs index 745d707014366..c3168cd4c0144 100644 --- a/tests/ui/lint/reference_casting.rs +++ b/tests/ui/lint/reference_casting.rs @@ -36,6 +36,9 @@ fn main() { //~^ ERROR casting `&T` to `&mut T` is undefined behavior *(std::ptr::from_ref({ num }) as *mut i32) += 1; //~^ ERROR casting `&T` to `&mut T` is undefined behavior + let value = num as *const i32 as *mut i32; + *value = 1; + //~^ ERROR casting `&T` to `&mut T` is undefined behavior // Shouldn't be warned against println!("{}", *(num as *const _ as *const i16)); diff --git a/tests/ui/lint/reference_casting.stderr b/tests/ui/lint/reference_casting.stderr index d1dd1b32ff44d..d9ce4b383875f 100644 --- a/tests/ui/lint/reference_casting.stderr +++ b/tests/ui/lint/reference_casting.stderr @@ -60,5 +60,13 @@ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is LL | *(std::ptr::from_ref({ num }) as *mut i32) += 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 10 previous errors +error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` + --> $DIR/reference_casting.rs:40:9 + | +LL | let value = num as *const i32 as *mut i32; + | ----------------------------- casting happend here +LL | *value = 1; + | ^^^^^^ + +error: aborting due to 11 previous errors From 50a46710a9c6942751930fa963e0d70d84128859 Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 14 Jul 2023 22:15:28 +0200 Subject: [PATCH 3/5] Avoid linting on expression that are only UB with SB/TB --- compiler/rustc_lint/src/reference_casting.rs | 17 ++++++- tests/ui/lint/reference_casting.rs | 5 +- tests/ui/lint/reference_casting.stderr | 48 ++++++++------------ 3 files changed, 34 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_lint/src/reference_casting.rs b/compiler/rustc_lint/src/reference_casting.rs index e6cab4cebe739..428bf750bbd1b 100644 --- a/compiler/rustc_lint/src/reference_casting.rs +++ b/compiler/rustc_lint/src/reference_casting.rs @@ -56,7 +56,20 @@ impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting { } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - let ExprKind::Unary(UnOp::Deref, e) = &expr.kind else { + // &mut + let inner = if let ExprKind::AddrOf(_, Mutability::Mut, expr) = expr.kind { + expr + // = ... + } else if let ExprKind::Assign(expr, _, _) = expr.kind { + expr + // += ... + } else if let ExprKind::AssignOp(_, expr, _) = expr.kind { + expr + } else { + return; + }; + + let ExprKind::Unary(UnOp::Deref, e) = &inner.kind else { return; }; @@ -103,5 +116,5 @@ fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) }; let e = e.peel_blocks(); - matches!(cx.typeck_results().node_type(e.hir_id).kind(), ty::Ref(..)) + matches!(cx.typeck_results().node_type(e.hir_id).kind(), ty::Ref(_, _, Mutability::Not)) } diff --git a/tests/ui/lint/reference_casting.rs b/tests/ui/lint/reference_casting.rs index c3168cd4c0144..aa946038c99da 100644 --- a/tests/ui/lint/reference_casting.rs +++ b/tests/ui/lint/reference_casting.rs @@ -16,8 +16,6 @@ fn main() { let num = &3i32; let mut_num = &mut 3i32; - (*(a as *const _ as *mut String)).push_str(" world"); - //~^ ERROR casting `&T` to `&mut T` is undefined behavior *(a as *const _ as *mut _) = String::from("Replaced"); //~^ ERROR casting `&T` to `&mut T` is undefined behavior *(a as *const _ as *mut String) += " world"; @@ -26,8 +24,6 @@ fn main() { //~^ ERROR casting `&T` to `&mut T` is undefined behavior let _num = &mut *(num as *const i32).cast_mut(); //~^ ERROR casting `&T` to `&mut T` is undefined behavior - let _num = *{ num as *const i32 }.cast_mut(); - //~^ ERROR casting `&T` to `&mut T` is undefined behavior *std::ptr::from_ref(num).cast_mut() += 1; //~^ ERROR casting `&T` to `&mut T` is undefined behavior *std::ptr::from_ref({ num }).cast_mut() += 1; @@ -41,6 +37,7 @@ fn main() { //~^ ERROR casting `&T` to `&mut T` is undefined behavior // Shouldn't be warned against + *(num as *const i32 as *mut i32); println!("{}", *(num as *const _ as *const i16)); println!("{}", *(mut_num as *mut _ as *mut i16)); ffi(a.as_ptr() as *mut _); diff --git a/tests/ui/lint/reference_casting.stderr b/tests/ui/lint/reference_casting.stderr index d9ce4b383875f..22bc66264d053 100644 --- a/tests/ui/lint/reference_casting.stderr +++ b/tests/ui/lint/reference_casting.stderr @@ -1,72 +1,60 @@ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` --> $DIR/reference_casting.rs:19:9 | -LL | (*(a as *const _ as *mut String)).push_str(" world"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | *(a as *const _ as *mut _) = String::from("Replaced"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[deny(invalid_reference_casting)]` on by default error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` --> $DIR/reference_casting.rs:21:9 | -LL | *(a as *const _ as *mut _) = String::from("Replaced"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:23:9 - | LL | *(a as *const _ as *mut String) += " world"; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:25:25 + --> $DIR/reference_casting.rs:23:20 | LL | let _num = &mut *(num as *const i32 as *mut i32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:27:25 + --> $DIR/reference_casting.rs:25:20 | LL | let _num = &mut *(num as *const i32).cast_mut(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:29:20 - | -LL | let _num = *{ num as *const i32 }.cast_mut(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:31:9 + --> $DIR/reference_casting.rs:27:9 | LL | *std::ptr::from_ref(num).cast_mut() += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:33:9 + --> $DIR/reference_casting.rs:29:9 | LL | *std::ptr::from_ref({ num }).cast_mut() += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:35:9 + --> $DIR/reference_casting.rs:31:9 | LL | *{ std::ptr::from_ref(num) }.cast_mut() += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:37:9 + --> $DIR/reference_casting.rs:33:9 | LL | *(std::ptr::from_ref({ num }) as *mut i32) += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:40:9 + --> $DIR/reference_casting.rs:36:9 | LL | let value = num as *const i32 as *mut i32; | ----------------------------- casting happend here LL | *value = 1; - | ^^^^^^ + | ^^^^^^^^^^ -error: aborting due to 11 previous errors +error: aborting due to 9 previous errors From 20a6b571063d33c4b1a786c558f74edbda3012ea Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 14 Jul 2023 22:25:47 +0200 Subject: [PATCH 4/5] Improve diagnostics of the invalid_reference_casting lint --- compiler/rustc_lint/messages.ftl | 5 +- compiler/rustc_lint/src/lints.rs | 15 ++- compiler/rustc_lint/src/reference_casting.rs | 20 +++- tests/ui/lint/reference_casting.rs | 93 +++++++++++-------- tests/ui/lint/reference_casting.stderr | 96 +++++++++++++------- 5 files changed, 152 insertions(+), 77 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 95f0b6519620c..f482e3d7c1280 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -318,7 +318,10 @@ lint_invalid_nan_comparisons_eq_ne = incorrect NaN comparison, NaN cannot be dir lint_invalid_nan_comparisons_lt_le_gt_ge = incorrect NaN comparison, NaN is not orderable -lint_invalid_reference_casting = casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` +lint_invalid_reference_casting_assign_to_ref = assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + .label = casting happend here + +lint_invalid_reference_casting_borrow_as_mut = casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` .label = casting happend here lint_lintpass_by_hand = implementing `LintPass` by hand diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 6d774070ec433..a6a48bf4ffa74 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -745,10 +745,17 @@ pub enum InvalidFromUtf8Diag { // reference_casting.rs #[derive(LintDiagnostic)] -#[diag(lint_invalid_reference_casting)] -pub struct InvalidReferenceCastingDiag { - #[label] - pub orig_cast: Option, +pub enum InvalidReferenceCastingDiag { + #[diag(lint_invalid_reference_casting_borrow_as_mut)] + BorrowAsMut { + #[label] + orig_cast: Option, + }, + #[diag(lint_invalid_reference_casting_assign_to_ref)] + AssignToRef { + #[label] + orig_cast: Option, + }, } // hidden_unicode_codepoints.rs diff --git a/compiler/rustc_lint/src/reference_casting.rs b/compiler/rustc_lint/src/reference_casting.rs index 428bf750bbd1b..ed3d47210494f 100644 --- a/compiler/rustc_lint/src/reference_casting.rs +++ b/compiler/rustc_lint/src/reference_casting.rs @@ -73,13 +73,25 @@ impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting { return; }; - if is_cast_from_const_to_mut(cx, e) { - cx.emit_spanned_lint(INVALID_REFERENCE_CASTING, expr.span, InvalidReferenceCastingDiag { orig_cast: None }); + let orig_cast = if is_cast_from_const_to_mut(cx, e) { + None } else if let ExprKind::Path(QPath::Resolved(_, path)) = e.kind && let Res::Local(hir_id) = &path.res && let Some(orig_cast) = self.casted.get(hir_id) { - cx.emit_spanned_lint(INVALID_REFERENCE_CASTING, expr.span, InvalidReferenceCastingDiag { orig_cast: Some(*orig_cast) }); - } + Some(*orig_cast) + } else { + return; + }; + + cx.emit_spanned_lint( + INVALID_REFERENCE_CASTING, + expr.span, + if matches!(expr.kind, ExprKind::AddrOf(..)) { + InvalidReferenceCastingDiag::BorrowAsMut { orig_cast } + } else { + InvalidReferenceCastingDiag::AssignToRef { orig_cast } + }, + ); } } diff --git a/tests/ui/lint/reference_casting.rs b/tests/ui/lint/reference_casting.rs index aa946038c99da..6e70626ef997f 100644 --- a/tests/ui/lint/reference_casting.rs +++ b/tests/ui/lint/reference_casting.rs @@ -9,42 +9,63 @@ extern "C" { fn int_ffi(c: *mut i32); } -fn main() { +unsafe fn ref_to_mut() { + let num = &3i32; + + let _num = &mut *(num as *const i32 as *mut i32); + //~^ ERROR casting `&T` to `&mut T` is undefined behavior + let _num = &mut *(num as *const i32).cast_mut(); + //~^ ERROR casting `&T` to `&mut T` is undefined behavior + let _num = &mut *std::ptr::from_ref(num).cast_mut(); + //~^ ERROR casting `&T` to `&mut T` is undefined behavior + let _num = &mut *std::ptr::from_ref({ num }).cast_mut(); + //~^ ERROR casting `&T` to `&mut T` is undefined behavior + let _num = &mut *{ std::ptr::from_ref(num) }.cast_mut(); + //~^ ERROR casting `&T` to `&mut T` is undefined behavior + let _num = &mut *(std::ptr::from_ref({ num }) as *mut i32); + //~^ ERROR casting `&T` to `&mut T` is undefined behavior + + let deferred = num as *const i32 as *mut i32; + let _num = &mut *deferred; + //~^ ERROR casting `&T` to `&mut T` is undefined behavior +} + +unsafe fn assign_to_ref() { let s = String::from("Hello"); let a = &s; - unsafe { - let num = &3i32; - let mut_num = &mut 3i32; - - *(a as *const _ as *mut _) = String::from("Replaced"); - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - *(a as *const _ as *mut String) += " world"; - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - let _num = &mut *(num as *const i32 as *mut i32); - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - let _num = &mut *(num as *const i32).cast_mut(); - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - *std::ptr::from_ref(num).cast_mut() += 1; - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - *std::ptr::from_ref({ num }).cast_mut() += 1; - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - *{ std::ptr::from_ref(num) }.cast_mut() += 1; - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - *(std::ptr::from_ref({ num }) as *mut i32) += 1; - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - let value = num as *const i32 as *mut i32; - *value = 1; - //~^ ERROR casting `&T` to `&mut T` is undefined behavior - - // Shouldn't be warned against - *(num as *const i32 as *mut i32); - println!("{}", *(num as *const _ as *const i16)); - println!("{}", *(mut_num as *mut _ as *mut i16)); - ffi(a.as_ptr() as *mut _); - int_ffi(num as *const _ as *mut _); - int_ffi(&3 as *const _ as *mut _); - let mut value = 3; - let value: *const i32 = &mut value; - *(value as *const i16 as *mut i16) = 42; - } + let num = &3i32; + + *(a as *const _ as *mut _) = String::from("Replaced"); + //~^ ERROR assigning to `&T` is undefined behavior + *(a as *const _ as *mut String) += " world"; + //~^ ERROR assigning to `&T` is undefined behavior + *std::ptr::from_ref(num).cast_mut() += 1; + //~^ ERROR assigning to `&T` is undefined behavior + *std::ptr::from_ref({ num }).cast_mut() += 1; + //~^ ERROR assigning to `&T` is undefined behavior + *{ std::ptr::from_ref(num) }.cast_mut() += 1; + //~^ ERROR assigning to `&T` is undefined behavior + *(std::ptr::from_ref({ num }) as *mut i32) += 1; + //~^ ERROR assigning to `&T` is undefined behavior + let value = num as *const i32 as *mut i32; + *value = 1; + //~^ ERROR assigning to `&T` is undefined behavior } + +unsafe fn no_warn() { + let num = &3i32; + let mut_num = &mut 3i32; + let a = &String::from("ffi"); + + *(num as *const i32 as *mut i32); + println!("{}", *(num as *const _ as *const i16)); + println!("{}", *(mut_num as *mut _ as *mut i16)); + ffi(a.as_ptr() as *mut _); + int_ffi(num as *const _ as *mut _); + int_ffi(&3 as *const _ as *mut _); + let mut value = 3; + let value: *const i32 = &mut value; + *(value as *const i16 as *mut i16) = 42; +} + +fn main() {} diff --git a/tests/ui/lint/reference_casting.stderr b/tests/ui/lint/reference_casting.stderr index 22bc66264d053..02b23600557d5 100644 --- a/tests/ui/lint/reference_casting.stderr +++ b/tests/ui/lint/reference_casting.stderr @@ -1,60 +1,92 @@ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:19:9 + --> $DIR/reference_casting.rs:15:16 | -LL | *(a as *const _ as *mut _) = String::from("Replaced"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _num = &mut *(num as *const i32 as *mut i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[deny(invalid_reference_casting)]` on by default error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:21:9 + --> $DIR/reference_casting.rs:17:16 | -LL | *(a as *const _ as *mut String) += " world"; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _num = &mut *(num as *const i32).cast_mut(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:23:20 + --> $DIR/reference_casting.rs:19:16 | -LL | let _num = &mut *(num as *const i32 as *mut i32); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _num = &mut *std::ptr::from_ref(num).cast_mut(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:25:20 + --> $DIR/reference_casting.rs:21:16 | -LL | let _num = &mut *(num as *const i32).cast_mut(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _num = &mut *std::ptr::from_ref({ num }).cast_mut(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:27:9 + --> $DIR/reference_casting.rs:23:16 | -LL | *std::ptr::from_ref(num).cast_mut() += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _num = &mut *{ std::ptr::from_ref(num) }.cast_mut(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:29:9 + --> $DIR/reference_casting.rs:25:16 | -LL | *std::ptr::from_ref({ num }).cast_mut() += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _num = &mut *(std::ptr::from_ref({ num }) as *mut i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:31:9 + --> $DIR/reference_casting.rs:29:16 | -LL | *{ std::ptr::from_ref(num) }.cast_mut() += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let deferred = num as *const i32 as *mut i32; + | ----------------------------- casting happend here +LL | let _num = &mut *deferred; + | ^^^^^^^^^^^^^^ -error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:33:9 +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + --> $DIR/reference_casting.rs:38:5 | -LL | *(std::ptr::from_ref({ num }) as *mut i32) += 1; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | *(a as *const _ as *mut _) = String::from("Replaced"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` - --> $DIR/reference_casting.rs:36:9 +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + --> $DIR/reference_casting.rs:40:5 + | +LL | *(a as *const _ as *mut String) += " world"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + --> $DIR/reference_casting.rs:42:5 + | +LL | *std::ptr::from_ref(num).cast_mut() += 1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + --> $DIR/reference_casting.rs:44:5 + | +LL | *std::ptr::from_ref({ num }).cast_mut() += 1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + --> $DIR/reference_casting.rs:46:5 + | +LL | *{ std::ptr::from_ref(num) }.cast_mut() += 1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + --> $DIR/reference_casting.rs:48:5 + | +LL | *(std::ptr::from_ref({ num }) as *mut i32) += 1; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` + --> $DIR/reference_casting.rs:51:5 | -LL | let value = num as *const i32 as *mut i32; - | ----------------------------- casting happend here -LL | *value = 1; - | ^^^^^^^^^^ +LL | let value = num as *const i32 as *mut i32; + | ----------------------------- casting happend here +LL | *value = 1; + | ^^^^^^^^^^ -error: aborting due to 9 previous errors +error: aborting due to 14 previous errors From 507d497cfa014514c5dd904f9d273810e888f757 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 8 Jun 2023 21:36:20 +0200 Subject: [PATCH 5/5] Adjust some tests for invalid_reference_casting improvements --- library/core/src/cell.rs | 3 ++- src/tools/miri/tests/fail/both_borrows/illegal_write1.rs | 2 ++ src/tools/miri/tests/fail/stacked_borrows/illegal_write3.rs | 2 ++ tests/ui/const-generics/issues/issue-100313.rs | 2 +- tests/ui/const-generics/issues/issue-100313.stderr | 4 ++-- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 909b32547e74c..bf4c682d33e5a 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -1893,7 +1893,8 @@ impl fmt::Display for RefMut<'_, T> { /// on an _exclusive_ `UnsafeCell`. Even though `T` and `UnsafeCell` have the /// same memory layout, the following is not allowed and undefined behavior: /// -/// ```rust,no_run +#[cfg_attr(bootstrap, doc = "```rust,no_run")] +#[cfg_attr(not(bootstrap), doc = "```rust,compile_fail")] /// # use std::cell::UnsafeCell; /// unsafe fn not_allowed(ptr: &UnsafeCell) -> &mut T { /// let t = ptr as *const UnsafeCell as *mut T; diff --git a/src/tools/miri/tests/fail/both_borrows/illegal_write1.rs b/src/tools/miri/tests/fail/both_borrows/illegal_write1.rs index d3991949cc39e..92f273f67bb22 100644 --- a/src/tools/miri/tests/fail/both_borrows/illegal_write1.rs +++ b/src/tools/miri/tests/fail/both_borrows/illegal_write1.rs @@ -1,6 +1,8 @@ //@revisions: stack tree //@[tree]compile-flags: -Zmiri-tree-borrows +#![allow(invalid_reference_casting)] + fn main() { let target = Box::new(42); // has an implicit raw let xref = &*target; diff --git a/src/tools/miri/tests/fail/stacked_borrows/illegal_write3.rs b/src/tools/miri/tests/fail/stacked_borrows/illegal_write3.rs index 6f55b63cb5c64..f79f7b561b834 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/illegal_write3.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/illegal_write3.rs @@ -1,3 +1,5 @@ +#![allow(invalid_reference_casting)] + fn main() { let target = 42; // Make sure raw ptr with raw tag cannot mutate frozen location without breaking the shared ref. diff --git a/tests/ui/const-generics/issues/issue-100313.rs b/tests/ui/const-generics/issues/issue-100313.rs index 9a9d4721c8494..9af9b5ca45851 100644 --- a/tests/ui/const-generics/issues/issue-100313.rs +++ b/tests/ui/const-generics/issues/issue-100313.rs @@ -9,7 +9,7 @@ impl T { unsafe { *(B as *const bool as *mut bool) = false; //~^ ERROR evaluation of constant value failed [E0080] - //~| ERROR casting `&T` to `&mut T` is undefined behavior + //~| ERROR assigning to `&T` is undefined behavior } } } diff --git a/tests/ui/const-generics/issues/issue-100313.stderr b/tests/ui/const-generics/issues/issue-100313.stderr index 17e4850bd12a4..42ad4d61c8e0c 100644 --- a/tests/ui/const-generics/issues/issue-100313.stderr +++ b/tests/ui/const-generics/issues/issue-100313.stderr @@ -1,8 +1,8 @@ -error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` +error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell` --> $DIR/issue-100313.rs:10:13 | LL | *(B as *const bool as *mut bool) = false; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[deny(invalid_reference_casting)]` on by default