From c8238ac017652ab7a6c72f11798ed41d7eac5445 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 23 Apr 2026 14:47:14 +0100 Subject: [PATCH 1/2] internal: move alignment check to `make_field_check` Instead of having the reference creation serving dual-purpose as both for let bindings and alignment check, detangle them so that the alignment check is done explicitly in `make_field_check`. This is more robust again refactors that may change the way let bindings are created. Signed-off-by: Gary Guo --- internal/src/init.rs | 78 +++++++++---------- tests/ui/compile-fail/init/packed_struct.rs | 5 ++ .../ui/compile-fail/init/packed_struct.stderr | 19 ++++- 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/internal/src/init.rs b/internal/src/init.rs index daa3f1c6..0a6600e8 100644 --- a/internal/src/init.rs +++ b/internal/src/init.rs @@ -249,10 +249,6 @@ fn init_fields( }); // Again span for better diagnostics let write = quote_spanned!(ident.span()=> ::core::ptr::write); - // NOTE: the field accessor ensures that the initialized field is properly aligned. - // Unaligned fields will cause the compiler to emit E0793. We do not support - // unaligned fields since `Init::__init` requires an aligned pointer; the call to - // `ptr::write` below has the same requirement. let accessor = if pinned { let project_ident = format_ident!("__project_{ident}"); quote! { @@ -367,49 +363,49 @@ fn init_fields( } } -/// Generate the check for ensuring that every field has been initialized. +/// Generate the check for ensuring that every field has been initialized and aligned. fn make_field_check( fields: &Punctuated, init_kind: InitKind, path: &Path, ) -> TokenStream { - let field_attrs = fields + let field_attrs: Vec<_> = fields .iter() - .filter_map(|f| f.kind.ident().map(|_| &f.attrs)); - let field_name = fields.iter().filter_map(|f| f.kind.ident()); - match init_kind { - InitKind::Normal => quote! { - // We use unreachable code to ensure that all fields have been mentioned exactly once, - // this struct initializer will still be type-checked and complain with a very natural - // error message if a field is forgotten/mentioned more than once. - #[allow(unreachable_code, clippy::diverging_sub_expression)] - // SAFETY: this code is never executed. - let _ = || unsafe { - ::core::ptr::write(slot, #path { - #( - #(#field_attrs)* - #field_name: ::core::panic!(), - )* - }) - }; - }, - InitKind::Zeroing => quote! { - // We use unreachable code to ensure that all fields have been mentioned at most once. - // Since the user specified `..Zeroable::zeroed()` at the end, all missing fields will - // be zeroed. This struct initializer will still be type-checked and complain with a - // very natural error message if a field is mentioned more than once, or doesn't exist. - #[allow(unreachable_code, clippy::diverging_sub_expression, unused_assignments)] - // SAFETY: this code is never executed. - let _ = || unsafe { - ::core::ptr::write(slot, #path { - #( - #(#field_attrs)* - #field_name: ::core::panic!(), - )* - ..::core::mem::zeroed() - }) - }; - }, + .filter_map(|f| f.kind.ident().map(|_| &f.attrs)) + .collect(); + let field_name: Vec<_> = fields.iter().filter_map(|f| f.kind.ident()).collect(); + let zeroing_trailer = match init_kind { + InitKind::Normal => None, + InitKind::Zeroing => Some(quote! { + ..::core::mem::zeroed() + }), + }; + quote! { + #[allow(unreachable_code, clippy::diverging_sub_expression)] + // We use unreachable code to perform field checks. They're still checked by the compiler. + // SAFETY: this code is never executed. + let _ = || unsafe { + // Create references to ensure that the initialized field is properly aligned. + // Unaligned fields will cause the compiler to emit E0793. We do not support + // unaligned fields since `Init::__init` requires an aligned pointer; the call to + // `ptr::write` for value-initialization case has the same requirement. + #( + #(#field_attrs)* + let _ = &(*slot).#field_name; + )* + + // If the zeroing trailer is not present, this checks that all fields have been + // mentioned exactly once. If the zeroing trailer is present, all missing fields will be + // zeroed, so this checks that all fields have been mentioned at most once. The use of + // struct initializer will still generate very natural error messages for any misuse. + ::core::ptr::write(slot, #path { + #( + #(#field_attrs)* + #field_name: ::core::panic!(), + )* + #zeroing_trailer + }) + }; } } diff --git a/tests/ui/compile-fail/init/packed_struct.rs b/tests/ui/compile-fail/init/packed_struct.rs index d8b1eee1..1587c8c2 100644 --- a/tests/ui/compile-fail/init/packed_struct.rs +++ b/tests/ui/compile-fail/init/packed_struct.rs @@ -1,6 +1,7 @@ use pin_init::*; #[repr(C, packed)] +#[derive(Zeroable)] struct Foo { a: i8, b: i32, @@ -8,4 +9,8 @@ struct Foo { fn main() { let _ = init!(Foo { a: -42, b: 42 }); + let _ = init!(Foo { + b: 42, + ..Zeroable::init_zeroed() + }); } diff --git a/tests/ui/compile-fail/init/packed_struct.stderr b/tests/ui/compile-fail/init/packed_struct.stderr index 1fc91d06..617c9eea 100644 --- a/tests/ui/compile-fail/init/packed_struct.stderr +++ b/tests/ui/compile-fail/init/packed_struct.stderr @@ -1,10 +1,25 @@ error[E0793]: reference to field of packed struct is unaligned - --> tests/ui/compile-fail/init/packed_struct.rs:10:13 + --> tests/ui/compile-fail/init/packed_struct.rs:11:13 | -10 | let _ = init!(Foo { a: -42, b: 42 }); +11 | let _ = init!(Foo { a: -42, b: 42 }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: this struct is 1-byte aligned, but the type of this field may require higher alignment = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers) = note: this error originates in the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0793]: reference to field of packed struct is unaligned + --> tests/ui/compile-fail/init/packed_struct.rs:12:13 + | +12 | let _ = init!(Foo { + | _____________^ +13 | | b: 42, +14 | | ..Zeroable::init_zeroed() +15 | | }); + | |______^ + | + = note: this struct is 1-byte aligned, but the type of this field may require higher alignment + = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced) + = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers) + = note: this error originates in the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info) From ac0190c13533d44bf0e74e5f8982047da3555df6 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sat, 18 Apr 2026 15:33:34 +0100 Subject: [PATCH 2/2] fix incorrect accessor reference lifetime When a field has been initialized, `init!`/`pin_init!` create a reference or pinned reference to the field so it can be accessed later during the initialization of other fields. However, the reference it created is incorrectly `&'static` rather than just the scope of the initializer. This means that you can do init!(Foo { a: 1, _: { let b: &'static u32 = a; } }) which is unsound. This is caused by `&mut (*#slot).#ident`, which actually allows arbitrary lifetime, so this is effectively `'static`. Somewhat ironically, the safety justification of creating the accessor is.. "SAFETY: TODO". Fix it by adding `let_binding` method on `DropGuard` to shorten lifetime. This results exactly what we want for these accessors. Fixes: db96c5103ae6 ("add references to previously initialized fields") Signed-off-by: Gary Guo --- internal/src/init.rs | 106 ++++++++---------- src/__internal.rs | 28 +++-- .../ui/compile-fail/init/accessor_lifetime.rs | 14 +++ .../init/accessor_lifetime.stderr | 17 +++ 4 files changed, 97 insertions(+), 68 deletions(-) create mode 100644 tests/ui/compile-fail/init/accessor_lifetime.rs create mode 100644 tests/ui/compile-fail/init/accessor_lifetime.stderr diff --git a/internal/src/init.rs b/internal/src/init.rs index 0a6600e8..487ee001 100644 --- a/internal/src/init.rs +++ b/internal/src/init.rs @@ -249,18 +249,6 @@ fn init_fields( }); // Again span for better diagnostics let write = quote_spanned!(ident.span()=> ::core::ptr::write); - let accessor = if pinned { - let project_ident = format_ident!("__project_{ident}"); - quote! { - // SAFETY: TODO - unsafe { #data.#project_ident(&mut (*#slot).#ident) } - } - } else { - quote! { - // SAFETY: TODO - unsafe { &mut (*#slot).#ident } - } - }; quote! { #(#attrs)* { @@ -268,51 +256,31 @@ fn init_fields( // SAFETY: TODO unsafe { #write(&raw mut (*#slot).#ident, #value_ident) }; } - #(#cfgs)* - #[allow(unused_variables)] - let #ident = #accessor; } } InitializerKind::Init { ident, value, .. } => { // Again span for better diagnostics let init = format_ident!("init", span = value.span()); - // NOTE: the field accessor ensures that the initialized field is properly aligned. - // Unaligned fields will cause the compiler to emit E0793. We do not support - // unaligned fields since `Init::__init` requires an aligned pointer; the call to - // `ptr::write` below has the same requirement. - let (value_init, accessor) = if pinned { - let project_ident = format_ident!("__project_{ident}"); - ( - quote! { - // SAFETY: - // - `slot` is valid, because we are inside of an initializer closure, we - // return when an error/panic occurs. - // - We also use `#data` to require the correct trait (`Init` or `PinInit`) - // for `#ident`. - unsafe { #data.#ident(&raw mut (*#slot).#ident, #init)? }; - }, - quote! { - // SAFETY: TODO - unsafe { #data.#project_ident(&mut (*#slot).#ident) } - }, - ) + let value_init = if pinned { + quote! { + // SAFETY: + // - `slot` is valid, because we are inside of an initializer closure, we + // return when an error/panic occurs. + // - We also use `#data` to require the correct trait (`Init` or `PinInit`) + // for `#ident`. + unsafe { #data.#ident(&raw mut (*#slot).#ident, #init)? }; + } } else { - ( - quote! { - // SAFETY: `slot` is valid, because we are inside of an initializer - // closure, we return when an error/panic occurs. - unsafe { - ::pin_init::Init::__init( - #init, - &raw mut (*#slot).#ident, - )? - }; - }, - quote! { - // SAFETY: TODO - unsafe { &mut (*#slot).#ident } - }, - ) + quote! { + // SAFETY: `slot` is valid, because we are inside of an initializer + // closure, we return when an error/panic occurs. + unsafe { + ::pin_init::Init::__init( + #init, + &raw mut (*#slot).#ident, + )? + }; + } }; quote! { #(#attrs)* @@ -320,9 +288,6 @@ fn init_fields( let #init = #value; #value_init } - #(#cfgs)* - #[allow(unused_variables)] - let #ident = #accessor; } } InitializerKind::Code { block: value, .. } => quote! { @@ -335,18 +300,41 @@ fn init_fields( if let Some(ident) = kind.ident() { // `mixed_site` ensures that the guard is not accessible to the user-controlled code. let guard = format_ident!("__{ident}_guard", span = Span::mixed_site()); + + // NOTE: The reference is derived from the guard so that it only lives as long as the + // guard does and cannot escape the scope. If it's created via `&mut (*#slot).#ident` + // like the unaligned field guard, it will become effectively `'static`. + let accessor = if pinned { + let project_ident = format_ident!("__project_{ident}"); + quote! { + // SAFETY: the initialization is pinned. + unsafe { #data.#project_ident(#guard.let_binding()) } + } + } else { + quote! { + #guard.let_binding() + } + }; + res.extend(quote! { #(#cfgs)* - // Create the drop guard: + // Create the drop guard. // - // We rely on macro hygiene to make it impossible for users to access this local - // variable. - // SAFETY: We forget the guard later when initialization has succeeded. - let #guard = unsafe { + // SAFETY: + // - `&raw mut (*slot).#ident` is valid. + // - `make_field_check` checks that `&raw mut (*slot).#ident` is properly aligned. + // - `(*slot).#ident` has been initialized above. + // - We only need the ownership to the pointee back when initialization has + // succeeded, where we `forget` the guard. + let mut #guard = unsafe { ::pin_init::__internal::DropGuard::new( &raw mut (*slot).#ident ) }; + + #(#cfgs)* + #[allow(unused_variables)] + let #ident = #accessor; }); guards.push(guard); guard_attrs.push(cfgs); diff --git a/src/__internal.rs b/src/__internal.rs index 90adbdc1..5720a621 100644 --- a/src/__internal.rs +++ b/src/__internal.rs @@ -238,32 +238,42 @@ fn stack_init_reuse() { /// When a value of this type is dropped, it drops a `T`. /// /// Can be forgotten to prevent the drop. +/// +/// # Invariants +/// +/// - `ptr` is valid and properly aligned. +/// - `*ptr` is initialized and owned by this guard. pub struct DropGuard { ptr: *mut T, } impl DropGuard { - /// Creates a new [`DropGuard`]. It will [`ptr::drop_in_place`] `ptr` when it gets dropped. + /// Creates a drop guard and transfer the ownership of the pointer content. /// - /// # Safety + /// The ownership is only relinguished if the guard is forgotten via [`core::mem::forget`]. /// - /// `ptr` must be a valid pointer. + /// # Safety /// - /// It is the callers responsibility that `self` will only get dropped if the pointee of `ptr`: - /// - has not been dropped, - /// - is not accessible by any other means, - /// - will not be dropped by any other means. + /// - `ptr` is valid and properly aligned. + /// - `*ptr` is initialized, and the ownership is transferred to this guard. #[inline] pub unsafe fn new(ptr: *mut T) -> Self { + // INVARIANT: By safety requirement. Self { ptr } } + + /// Create a let binding for accessor use. + #[inline] + pub fn let_binding(&mut self) -> &mut T { + // SAFETY: Per type invariant. + unsafe { &mut *self.ptr } + } } impl Drop for DropGuard { #[inline] fn drop(&mut self) { - // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function - // ensuring that this operation is safe. + // SAFETY: `self.ptr` is valid, properly aligned and `*self.ptr` is owned by this guard. unsafe { ptr::drop_in_place(self.ptr) } } } diff --git a/tests/ui/compile-fail/init/accessor_lifetime.rs b/tests/ui/compile-fail/init/accessor_lifetime.rs new file mode 100644 index 00000000..21f13a04 --- /dev/null +++ b/tests/ui/compile-fail/init/accessor_lifetime.rs @@ -0,0 +1,14 @@ +use pin_init::*; + +struct Foo { + x: usize, +} + +fn main() { + let _ = init!(Foo { + x: 0, + _: { + let _: &'static usize = x; + }, + }); +} diff --git a/tests/ui/compile-fail/init/accessor_lifetime.stderr b/tests/ui/compile-fail/init/accessor_lifetime.stderr new file mode 100644 index 00000000..3c9e0847 --- /dev/null +++ b/tests/ui/compile-fail/init/accessor_lifetime.stderr @@ -0,0 +1,17 @@ +error[E0505]: cannot move out of value because it is borrowed + --> tests/ui/compile-fail/init/accessor_lifetime.rs:8:13 + | + 8 | let _ = init!(Foo { + | _____________^ + 9 | | x: 0, +10 | | _: { +11 | | let _: &'static usize = x; + | | -------------- type annotation requires that borrow lasts for `'static` +12 | | }, +13 | | }); + | | ^ + | | | + | |______move out of value occurs here + | borrow of value occurs here + | + = note: this error originates in the macro `init` (in Nightly builds, run with -Z macro-backtrace for more info)