completly revised logic

Kivooeo · Kivooeo · commit 43892d3edf66 · 2025-05-24T16:46:33.000+05:00
diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs
@@ -45,9 +45,6 @@ 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> {
@@ -221,7 +218,6 @@ 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 {
@@ -542,22 +538,28 @@ 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 = is_direct_place_expr(self.thir, 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
                 {
                     // 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;
                 }
-
-                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) =
@@ -654,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
@@ -663,10 +667,8 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                                     "union fields that need dropping should be impossible: {assigned_ty}"
                                 );
                             }
-                        } 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.
+                        } else {
+                            // Only require unsafe if this is not a raw borrow operation
                             self.requires_unsafe(expr.span, AccessToUnionField);
                         }
                     }
@@ -720,16 +722,43 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
     }
 }
 
-fn is_direct_place_expr<'a, 'tcx>(thir: &'a Thir<'tcx>, expr_id: ExprId) -> bool {
-    match thir[expr_id].kind {
-        // Direct place expressions that don't read values
-        ExprKind::Field { .. } | ExprKind::VarRef { .. } | ExprKind::UpvarRef { .. } => true,
-
-        // Scope is transparent for place expressions
-        ExprKind::Scope { value, .. } => is_direct_place_expr(thir, value),
+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,
+        }
+    }
 
-        // Any other expression (including Deref, method calls, etc.) reads values
-        _ => 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]);
+            }
+        }
     }
 }
 
