Skip to content

Lint on invalid usage of UnsafeCell::raw_get in reference casting #115166

New issue

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

#

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Lint on invalid usage of UnsafeCell::raw_get in reference casting #115166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 39 additions & 5 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 @@ -128,7 +128,11 @@ fn is_operation_we_care_about<'tcx>(
fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
let e = e.peel_blocks();

fn from_casts<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
fn from_casts<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'tcx>,
need_check_freeze: &mut bool,
) -> Option<&'tcx Expr<'tcx>> {
// <expr> as *mut ...
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() {
Expand All @@ -138,6 +142,14 @@ fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>)
&& 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
// UnsafeCell::raw_get(<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::unsafe_cell_raw_get, def_id)
{
*need_check_freeze = true;
arg
} else {
return None;
};
Expand All @@ -160,11 +172,18 @@ fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>)
{
had_at_least_one_cast = true;
expr
// ptr::from_ref(<expr>)
// ptr::from_ref(<expr>) or UnsafeCell::raw_get(<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) {
&& matches!(
cx.tcx.get_diagnostic_name(def_id),
Some(sym::ptr_from_ref | sym::unsafe_cell_raw_get)
)
{
if cx.tcx.is_diagnostic_item(sym::unsafe_cell_raw_get, def_id) {
*need_check_freeze = true;
}
return Some(arg);
} else if had_at_least_one_cast {
return Some(e);
Expand All @@ -190,10 +209,25 @@ fn is_cast_from_const_to_mut<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>)
}
}

