Auto merge of #112431 - Urgau:cast_ref_to_mut_improvments, r=Nilstrieb

Improve `invalid_reference_casting` lint This PR is a follow-up to #111567 and #113422. This PR does multiple things: - First it adds support for deferred de-reference, the goal is to support code like this, where the casting and de-reference are not done on the same expression ```rust let myself = self as *const Self as *mut Self; *myself = Self::Ready(value); ``` - Second it does not lint anymore on SB/TB UB code by only checking assignments (`=`, ` =`, ...) and creation of mutable references `&mut *` - Thirdly it greatly improves the diagnostics in particular for cast from `&mut` to `&mut` or assignments - ~~And lastly it renames the lint from `cast_ref_to_mut` to `invalid_reference_casting` which is more consistent with the ["rules"](rust-lang/rust-clippy#2845) and also more consistent with what the lint checks~~ *#113422 This PR is best reviewed commit by commit. r? compiler
rust-lang · Aug 2, 2023 · d170833 · d170833
2 parents 64ad036 507d497
commit d170833
Show file tree

Hide file tree

Showing 11 changed files with 240 additions and 111 deletions.
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -318,7 318,11 @@ 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
  .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
@@ -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
@@ -745,8 745,18 @@ pub enum InvalidFromUtf8Diag {
 
 // reference_casting.rs
 #[derive(LintDiagnostic)]
-#[diag(lint_invalid_reference_casting)]
-pub struct InvalidReferenceCastingDiag;
 pub enum InvalidReferenceCastingDiag {
  #[diag(lint_invalid_reference_casting_borrow_as_mut)]
  BorrowAsMut {
  #[label]
  orig_cast: Option<Span>,
  },
  #[diag(lint_invalid_reference_casting_assign_to_ref)]
  AssignToRef {
  #[label]
  orig_cast: Option<Span>,
  },
 }
 
 // hidden_unicode_codepoints.rs
 #[derive(LintDiagnostic)]

diff --git 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};
 
@@ -12,7 13,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,46 30,103 @@ 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"
 }
 
-declare_lint_pass!(InvalidReferenceCasting => [INVALID_REFERENCE_CASTING]);
 #[derive(Default)]
 pub struct InvalidReferenceCasting {
  casted: FxHashMap<HirId, Span>,
 }
 
 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 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::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) {
  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>) {
  // &mut <expr>
  let inner = if let ExprKind::AddrOf(_, Mutability::Mut, expr) = expr.kind {
  expr
  // <expr> = ...
  } else if let ExprKind::Assign(expr, _, _) = expr.kind {
  expr
  // <expr> = ...
  } else if let ExprKind::AssignOp(_, expr, _) = expr.kind {
  expr
  } 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
  let ExprKind::Unary(UnOp::Deref, e) = &inner.kind else {
  return;
  };
 
  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) {
  Some(*orig_cast)
  } 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);
- }
  cx.emit_spanned_lint(
  INVALID_REFERENCE_CASTING,
  expr.span,
  if matches!(expr.kind, ExprKind::AddrOf(..)) {
  InvalidReferenceCastingDiag::BorrowAsMut { orig_cast }
  } else {
  InvalidReferenceCastingDiag::AssignToRef { orig_cast }
  },
  );
  }
 }
 
 fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
  let e = e.peel_blocks();
 
  // <expr> 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
  // <expr>.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();
 
  // <expr> 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(<expr>)
  } 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(_, _, Mutability::Not))
 }
diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs
@@ -1893,7 1893,8 @@ impl<T: ?Sized fmt::Display> fmt::Display for RefMut<'_, T> {
 /// on an _exclusive_ `UnsafeCell<T>`. Even though `T` and `UnsafeCell<T>` 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<T>(ptr: &UnsafeCell<T>) -> &mut T {
 /// let t = ptr as *const UnsafeCell<T> 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
@@ -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
@@ -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
@@ -9,6 9,7 @@ impl <const B: &'static bool> T<B> {
  unsafe {
  *(B as *const bool as *mut bool) = false;
  //~^ ERROR evaluation of constant value failed [E0080]
  //~| 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
@@ -1,3 1,11 @@
 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
 
 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
@@ -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
@@ -10,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)).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";
- //~^ 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
- 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;
- //~^ 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
-
- // Shouldn't be warned against
- 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() {}