Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for ptr::writes for the invalid_reference_casting lint #115100

Merged
merged 1 commit into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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