Skip to content

Allow &raw [mut | const] for union field in safe #141469

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
57 changes: 57 additions & 0 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,20 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
}
ExprKind::RawBorrow { arg, .. } => {
// Handle the case where we're taking a raw pointer to a union field
if let ExprKind::Scope { value: arg, .. } = self.thir[arg].kind {
if self.is_union_field_access(arg) {
// Taking a raw pointer to a union field is safe - just check the base expression
// but skip the union field safety check
self.visit_union_field_for_raw_borrow(arg);
return;
}
} else if self.is_union_field_access(arg) {
// Direct raw borrow of union field
self.visit_union_field_for_raw_borrow(arg);
return;
}

if let ExprKind::Scope { value: arg, .. } = self.thir[arg].kind
&& let ExprKind::Deref { arg } = self.thir[arg].kind
{
Expand Down Expand Up @@ -642,6 +656,8 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
if adt_def.variant(variant_index).fields[name].safety.is_unsafe() {
self.requires_unsafe(expr.span, UseOfUnsafeField);
} else if adt_def.is_union() {
// Check if this field access is part of a raw borrow operation
// If so, we've already handled it above and shouldn't reach here
if let Some(assigned_ty) = self.assignment_info {
if assigned_ty.needs_drop(self.tcx, self.typing_env) {
// This would be unsafe, but should be outright impossible since we
Expand All @@ -652,6 +668,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
);
}
} else {
// Only require unsafe if this is not a raw borrow operation
self.requires_unsafe(expr.span, AccessToUnionField);
}
}
Expand Down Expand Up @@ -705,6 +722,46 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
}

impl<'a, 'tcx> UnsafetyVisitor<'a, 'tcx> {
/// Check if an expression is a union field access
fn is_union_field_access(&self, expr_id: ExprId) -> bool {
match self.thir[expr_id].kind {
ExprKind::Field { lhs, .. } => {
let lhs = &self.thir[lhs];
if let ty::Adt(adt_def, _) = lhs.ty.kind() { adt_def.is_union() } else { false }
}
_ => false,
}
}

/// Visit a union field access in the context of a raw borrow operation
/// This ensures we still check safety of nested operations while allowing
/// the raw pointer creation itself
fn visit_union_field_for_raw_borrow(&mut self, expr_id: ExprId) {
match self.thir[expr_id].kind {
ExprKind::Field { lhs, variant_index, name } => {
let lhs_expr = &self.thir[lhs];
if let ty::Adt(adt_def, _) = lhs_expr.ty.kind() {
// Check for unsafe fields but skip the union access check
if adt_def.variant(variant_index).fields[name].safety.is_unsafe() {
self.requires_unsafe(self.thir[expr_id].span, UseOfUnsafeField);
}
// For unions, we don't require unsafe for raw pointer creation
// But we still need to check the LHS for safety
self.visit_expr(lhs_expr);
} else {
// Not a union, use normal visiting
visit::walk_expr(self, &self.thir[expr_id]);
}
}
_ => {
// Not a field access, use normal visiting
visit::walk_expr(self, &self.thir[expr_id]);
}
}
}
}

#[derive(Clone)]
enum SafetyContext {
Safe,
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/union/union-unsafe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ union U4<T: Copy> {
a: T,
}

union U5 {
a: usize,
}

union URef {
p: &'static mut i32,
}
Expand All @@ -31,6 +35,11 @@ fn deref_union_field(mut u: URef) {
*(u.p) = 13; //~ ERROR access to union field is unsafe
}

fn raw_deref_union_field(mut u: URef) {
// This is unsafe because we first dereference u.p (reading uninitialized memory)
let _p = &raw const *(u.p); //~ ERROR access to union field is unsafe
}

fn assign_noncopy_union_field(mut u: URefCell) {
u.a = (ManuallyDrop::new(RefCell::new(0)), 1); // OK (assignment does not drop)
u.a.0 = ManuallyDrop::new(RefCell::new(0)); // OK (assignment does not drop)
Expand All @@ -57,6 +66,20 @@ fn main() {
let a = u1.a; //~ ERROR access to union field is unsafe
u1.a = 11; // OK

let mut u2 = U1 { a: 10 };
let a = &raw mut u2.a; // OK
unsafe { *a = 3 };

let mut u3 = U1 { a: 10 };
let a = std::ptr::addr_of_mut!(u3.a); // OK
unsafe { *a = 14 };

let u4 = U5 { a: 2 };
let vec = vec![1, 2, 3];
// This is unsafe because we read u4.a (potentially uninitialized memory)
// to use as an array index
let _a = &raw const vec[u4.a]; //~ ERROR access to union field is unsafe

let U1 { a } = u1; //~ ERROR access to union field is unsafe
if let U1 { a: 12 } = u1 {} //~ ERROR access to union field is unsafe
if let Some(U1 { a: 13 }) = Some(u1) {} //~ ERROR access to union field is unsafe
Expand Down
38 changes: 27 additions & 11 deletions tests/ui/union/union-unsafe.stderr
Original file line number Diff line number Diff line change
@@ -1,83 +1,99 @@
error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:31:6
--> $DIR/union-unsafe.rs:35:6
|
LL | *(u.p) = 13;
| ^^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:43:6
--> $DIR/union-unsafe.rs:40:26
|
LL | let _p = &raw const *(u.p);
| ^^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:52:6
|
LL | *u3.a = T::default();
| ^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:49:6
--> $DIR/union-unsafe.rs:58:6
|
LL | *u3.a = T::default();
| ^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:57:13
--> $DIR/union-unsafe.rs:66:13
|
LL | let a = u1.a;
| ^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:60:14
--> $DIR/union-unsafe.rs:81:29
|
LL | let _a = &raw const vec[u4.a];
| ^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:83:14
|
LL | let U1 { a } = u1;
| ^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:61:20
--> $DIR/union-unsafe.rs:84:20
|
LL | if let U1 { a: 12 } = u1 {}
| ^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:62:25
--> $DIR/union-unsafe.rs:85:25
|
LL | if let Some(U1 { a: 13 }) = Some(u1) {}
| ^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:67:6
--> $DIR/union-unsafe.rs:90:6
|
LL | *u2.a = String::from("new");
| ^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:71:6
--> $DIR/union-unsafe.rs:94:6
|
LL | *u3.a = 1;
| ^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error[E0133]: access to union field is unsafe and requires unsafe function or block
--> $DIR/union-unsafe.rs:75:6
--> $DIR/union-unsafe.rs:98:6
|
LL | *u3.a = String::from("new");
| ^^^^ access to union field
|
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior

error: aborting due to 10 previous errors
error: aborting due to 12 previous errors

For more information about this error, try `rustc --explain E0133`.
Loading