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 1 commit
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
22 changes: 20 additions & 2 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ struct UnsafetyVisitor<'a, 'tcx> {
/// Flag to ensure that we only suggest wrapping the entire function body in
/// an unsafe block once.
suggest_unsafe_block: bool,
/// Track whether we're currently inside a `&raw const/mut` expression.
/// Used to allow safe access to union fields when only taking their address.
in_raw_borrow: bool,
}

impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
Expand Down Expand Up @@ -218,6 +221,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
inside_adt: false,
warnings: self.warnings,
suggest_unsafe_block: self.suggest_unsafe_block,
in_raw_borrow: false,
};
// params in THIR may be unsafe, e.g. a union pattern.
for param in &inner_thir.params {
Expand Down Expand Up @@ -538,14 +542,24 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
}
ExprKind::RawBorrow { arg, .. } => {
// Set flag when entering raw borrow context
let old_in_raw_borrow = self.in_raw_borrow;
self.in_raw_borrow = true;

if let ExprKind::Scope { value: arg, .. } = self.thir[arg].kind
&& let ExprKind::Deref { arg } = self.thir[arg].kind
{
// Taking a raw ref to a deref place expr is always safe.
// Make sure the expression we're deref'ing is safe, though.
visit::walk_expr(self, &self.thir[arg]);
return;
} else {
// Handle other raw borrow cases (including field access)
visit::walk_expr(self, &self.thir[arg]);
}

// Restore previous state
self.in_raw_borrow = old_in_raw_borrow;
return;
}
ExprKind::Deref { arg } => {
if let ExprKind::StaticRef { def_id, .. } | ExprKind::ThreadLocalRef(def_id) =
Expand Down Expand Up @@ -651,7 +665,10 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
"union fields that need dropping should be impossible: {assigned_ty}"
);
}
} else {
} else if !self.in_raw_borrow {
// Union field access is unsafe because the field may be uninitialized.
// However, `&raw const/mut union.field` is safe since it only computes
// the field's address without reading the potentially uninitialized value.
self.requires_unsafe(expr.span, AccessToUnionField);
}
}
Expand Down Expand Up @@ -1187,6 +1204,7 @@ pub(crate) fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
inside_adt: false,
warnings: &mut warnings,
suggest_unsafe_block: true,
in_raw_borrow: false,
};
// params in THIR may be unsafe, e.g. a union pattern.
for param in &thir.params {
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/union/union-unsafe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ 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 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
12 changes: 6 additions & 6 deletions tests/ui/union/union-unsafe.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -31,47 +31,47 @@ LL | let a = u1.a;
= 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:68: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:69: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:70: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:75: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:79: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:83:6
|
LL | *u3.a = String::from("new");
| ^^^^ access to union field
Expand Down
Loading