Skip to content

Improve invalid_reference_casting lint #114784

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 16, 2023
Merged

Improve invalid_reference_casting lint #114784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6933848
Add diagnostic items for `<*const _>::cast` and `<*mut _>::cast_const`
Urgau Aug 14, 2023
91b05f8
Improve `invalid_reference_casting` lint
Urgau Aug 13, 2023
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
49 changes: 31 additions & 18 deletions compiler/rustc_lint/src/reference_casting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>)

fn from_casts<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
// <expr> as *mut ...
let e = if let ExprKind::Cast(e, t) = e.kind
let mut 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()
Expand All @@ -112,23 +112,36 @@ fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>)
return None;
};

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 None;
};

Some(e)
let mut had_at_least_one_cast = false;
loop {
e = e.peel_blocks();
// <expr> as *mut/const ... or <expr> as <uint>
e = if let ExprKind::Cast(expr, t) = e.kind
&& matches!(cx.typeck_results().node_type(t.hir_id).kind(), ty::RawPtr(_) | ty::Uint(_)) {
had_at_least_one_cast = true;
expr
// <expr>.cast(), <expr>.cast_mut() or <expr>.cast_const()
} else if let ExprKind::MethodCall(_, expr, [], _) = e.kind
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
&& matches!(
cx.tcx.get_diagnostic_name(def_id),
Some(sym::ptr_cast | sym::const_ptr_cast | sym::ptr_cast_mut | sym::ptr_cast_const)
)
{
had_at_least_one_cast = true;
expr
// 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) {
return Some(arg);
} else if had_at_least_one_cast {
return Some(e);
} else {
return None;
};
}
}

fn from_transmute<'tcx>(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ symbols! {
const_panic_fmt,
const_param_ty,
const_precise_live_drops,
const_ptr_cast,
const_raw_ptr_deref,
const_raw_ptr_to_usize_cast,
const_refs_to_cell,
Expand Down Expand Up @@ -1159,6 +1160,7 @@ symbols! {
profiler_runtime,
ptr,
ptr_cast,
ptr_cast_const,
ptr_cast_mut,
ptr_const_is_null,
ptr_from_mut,
Expand Down
1 change: 1 addition & 0 deletions library/core/src/ptr/const_ptr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ impl<T: ?Sized> *const T {
/// Casts to a pointer of another type.
#[stable(feature = "ptr_cast", since = "1.38.0")]
#[rustc_const_stable(feature = "const_ptr_cast", since = "1.38.0")]
#[rustc_diagnostic_item = "const_ptr_cast"]
#[inline(always)]
pub const fn cast<U>(self) -> *const U {
self as _
Expand Down
1 change: 1 addition & 0 deletions library/core/src/ptr/mut_ptr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ impl<T: ?Sized> *mut T {
/// [`cast_mut`]: #method.cast_mut
#[stable(feature = "ptr_const_cast", since = "1.65.0")]
#[rustc_const_stable(feature = "ptr_const_cast", since = "1.65.0")]
#[rustc_diagnostic_item = "ptr_cast_const"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is high confusion potential between ptr_const_cast and ptr_cast_const. Maybe this one could be renamed to ptr_mut_cast_const?

Copy link
Member Author

@Urgau Urgau Aug 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to rename the other one from ptr_const_cast to const_ptr_cast, so that every diagnostic item finishes with the name of it's method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that rename improved the situation but the two names are still only distinguished by the order of the components. I think the mut should still be added to this one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that the const part is now on the opposite side of the word: in front for const_ptr_cast and behind for ptr_cast_const, there is still the possibility that someone might mistake one for the other, but I think it's sufficiently diffused.

PS: I'm not against changing it, but I think adding mut would make the name out of place, and could definitely confuse a future me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhh yeah the other names are also not prefixed with mut_, even though IMO they should be. I will approve this.

#[inline(always)]
pub const fn cast_const(self) -> *const T {
self as _
Expand Down
29 changes: 29 additions & 0 deletions tests/ui/lint/reference_casting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ extern "C" {
fn int_ffi(c: *mut i32);
}

fn static_u8() -> &'static u8 {
&8
}

unsafe fn ref_to_mut() {
let num = &3i32;

Expand All @@ -24,12 +28,28 @@ unsafe fn ref_to_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 _num = &mut *(num as *const i32).cast::<i32>().cast_mut();
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
let _num = &mut *(num as *const i32).cast::<i32>().cast_mut().cast_const().cast_mut();
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
let _num = &mut *(std::ptr::from_ref(static_u8()) as *mut i32);
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
let _num = &mut *std::mem::transmute::<_, *mut i32>(num);
//~^ 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
let deferred = (std::ptr::from_ref(num) as *const i32 as *const i32).cast_mut() as *mut i32;
let _num = &mut *deferred;
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
let _num = &mut *(num as *const _ as usize as *mut i32);
//~^ ERROR casting `&T` to `&mut T` is undefined behavior

unsafe fn generic_ref_cast_mut<T>(this: &T) -> &mut T {
&mut *((this as *const _) as *mut _)
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
}
}

unsafe fn assign_to_ref() {
Expand All @@ -55,6 +75,15 @@ unsafe fn assign_to_ref() {
let value = num as *const i32 as *mut i32;
*value = 1;
//~^ ERROR assigning to `&T` is undefined behavior
*(num as *const i32).cast::<i32>().cast_mut() = 2;
//~^ ERROR assigning to `&T` is undefined behavior
*(num as *const _ as usize as *mut i32) = 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;
//~^ ERROR assigning to `&T` is undefined behavior
}
}

unsafe fn no_warn() {
Expand Down
90 changes: 73 additions & 17 deletions tests/ui/lint/reference_casting.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,104 +1,160 @@
error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell`
--> $DIR/reference_casting.rs:15:16
--> $DIR/reference_casting.rs:19: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:17:16
--> $DIR/reference_casting.rs:21: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:19:16
--> $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:21:16
--> $DIR/reference_casting.rs:25: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
--> $DIR/reference_casting.rs:27: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
--> $DIR/reference_casting.rs:29: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:27:16
--> $DIR/reference_casting.rs:31:16
|
LL | let _num = &mut *(num as *const i32).cast::<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:33:16
|
LL | let _num = &mut *(num as *const i32).cast::<i32>().cast_mut().cast_const().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:35:16
|
LL | let _num = &mut *(std::ptr::from_ref(static_u8()) 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:37:16
|
LL | let _num = &mut *std::mem::transmute::<_, *mut i32>(num);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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:16
--> $DIR/reference_casting.rs:41:16
|
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:44:16
|
LL | let deferred = (std::ptr::from_ref(num) as *const i32 as *const i32).cast_mut() 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:46:16
|
LL | let _num = &mut *(num as *const _ as usize 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:50:9
|
LL | &mut *((this as *const _) as *mut _)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:40:5
--> $DIR/reference_casting.rs:60: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:42:5
--> $DIR/reference_casting.rs:62:5
|
LL | *(a as *const _ as *mut String) += " world";
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:44:5
--> $DIR/reference_casting.rs:64: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
--> $DIR/reference_casting.rs:66: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
--> $DIR/reference_casting.rs:68: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:50:5
--> $DIR/reference_casting.rs:70: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:52:5
--> $DIR/reference_casting.rs:72:5
|
LL | *std::mem::transmute::<_, *mut i32>(num) += 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

error: aborting due to 16 previous errors
error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:78: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
|
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
|
LL | *(this as *const _ as *mut _) = a;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 25 previous errors