let Some(e) = from_casts(cx, e).or_else(|| from_transmute(cx, e)) else {
let mut need_check_freeze = false;
let Some(e) = from_casts(cx, e, &mut need_check_freeze).or_else(|| from_transmute(cx, e))
else {
return false;
};

let e = e.peel_blocks();
matches!(cx.typeck_results().node_type(e.hir_id).kind(), ty::Ref(_, _, Mutability::Not))
let node_type = cx.typeck_results().node_type(e.hir_id);
if let ty::Ref(_, inner_ty, Mutability::Not) = node_type.kind() {
// If an UnsafeCell method is involved we need to additionaly check the
// inner type for the presence of the Freeze trait (ie does NOT contain
// an UnsafeCell), since in that case we would incorrectly lint on valid casts.
//
// We also consider non concrete skeleton types (ie generics)
// to be an issue since there is no way to make it safe for abitrary types.
!need_check_freeze
|| inner_ty.is_freeze(cx.tcx, cx.param_env)
|| !inner_ty.has_concrete_skeleton()
} else {
false
}
}
1 change: 1 addition & 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 @@ -1630,6 +1630,7 @@ symbols! {
unsafe_block_in_unsafe_fn,
unsafe_cell,
unsafe_cell_from_mut,
unsafe_cell_raw_get,
unsafe_no_drop_flag,
unsafe_pin_internals,
unsize,
Expand Down
1 change: 1 addition & 0 deletions library/core/src/cell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2131,6 +2131,7 @@ impl<T: ?Sized> UnsafeCell<T> {
#[inline(always)]
#[stable(feature = "unsafe_cell_raw_get", since = "1.56.0")]
#[rustc_const_stable(feature = "unsafe_cell_raw_get", since = "1.56.0")]
#[rustc_diagnostic_item = "unsafe_cell_raw_get"]
pub const fn raw_get(this: *const Self) -> *mut T {
// We can just cast the pointer from `UnsafeCell<T>` to `T` because of
// #[repr(transparent)]. This exploits std's special status, there is
Expand Down
28 changes: 28 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 @@ -36,6 +36,10 @@ unsafe fn ref_to_mut() {
//~^ 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 _num = &mut *std::cell::UnsafeCell::raw_get(
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
num as *const i32 as *const std::cell::UnsafeCell<i32>
);

let deferred = num as *const i32 as *mut i32;
let _num = &mut *deferred;
Expand All @@ -50,6 +54,16 @@ unsafe fn ref_to_mut() {
&mut *((this as *const _) as *mut _)
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
}

fn as_mut<T>(x: &T) -> &mut T {
unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
}

fn as_mut_i32(x: &i32) -> &mut i32 {
unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
//~^ ERROR casting `&T` to `&mut T` is undefined behavior
}
Comment on lines +58 to +66
Copy link
Member Author

@Urgau Urgau Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@RalfJung can you confirm (or deny) that we can lint on those expressions ?

Copy link
Member

Choose a reason for hiding this comment

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

Those expressions are definitely bogus.

Can you confirm that the lint will not fire if the type of x is changed to &Cell<i32>?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I can.

I already added some tests below to make sure we don't lint on them, but added a test with &Cell<i32> anyway (just to be on the safe side).

}

unsafe fn assign_to_ref() {
Expand Down Expand Up @@ -111,6 +125,20 @@ unsafe fn no_warn() {
let mut value = 3;
let value: *const i32 = &mut value;
*(value as *const i16 as *mut i16) = 42;

fn safe_as_mut<T>(x: &std::cell::UnsafeCell<T>) -> &mut T {
unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
}

fn cell_as_mut(x: &std::cell::Cell<i32>) -> &mut i32 {
unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
}

#[repr(transparent)]
struct DoesContainUnsafeCell(std::cell::UnsafeCell<i32>);
fn safe_as_mut2(x: &DoesContainUnsafeCell) -> &mut DoesContainUnsafeCell {
unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
}
}

fn main() {}
68 changes: 48 additions & 20 deletions tests/ui/lint/reference_casting.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,19 @@ LL | let _num = &mut *std::mem::transmute::<_, *mut i32>(num);
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell`
--> $DIR/reference_casting.rs:41:16
--> $DIR/reference_casting.rs:39:16
|
LL | let _num = &mut *std::cell::UnsafeCell::raw_get(
| ________________^
LL | |
LL | | num as *const i32 as *const std::cell::UnsafeCell<i32>
LL | | );
| |_____^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell`
--> $DIR/reference_casting.rs:45:16
|
LL | let deferred = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
Expand All @@ -90,7 +102,7 @@ LL | let _num = &mut *deferred;
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

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
--> $DIR/reference_casting.rs:48:16
|
LL | let deferred = (std::ptr::from_ref(num) as *const i32 as *const i32).cast_mut() as *mut i32;
| ---------------------------------------------------------------------------- casting happend here
Expand All @@ -100,79 +112,95 @@ LL | let _num = &mut *deferred;
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

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
--> $DIR/reference_casting.rs:50:16
|
LL | let _num = &mut *(num as *const _ as usize as *mut i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

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
--> $DIR/reference_casting.rs:54:9
|
LL | &mut *((this as *const _) as *mut _)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell`
--> $DIR/reference_casting.rs:59:18
|
LL | unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell`
--> $DIR/reference_casting.rs:64:18
|
LL | unsafe { &mut *std::cell::UnsafeCell::raw_get(x as *const _ as *const _) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:60:5
--> $DIR/reference_casting.rs:74:5
|
LL | *(a as *const _ as *mut _) = String::from("Replaced");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:62:5
--> $DIR/reference_casting.rs:76:5
|
LL | *(a as *const _ as *mut String) += " world";
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:64:5
--> $DIR/reference_casting.rs:78:5
|
LL | *std::ptr::from_ref(num).cast_mut() += 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:66:5
--> $DIR/reference_casting.rs:80:5
|
LL | *std::ptr::from_ref({ num }).cast_mut() += 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:68:5
--> $DIR/reference_casting.rs:82:5
|
LL | *{ std::ptr::from_ref(num) }.cast_mut() += 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:70:5
--> $DIR/reference_casting.rs:84:5
|
LL | *(std::ptr::from_ref({ num }) as *mut i32) += 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:72:5
--> $DIR/reference_casting.rs:86:5
|
LL | *std::mem::transmute::<_, *mut i32>(num) += 1;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:74:5
--> $DIR/reference_casting.rs:88:5
|
LL | / std::ptr::write(
LL | |
Expand All @@ -184,7 +212,7 @@ LL | | );
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:81:5
--> $DIR/reference_casting.rs:95:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
Expand All @@ -194,23 +222,23 @@ LL | *value = 1;
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:83:5
--> $DIR/reference_casting.rs:97:5
|
LL | *(num as *const i32).cast::<i32>().cast_mut() = 2;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:85:5
--> $DIR/reference_casting.rs:99:5
|
LL | *(num as *const _ as usize as *mut i32) = 2;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:87:5
--> $DIR/reference_casting.rs:101:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
Expand All @@ -221,7 +249,7 @@ LL | std::ptr::write(value, 2);
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:89:5
--> $DIR/reference_casting.rs:103:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
Expand All @@ -232,7 +260,7 @@ LL | std::ptr::write_unaligned(value, 2);
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:91:5
--> $DIR/reference_casting.rs:105:5
|
LL | let value = num as *const i32 as *mut i32;
| ----------------------------- casting happend here
Expand All @@ -243,12 +271,12 @@ LL | std::ptr::write_volatile(value, 2);
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

error: assigning to `&T` is undefined behavior, consider using an `UnsafeCell`
--> $DIR/reference_casting.rs:95:9
--> $DIR/reference_casting.rs:109:9
|
LL | *(this as *const _ as *mut _) = a;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>

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