@@ -1215,7 +1244,6 @@ 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 {
diff --git a/main b/main
diff --git a/tests/ui/union/union-unsafe.rs b/tests/ui/union/union-unsafe.rs
@@ -36,9 +36,8 @@ fn deref_union_field(mut u: URef) {
 }
 
 fn raw_deref_union_field(mut u: URef) {
-    let _p = &raw const *(u.p);
-    //~^ ERROR access to union field is unsafe
-    //~| ERROR access to union field is unsafe
+    // 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) {
@@ -77,8 +76,9 @@ fn main() {
 
     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
-    //~^ 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
diff --git a/tests/ui/union/union-unsafe.stderr b/tests/ui/union/union-unsafe.stderr
@@ -15,103 +15,85 @@ LL |     let _p = &raw const *(u.p);
    = 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:39: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
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
-
-error[E0133]: access to union field is unsafe and requires unsafe function or block
-  --> $DIR/union-unsafe.rs:53:6
+  --> $DIR/union-unsafe.rs:51: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:59:6
+  --> $DIR/union-unsafe.rs:57: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:67:13
+  --> $DIR/union-unsafe.rs:65: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:80: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:80:29
+  --> $DIR/union-unsafe.rs:78: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
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
 error[E0133]: access to union field is unsafe and requires unsafe function or block
-  --> $DIR/union-unsafe.rs:83:14
+  --> $DIR/union-unsafe.rs:80: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:84:20
+  --> $DIR/union-unsafe.rs:81: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:85:25
+  --> $DIR/union-unsafe.rs:82: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:90:6
+  --> $DIR/union-unsafe.rs:87: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:94:6
+  --> $DIR/union-unsafe.rs:91: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:98:6
+  --> $DIR/union-unsafe.rs:95: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 14 previous errors
+error: aborting due to 12 previous errors
 
 For more information about this error, try `rustc --explain E0133`.

Original file line number	Diff line number	Diff line change
`@@ -15,103 +15,85 @@ LL \| let _p = &raw const *(u.p);`
`15`	`15`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`16`	`16`
`17`	`17`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`18`		`- --> $DIR/union-unsafe.rs:39:26`
`19`		`- \|`
`20`		`-LL \| let _p = &raw const *(u.p);`
`21`		`- \| ^^^^^ access to union field`
`22`		`- \|`
`23`		`- = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`24`		- = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
`25`		`-`
`26`		`-error[E0133]: access to union field is unsafe and requires unsafe function or block`
`27`		`- --> $DIR/union-unsafe.rs:53:6`
	`18`	`+ --> $DIR/union-unsafe.rs:51:6`
`28`	`19`	`\|`
`29`	`20`	`LL \| *u3.a = T::default();`
`30`	`21`	`\| ^^^^ access to union field`
`31`	`22`	`\|`
`32`	`23`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`33`	`24`
`34`	`25`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`35`		`- --> $DIR/union-unsafe.rs:59:6`
	`26`	`+ --> $DIR/union-unsafe.rs:57:6`
`36`	`27`	`\|`
`37`	`28`	`LL \| *u3.a = T::default();`
`38`	`29`	`\| ^^^^ access to union field`
`39`	`30`	`\|`
`40`	`31`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`41`	`32`
`42`	`33`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`43`		`- --> $DIR/union-unsafe.rs:67:13`
	`34`	`+ --> $DIR/union-unsafe.rs:65:13`
`44`	`35`	`\|`
`45`	`36`	`LL \| let a = u1.a;`
`46`	`37`	`\| ^^^^ access to union field`
`47`	`38`	`\|`
`48`	`39`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`49`	`40`
`50`	`41`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`51`		`- --> $DIR/union-unsafe.rs:80:29`
`52`		`- \|`
`53`		`-LL \| let _a = &raw const vec[u4.a];`
`54`		`- \| ^^^^ access to union field`
`55`		`- \|`
`56`		`- = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`57`		`-`
`58`		`-error[E0133]: access to union field is unsafe and requires unsafe function or block`
`59`		`- --> $DIR/union-unsafe.rs:80:29`
	`42`	`+ --> $DIR/union-unsafe.rs:78:29`
`60`	`43`	`\|`
`61`	`44`	`LL \| let _a = &raw const vec[u4.a];`
`62`	`45`	`\| ^^^^ access to union field`
`63`	`46`	`\|`
`64`	`47`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`65`		- = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
`66`	`48`
`67`	`49`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`68`		`- --> $DIR/union-unsafe.rs:83:14`
	`50`	`+ --> $DIR/union-unsafe.rs:80:14`
`69`	`51`	`\|`
`70`	`52`	`LL \| let U1 { a } = u1;`
`71`	`53`	`\| ^ access to union field`
`72`	`54`	`\|`
`73`	`55`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`74`	`56`
`75`	`57`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`76`		`- --> $DIR/union-unsafe.rs:84:20`
	`58`	`+ --> $DIR/union-unsafe.rs:81:20`
`77`	`59`	`\|`
`78`	`60`	`LL \| if let U1 { a: 12 } = u1 {}`
`79`	`61`	`\| ^^ access to union field`
`80`	`62`	`\|`
`81`	`63`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`82`	`64`
`83`	`65`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`84`		`- --> $DIR/union-unsafe.rs:85:25`
	`66`	`+ --> $DIR/union-unsafe.rs:82:25`
`85`	`67`	`\|`
`86`	`68`	`LL \| if let Some(U1 { a: 13 }) = Some(u1) {}`
`87`	`69`	`\| ^^ access to union field`
`88`	`70`	`\|`
`89`	`71`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`90`	`72`
`91`	`73`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`92`		`- --> $DIR/union-unsafe.rs:90:6`
	`74`	`+ --> $DIR/union-unsafe.rs:87:6`
`93`	`75`	`\|`
`94`	`76`	`LL \| *u2.a = String::from("new");`
`95`	`77`	`\| ^^^^ access to union field`
`96`	`78`	`\|`
`97`	`79`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`98`	`80`
`99`	`81`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`100`		`- --> $DIR/union-unsafe.rs:94:6`
	`82`	`+ --> $DIR/union-unsafe.rs:91:6`
`101`	`83`	`\|`
`102`	`84`	`LL \| *u3.a = 1;`
`103`	`85`	`\| ^^^^ access to union field`
`104`	`86`	`\|`
`105`	`87`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`106`	`88`
`107`	`89`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`108`		`- --> $DIR/union-unsafe.rs:98:6`
	`90`	`+ --> $DIR/union-unsafe.rs:95:6`
`109`	`91`	`\|`
`110`	`92`	`LL \| *u3.a = String::from("new");`
`111`	`93`	`\| ^^^^ access to union field`
`112`	`94`	`\|`
`113`	`95`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`114`	`96`
`115`		`-error: aborting due to 14 previous errors`
	`97`	`+error: aborting due to 12 previous errors`
`116`	`98`
`117`	`99`	For more information about this error, try `rustc --explain E0133`.