Skip to content

Commit

Permalink
Rollup merge of rust-lang#115100 - Urgau:invalid_ref_casting-ptr-writ…
Browse files Browse the repository at this point in the history
…es, r=est31

Add support for `ptr::write`s for the `invalid_reference_casting` lint

This PR adds support for `ptr::write` and others for the `invalid_reference_casting` lint.

Detecting instances where instead of using the deref (`*`) operator to assign someone uses `ptr::write`, `ptr::write_unaligned` or `ptr::write_volatile`.

```rust
let data_len = 5u64;

std::ptr::write(
    std::mem::transmute::<*const u64, *mut u64>(&data_len),
    new_data_len,
);
```

r? `@est31`
  • Loading branch information
matthiaskrgr authored Aug 23, 2023
2 parents 92af886 + 7ee77b5 commit a9c69ee
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 22 deletions.
64 changes: 47 additions & 17 deletions compiler/rustc_lint/src/reference_casting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting {
}

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 {
let Some((is_assignment, e)) = is_operation_we_care_about(cx, expr) else {
return;
};

Expand All @@ -86,15 +73,58 @@ impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting {
cx.emit_spanned_lint(
INVALID_REFERENCE_CASTING,
expr.span,
if matches!(expr.kind, ExprKind::AddrOf(..)) {
InvalidReferenceCastingDiag::BorrowAsMut { orig_cast }
} else {
if is_assignment {
InvalidReferenceCastingDiag::AssignToRef { orig_cast }
} else {
InvalidReferenceCastingDiag::BorrowAsMut { orig_cast }
},
);
}
}

fn is_operation_we_care_about<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'tcx>,
) -> Option<(bool, &'tcx Expr<'tcx>)> {
fn deref_assign_or_addr_of<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<(bool, &'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 None;
};

if let ExprKind::Unary(UnOp::Deref, e) = &inner.kind {
Some((!matches!(expr.kind, ExprKind::AddrOf(..)), e))
} else {
None
}
}

fn ptr_write<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'tcx>,
) -> Option<(bool, &'tcx Expr<'tcx>)> {
if let ExprKind::Call(path, [arg_ptr, _arg_val]) = e.kind
&& let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
&& matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::ptr_write | sym::ptr_write_volatile | sym::ptr_write_unaligned))
{
Some((true, arg_ptr))
} else {
None
}
}

deref_assign_or_addr_of(e).or_else(|| ptr_write(cx, e))
}

fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
let e = e.peel_blocks();

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1179,6 +1179,9 @@ symbols! {
ptr_offset_from,
ptr_offset_from_unsigned,
ptr_unique,
ptr_write,
ptr_write_unaligned,
ptr_write_volatile,
pub_macro_rules,
pub_restricted,
public,
Expand Down
3 changes: 3 additions & 0 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,7 @@ pub const unsafe fn read_unaligned<T>(src: *const T) -> T {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[rustc_diagnostic_item = "ptr_write"]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write<T>(dst: *mut T, src: T) {
// Semantically, it would be fine for this to be implemented as a
Expand Down Expand Up @@ -1459,6 +1460,7 @@ pub const unsafe fn write<T>(dst: *mut T, src: T) {
#[inline]
#[stable(feature = "ptr_unaligned", since = "1.17.0")]
#[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]
#[rustc_diagnostic_item = "ptr_write_unaligned"]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn write_unaligned<T>(dst: *mut T, src: T) {
// SAFETY: the caller must guarantee that `dst` is valid for writes.
Expand Down Expand Up @@ -1607,6 +1609,7 @@ pub unsafe fn read_volatile<T>(src: *const T) -> T {
/// ```
#[inline]
#[stable(feature = "volatile", since = "1.9.0")]
#[rustc_diagnostic_item = "ptr_write_volatile"]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
// SAFETY: the caller must uphold the safety contract for `volatile_store`.
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/lint/reference_casting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ unsafe fn assign_to_ref() {
//~^ ERROR assigning to `&T` is undefined behavior
*std::mem::transmute::<_, *mut i32>(num) += 1;
//~^ ERROR assigning to `&T` is undefined behavior
std::ptr::write(
//~^ ERROR assigning to `&T` is undefined behavior
std::mem::transmute::<*const i32, *mut i32>(num),
-1i32,
);

let value = num as *const i32 as *mut i32;
*value = 1;
Expand All @@ -79,6 +84,12 @@ unsafe fn assign_to_ref() {
//~^ ERROR assigning to `&T` is undefined behavior
*(num as *const _ as usize as *mut i32) = 2;
//~^ ERROR assigning to `&T` is undefined behavior
std::ptr::write(value, 2);
//~^ ERROR assigning to `&T` is undefined behavior
std::ptr::write_unaligned(value, 2);
//~^ ERROR assigning to `&T` is undefined behavior
std::ptr::write_volatile(value, 2);
//~^ ERROR assigning to `&T` is undefined behavior

unsafe fn generic_assign_to_ref<T>(this: &T, a: T) {
*(this as *const _ as *mut _) = a;
Expand Down
47 changes: 42 additions & 5 deletions tests/ui/lint/reference_casting.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -131,30 +131,67 @@ LL | *std::mem::transmute::<_, *mut i32>(num) += 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:76:5
--> $DIR/reference_casting.rs:74:5
|
LL | / std::ptr::write(
LL | |
LL | | std::mem::transmute::<*const i32, *mut i32>(num),
LL | | -1i32,
LL | | );
| |_____^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:81:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
LL | *value = 1;
| ^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:78:5
--> $DIR/reference_casting.rs:83:5
|
LL | *(num as *const i32).cast::<i32>().cast_mut() = 2;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:80:5
--> $DIR/reference_casting.rs:85:5
|
LL | *(num as *const _ as usize as *mut i32) = 2;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:84:9
--> $DIR/reference_casting.rs:87:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
...
LL | std::ptr::write(value, 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:89:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
...
LL | std::ptr::write_unaligned(value, 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:91:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
...
LL | std::ptr::write_volatile(value, 2);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:95:9
|
LL | *(this as *const _ as *mut _) = a;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 25 previous errors
error: aborting due to 29 previous errors

0 comments on commit a9c69ee

Please sign in to comment.