Skip to content
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
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/dangling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ fn lint_addr_of_local<'a>(
expr: &'a Expr<'a>,
) {
// peel casts as they do not interest us here, we want the inner expression.
let (inner, _) = super::utils::peel_casts(cx, expr);
let inner = super::utils::peel_casts(cx, expr);

if let ExprKind::AddrOf(_, _, inner_of) = inner.kind
&& let ExprKind::Path(ref qpath) = inner_of.peel_blocks().kind
Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,10 +993,6 @@ pub(crate) enum InvalidReferenceCastingDiag<'tcx> {
BorrowAsMut {
#[label("casting happened here")]
orig_cast: Option<Span>,
#[note(
"even for types with interior mutability, the only legal way to obtain a mutable pointer from a shared reference is through `UnsafeCell::get`"
)]
ty_has_interior_mutability: bool,
},
#[diag("assigning to `&T` is undefined behavior, consider using an `UnsafeCell`")]
#[note(
Expand All @@ -1005,10 +1001,6 @@ pub(crate) enum InvalidReferenceCastingDiag<'tcx> {
AssignToRef {
#[label("casting happened here")]
orig_cast: Option<Span>,
#[note(
"even for types with interior mutability, the only legal way to obtain a mutable pointer from a shared reference is through `UnsafeCell::get`"
)]
ty_has_interior_mutability: bool,
},
#[diag(
"casting references to a bigger memory layout than the backing allocation is undefined behavior, even if the reference is unused"
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/ptr_nulls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ fn useless_check<'a, 'tcx: 'a>(

/// Checks if the given expression is a null pointer (modulo casting)
fn is_null_ptr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Span> {
let (expr, _) = peel_casts(cx, expr);
let expr = peel_casts(cx, expr);

if let ExprKind::Call(path, []) = expr.kind
&& let ExprKind::Path(ref qpath) = path.kind
Expand Down
43 changes: 19 additions & 24 deletions compiler/rustc_lint/src/reference_casting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,15 @@ impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting {
};

if matches!(pat, PatternKind::Borrow { mutbl: Mutability::Mut } | PatternKind::Assign)
&& let Some(ty_has_interior_mutability) =
is_cast_from_ref_to_mut_ptr(cx, init, &mut peel_casts)
&& is_invalid_cast_from_ref_to_mut_ptr(cx, init, &mut peel_casts)
{
cx.emit_span_lint(
INVALID_REFERENCE_CASTING,
expr.span,
if pat == PatternKind::Assign {
InvalidReferenceCastingDiag::AssignToRef {
orig_cast,
ty_has_interior_mutability,
}
InvalidReferenceCastingDiag::AssignToRef { orig_cast }
} else {
InvalidReferenceCastingDiag::BorrowAsMut {
orig_cast,
ty_has_interior_mutability,
}
InvalidReferenceCastingDiag::BorrowAsMut { orig_cast }
},
);
}
Expand Down Expand Up @@ -146,49 +139,51 @@ fn borrow_or_assign<'tcx>(
deref_assign_or_addr_of(e).or_else(|| ptr_write(cx, e))
}

fn is_cast_from_ref_to_mut_ptr<'tcx>(
fn is_invalid_cast_from_ref_to_mut_ptr<'tcx>(
cx: &LateContext<'tcx>,
orig_expr: &'tcx Expr<'tcx>,
mut peel_casts: impl FnMut() -> (&'tcx Expr<'tcx>, bool),
) -> Option<bool> {
mut peel_casts: impl FnMut() -> &'tcx Expr<'tcx>,
) -> bool {
let end_ty = cx.typeck_results().node_type(orig_expr.hir_id);

// Bail out early if the end type is **not** a mutable pointer.
if !matches!(end_ty.kind(), ty::RawPtr(_, Mutability::Mut)) {
return None;
return false;
}

let (e, need_check_freeze) = peel_casts();

let e = peel_casts();
let start_ty = cx.typeck_results().node_type(e.hir_id);

if let ty::Ref(_, inner_ty, Mutability::Not) = start_ty.kind() {
// If an UnsafeCell method is involved, we need to additionally 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 need to additionally 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 (see https://github.com/rust-lang/unsafe-code-guidelines/issues/281).
//
Comment thread
Urgau marked this conversation as resolved.
// Except on the presence of non concrete skeleton types (ie generics)
// since there is no way to make it safe for arbitrary types.
//
// However this does mean we miss out on some cases where the user doesn't go
// through an UnsafeCell but there's an UnsafeCell somewhere else in the type.
let inner_ty_has_interior_mutability =
!inner_ty.is_freeze(cx.tcx, cx.typing_env()) && inner_ty.has_concrete_skeleton();
(!need_check_freeze || !inner_ty_has_interior_mutability)
.then_some(inner_ty_has_interior_mutability)
!inner_ty_has_interior_mutability
} else {
None
false
}
}

