mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Browse Source 
Improve `invalid_reference_casting` lint

This PR is a follow-up to https://github.com/rust-lang/rust/pull/111567 and https://github.com/rust-lang/rust/pull/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"](https://github.com/rust-lang/rust-clippy/issues/2845) and also more consistent with what the lint checks~~ *https://github.com/rust-lang/rust/pull/113422*

This PR is best reviewed commit by commit.

r? compiler
This commit is contained in:
bors 2023-08-02 11:25:13 +00:00
commit d170833431
11 changed files with 239 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
compiler/rustc_lint/messages.ftl
View File

@ -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

2
compiler/rustc_lint/src/lib.rs
View File

@ -218,7 +218,7 @@ fn lint_mod(tcx: TyCtxt<'_>, module_def_id: LocalDefId) {
BoxPointers: BoxPointers,
PathStatements: PathStatements,
LetUnderscore: LetUnderscore,
InvalidReferenceCasting: InvalidReferenceCasting,
InvalidReferenceCasting: InvalidReferenceCasting::default(),
// Depends on referenced function signatures in expressions
UnusedResults: UnusedResults,
NonUpperCaseGlobals: NonUpperCaseGlobals,

14
compiler/rustc_lint/src/lints.rs
View File

@ -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)]

99
compiler/rustc_lint/src/reference_casting.rs
View File

@ -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 @@
/// ### 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 @@
/// `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 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;
};
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 TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.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;
return false;
};
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);
}
}
matches!(cx.typeck_results().node_type(e.hir_id).kind(), ty::Ref(_, _, Mutability::Not))
}

3
library/core/src/cell.rs
View File

@ -1893,7 +1893,8 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// 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;

2
src/tools/miri/tests/fail/both_borrows/illegal_write1.rs
View File

@ -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;

2
src/tools/miri/tests/fail/stacked_borrows/illegal_write3.rs
View File

@ -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.

1
tests/ui/const-generics/issues/issue-100313.rs
View File

@ -9,6 +9,7 @@ const fn set_false(&self) {
unsafe {
*(B as *const bool as *mut bool) = false;
//~^ ERROR evaluation of constant value failed [E0080]
//~| ERROR assigning to `&T` is undefined behavior
}
}
}

12
tests/ui/const-generics/issues/issue-100313.stderr
View File

@ -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`.

60
tests/ui/lint/reference_casting.rs
View File

@ -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,35 +9,55 @@
fn int_ffi(c: *mut i32);
}
fn main() {
let s = String::from("Hello");
let a = &s;
unsafe {
unsafe fn ref_to_mut() {
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();
let _num = &mut *std::ptr::from_ref(num).cast_mut();
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
*std::ptr::from_ref(num).cast_mut() += 1;
let _num = &mut *std::ptr::from_ref({ num }).cast_mut();
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
*std::ptr::from_ref({ num }).cast_mut() += 1;
let _num = &mut *{ std::ptr::from_ref(num) }.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 }) as *mut i32) += 1;
let _num = &mut *(std::ptr::from_ref({ num }) as *mut i32);
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
// Shouldn't be warned against
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;
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 _);
@ -47,5 +66,6 @@ fn main() {
let mut value = 3;
let value: *const i32 = &mut value;
*(value as *const i16 as *mut i16) = 42;
}
}
fn main() {}

110
tests/ui/lint/reference_casting.stderr
View File

@ -1,68 +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:20: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)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
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
|
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
|
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:15:16
|
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:28:25
--> $DIR/reference_casting.rs:17:16
|
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:19:16
|
LL | let _num = *{ 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:32:9
--> $DIR/reference_casting.rs:21:16
|
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:23:16
|
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:16
|
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:29:16
|
LL | let deferred = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
LL | let _num = &mut *deferred;
| ^^^^^^^^^^^^^^
error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:38:5
|
LL | *(a as *const _ as *mut _) = String::from("Replaced");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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: 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
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: 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:46:5
|
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
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: aborting due to 10 previous errors
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;
| ^^^^^^^^^^
error: aborting due to 14 previous errors