fn is_cast_to_bigger_memory_layout<'tcx>(
cx: &LateContext<'tcx>,
orig_expr: &'tcx Expr<'tcx>,
mut peel_casts: impl FnMut() -> (&'tcx Expr<'tcx>, bool),
mut peel_casts: impl FnMut() -> &'tcx Expr<'tcx>,
) -> Option<(TyAndLayout<'tcx>, TyAndLayout<'tcx>, Expr<'tcx>)> {
let end_ty = cx.typeck_results().node_type(orig_expr.hir_id);

let ty::RawPtr(inner_end_ty, _) = end_ty.kind() else {
return None;
};

let (e, _) = peel_casts();
let e = peel_casts();
let start_ty = cx.typeck_results().node_type(e.hir_id);

let ty::Ref(_, inner_start_ty, _) = start_ty.kind() else {
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_lint/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ use crate::LateContext;
/// Given an expression, peel all of casts (`<expr> as ...`, `<expr>.cast{,_mut,_const}()`,
/// `ptr::from_ref(<expr>)`, ...) and init expressions.
///
/// Returns the innermost expression and a boolean representing if one of the casts was
/// `UnsafeCell::raw_get(<expr>)`
/// Returns the innermost expression.
pub(crate) fn peel_casts<'tcx>(
cx: &LateContext<'tcx>,
mut e: &'tcx Expr<'tcx>,
) -> (&'tcx Expr<'tcx>, bool) {
let mut gone_trough_unsafe_cell_raw_get = false;

) -> &'tcx Expr<'tcx> {
loop {
e = e.peel_blocks();
// <expr> as ...
Expand All @@ -37,9 +34,6 @@ pub(crate) fn peel_casts<'tcx>(
Some(sym::ptr_from_ref | sym::unsafe_cell_raw_get | sym::transmute)
)
{
if cx.tcx.is_diagnostic_item(sym::unsafe_cell_raw_get, def_id) {
gone_trough_unsafe_cell_raw_get = true;
}
arg
} else {
let init = cx.expr_or_init(e);
Expand All @@ -51,5 +45,5 @@ pub(crate) fn peel_casts<'tcx>(
};
}

(e, gone_trough_unsafe_cell_raw_get)
e
}
1 change: 0 additions & 1 deletion library/core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2236,7 +2236,6 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
/// /// # Safety
/// /// The caller must not call `get_mut_unchecked` again (on any alias of `ptr`) for the duration
/// /// of the lifetime of the returned reference.
/// # #[allow(invalid_reference_casting)] // FIXME should the lint really fire here?
/// unsafe fn get_mut_unchecked<T>(ptr: &UnsafeCell<T>) -> &mut T {
/// let t = ptr as *const UnsafeCell<T> as *mut T;
/// unsafe { &mut *t }
Expand Down
55 changes: 36 additions & 19 deletions tests/ui/lint/reference_casting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ unsafe fn ref_to_mut() {
let _num = &mut *(std::mem::transmute::<_, *mut i32>(num) as *mut i32);
//~^ 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>
//~^ 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;
Expand All @@ -62,10 +62,6 @@ unsafe fn ref_to_mut() {
let _num = &mut *num;
//~^ ERROR casting `&T` to `&mut T` is undefined behavior

let cell = &std::cell::UnsafeCell::new(0);
let _num = &mut *(cell as *const _ 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
Expand Down Expand Up @@ -104,12 +100,10 @@ unsafe fn assign_to_ref() {
*(std::mem::transmute::<_, *mut i32>(num) as *mut i32) += 1;
//~^ ERROR assigning to `&T` is undefined behavior
std::ptr::write(
//~^ ERROR assigning to `&T` is undefined behavior
//~^ ERROR assigning to `&T` is undefined behavior
std::mem::transmute::<*const i32, *mut i32>(num),
-1i32,
);
*((&std::cell::UnsafeCell::new(0)) as *const _ as *mut i32) = 5;
//~^ ERROR assigning to `&T` is undefined behavior

let value = num as *const i32 as *mut i32;
*value = 1;
Expand Down Expand Up @@ -207,14 +201,16 @@ unsafe fn bigger_layout() {
}

{
let mut l: [u8; 2] = [0,1];
let mut l: [u8; 2] = [0, 1];
let w: *mut [u16; 2] = &mut l as *mut [u8; 2] as *mut _;
let w: *mut [u16] = unsafe {&mut *w};
let w: *mut [u16] = unsafe { &mut *w };
//~^ ERROR casting references to a bigger memory layout
}

{
fn foo() -> [i32; 1] { todo!() }
fn foo() -> [i32; 1] {
todo!()
}

let num = foo();
let _num = &*(&num as *const i32 as *const i64);
Expand All @@ -224,15 +220,19 @@ unsafe fn bigger_layout() {
}

{
fn bar(_a: &[i32; 2]) -> &[i32; 1] { todo!() }
fn bar(_a: &[i32; 2]) -> &[i32; 1] {
todo!()
}

let num = bar(&[0, 0]);
let _num = &*(num as *const i32 as *const i64);
let _num = &*(bar(&[0, 0]) as *const i32 as *const i64);
}

{
fn foi<T>() -> T { todo!() }
fn foi<T>() -> T {
todo!()
}

let num = foi::<i32>();
let _num = &*(&num as *const i32 as *const i64);
Expand Down Expand Up @@ -286,24 +286,41 @@ unsafe fn no_warn() {
let value: *const i32 = &mut value;
*(value as *const i16 as *mut i16) = 42;
*RAW_PTR = 42; // RAW_PTR is defined outside the function body,
// make sure we don't ICE on it when trying to
// determine if we should lint on it or not.
// make sure we don't ICE on it when trying to
// determine if we should lint on it or not.
*((&std::cell::UnsafeCell::new(0)) as *const _ as *mut i32) = 5;

let cell = &std::cell::UnsafeCell::new(0);
let _num = &mut *(cell.get() as *mut i32);
let _num = &mut *(cell as *const _ as *mut i32);

fn safe_as_mut<T>(x: &std::cell::UnsafeCell<T>) -> &mut T {
unsafe fn get_mut_unchecked<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 fn get_mut_unchecked2<T>(ptr: &std::cell::UnsafeCell<T>) -> &mut T {
let t = ptr as *const std::cell::UnsafeCell<T> as *mut T;
unsafe { &mut *t }
}

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

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

#[repr(transparent)]
struct DoesContainUnsafeCell(std::cell::UnsafeCell<i32>);
fn safe_as_mut2(x: &DoesContainUnsafeCell) -> &mut DoesContainUnsafeCell {

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

unsafe fn get_mut_unchecked4(x: &DoesContainUnsafeCell) -> &mut DoesContainUnsafeCell {
unsafe { &mut *(x as *const DoesContainUnsafeCell as *mut _) }
}
Comment on lines +289 to +323

@Urgau Urgau Jul 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@RalfJung could you double-check and confirm that all of those casts are now fine.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Depends on what you mean by "fine". ;)

They are not immediate UB. But getting a reference into a Cell is still incredibly dangerous. Just as dangerous as Cell::as_ptr though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, by "fine" I mean no longer immediate UB, it can still be very UB. Thanks for checking.

}

fn main() {}
Loading
Loading