* [PATCH 1/7] rust: init: consolidate init macros
@ 2023-06-24 9:24 Benno Lossin
2023-06-24 9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
` (6 more replies)
0 siblings, 7 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 9:24 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Andreas Hindborg, rust-for-linux, linux-kernel,
patches
Merges the implementations of `try_init!` and `try_pin_init!`. These two
macros are very similar, but use different traits. The new macro
`__init_internal!` that is now the implementation for both takes these
traits as parameters.
This change does not affect any users, as no public API has been
changed, but it should simplify maintaining the init macros.
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/kernel/init.rs | 388 +++----------------------------------
rust/kernel/init/macros.rs | 237 +++++++++++++++++++++-
2 files changed, 259 insertions(+), 366 deletions(-)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index b4332a4ec1f4..d9a91950cba2 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -540,11 +540,14 @@ macro_rules! pin_init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_pin_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error(::core::convert::Infallible),
+ @data(PinData, use_data),
+ @has_data(HasPinData, __pin_data),
+ @construct_closure(pin_init_from_closure),
)
};
}
@@ -593,205 +596,29 @@ macro_rules! try_pin_init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_pin_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)? ),
@fields($($fields)*),
@error($crate::error::Error),
+ @data(PinData, use_data),
+ @has_data(HasPinData, __pin_data),
+ @construct_closure(pin_init_from_closure),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}? $err:ty) => {
- $crate::try_pin_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)? ),
@fields($($fields)*),
@error($err),
+ @data(PinData, use_data),
+ @has_data(HasPinData, __pin_data),
+ @construct_closure(pin_init_from_closure),
)
};
- (
- @this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
- @fields($($fields:tt)*),
- @error($err:ty),
- ) => {{
- // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
- // type and shadow it later when we insert the arbitrary user code. That way there will be
- // no possibility of returning without `unsafe`.
- struct __InitOk;
- // Get the pin data from the supplied type.
- let data = unsafe {
- use $crate::init::__internal::HasPinData;
- $t$(::<$($generics),*>)?::__pin_data()
- };
- // Ensure that `data` really is of type `PinData` and help with type inference:
- let init = $crate::init::__internal::PinData::make_closure::<_, __InitOk, $err>(
- data,
- move |slot| {
- {
- // Shadow the structure so it cannot be used to return early.
- struct __InitOk;
- // Create the `this` so it can be referenced by the user inside of the
- // expressions creating the individual fields.
- $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
- // Initialize every field.
- $crate::try_pin_init!(init_slot:
- @data(data),
- @slot(slot),
- @munch_fields($($fields)*,),
- );
- // 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)]
- if false {
- $crate::try_pin_init!(make_initializer:
- @slot(slot),
- @type_name($t),
- @munch_fields($($fields)*,),
- @acc(),
- );
- }
- // Forget all guards, since initialization was a success.
- $crate::try_pin_init!(forget_guards:
- @munch_fields($($fields)*,),
- );
- }
- Ok(__InitOk)
- }
- );
- let init = move |slot| -> ::core::result::Result<(), $err> {
- init(slot).map(|__InitOk| ())
- };
- let init = unsafe { $crate::init::pin_init_from_closure::<_, $err>(init) };
- init
- }};
- (init_slot:
- @data($data:ident),
- @slot($slot:ident),
- @munch_fields($(,)?),
- ) => {
- // Endpoint of munching, no fields are left.
- };
- (init_slot:
- @data($data:ident),
- @slot($slot:ident),
- // In-place initialization syntax.
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- let $field = $val;
- // Call the initializer.
- //
- // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
- // return when an error/panic occurs.
- // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
- unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
- // Create the drop guard.
- //
- // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_pin_init!(init_slot:
- @data($data),
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (init_slot:
- @data($data:ident),
- @slot($slot:ident),
- // Direct value init, this is safe for every field.
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- $(let $field = $val;)?
- // Initialize the field.
- //
- // SAFETY: The memory at `slot` is uninitialized.
- unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
- // Create the drop guard:
- //
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_pin_init!(init_slot:
- @data($data),
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($(,)?),
- @acc($($acc:tt)*),
- ) => {
- // Endpoint, nothing more to munch, create the initializer.
- // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
- // get the correct type inference here:
- unsafe {
- ::core::ptr::write($slot, $t {
- $($acc)*
- });
- }
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_pin_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)* $field: ::core::panic!(),),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_pin_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)* $field: ::core::panic!(),),
- );
- };
- (forget_guards:
- @munch_fields($(,)?),
- ) => {
- // Munching finished.
- };
- (forget_guards:
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_pin_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
- (forget_guards:
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_pin_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
}
/// Construct an in-place initializer for `struct`s.
@@ -816,11 +643,14 @@ macro_rules! init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error(::core::convert::Infallible),
+ @data(InitData, /*no use_data*/),
+ @has_data(HasInitData, __init_data),
+ @construct_closure(init_from_closure),
)
}
}
@@ -863,199 +693,29 @@ macro_rules! try_init {
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}) => {
- $crate::try_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error($crate::error::Error),
+ @data(InitData, /*no use_data*/),
+ @has_data(HasInitData, __init_data),
+ @construct_closure(init_from_closure),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
$($fields:tt)*
}? $err:ty) => {
- $crate::try_init!(
+ $crate::__init_internal!(
@this($($this)?),
@typ($t $(::<$($generics),*>)?),
@fields($($fields)*),
@error($err),
+ @data(InitData, /*no use_data*/),
+ @has_data(HasInitData, __init_data),
+ @construct_closure(init_from_closure),
)
};
- (
- @this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
- @fields($($fields:tt)*),
- @error($err:ty),
- ) => {{
- // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
- // type and shadow it later when we insert the arbitrary user code. That way there will be
- // no possibility of returning without `unsafe`.
- struct __InitOk;
- // Get the init data from the supplied type.
- let data = unsafe {
- use $crate::init::__internal::HasInitData;
- $t$(::<$($generics),*>)?::__init_data()
- };
- // Ensure that `data` really is of type `InitData` and help with type inference:
- let init = $crate::init::__internal::InitData::make_closure::<_, __InitOk, $err>(
- data,
- move |slot| {
- {
- // Shadow the structure so it cannot be used to return early.
- struct __InitOk;
- // Create the `this` so it can be referenced by the user inside of the
- // expressions creating the individual fields.
- $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
- // Initialize every field.
- $crate::try_init!(init_slot:
- @slot(slot),
- @munch_fields($($fields)*,),
- );
- // 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)]
- if false {
- $crate::try_init!(make_initializer:
- @slot(slot),
- @type_name($t),
- @munch_fields($($fields)*,),
- @acc(),
- );
- }
- // Forget all guards, since initialization was a success.
- $crate::try_init!(forget_guards:
- @munch_fields($($fields)*,),
- );
- }
- Ok(__InitOk)
- }
- );
- let init = move |slot| -> ::core::result::Result<(), $err> {
- init(slot).map(|__InitOk| ())
- };
- let init = unsafe { $crate::init::init_from_closure::<_, $err>(init) };
- init
- }};
- (init_slot:
- @slot($slot:ident),
- @munch_fields( $(,)?),
- ) => {
- // Endpoint of munching, no fields are left.
- };
- (init_slot:
- @slot($slot:ident),
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- let $field = $val;
- // Call the initializer.
- //
- // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
- // return when an error/panic occurs.
- unsafe {
- $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))?;
- }
- // Create the drop guard.
- //
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_init!(init_slot:
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (init_slot:
- @slot($slot:ident),
- // Direct value init.
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- $(let $field = $val;)?
- // Call the initializer.
- //
- // SAFETY: The memory at `slot` is uninitialized.
- unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
- // Create the drop guard.
- //
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
- //
- // SAFETY: We forget the guard later when initialization has succeeded.
- let $field = &unsafe {
- $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
- };
-
- $crate::try_init!(init_slot:
- @slot($slot),
- @munch_fields($($rest)*),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields( $(,)?),
- @acc($($acc:tt)*),
- ) => {
- // Endpoint, nothing more to munch, create the initializer.
- // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
- // get the correct type inference here:
- unsafe {
- ::core::ptr::write($slot, $t {
- $($acc)*
- });
- }
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)*$field: ::core::panic!(),),
- );
- };
- (make_initializer:
- @slot($slot:ident),
- @type_name($t:ident),
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- @acc($($acc:tt)*),
- ) => {
- $crate::try_init!(make_initializer:
- @slot($slot),
- @type_name($t),
- @munch_fields($($rest)*),
- @acc($($acc)*$field: ::core::panic!(),),
- );
- };
- (forget_guards:
- @munch_fields($(,)?),
- ) => {
- // Munching finished.
- };
- (forget_guards:
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
- (forget_guards:
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::try_init!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
}
/// A pin-initializer for the type `T`.
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 00aa4e956c0a..fbaebd34f218 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -1,10 +1,12 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT
//! This module provides the macros that actually implement the proc-macros `pin_data` and
-//! `pinned_drop`.
+//! `pinned_drop`. It also contains `__init_internal` the implementation of the `{try_}{pin_}init!`
+//! macros.
//!
//! These macros should never be called directly, since they expect their input to be
-//! in a certain format which is internal. Use the proc-macros instead.
+//! in a certain format which is internal. If used incorrectly, these macros can lead to UB even in
+//! safe code! Use the public facing macros instead.
//!
//! This architecture has been chosen because the kernel does not yet have access to `syn` which
//! would make matters a lot easier for implementing these as proc-macros.
@@ -980,3 +982,234 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
}
};
}
+
+/// The internal init macro. Do not call manually!
+///
+/// This is called by the `{try_}{pin_}init!` macros with various inputs.
+///
+/// This macro has multiple internal call configurations, these are always the very first ident:
+/// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
+/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
+/// - `make_initializer`: recursively create the struct initializer that guarantees that every
+/// field has been initialized exactly once.
+/// - `forget_guards`: recursively forget the drop guards for every field.
+#[doc(hidden)]
+#[macro_export]
+macro_rules! __init_internal {
+ (
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ ) => {{
+ // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
+ // type and shadow it later when we insert the arbitrary user code. That way there will be
+ // no possibility of returning without `unsafe`.
+ struct __InitOk;
+ // Get the data about fields from the supplied type.
+ let data = unsafe {
+ use $crate::init::__internal::$has_data;
+ $t$(::<$($generics),*>)?::$get_data()
+ };
+ // Ensure that `data` really is of type `$data` and help with type inference:
+ let init = $crate::init::__internal::$data::make_closure::<_, __InitOk, $err>(
+ data,
+ move |slot| {
+ {
+ // Shadow the structure so it cannot be used to return early.
+ struct __InitOk;
+ // Create the `this` so it can be referenced by the user inside of the
+ // expressions creating the individual fields.
+ $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
+ // Initialize every field.
+ $crate::__init_internal!(init_slot($($use_data)?):
+ @data(data),
+ @slot(slot),
+ @munch_fields($($fields)*,),
+ );
+ // 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)]
+ if false {
+ $crate::__init_internal!(make_initializer:
+ @slot(slot),
+ @type_name($t),
+ @munch_fields($($fields)*,),
+ @acc(),
+ );
+ }
+ // Forget all guards, since initialization was a success.
+ $crate::__init_internal!(forget_guards:
+ @munch_fields($($fields)*,),
+ );
+ }
+ Ok(__InitOk)
+ }
+ );
+ let init = move |slot| -> ::core::result::Result<(), $err> {
+ init(slot).map(|__InitOk| ())
+ };
+ let init = unsafe { $crate::init::$construct_closure::<_, $err>(init) };
+ init
+ }};
+ (init_slot($($use_data:ident)?):
+ @data($data:ident),
+ @slot($slot:ident),
+ @munch_fields($(,)?),
+ ) => {
+ // Endpoint of munching, no fields are left.
+ };
+ (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
+ @data($data:ident),
+ @slot($slot:ident),
+ // In-place initialization syntax.
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ ) => {
+ let $field = $val;
+ // Call the initializer.
+ //
+ // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
+ // return when an error/panic occurs.
+ // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
+ unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
+ // Create the drop guard.
+ //
+ // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ //
+ // SAFETY: We forget the guard later when initialization has succeeded.
+ let $field = &unsafe {
+ $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
+ };
+
+ $crate::__init_internal!(init_slot($use_data):
+ @data($data),
+ @slot($slot),
+ @munch_fields($($rest)*),
+ );
+ };
+ (init_slot(): // no use_data, so we use `Init::__init` directly.
+ @data($data:ident),
+ @slot($slot:ident),
+ // In-place initialization syntax.
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ ) => {
+ let $field = $val;
+ // Call the initializer.
+ //
+ // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
+ // return when an error/panic occurs.
+ unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
+ // Create the drop guard.
+ //
+ // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ //
+ // SAFETY: We forget the guard later when initialization has succeeded.
+ let $field = &unsafe {
+ $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
+ };
+
+ $crate::__init_internal!(init_slot():
+ @data($data),
+ @slot($slot),
+ @munch_fields($($rest)*),
+ );
+ };
+ (init_slot($($use_data:ident)?):
+ @data($data:ident),
+ @slot($slot:ident),
+ // Init by-value.
+ @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
+ ) => {
+ $(let $field = $val;)?
+ // Initialize the field.
+ //
+ // SAFETY: The memory at `slot` is uninitialized.
+ unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
+ // Create the drop guard:
+ //
+ // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
+ //
+ // SAFETY: We forget the guard later when initialization has succeeded.
+ let $field = &unsafe {
+ $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
+ };
+
+ $crate::__init_internal!(init_slot($($use_data)?):
+ @data($data),
+ @slot($slot),
+ @munch_fields($($rest)*),
+ );
+ };
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields($(,)?),
+ @acc($($acc:tt)*),
+ ) => {
+ // Endpoint, nothing more to munch, create the initializer.
+ // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
+ // get the correct type inference here:
+ unsafe {
+ ::core::ptr::write($slot, $t {
+ $($acc)*
+ });
+ }
+ };
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ @acc($($acc:tt)*),
+ ) => {
+ $crate::__init_internal!(make_initializer:
+ @slot($slot),
+ @type_name($t),
+ @munch_fields($($rest)*),
+ @acc($($acc)* $field: ::core::panic!(),),
+ );
+ };
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
+ @acc($($acc:tt)*),
+ ) => {
+ $crate::__init_internal!(make_initializer:
+ @slot($slot),
+ @type_name($t),
+ @munch_fields($($rest)*),
+ @acc($($acc)* $field: ::core::panic!(),),
+ );
+ };
+ (forget_guards:
+ @munch_fields($(,)?),
+ ) => {
+ // Munching finished.
+ };
+ (forget_guards:
+ @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
+ ) => {
+ unsafe { $crate::init::__internal::DropGuard::forget($field) };
+
+ $crate::__init_internal!(forget_guards:
+ @munch_fields($($rest)*),
+ );
+ };
+ (forget_guards:
+ @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
+ ) => {
+ unsafe { $crate::init::__internal::DropGuard::forget($field) };
+
+ $crate::__init_internal!(forget_guards:
+ @munch_fields($($rest)*),
+ );
+ };
+}
base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
--
2.41.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 2/7] rust: add derive macro for `Zeroable`
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
@ 2023-06-24 9:25 ` Benno Lossin
2023-06-24 14:55 ` Björn Roy Baron
` (2 more replies)
2023-06-24 9:25 ` [PATCH 3/7] rust: init: make guards in the init macros hygienic Benno Lossin
` (5 subsequent siblings)
6 siblings, 3 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 9:25 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Andreas Hindborg, rust-for-linux, linux-kernel,
patches, Asahi Lina
Add a derive proc-macro for the `Zeroable` trait. The macro supports
structs where every field implements the `Zeroable` trait. This way
`unsafe` implementations can be avoided.
The macro is split into two parts:
- a proc-macro to parse generics into impl and ty generics,
- a declarative macro that expands to the impl block.
Suggested-by: Asahi Lina <lina@asahilina.net>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/kernel/init/macros.rs | 28 ++++++++++++++++++++++++++++
rust/kernel/prelude.rs | 2 +-
rust/macros/lib.rs | 20 ++++++++++++++++++++
rust/macros/quote.rs | 6 ++++++
rust/macros/zeroable.rs | 25 +++++++++++++++++++++++++
5 files changed, 80 insertions(+), 1 deletion(-)
create mode 100644 rust/macros/zeroable.rs
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index fbaebd34f218..e8165ff53a94 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -1213,3 +1213,31 @@ macro_rules! __init_internal {
);
};
}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! __derive_zeroable {
+ (parse_input:
+ @sig(
+ $(#[$($struct_attr:tt)*])*
+ $vis:vis struct $name:ident
+ $(where $($whr:tt)*)?
+ ),
+ @impl_generics($($impl_generics:tt)*),
+ @ty_generics($($ty_generics:tt)*),
+ @body({
+ $(
+ $(#[$($field_attr:tt)*])*
+ $field:ident : $field_ty:ty
+ ),* $(,)?
+ }),
+ ) => {
+ // SAFETY: every field type implements `Zeroable` and padding bytes may be zero.
+ #[automatically_derived]
+ unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
+ where
+ $($field_ty: $crate::Zeroable,)*
+ $($($whr)*)?
+ {}
+ };
+}
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index c28587d68ebc..ae21600970b3 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -18,7 +18,7 @@
pub use alloc::{boxed::Box, vec::Vec};
#[doc(no_inline)]
-pub use macros::{module, pin_data, pinned_drop, vtable};
+pub use macros::{module, pin_data, pinned_drop, vtable, Zeroable};
pub use super::build_assert;
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 3fc74cb4ea19..9f056a5c780a 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -10,6 +10,7 @@
mod pin_data;
mod pinned_drop;
mod vtable;
+mod zeroable;
use proc_macro::TokenStream;
@@ -246,3 +247,22 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
pinned_drop::pinned_drop(args, input)
}
+
+/// Derives the [`Zeroable`] trait for the given struct.
+///
+/// This can only be used for structs where every field implements the [`Zeroable`] trait.
+///
+/// # Examples
+///
+/// ```rust
+/// #[derive(Zeroable)]
+/// pub struct DriverData {
+/// id: i64,
+/// buf_ptr: *mut u8,
+/// len: usize,
+/// }
+/// ```
+#[proc_macro_derive(Zeroable)]
+pub fn derive_zeroable(input: TokenStream) -> TokenStream {
+ zeroable::derive(input)
+}
diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
index dddbb4e6f4cb..b76c198a4ed5 100644
--- a/rust/macros/quote.rs
+++ b/rust/macros/quote.rs
@@ -124,6 +124,12 @@ macro_rules! quote_spanned {
));
quote_spanned!(@proc $v $span $($tt)*);
};
+ (@proc $v:ident $span:ident ; $($tt:tt)*) => {
+ $v.push(::proc_macro::TokenTree::Punct(
+ ::proc_macro::Punct::new(';', ::proc_macro::Spacing::Alone)
+ ));
+ quote_spanned!(@proc $v $span $($tt)*);
+ };
(@proc $v:ident $span:ident $id:ident $($tt:tt)*) => {
$v.push(::proc_macro::TokenTree::Ident(::proc_macro::Ident::new(stringify!($id), $span)));
quote_spanned!(@proc $v $span $($tt)*);
diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
new file mode 100644
index 000000000000..cddb866c44ef
--- /dev/null
+++ b/rust/macros/zeroable.rs
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use crate::helpers::{parse_generics, Generics};
+use proc_macro::TokenStream;
+
+pub(crate) fn derive(input: TokenStream) -> TokenStream {
+ let (
+ Generics {
+ impl_generics,
+ ty_generics,
+ },
+ mut rest,
+ ) = parse_generics(input);
+ // This should be the body of the struct `{...}`.
+ let last = rest.pop();
+ quote! {
+ ::kernel::__derive_zeroable!(
+ parse_input:
+ @sig(#(#rest)*),
+ @impl_generics(#(#impl_generics)*),
+ @ty_generics(#(#ty_generics)*),
+ @body(#last),
+ );
+ }
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 3/7] rust: init: make guards in the init macros hygienic
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
2023-06-24 9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
@ 2023-06-24 9:25 ` Benno Lossin
2023-06-24 14:58 ` Björn Roy Baron
2023-06-25 20:54 ` Gary Guo
2023-06-24 9:25 ` [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure Benno Lossin
` (4 subsequent siblings)
6 siblings, 2 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 9:25 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Andreas Hindborg, rust-for-linux, linux-kernel,
patches, Asahi Lina
Use hygienic identifiers for the guards instead of the field names. This
makes the init macros feel more like normal struct initializers, since
assigning identifiers with the name of a field does not create
conflicts.
Also change the internals of the guards, no need to make the `forget`
function `unsafe`, since users cannot access the guards anyways. Now the
guards are carried directly on the stack and have no extra `Cell<bool>`
field that marks if they have been forgotten or not, instead they are
just forgotten via `mem::forget`.
Suggested-by: Asahi Lina <lina@asahilina.net>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/kernel/init.rs | 1 -
rust/kernel/init/__internal.rs | 25 +++------------
rust/kernel/init/macros.rs | 56 ++++++++++++----------------------
3 files changed, 23 insertions(+), 59 deletions(-)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index d9a91950cba2..ecf6a4bd0ce4 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -206,7 +206,6 @@
use alloc::boxed::Box;
use core::{
alloc::AllocError,
- cell::Cell,
convert::Infallible,
marker::PhantomData,
mem::MaybeUninit,
diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
index 44751fb62b51..7abd1fb65e41 100644
--- a/rust/kernel/init/__internal.rs
+++ b/rust/kernel/init/__internal.rs
@@ -174,7 +174,6 @@ pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mu
/// Can be forgotten to prevent the drop.
pub struct DropGuard<T: ?Sized> {
ptr: *mut T,
- do_drop: Cell<bool>,
}
impl<T: ?Sized> DropGuard<T> {
@@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> {
/// - will not be dropped by any other means.
#[inline]
pub unsafe fn new(ptr: *mut T) -> Self {
- Self {
- ptr,
- do_drop: Cell::new(true),
- }
- }
-
- /// Prevents this guard from dropping the supplied pointer.
- ///
- /// # Safety
- ///
- /// This function is unsafe in order to prevent safe code from forgetting this guard. It should
- /// only be called by the macros in this module.
- #[inline]
- pub unsafe fn forget(&self) {
- self.do_drop.set(false);
+ Self { ptr }
}
}
impl<T: ?Sized> Drop for DropGuard<T> {
#[inline]
fn drop(&mut self) {
- if self.do_drop.get() {
- // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
- // ensuring that this operation is safe.
- unsafe { ptr::drop_in_place(self.ptr) }
- }
+ // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
+ // ensuring that this operation is safe.
+ unsafe { ptr::drop_in_place(self.ptr) }
}
}
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index e8165ff53a94..df4281743175 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -992,7 +992,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
/// - `make_initializer`: recursively create the struct initializer that guarantees that every
/// field has been initialized exactly once.
-/// - `forget_guards`: recursively forget the drop guards for every field.
#[doc(hidden)]
#[macro_export]
macro_rules! __init_internal {
@@ -1032,6 +1031,7 @@ macro_rules! __init_internal {
$crate::__init_internal!(init_slot($($use_data)?):
@data(data),
@slot(slot),
+ @guards(),
@munch_fields($($fields)*,),
);
// We use unreachable code to ensure that all fields have been mentioned exactly
@@ -1046,10 +1046,6 @@ macro_rules! __init_internal {
@acc(),
);
}
- // Forget all guards, since initialization was a success.
- $crate::__init_internal!(forget_guards:
- @munch_fields($($fields)*,),
- );
}
Ok(__InitOk)
}
@@ -1063,13 +1059,17 @@ macro_rules! __init_internal {
(init_slot($($use_data:ident)?):
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
@munch_fields($(,)?),
) => {
- // Endpoint of munching, no fields are left.
+ // Endpoint of munching, no fields are left. If execution reaches this point, all fields
+ // have been initialized. Therefore we can now dismiss the guards by forgetting them.
+ $(::core::mem::forget($guards);)*
};
(init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
// In-place initialization syntax.
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
) => {
@@ -1080,24 +1080,26 @@ macro_rules! __init_internal {
// return when an error/panic occurs.
// We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
- // Create the drop guard.
+ // Create the drop guard:
//
- // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ // 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 $field = &unsafe {
+ let guard = unsafe {
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot($use_data):
@data($data),
@slot($slot),
+ @guards(guard, $($guards,)*),
@munch_fields($($rest)*),
);
};
(init_slot(): // no use_data, so we use `Init::__init` directly.
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
// In-place initialization syntax.
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
) => {
@@ -1107,24 +1109,26 @@ macro_rules! __init_internal {
// SAFETY: `slot` is valid, because we are inside of an initializer closure, we
// return when an error/panic occurs.
unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
- // Create the drop guard.
+ // Create the drop guard:
//
- // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
+ // 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 $field = &unsafe {
+ let guard = unsafe {
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot():
@data($data),
@slot($slot),
+ @guards(guard, $($guards,)*),
@munch_fields($($rest)*),
);
};
(init_slot($($use_data:ident)?):
@data($data:ident),
@slot($slot:ident),
+ @guards($($guards:ident,)*),
// Init by-value.
@munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
) => {
@@ -1135,16 +1139,17 @@ macro_rules! __init_internal {
unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
// Create the drop guard:
//
- // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
+ // 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 $field = &unsafe {
+ let guard = unsafe {
$crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot($($use_data)?):
@data($data),
@slot($slot),
+ @guards(guard, $($guards,)*),
@munch_fields($($rest)*),
);
};
@@ -1189,29 +1194,6 @@ macro_rules! __init_internal {
@acc($($acc)* $field: ::core::panic!(),),
);
};
- (forget_guards:
- @munch_fields($(,)?),
- ) => {
- // Munching finished.
- };
- (forget_guards:
- @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::__init_internal!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
- (forget_guards:
- @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
- ) => {
- unsafe { $crate::init::__internal::DropGuard::forget($field) };
-
- $crate::__init_internal!(forget_guards:
- @munch_fields($($rest)*),
- );
- };
}
#[doc(hidden)]
--
2.41.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
2023-06-24 9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
2023-06-24 9:25 ` [PATCH 3/7] rust: init: make guards in the init macros hygienic Benno Lossin
@ 2023-06-24 9:25 ` Benno Lossin
2023-06-24 15:03 ` Björn Roy Baron
2023-06-24 9:25 ` [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
` (3 subsequent siblings)
6 siblings, 1 reply; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 9:25 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Andreas Hindborg, rust-for-linux, linux-kernel,
patches
In the implementation of the init macros there is a `if false` statement
that type checks the initializer to ensure every field is initialized.
Since the next patch has a stack variable to store the struct, the
function might allocate too much memory on debug builds. Putting the
struct into a closure ensures that even in debug builds no stack
overflow error is caused. In release builds this was not a problem since
the code was optimized away due to the `if false`.
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/kernel/init/macros.rs | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index df4281743175..1e0c4aca055a 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -1037,14 +1037,18 @@ macro_rules! __init_internal {
// 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)]
+ #[allow(unreachable_code,
+ clippy::diverging_sub_expression,
+ clippy::redundant_closure_call)]
if false {
- $crate::__init_internal!(make_initializer:
- @slot(slot),
- @type_name($t),
- @munch_fields($($fields)*,),
- @acc(),
- );
+ (|| {
+ $crate::__init_internal!(make_initializer:
+ @slot(slot),
+ @type_name($t),
+ @munch_fields($($fields)*,),
+ @acc(),
+ );
+ })();
}
}
Ok(__InitOk)
--
2.41.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
` (2 preceding siblings ...)
2023-06-24 9:25 ` [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure Benno Lossin
@ 2023-06-24 9:25 ` Benno Lossin
2023-06-24 15:11 ` Björn Roy Baron
` (2 more replies)
2023-06-24 9:25 ` [PATCH 6/7] rust: init: Add functions to create array initializers Benno Lossin
` (2 subsequent siblings)
6 siblings, 3 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 9:25 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Andreas Hindborg, rust-for-linux, linux-kernel,
patches, Asahi Lina
Add the struct update syntax to the init macros, but only for
`..Zeroable::zeroed()`. Adding this at the end of the struct initializer
allows one to omit fields from the initializer, these fields will be
initialized with 0x00 set to every byte. Only types that implement the
`Zeroable` trait can utilize this.
Suggested-by: Asahi Lina <lina@asahilina.net>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/kernel/init.rs | 16 +++++-
rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
2 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index ecf6a4bd0ce4..44bc3e77419a 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
/// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
/// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
/// pointer named `this` inside of the initializer.
+/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
+/// struct, this initializes every field with 0 and then runs all initializers specified in the
+/// body. This can only be done if [`Zeroable`] is implemented for the struct.
///
/// For instance:
///
/// ```rust
/// # use kernel::pin_init;
-/// # use macros::pin_data;
+/// # use macros::{pin_data, Zeroable};
/// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
/// #[pin_data]
+/// #[derive(Zeroable)]
/// struct Buf {
/// // `ptr` points into `buf`.
/// ptr: *mut u8,
@@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
/// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
/// pin: PhantomPinned,
/// });
+/// pin_init!(Buf {
+/// buf: [1; 64],
+/// ..Zeroable::zeroed(),
+/// });
/// ```
///
/// [`try_pin_init!`]: kernel::try_pin_init
@@ -547,6 +555,7 @@ macro_rules! pin_init {
@data(PinData, use_data),
@has_data(HasPinData, __pin_data),
@construct_closure(pin_init_from_closure),
+ @munch_fields($($fields)*),
)
};
}
@@ -603,6 +612,7 @@ macro_rules! try_pin_init {
@data(PinData, use_data),
@has_data(HasPinData, __pin_data),
@construct_closure(pin_init_from_closure),
+ @munch_fields($($fields)*),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
@@ -616,6 +626,7 @@ macro_rules! try_pin_init {
@data(PinData, use_data),
@has_data(HasPinData, __pin_data),
@construct_closure(pin_init_from_closure),
+ @munch_fields($($fields)*),
)
};
}
@@ -650,6 +661,7 @@ macro_rules! init {
@data(InitData, /*no use_data*/),
@has_data(HasInitData, __init_data),
@construct_closure(init_from_closure),
+ @munch_fields($($fields)*),
)
}
}
@@ -700,6 +712,7 @@ macro_rules! try_init {
@data(InitData, /*no use_data*/),
@has_data(HasInitData, __init_data),
@construct_closure(init_from_closure),
+ @munch_fields($($fields)*),
)
};
($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
@@ -713,6 +726,7 @@ macro_rules! try_init {
@data(InitData, /*no use_data*/),
@has_data(HasInitData, __init_data),
@construct_closure(init_from_closure),
+ @munch_fields($($fields)*),
)
};
}
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 1e0c4aca055a..5dcb2e513f26 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
///
/// This macro has multiple internal call configurations, these are always the very first ident:
/// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
+/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
/// - `make_initializer`: recursively create the struct initializer that guarantees that every
/// field has been initialized exactly once.
@@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
@has_data($has_data:ident, $get_data:ident),
// `pin_init_from_closure` or `init_from_closure`.
@construct_closure($construct_closure:ident),
+ @munch_fields(),
+ ) => {
+ $crate::__init_internal!(with_update_parsed:
+ @this($($this)?),
+ @typ($t $(::<$($generics),*>)? ),
+ @fields($($fields)*),
+ @error($err),
+ @data($data, $($use_data)?),
+ @has_data($has_data, $get_data),
+ @construct_closure($construct_closure),
+ @zeroed(), // nothing means default behavior.
+ )
+ };
+ (
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ @munch_fields(..Zeroable::zeroed()),
+ ) => {
+ $crate::__init_internal!(with_update_parsed:
+ @this($($this)?),
+ @typ($t $(::<$($generics),*>)? ),
+ @fields($($fields)*),
+ @error($err),
+ @data($data, $($use_data)?),
+ @has_data($has_data, $get_data),
+ @construct_closure($construct_closure),
+ @zeroed(()), // `()` means zero all fields not mentioned.
+ )
+ };
+ (
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ @munch_fields($ignore:tt $($rest:tt)*),
+ ) => {
+ $crate::__init_internal!(
+ @this($($this)?),
+ @typ($t $(::<$($generics),*>)? ),
+ @fields($($fields)*),
+ @error($err),
+ @data($data, $($use_data)?),
+ @has_data($has_data, $get_data),
+ @construct_closure($construct_closure),
+ @munch_fields($($rest)*),
+ )
+ };
+ (with_update_parsed:
+ @this($($this:ident)?),
+ @typ($t:ident $(::<$($generics:ty),*>)?),
+ @fields($($fields:tt)*),
+ @error($err:ty),
+ // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
+ // case.
+ @data($data:ident, $($use_data:ident)?),
+ // `HasPinData` or `HasInitData`.
+ @has_data($has_data:ident, $get_data:ident),
+ // `pin_init_from_closure` or `init_from_closure`.
+ @construct_closure($construct_closure:ident),
+ @zeroed($($init_zeroed:expr)?),
) => {{
// We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
// type and shadow it later when we insert the arbitrary user code. That way there will be
@@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
{
// Shadow the structure so it cannot be used to return early.
struct __InitOk;
+ // If `$init_zeroed` is present we should zero the slot now and not emit an
+ // error when fields are missing (since they will be zeroed). We also have to
+ // check that the type actually implements `Zeroable`.
+ $(
+ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
+ // Ensure that the struct is indeed `Zeroable`.
+ is_zeroable(slot);
+ // SAFETY: The type implements `Zeroable` by the check above.
+ unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
+ $init_zeroed // this will be `()` if set.
+ )?
// Create the `this` so it can be referenced by the user inside of the
// expressions creating the individual fields.
$(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
@@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
@data($data:ident),
@slot($slot:ident),
@guards($($guards:ident,)*),
- @munch_fields($(,)?),
+ @munch_fields($(..Zeroable::zeroed())? $(,)?),
) => {
// Endpoint of munching, no fields are left. If execution reaches this point, all fields
// have been initialized. Therefore we can now dismiss the guards by forgetting them.
@@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
@munch_fields($($rest)*),
);
};
+ (make_initializer:
+ @slot($slot:ident),
+ @type_name($t:ident),
+ @munch_fields(..Zeroable::zeroed() $(,)?),
+ @acc($($acc:tt)*),
+ ) => {
+ // Endpoint, nothing more to munch, create the initializer. Since the users specified
+ // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
+ // not been overwritten are thus zero and initialized. We still check that all fields are
+ // actually accessible by using the struct update syntax ourselves.
+ // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
+ // get the correct type inference here:
+ unsafe {
+ let mut zeroed = ::core::mem::zeroed();
+ // We have to use type inference her to make zeroed have the correct type. This does
+ // not get executed, so it has no effect.
+ ::core::ptr::write($slot, zeroed);
+ zeroed = ::core::mem::zeroed();
+ ::core::ptr::write($slot, $t {
+ $($acc)*
+ ..zeroed
+ });
+ }
+ };
(make_initializer:
@slot($slot:ident),
@type_name($t:ident),
--
2.41.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 6/7] rust: init: Add functions to create array initializers
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
` (3 preceding siblings ...)
2023-06-24 9:25 ` [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
@ 2023-06-24 9:25 ` Benno Lossin
2023-06-24 15:17 ` Björn Roy Baron
2023-07-03 12:03 ` Alice Ryhl
2023-06-24 9:25 ` [PATCH 7/7] rust: init: add support for arbitrary paths in init macros Benno Lossin
2023-06-24 14:49 ` [PATCH 1/7] rust: init: consolidate " Björn Roy Baron
6 siblings, 2 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 9:25 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Andreas Hindborg, rust-for-linux, linux-kernel,
patches, Asahi Lina
Add two functions `pin_init_array_from_fn` and `init_array_from_fn` that
take a function that generates initializers for `T` from usize, the added
functions then return an initializer for `[T; N]` where every element is
initialized by an element returned from the generator function.
Suggested-by: Asahi Lina <lina@asahilina.net>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/kernel/init.rs | 90 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 44bc3e77419a..c9ea4bf71987 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -867,6 +867,96 @@ pub fn uninit<T, E>() -> impl Init<MaybeUninit<T>, E> {
unsafe { init_from_closure(|_| Ok(())) }
}
+/// Initializes an array by initializing each element via the provided initializer.
+///
+/// # Examples
+///
+/// ```rust
+/// let array: Box<[usize; 1000_000_000]>= Box::init(init_array_from_fn(|i| i)).unwrap();
+/// println!("{array:?}");
+/// ```
+pub fn init_array_from_fn<I, const N: usize, T, E>(
+ mut make_init: impl FnMut(usize) -> I,
+) -> impl Init<[T; N], E>
+where
+ I: Init<T, E>,
+{
+ let init = move |slot: *mut [T; N]| {
+ let slot = slot.cast::<T>();
+ for i in 0..N {
+ let init = make_init(i);
+ // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
+ let ptr = unsafe { slot.add(i) };
+ // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init`
+ // requirements.
+ match unsafe { init.__init(ptr) } {
+ Ok(()) => {}
+ Err(e) => {
+ // We now free every element that has been initialized before:
+ for j in 0..i {
+ let ptr = unsafe { slot.add(j) };
+ // SAFETY: The value was initialized in a previous iteration of the loop
+ // and since we return `Err` below, the caller will consider the memory at
+ // `slot` as uninitialized.
+ unsafe { ptr::drop_in_place(ptr) };
+ }
+ return Err(e);
+ }
+ }
+ }
+ Ok(())
+ };
+ // SAFETY: The initializer above initializes every element of the array. On failure it drops
+ // any initialized elements and returns `Err`.
+ unsafe { init_from_closure(init) }
+}
+
+/// Initializes an array by initializing each element via the provided initializer.
+///
+/// # Examples
+///
+/// ```rust
+/// let array: Arc<[Mutex<usize>; 1000_000_000]>=
+/// Arc::pin_init(init_array_from_fn(|i| new_mutex!(i))).unwrap();
+/// println!("{array:?}");
+/// ```
+pub fn pin_init_array_from_fn<I, const N: usize, T, E>(
+ mut make_init: impl FnMut(usize) -> I,
+) -> impl PinInit<[T; N], E>
+where
+ I: PinInit<T, E>,
+{
+ let init = move |slot: *mut [T; N]| {
+ let slot = slot.cast::<T>();
+ for i in 0..N {
+ let init = make_init(i);
+ // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
+ let ptr = unsafe { slot.add(i) };
+ // SAFETY: The pointer is derived from `slot` and thus satisfies the `__pinned_init`
+ // requirements.
+ match unsafe { init.__pinned_init(ptr) } {
+ Ok(()) => {}
+ Err(e) => {
+ // We now have to free every element that has been initialized before, since we
+ // have to abide by the drop guarantee.
+ for j in 0..i {
+ let ptr = unsafe { slot.add(j) };
+ // SAFETY: The value was initialized in a previous iteration of the loop
+ // and since we return `Err` below, the caller will consider the memory at
+ // `slot` as uninitialized.
+ unsafe { ptr::drop_in_place(ptr) };
+ }
+ return Err(e);
+ }
+ }
+ }
+ Ok(())
+ };
+ // SAFETY: The initializer above initializes every element of the array. On failure it drops
+ // any initialized elements and returns `Err`.
+ unsafe { pin_init_from_closure(init) }
+}
+
// SAFETY: Every type can be initialized by-value.
unsafe impl<T, E> Init<T, E> for T {
unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
--
2.41.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 7/7] rust: init: add support for arbitrary paths in init macros
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
` (4 preceding siblings ...)
2023-06-24 9:25 ` [PATCH 6/7] rust: init: Add functions to create array initializers Benno Lossin
@ 2023-06-24 9:25 ` Benno Lossin
2023-06-24 15:20 ` Björn Roy Baron
2023-06-25 21:01 ` Gary Guo
2023-06-24 14:49 ` [PATCH 1/7] rust: init: consolidate " Björn Roy Baron
6 siblings, 2 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 9:25 UTC (permalink / raw)
To: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Andreas Hindborg, rust-for-linux, linux-kernel,
patches, Asahi Lina
Previously only `ident` and generic types were supported in the
`{try_}{pin_}init!` macros. This patch allows arbitrary path fragments,
so for example `Foo::Bar` but also very complex paths such as
`<Foo as Baz>::Bar::<0, i32>`.
Internally this is accomplished by using `path` fragments. Due to some
peculiar declarative macro limitations, we have to "forget" certain
additional parsing information in the token trees. This is achieved by
the new `retokenize` proc macro. It does not modify the input, but just
strips this information. For example, if a declarative macro takes
`$t:path` as its input, it cannot sensibly propagate this to a macro that
takes `$($p:tt)*` as its input, since the `$t` token will only be
considered one `tt` token for the second macro. If we first pipe the
tokens through `retokenize`, then it parses as expected.
Suggested-by: Asahi Lina <lina@asahilina.net>
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/kernel/init/__internal.rs | 2 ++
rust/kernel/init/macros.rs | 42 +++++++++++++++++++---------------
rust/macros/lib.rs | 17 +++++++++++++-
3 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
index 7abd1fb65e41..e36a706a4a1b 100644
--- a/rust/kernel/init/__internal.rs
+++ b/rust/kernel/init/__internal.rs
@@ -9,6 +9,8 @@
use super::*;
+pub use ::macros::retokenize;
+
/// See the [nomicon] for what subtyping is. See also [this table].
///
/// [nomicon]: https://doc.rust-lang.org/nomicon/subtyping.html
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 5dcb2e513f26..6a82be675808 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -998,7 +998,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
macro_rules! __init_internal {
(
@this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
+ @typ($t:path),
@fields($($fields:tt)*),
@error($err:ty),
// Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
@@ -1012,7 +1012,7 @@ macro_rules! __init_internal {
) => {
$crate::__init_internal!(with_update_parsed:
@this($($this)?),
- @typ($t $(::<$($generics),*>)? ),
+ @typ($t),
@fields($($fields)*),
@error($err),
@data($data, $($use_data)?),
@@ -1023,7 +1023,7 @@ macro_rules! __init_internal {
};
(
@this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
+ @typ($t:path),
@fields($($fields:tt)*),
@error($err:ty),
// Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
@@ -1037,7 +1037,7 @@ macro_rules! __init_internal {
) => {
$crate::__init_internal!(with_update_parsed:
@this($($this)?),
- @typ($t $(::<$($generics),*>)? ),
+ @typ($t),
@fields($($fields)*),
@error($err),
@data($data, $($use_data)?),
@@ -1048,7 +1048,7 @@ macro_rules! __init_internal {
};
(
@this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
+ @typ($t:path),
@fields($($fields:tt)*),
@error($err:ty),
// Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
@@ -1062,7 +1062,7 @@ macro_rules! __init_internal {
) => {
$crate::__init_internal!(
@this($($this)?),
- @typ($t $(::<$($generics),*>)? ),
+ @typ($t),
@fields($($fields)*),
@error($err),
@data($data, $($use_data)?),
@@ -1073,7 +1073,7 @@ macro_rules! __init_internal {
};
(with_update_parsed:
@this($($this:ident)?),
- @typ($t:ident $(::<$($generics:ty),*>)?),
+ @typ($t:path),
@fields($($fields:tt)*),
@error($err:ty),
// Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
@@ -1092,7 +1092,7 @@ macro_rules! __init_internal {
// Get the data about fields from the supplied type.
let data = unsafe {
use $crate::init::__internal::$has_data;
- $t$(::<$($generics),*>)?::$get_data()
+ $crate::init::__internal::retokenize!($t::$get_data())
};
// Ensure that `data` really is of type `$data` and help with type inference:
let init = $crate::init::__internal::$data::make_closure::<_, __InitOk, $err>(
@@ -1247,7 +1247,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
};
(make_initializer:
@slot($slot:ident),
- @type_name($t:ident),
+ @type_name($t:path),
@munch_fields(..Zeroable::zeroed() $(,)?),
@acc($($acc:tt)*),
) => {
@@ -1263,15 +1263,17 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
// not get executed, so it has no effect.
::core::ptr::write($slot, zeroed);
zeroed = ::core::mem::zeroed();
- ::core::ptr::write($slot, $t {
- $($acc)*
- ..zeroed
- });
+ $crate::init::__internal::retokenize!(
+ ::core::ptr::write($slot, $t {
+ $($acc)*
+ ..zeroed
+ });
+ );
}
};
(make_initializer:
@slot($slot:ident),
- @type_name($t:ident),
+ @type_name($t:path),
@munch_fields($(,)?),
@acc($($acc:tt)*),
) => {
@@ -1279,14 +1281,16 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
// Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
// get the correct type inference here:
unsafe {
- ::core::ptr::write($slot, $t {
- $($acc)*
- });
+ $crate::init::__internal::retokenize!(
+ ::core::ptr::write($slot, $t {
+ $($acc)*
+ });
+ );
}
};
(make_initializer:
@slot($slot:ident),
- @type_name($t:ident),
+ @type_name($t:path),
@munch_fields($field:ident <- $val:expr, $($rest:tt)*),
@acc($($acc:tt)*),
) => {
@@ -1299,7 +1303,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
};
(make_initializer:
@slot($slot:ident),
- @type_name($t:ident),
+ @type_name($t:path),
@munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
@acc($($acc:tt)*),
) => {
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 9f056a5c780a..d329ab622fd4 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -12,7 +12,7 @@
mod vtable;
mod zeroable;
-use proc_macro::TokenStream;
+use proc_macro::{Group, TokenStream, TokenTree};
/// Declares a kernel module.
///
@@ -266,3 +266,18 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
pub fn derive_zeroable(input: TokenStream) -> TokenStream {
zeroable::derive(input)
}
+
+/// Does not modify the given TokenStream, but removes any declarative macro information.
+#[proc_macro]
+pub fn retokenize(input: TokenStream) -> TokenStream {
+ fn id(tt: TokenTree) -> TokenTree {
+ match tt {
+ TokenTree::Group(g) => TokenTree::Group(Group::new(
+ g.delimiter(),
+ g.stream().into_iter().map(id).collect(),
+ )),
+ x => x,
+ }
+ }
+ input.into_iter().map(id).collect()
+}
--
2.41.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/7] rust: init: consolidate init macros
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
` (5 preceding siblings ...)
2023-06-24 9:25 ` [PATCH 7/7] rust: init: add support for arbitrary paths in init macros Benno Lossin
@ 2023-06-24 14:49 ` Björn Roy Baron
6 siblings, 0 replies; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-24 14:49 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches
On Saturday, June 24th, 2023 at 11:24, Benno Lossin <benno.lossin@proton.me> wrote:
> Merges the implementations of `try_init!` and `try_pin_init!`. These two
> macros are very similar, but use different traits. The new macro
> `__init_internal!` that is now the implementation for both takes these
> traits as parameters.
>
> This change does not affect any users, as no public API has been
> changed, but it should simplify maintaining the init macros.
>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
A bit hard to review due to the large blocks of code that were moved, but git show --color-moved didn't show anything weird. Nice to see less code duplication in any case.
Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com>
> ---
> rust/kernel/init.rs | 388 +++----------------------------------
> rust/kernel/init/macros.rs | 237 +++++++++++++++++++++-
> 2 files changed, 259 insertions(+), 366 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index b4332a4ec1f4..d9a91950cba2 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -540,11 +540,14 @@ macro_rules! pin_init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_pin_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error(::core::convert::Infallible),
> + @data(PinData, use_data),
> + @has_data(HasPinData, __pin_data),
> + @construct_closure(pin_init_from_closure),
> )
> };
> }
> @@ -593,205 +596,29 @@ macro_rules! try_pin_init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_pin_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)? ),
> @fields($($fields)*),
> @error($crate::error::Error),
> + @data(PinData, use_data),
> + @has_data(HasPinData, __pin_data),
> + @construct_closure(pin_init_from_closure),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }? $err:ty) => {
> - $crate::try_pin_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)? ),
> @fields($($fields)*),
> @error($err),
> + @data(PinData, use_data),
> + @has_data(HasPinData, __pin_data),
> + @construct_closure(pin_init_from_closure),
> )
> };
> - (
> - @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> - @fields($($fields:tt)*),
> - @error($err:ty),
> - ) => {{
> - // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> - // type and shadow it later when we insert the arbitrary user code. That way there will be
> - // no possibility of returning without `unsafe`.
> - struct __InitOk;
> - // Get the pin data from the supplied type.
> - let data = unsafe {
> - use $crate::init::__internal::HasPinData;
> - $t$(::<$($generics),*>)?::__pin_data()
> - };
> - // Ensure that `data` really is of type `PinData` and help with type inference:
> - let init = $crate::init::__internal::PinData::make_closure::<_, __InitOk, $err>(
> - data,
> - move |slot| {
> - {
> - // Shadow the structure so it cannot be used to return early.
> - struct __InitOk;
> - // Create the `this` so it can be referenced by the user inside of the
> - // expressions creating the individual fields.
> - $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> - // Initialize every field.
> - $crate::try_pin_init!(init_slot:
> - @data(data),
> - @slot(slot),
> - @munch_fields($($fields)*,),
> - );
> - // 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)]
> - if false {
> - $crate::try_pin_init!(make_initializer:
> - @slot(slot),
> - @type_name($t),
> - @munch_fields($($fields)*,),
> - @acc(),
> - );
> - }
> - // Forget all guards, since initialization was a success.
> - $crate::try_pin_init!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> - }
> - Ok(__InitOk)
> - }
> - );
> - let init = move |slot| -> ::core::result::Result<(), $err> {
> - init(slot).map(|__InitOk| ())
> - };
> - let init = unsafe { $crate::init::pin_init_from_closure::<_, $err>(init) };
> - init
> - }};
> - (init_slot:
> - @data($data:ident),
> - @slot($slot:ident),
> - @munch_fields($(,)?),
> - ) => {
> - // Endpoint of munching, no fields are left.
> - };
> - (init_slot:
> - @data($data:ident),
> - @slot($slot:ident),
> - // In-place initialization syntax.
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - let $field = $val;
> - // Call the initializer.
> - //
> - // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> - // return when an error/panic occurs.
> - // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> - unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> - // Create the drop guard.
> - //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_pin_init!(init_slot:
> - @data($data),
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (init_slot:
> - @data($data:ident),
> - @slot($slot:ident),
> - // Direct value init, this is safe for every field.
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - $(let $field = $val;)?
> - // Initialize the field.
> - //
> - // SAFETY: The memory at `slot` is uninitialized.
> - unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> - // Create the drop guard:
> - //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_pin_init!(init_slot:
> - @data($data),
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($(,)?),
> - @acc($($acc:tt)*),
> - ) => {
> - // Endpoint, nothing more to munch, create the initializer.
> - // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> - // get the correct type inference here:
> - unsafe {
> - ::core::ptr::write($slot, $t {
> - $($acc)*
> - });
> - }
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_pin_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)* $field: ::core::panic!(),),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_pin_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)* $field: ::core::panic!(),),
> - );
> - };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_pin_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_pin_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> /// Construct an in-place initializer for `struct`s.
> @@ -816,11 +643,14 @@ macro_rules! init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error(::core::convert::Infallible),
> + @data(InitData, /*no use_data*/),
> + @has_data(HasInitData, __init_data),
> + @construct_closure(init_from_closure),
> )
> }
> }
> @@ -863,199 +693,29 @@ macro_rules! try_init {
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }) => {
> - $crate::try_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error($crate::error::Error),
> + @data(InitData, /*no use_data*/),
> + @has_data(HasInitData, __init_data),
> + @construct_closure(init_from_closure),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> $($fields:tt)*
> }? $err:ty) => {
> - $crate::try_init!(
> + $crate::__init_internal!(
> @this($($this)?),
> @typ($t $(::<$($generics),*>)?),
> @fields($($fields)*),
> @error($err),
> + @data(InitData, /*no use_data*/),
> + @has_data(HasInitData, __init_data),
> + @construct_closure(init_from_closure),
> )
> };
> - (
> - @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> - @fields($($fields:tt)*),
> - @error($err:ty),
> - ) => {{
> - // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> - // type and shadow it later when we insert the arbitrary user code. That way there will be
> - // no possibility of returning without `unsafe`.
> - struct __InitOk;
> - // Get the init data from the supplied type.
> - let data = unsafe {
> - use $crate::init::__internal::HasInitData;
> - $t$(::<$($generics),*>)?::__init_data()
> - };
> - // Ensure that `data` really is of type `InitData` and help with type inference:
> - let init = $crate::init::__internal::InitData::make_closure::<_, __InitOk, $err>(
> - data,
> - move |slot| {
> - {
> - // Shadow the structure so it cannot be used to return early.
> - struct __InitOk;
> - // Create the `this` so it can be referenced by the user inside of the
> - // expressions creating the individual fields.
> - $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> - // Initialize every field.
> - $crate::try_init!(init_slot:
> - @slot(slot),
> - @munch_fields($($fields)*,),
> - );
> - // 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)]
> - if false {
> - $crate::try_init!(make_initializer:
> - @slot(slot),
> - @type_name($t),
> - @munch_fields($($fields)*,),
> - @acc(),
> - );
> - }
> - // Forget all guards, since initialization was a success.
> - $crate::try_init!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> - }
> - Ok(__InitOk)
> - }
> - );
> - let init = move |slot| -> ::core::result::Result<(), $err> {
> - init(slot).map(|__InitOk| ())
> - };
> - let init = unsafe { $crate::init::init_from_closure::<_, $err>(init) };
> - init
> - }};
> - (init_slot:
> - @slot($slot:ident),
> - @munch_fields( $(,)?),
> - ) => {
> - // Endpoint of munching, no fields are left.
> - };
> - (init_slot:
> - @slot($slot:ident),
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - let $field = $val;
> - // Call the initializer.
> - //
> - // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> - // return when an error/panic occurs.
> - unsafe {
> - $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))?;
> - }
> - // Create the drop guard.
> - //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_init!(init_slot:
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (init_slot:
> - @slot($slot:ident),
> - // Direct value init.
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - $(let $field = $val;)?
> - // Call the initializer.
> - //
> - // SAFETY: The memory at `slot` is uninitialized.
> - unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> - // Create the drop guard.
> - //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> - //
> - // SAFETY: We forget the guard later when initialization has succeeded.
> - let $field = &unsafe {
> - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> - };
> -
> - $crate::try_init!(init_slot:
> - @slot($slot),
> - @munch_fields($($rest)*),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields( $(,)?),
> - @acc($($acc:tt)*),
> - ) => {
> - // Endpoint, nothing more to munch, create the initializer.
> - // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> - // get the correct type inference here:
> - unsafe {
> - ::core::ptr::write($slot, $t {
> - $($acc)*
> - });
> - }
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)*$field: ::core::panic!(),),
> - );
> - };
> - (make_initializer:
> - @slot($slot:ident),
> - @type_name($t:ident),
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - @acc($($acc:tt)*),
> - ) => {
> - $crate::try_init!(make_initializer:
> - @slot($slot),
> - @type_name($t),
> - @munch_fields($($rest)*),
> - @acc($($acc)*$field: ::core::panic!(),),
> - );
> - };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::try_init!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> /// A pin-initializer for the type `T`.
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 00aa4e956c0a..fbaebd34f218 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1,10 +1,12 @@
> // SPDX-License-Identifier: Apache-2.0 OR MIT
>
> //! This module provides the macros that actually implement the proc-macros `pin_data` and
> -//! `pinned_drop`.
> +//! `pinned_drop`. It also contains `__init_internal` the implementation of the `{try_}{pin_}init!`
> +//! macros.
> //!
> //! These macros should never be called directly, since they expect their input to be
> -//! in a certain format which is internal. Use the proc-macros instead.
> +//! in a certain format which is internal. If used incorrectly, these macros can lead to UB even in
> +//! safe code! Use the public facing macros instead.
> //!
> //! This architecture has been chosen because the kernel does not yet have access to `syn` which
> //! would make matters a lot easier for implementing these as proc-macros.
> @@ -980,3 +982,234 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> }
> };
> }
> +
> +/// The internal init macro. Do not call manually!
> +///
> +/// This is called by the `{try_}{pin_}init!` macros with various inputs.
> +///
> +/// This macro has multiple internal call configurations, these are always the very first ident:
> +/// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
> +/// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> +/// - `make_initializer`: recursively create the struct initializer that guarantees that every
> +/// field has been initialized exactly once.
> +/// - `forget_guards`: recursively forget the drop guards for every field.
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __init_internal {
> + (
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + ) => {{
> + // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> + // type and shadow it later when we insert the arbitrary user code. That way there will be
> + // no possibility of returning without `unsafe`.
> + struct __InitOk;
> + // Get the data about fields from the supplied type.
> + let data = unsafe {
> + use $crate::init::__internal::$has_data;
> + $t$(::<$($generics),*>)?::$get_data()
> + };
> + // Ensure that `data` really is of type `$data` and help with type inference:
> + let init = $crate::init::__internal::$data::make_closure::<_, __InitOk, $err>(
> + data,
> + move |slot| {
> + {
> + // Shadow the structure so it cannot be used to return early.
> + struct __InitOk;
> + // Create the `this` so it can be referenced by the user inside of the
> + // expressions creating the individual fields.
> + $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> + // Initialize every field.
> + $crate::__init_internal!(init_slot($($use_data)?):
> + @data(data),
> + @slot(slot),
> + @munch_fields($($fields)*,),
> + );
> + // 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)]
> + if false {
> + $crate::__init_internal!(make_initializer:
> + @slot(slot),
> + @type_name($t),
> + @munch_fields($($fields)*,),
> + @acc(),
> + );
> + }
> + // Forget all guards, since initialization was a success.
> + $crate::__init_internal!(forget_guards:
> + @munch_fields($($fields)*,),
> + );
> + }
> + Ok(__InitOk)
> + }
> + );
> + let init = move |slot| -> ::core::result::Result<(), $err> {
> + init(slot).map(|__InitOk| ())
> + };
> + let init = unsafe { $crate::init::$construct_closure::<_, $err>(init) };
> + init
> + }};
> + (init_slot($($use_data:ident)?):
> + @data($data:ident),
> + @slot($slot:ident),
> + @munch_fields($(,)?),
> + ) => {
> + // Endpoint of munching, no fields are left.
> + };
> + (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
> + @data($data:ident),
> + @slot($slot:ident),
> + // In-place initialization syntax.
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + ) => {
> + let $field = $val;
> + // Call the initializer.
> + //
> + // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> + // return when an error/panic occurs.
> + // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> + unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> + // Create the drop guard.
> + //
> + // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + //
> + // SAFETY: We forget the guard later when initialization has succeeded.
> + let $field = &unsafe {
> + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> + };
> +
> + $crate::__init_internal!(init_slot($use_data):
> + @data($data),
> + @slot($slot),
> + @munch_fields($($rest)*),
> + );
> + };
> + (init_slot(): // no use_data, so we use `Init::__init` directly.
> + @data($data:ident),
> + @slot($slot:ident),
> + // In-place initialization syntax.
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + ) => {
> + let $field = $val;
> + // Call the initializer.
> + //
> + // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> + // return when an error/panic occurs.
> + unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
> + // Create the drop guard.
> + //
> + // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + //
> + // SAFETY: We forget the guard later when initialization has succeeded.
> + let $field = &unsafe {
> + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> + };
> +
> + $crate::__init_internal!(init_slot():
> + @data($data),
> + @slot($slot),
> + @munch_fields($($rest)*),
> + );
> + };
> + (init_slot($($use_data:ident)?):
> + @data($data:ident),
> + @slot($slot:ident),
> + // Init by-value.
> + @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> + ) => {
> + $(let $field = $val;)?
> + // Initialize the field.
> + //
> + // SAFETY: The memory at `slot` is uninitialized.
> + unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> + // Create the drop guard:
> + //
> + // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> + //
> + // SAFETY: We forget the guard later when initialization has succeeded.
> + let $field = &unsafe {
> + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> + };
> +
> + $crate::__init_internal!(init_slot($($use_data)?):
> + @data($data),
> + @slot($slot),
> + @munch_fields($($rest)*),
> + );
> + };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields($(,)?),
> + @acc($($acc:tt)*),
> + ) => {
> + // Endpoint, nothing more to munch, create the initializer.
> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> + // get the correct type inference here:
> + unsafe {
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + });
> + }
> + };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + @acc($($acc:tt)*),
> + ) => {
> + $crate::__init_internal!(make_initializer:
> + @slot($slot),
> + @type_name($t),
> + @munch_fields($($rest)*),
> + @acc($($acc)* $field: ::core::panic!(),),
> + );
> + };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> + @acc($($acc:tt)*),
> + ) => {
> + $crate::__init_internal!(make_initializer:
> + @slot($slot),
> + @type_name($t),
> + @munch_fields($($rest)*),
> + @acc($($acc)* $field: ::core::panic!(),),
> + );
> + };
> + (forget_guards:
> + @munch_fields($(,)?),
> + ) => {
> + // Munching finished.
> + };
> + (forget_guards:
> + @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> + ) => {
> + unsafe { $crate::init::__internal::DropGuard::forget($field) };
> +
> + $crate::__init_internal!(forget_guards:
> + @munch_fields($($rest)*),
> + );
> + };
> + (forget_guards:
> + @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> + ) => {
> + unsafe { $crate::init::__internal::DropGuard::forget($field) };
> +
> + $crate::__init_internal!(forget_guards:
> + @munch_fields($($rest)*),
> + );
> + };
> +}
>
> base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
> --
> 2.41.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/7] rust: add derive macro for `Zeroable`
2023-06-24 9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
@ 2023-06-24 14:55 ` Björn Roy Baron
2023-06-25 20:46 ` Gary Guo
2023-07-03 11:50 ` Alice Ryhl
2 siblings, 0 replies; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-24 14:55 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> Add a derive proc-macro for the `Zeroable` trait. The macro supports
> structs where every field implements the `Zeroable` trait. This way
> `unsafe` implementations can be avoided.
>
> The macro is split into two parts:
> - a proc-macro to parse generics into impl and ty generics,
> - a declarative macro that expands to the impl block.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com>
> ---
> rust/kernel/init/macros.rs | 28 ++++++++++++++++++++++++++++
> rust/kernel/prelude.rs | 2 +-
> rust/macros/lib.rs | 20 ++++++++++++++++++++
> rust/macros/quote.rs | 6 ++++++
> rust/macros/zeroable.rs | 25 +++++++++++++++++++++++++
> 5 files changed, 80 insertions(+), 1 deletion(-)
> create mode 100644 rust/macros/zeroable.rs
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index fbaebd34f218..e8165ff53a94 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1213,3 +1213,31 @@ macro_rules! __init_internal {
> );
> };
> }
> +
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __derive_zeroable {
> + (parse_input:
> + @sig(
> + $(#[$($struct_attr:tt)*])*
> + $vis:vis struct $name:ident
> + $(where $($whr:tt)*)?
> + ),
> + @impl_generics($($impl_generics:tt)*),
> + @ty_generics($($ty_generics:tt)*),
> + @body({
> + $(
> + $(#[$($field_attr:tt)*])*
> + $field:ident : $field_ty:ty
> + ),* $(,)?
> + }),
> + ) => {
> + // SAFETY: every field type implements `Zeroable` and padding bytes may be zero.
> + #[automatically_derived]
> + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> + where
> + $($field_ty: $crate::Zeroable,)*
> + $($($whr)*)?
> + {}
> + };
> +}
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index c28587d68ebc..ae21600970b3 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -18,7 +18,7 @@
> pub use alloc::{boxed::Box, vec::Vec};
>
> #[doc(no_inline)]
> -pub use macros::{module, pin_data, pinned_drop, vtable};
> +pub use macros::{module, pin_data, pinned_drop, vtable, Zeroable};
>
> pub use super::build_assert;
>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 3fc74cb4ea19..9f056a5c780a 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -10,6 +10,7 @@
> mod pin_data;
> mod pinned_drop;
> mod vtable;
> +mod zeroable;
>
> use proc_macro::TokenStream;
>
> @@ -246,3 +247,22 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
> pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pinned_drop::pinned_drop(args, input)
> }
> +
> +/// Derives the [`Zeroable`] trait for the given struct.
> +///
> +/// This can only be used for structs where every field implements the [`Zeroable`] trait.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// #[derive(Zeroable)]
> +/// pub struct DriverData {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// }
> +/// ```
> +#[proc_macro_derive(Zeroable)]
> +pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> + zeroable::derive(input)
> +}
> diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
> index dddbb4e6f4cb..b76c198a4ed5 100644
> --- a/rust/macros/quote.rs
> +++ b/rust/macros/quote.rs
> @@ -124,6 +124,12 @@ macro_rules! quote_spanned {
> ));
> quote_spanned!(@proc $v $span $($tt)*);
> };
> + (@proc $v:ident $span:ident ; $($tt:tt)*) => {
> + $v.push(::proc_macro::TokenTree::Punct(
> + ::proc_macro::Punct::new(';', ::proc_macro::Spacing::Alone)
> + ));
> + quote_spanned!(@proc $v $span $($tt)*);
> + };
> (@proc $v:ident $span:ident $id:ident $($tt:tt)*) => {
> $v.push(::proc_macro::TokenTree::Ident(::proc_macro::Ident::new(stringify!($id), $span)));
> quote_spanned!(@proc $v $span $($tt)*);
> diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
> new file mode 100644
> index 000000000000..cddb866c44ef
> --- /dev/null
> +++ b/rust/macros/zeroable.rs
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use crate::helpers::{parse_generics, Generics};
> +use proc_macro::TokenStream;
> +
> +pub(crate) fn derive(input: TokenStream) -> TokenStream {
> + let (
> + Generics {
> + impl_generics,
> + ty_generics,
> + },
> + mut rest,
> + ) = parse_generics(input);
> + // This should be the body of the struct `{...}`.
> + let last = rest.pop();
> + quote! {
> + ::kernel::__derive_zeroable!(
> + parse_input:
> + @sig(#(#rest)*),
> + @impl_generics(#(#impl_generics)*),
> + @ty_generics(#(#ty_generics)*),
> + @body(#last),
> + );
> + }
> +}
> --
> 2.41.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/7] rust: init: make guards in the init macros hygienic
2023-06-24 9:25 ` [PATCH 3/7] rust: init: make guards in the init macros hygienic Benno Lossin
@ 2023-06-24 14:58 ` Björn Roy Baron
2023-06-25 20:54 ` Gary Guo
1 sibling, 0 replies; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-24 14:58 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> Use hygienic identifiers for the guards instead of the field names. This
> makes the init macros feel more like normal struct initializers, since
> assigning identifiers with the name of a field does not create
> conflicts.
> Also change the internals of the guards, no need to make the `forget`
> function `unsafe`, since users cannot access the guards anyways. Now the
> guards are carried directly on the stack and have no extra `Cell<bool>`
> field that marks if they have been forgotten or not, instead they are
> just forgotten via `mem::forget`.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com>
> ---
> rust/kernel/init.rs | 1 -
> rust/kernel/init/__internal.rs | 25 +++------------
> rust/kernel/init/macros.rs | 56 ++++++++++++----------------------
> 3 files changed, 23 insertions(+), 59 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index d9a91950cba2..ecf6a4bd0ce4 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -206,7 +206,6 @@
> use alloc::boxed::Box;
> use core::{
> alloc::AllocError,
> - cell::Cell,
> convert::Infallible,
> marker::PhantomData,
> mem::MaybeUninit,
> diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
> index 44751fb62b51..7abd1fb65e41 100644
> --- a/rust/kernel/init/__internal.rs
> +++ b/rust/kernel/init/__internal.rs
> @@ -174,7 +174,6 @@ pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mu
> /// Can be forgotten to prevent the drop.
> pub struct DropGuard<T: ?Sized> {
> ptr: *mut T,
> - do_drop: Cell<bool>,
> }
>
> impl<T: ?Sized> DropGuard<T> {
> @@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> {
> /// - will not be dropped by any other means.
> #[inline]
> pub unsafe fn new(ptr: *mut T) -> Self {
> - Self {
> - ptr,
> - do_drop: Cell::new(true),
> - }
> - }
> -
> - /// Prevents this guard from dropping the supplied pointer.
> - ///
> - /// # Safety
> - ///
> - /// This function is unsafe in order to prevent safe code from forgetting this guard. It should
> - /// only be called by the macros in this module.
> - #[inline]
> - pub unsafe fn forget(&self) {
> - self.do_drop.set(false);
> + Self { ptr }
> }
> }
>
> impl<T: ?Sized> Drop for DropGuard<T> {
> #[inline]
> fn drop(&mut self) {
> - if self.do_drop.get() {
> - // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> - // ensuring that this operation is safe.
> - unsafe { ptr::drop_in_place(self.ptr) }
> - }
> + // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> + // ensuring that this operation is safe.
> + unsafe { ptr::drop_in_place(self.ptr) }
> }
> }
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index e8165ff53a94..df4281743175 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -992,7 +992,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> /// field has been initialized exactly once.
> -/// - `forget_guards`: recursively forget the drop guards for every field.
> #[doc(hidden)]
> #[macro_export]
> macro_rules! __init_internal {
> @@ -1032,6 +1031,7 @@ macro_rules! __init_internal {
> $crate::__init_internal!(init_slot($($use_data)?):
> @data(data),
> @slot(slot),
> + @guards(),
> @munch_fields($($fields)*,),
> );
> // We use unreachable code to ensure that all fields have been mentioned exactly
> @@ -1046,10 +1046,6 @@ macro_rules! __init_internal {
> @acc(),
> );
> }
> - // Forget all guards, since initialization was a success.
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> }
> Ok(__InitOk)
> }
> @@ -1063,13 +1059,17 @@ macro_rules! __init_internal {
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> @munch_fields($(,)?),
> ) => {
> - // Endpoint of munching, no fields are left.
> + // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> + // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> + $(::core::mem::forget($guards);)*
> };
> (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1080,24 +1080,26 @@ macro_rules! __init_internal {
> // return when an error/panic occurs.
> // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // 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 $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($use_data):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot(): // no use_data, so we use `Init::__init` directly.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1107,24 +1109,26 @@ macro_rules! __init_internal {
> // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> // return when an error/panic occurs.
> unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // 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 $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot():
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // Init by-value.
> @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> ) => {
> @@ -1135,16 +1139,17 @@ macro_rules! __init_internal {
> unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> + // 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 $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($($use_data)?):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> @@ -1189,29 +1194,6 @@ macro_rules! __init_internal {
> @acc($($acc)* $field: ::core::panic!(),),
> );
> };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> #[doc(hidden)]
> --
> 2.41.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure
2023-06-24 9:25 ` [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure Benno Lossin
@ 2023-06-24 15:03 ` Björn Roy Baron
2023-06-24 21:05 ` Benno Lossin
0 siblings, 1 reply; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-24 15:03 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> In the implementation of the init macros there is a `if false` statement
> that type checks the initializer to ensure every field is initialized.
> Since the next patch has a stack variable to store the struct, the
> function might allocate too much memory on debug builds. Putting the
> struct into a closure ensures that even in debug builds no stack
> overflow error is caused. In release builds this was not a problem since
> the code was optimized away due to the `if false`.
>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/kernel/init/macros.rs | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index df4281743175..1e0c4aca055a 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1037,14 +1037,18 @@ macro_rules! __init_internal {
> // 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)]
> + #[allow(unreachable_code,
> + clippy::diverging_sub_expression,
> + clippy::redundant_closure_call)]
> if false {
> - $crate::__init_internal!(make_initializer:
> - @slot(slot),
> - @type_name($t),
> - @munch_fields($($fields)*,),
> - @acc(),
> - );
> + (|| {
> + $crate::__init_internal!(make_initializer:
> + @slot(slot),
> + @type_name($t),
> + @munch_fields($($fields)*,),
> + @acc(),
> + );
> + })();
Is it necessary to call this closure? Typechecking of the closure should happen even without calling it.
> }
> }
> Ok(__InitOk)
> --
> 2.41.0
Cheers,
Björn
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-24 9:25 ` [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
@ 2023-06-24 15:11 ` Björn Roy Baron
2023-06-24 21:14 ` Benno Lossin
2023-07-03 11:58 ` Alice Ryhl
2023-07-03 18:15 ` Boqun Feng
2 siblings, 1 reply; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-24 15:11 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> Add the struct update syntax to the init macros, but only for
> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
> allows one to omit fields from the initializer, these fields will be
> initialized with 0x00 set to every byte. Only types that implement the
> `Zeroable` trait can utilize this.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/kernel/init.rs | 16 +++++-
> rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index ecf6a4bd0ce4..44bc3e77419a 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
> /// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
> /// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
> /// pointer named `this` inside of the initializer.
> +/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
> +/// struct, this initializes every field with 0 and then runs all initializers specified in the
> +/// body. This can only be done if [`Zeroable`] is implemented for the struct.
> ///
> /// For instance:
> ///
> /// ```rust
> /// # use kernel::pin_init;
> -/// # use macros::pin_data;
> +/// # use macros::{pin_data, Zeroable};
> /// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
> /// #[pin_data]
> +/// #[derive(Zeroable)]
> /// struct Buf {
> /// // `ptr` points into `buf`.
> /// ptr: *mut u8,
> @@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
> /// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
> /// pin: PhantomPinned,
> /// });
> +/// pin_init!(Buf {
> +/// buf: [1; 64],
> +/// ..Zeroable::zeroed(),
> +/// });
> /// ```
> ///
> /// [`try_pin_init!`]: kernel::try_pin_init
> @@ -547,6 +555,7 @@ macro_rules! pin_init {
> @data(PinData, use_data),
> @has_data(HasPinData, __pin_data),
> @construct_closure(pin_init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> }
> @@ -603,6 +612,7 @@ macro_rules! try_pin_init {
> @data(PinData, use_data),
> @has_data(HasPinData, __pin_data),
> @construct_closure(pin_init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> @@ -616,6 +626,7 @@ macro_rules! try_pin_init {
> @data(PinData, use_data),
> @has_data(HasPinData, __pin_data),
> @construct_closure(pin_init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> }
> @@ -650,6 +661,7 @@ macro_rules! init {
> @data(InitData, /*no use_data*/),
> @has_data(HasInitData, __init_data),
> @construct_closure(init_from_closure),
> + @munch_fields($($fields)*),
> )
> }
> }
> @@ -700,6 +712,7 @@ macro_rules! try_init {
> @data(InitData, /*no use_data*/),
> @has_data(HasInitData, __init_data),
> @construct_closure(init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> @@ -713,6 +726,7 @@ macro_rules! try_init {
> @data(InitData, /*no use_data*/),
> @has_data(HasInitData, __init_data),
> @construct_closure(init_from_closure),
> + @munch_fields($($fields)*),
> )
> };
> }
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 1e0c4aca055a..5dcb2e513f26 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> ///
> /// This macro has multiple internal call configurations, these are always the very first ident:
> /// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
> +/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> /// field has been initialized exactly once.
> @@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
> @has_data($has_data:ident, $get_data:ident),
> // `pin_init_from_closure` or `init_from_closure`.
> @construct_closure($construct_closure:ident),
> + @munch_fields(),
> + ) => {
> + $crate::__init_internal!(with_update_parsed:
> + @this($($this)?),
> + @typ($t $(::<$($generics),*>)? ),
> + @fields($($fields)*),
> + @error($err),
> + @data($data, $($use_data)?),
> + @has_data($has_data, $get_data),
> + @construct_closure($construct_closure),
> + @zeroed(), // nothing means default behavior.
> + )
> + };
> + (
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + @munch_fields(..Zeroable::zeroed()),
> + ) => {
> + $crate::__init_internal!(with_update_parsed:
> + @this($($this)?),
> + @typ($t $(::<$($generics),*>)? ),
> + @fields($($fields)*),
> + @error($err),
> + @data($data, $($use_data)?),
> + @has_data($has_data, $get_data),
> + @construct_closure($construct_closure),
> + @zeroed(()), // `()` means zero all fields not mentioned.
> + )
> + };
> + (
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + @munch_fields($ignore:tt $($rest:tt)*),
> + ) => {
> + $crate::__init_internal!(
> + @this($($this)?),
> + @typ($t $(::<$($generics),*>)? ),
> + @fields($($fields)*),
> + @error($err),
> + @data($data, $($use_data)?),
> + @has_data($has_data, $get_data),
> + @construct_closure($construct_closure),
> + @munch_fields($($rest)*),
> + )
> + };
> + (with_update_parsed:
> + @this($($this:ident)?),
> + @typ($t:ident $(::<$($generics:ty),*>)?),
> + @fields($($fields:tt)*),
> + @error($err:ty),
> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> + // case.
> + @data($data:ident, $($use_data:ident)?),
> + // `HasPinData` or `HasInitData`.
> + @has_data($has_data:ident, $get_data:ident),
> + // `pin_init_from_closure` or `init_from_closure`.
> + @construct_closure($construct_closure:ident),
> + @zeroed($($init_zeroed:expr)?),
> ) => {{
> // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> // type and shadow it later when we insert the arbitrary user code. That way there will be
> @@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
> {
> // Shadow the structure so it cannot be used to return early.
> struct __InitOk;
> + // If `$init_zeroed` is present we should zero the slot now and not emit an
> + // error when fields are missing (since they will be zeroed). We also have to
> + // check that the type actually implements `Zeroable`.
> + $(
> + fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
Maybe call this assert_zeroable?
> + // Ensure that the struct is indeed `Zeroable`.
> + is_zeroable(slot);
> + // SAFETY: The type implements `Zeroable` by the check above.
> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
> + $init_zeroed // this will be `()` if set.
How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
> + )?
> // Create the `this` so it can be referenced by the user inside of the
> // expressions creating the individual fields.
> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> @data($data:ident),
> @slot($slot:ident),
> @guards($($guards:ident,)*),
> - @munch_fields($(,)?),
> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
> ) => {
> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> @munch_fields($($rest)*),
> );
> };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields(..Zeroable::zeroed() $(,)?),
> + @acc($($acc:tt)*),
> + ) => {
> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> + // not been overwritten are thus zero and initialized. We still check that all fields are
> + // actually accessible by using the struct update syntax ourselves.
> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> + // get the correct type inference here:
> + unsafe {
> + let mut zeroed = ::core::mem::zeroed();
> + // We have to use type inference her to make zeroed have the correct type. This does
*here
> + // not get executed, so it has no effect.
> + ::core::ptr::write($slot, zeroed);
> + zeroed = ::core::mem::zeroed();
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + ..zeroed
> + });
> + }
> + };
> (make_initializer:
> @slot($slot:ident),
> @type_name($t:ident),
> --
> 2.41.0
Cheers,
Björn
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/7] rust: init: Add functions to create array initializers
2023-06-24 9:25 ` [PATCH 6/7] rust: init: Add functions to create array initializers Benno Lossin
@ 2023-06-24 15:17 ` Björn Roy Baron
2023-07-03 12:03 ` Alice Ryhl
1 sibling, 0 replies; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-24 15:17 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> Add two functions `pin_init_array_from_fn` and `init_array_from_fn` that
> take a function that generates initializers for `T` from usize, the added
> functions then return an initializer for `[T; N]` where every element is
> initialized by an element returned from the generator function.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com>
> ---
> rust/kernel/init.rs | 90 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 90 insertions(+)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 44bc3e77419a..c9ea4bf71987 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -867,6 +867,96 @@ pub fn uninit<T, E>() -> impl Init<MaybeUninit<T>, E> {
> unsafe { init_from_closure(|_| Ok(())) }
> }
>
> +/// Initializes an array by initializing each element via the provided initializer.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// let array: Box<[usize; 1000_000_000]>= Box::init(init_array_from_fn(|i| i)).unwrap();
> +/// println!("{array:?}");
> +/// ```
> +pub fn init_array_from_fn<I, const N: usize, T, E>(
> + mut make_init: impl FnMut(usize) -> I,
> +) -> impl Init<[T; N], E>
> +where
> + I: Init<T, E>,
> +{
> + let init = move |slot: *mut [T; N]| {
> + let slot = slot.cast::<T>();
> + for i in 0..N {
> + let init = make_init(i);
> + // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
> + let ptr = unsafe { slot.add(i) };
> + // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init`
> + // requirements.
> + match unsafe { init.__init(ptr) } {
> + Ok(()) => {}
> + Err(e) => {
> + // We now free every element that has been initialized before:
> + for j in 0..i {
> + let ptr = unsafe { slot.add(j) };
> + // SAFETY: The value was initialized in a previous iteration of the loop
> + // and since we return `Err` below, the caller will consider the memory at
> + // `slot` as uninitialized.
> + unsafe { ptr::drop_in_place(ptr) };
> + }
> + return Err(e);
> + }
> + }
> + }
> + Ok(())
> + };
> + // SAFETY: The initializer above initializes every element of the array. On failure it drops
> + // any initialized elements and returns `Err`.
> + unsafe { init_from_closure(init) }
> +}
> +
> +/// Initializes an array by initializing each element via the provided initializer.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// let array: Arc<[Mutex<usize>; 1000_000_000]>=
> +/// Arc::pin_init(init_array_from_fn(|i| new_mutex!(i))).unwrap();
> +/// println!("{array:?}");
> +/// ```
> +pub fn pin_init_array_from_fn<I, const N: usize, T, E>(
> + mut make_init: impl FnMut(usize) -> I,
> +) -> impl PinInit<[T; N], E>
> +where
> + I: PinInit<T, E>,
> +{
> + let init = move |slot: *mut [T; N]| {
> + let slot = slot.cast::<T>();
> + for i in 0..N {
> + let init = make_init(i);
> + // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
> + let ptr = unsafe { slot.add(i) };
> + // SAFETY: The pointer is derived from `slot` and thus satisfies the `__pinned_init`
> + // requirements.
> + match unsafe { init.__pinned_init(ptr) } {
> + Ok(()) => {}
> + Err(e) => {
> + // We now have to free every element that has been initialized before, since we
> + // have to abide by the drop guarantee.
> + for j in 0..i {
> + let ptr = unsafe { slot.add(j) };
> + // SAFETY: The value was initialized in a previous iteration of the loop
> + // and since we return `Err` below, the caller will consider the memory at
> + // `slot` as uninitialized.
> + unsafe { ptr::drop_in_place(ptr) };
> + }
> + return Err(e);
> + }
> + }
> + }
> + Ok(())
> + };
> + // SAFETY: The initializer above initializes every element of the array. On failure it drops
> + // any initialized elements and returns `Err`.
> + unsafe { pin_init_from_closure(init) }
> +}
> +
> // SAFETY: Every type can be initialized by-value.
> unsafe impl<T, E> Init<T, E> for T {
> unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
> --
> 2.41.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 7/7] rust: init: add support for arbitrary paths in init macros
2023-06-24 9:25 ` [PATCH 7/7] rust: init: add support for arbitrary paths in init macros Benno Lossin
@ 2023-06-24 15:20 ` Björn Roy Baron
2023-06-25 21:01 ` Gary Guo
1 sibling, 0 replies; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-24 15:20 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> Previously only `ident` and generic types were supported in the
> `{try_}{pin_}init!` macros. This patch allows arbitrary path fragments,
> so for example `Foo::Bar` but also very complex paths such as
> `<Foo as Baz>::Bar::<0, i32>`.
>
> Internally this is accomplished by using `path` fragments. Due to some
> peculiar declarative macro limitations, we have to "forget" certain
> additional parsing information in the token trees. This is achieved by
> the new `retokenize` proc macro. It does not modify the input, but just
> strips this information. For example, if a declarative macro takes
> `$t:path` as its input, it cannot sensibly propagate this to a macro that
> takes `$($p:tt)*` as its input, since the `$t` token will only be
> considered one `tt` token for the second macro. If we first pipe the
> tokens through `retokenize`, then it parses as expected.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com>
> ---
> rust/kernel/init/__internal.rs | 2 ++
> rust/kernel/init/macros.rs | 42 +++++++++++++++++++---------------
> rust/macros/lib.rs | 17 +++++++++++++-
> 3 files changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
> index 7abd1fb65e41..e36a706a4a1b 100644
> --- a/rust/kernel/init/__internal.rs
> +++ b/rust/kernel/init/__internal.rs
> @@ -9,6 +9,8 @@
>
> use super::*;
>
> +pub use ::macros::retokenize;
> +
> /// See the [nomicon] for what subtyping is. See also [this table].
> ///
> /// [nomicon]: https://doc.rust-lang.org/nomicon/subtyping.html
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 5dcb2e513f26..6a82be675808 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -998,7 +998,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> macro_rules! __init_internal {
> (
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1012,7 +1012,7 @@ macro_rules! __init_internal {
> ) => {
> $crate::__init_internal!(with_update_parsed:
> @this($($this)?),
> - @typ($t $(::<$($generics),*>)? ),
> + @typ($t),
> @fields($($fields)*),
> @error($err),
> @data($data, $($use_data)?),
> @@ -1023,7 +1023,7 @@ macro_rules! __init_internal {
> };
> (
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1037,7 +1037,7 @@ macro_rules! __init_internal {
> ) => {
> $crate::__init_internal!(with_update_parsed:
> @this($($this)?),
> - @typ($t $(::<$($generics),*>)? ),
> + @typ($t),
> @fields($($fields)*),
> @error($err),
> @data($data, $($use_data)?),
> @@ -1048,7 +1048,7 @@ macro_rules! __init_internal {
> };
> (
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1062,7 +1062,7 @@ macro_rules! __init_internal {
> ) => {
> $crate::__init_internal!(
> @this($($this)?),
> - @typ($t $(::<$($generics),*>)? ),
> + @typ($t),
> @fields($($fields)*),
> @error($err),
> @data($data, $($use_data)?),
> @@ -1073,7 +1073,7 @@ macro_rules! __init_internal {
> };
> (with_update_parsed:
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1092,7 +1092,7 @@ macro_rules! __init_internal {
> // Get the data about fields from the supplied type.
> let data = unsafe {
> use $crate::init::__internal::$has_data;
> - $t$(::<$($generics),*>)?::$get_data()
> + $crate::init::__internal::retokenize!($t::$get_data())
> };
> // Ensure that `data` really is of type `$data` and help with type inference:
> let init = $crate::init::__internal::$data::make_closure::<_, __InitOk, $err>(
> @@ -1247,7 +1247,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields(..Zeroable::zeroed() $(,)?),
> @acc($($acc:tt)*),
> ) => {
> @@ -1263,15 +1263,17 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> // not get executed, so it has no effect.
> ::core::ptr::write($slot, zeroed);
> zeroed = ::core::mem::zeroed();
> - ::core::ptr::write($slot, $t {
> - $($acc)*
> - ..zeroed
> - });
> + $crate::init::__internal::retokenize!(
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + ..zeroed
> + });
> + );
> }
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields($(,)?),
> @acc($($acc:tt)*),
> ) => {
> @@ -1279,14 +1281,16 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> // get the correct type inference here:
> unsafe {
> - ::core::ptr::write($slot, $t {
> - $($acc)*
> - });
> + $crate::init::__internal::retokenize!(
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + });
> + );
> }
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> @acc($($acc:tt)*),
> ) => {
> @@ -1299,7 +1303,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> @acc($($acc:tt)*),
> ) => {
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 9f056a5c780a..d329ab622fd4 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -12,7 +12,7 @@
> mod vtable;
> mod zeroable;
>
> -use proc_macro::TokenStream;
> +use proc_macro::{Group, TokenStream, TokenTree};
>
> /// Declares a kernel module.
> ///
> @@ -266,3 +266,18 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> zeroable::derive(input)
> }
> +
> +/// Does not modify the given TokenStream, but removes any declarative macro information.
> +#[proc_macro]
> +pub fn retokenize(input: TokenStream) -> TokenStream {
> + fn id(tt: TokenTree) -> TokenTree {
> + match tt {
> + TokenTree::Group(g) => TokenTree::Group(Group::new(
> + g.delimiter(),
> + g.stream().into_iter().map(id).collect(),
> + )),
> + x => x,
> + }
> + }
> + input.into_iter().map(id).collect()
> +}
> --
> 2.41.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure
2023-06-24 15:03 ` Björn Roy Baron
@ 2023-06-24 21:05 ` Benno Lossin
0 siblings, 0 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 21:05 UTC (permalink / raw)
To: Björn Roy Baron
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches
On 6/24/23 17:03, Björn Roy Baron wrote:
> On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
>
>> In the implementation of the init macros there is a `if false` statement
>> that type checks the initializer to ensure every field is initialized.
>> Since the next patch has a stack variable to store the struct, the
>> function might allocate too much memory on debug builds. Putting the
>> struct into a closure ensures that even in debug builds no stack
>> overflow error is caused. In release builds this was not a problem since
>> the code was optimized away due to the `if false`.
>>
>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> ---
>> rust/kernel/init/macros.rs | 18 +++++++++++-------
>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
>> index df4281743175..1e0c4aca055a 100644
>> --- a/rust/kernel/init/macros.rs
>> +++ b/rust/kernel/init/macros.rs
>> @@ -1037,14 +1037,18 @@ macro_rules! __init_internal {
>> // 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)]
>> + #[allow(unreachable_code,
>> + clippy::diverging_sub_expression,
>> + clippy::redundant_closure_call)]
>> if false {
>> - $crate::__init_internal!(make_initializer:
>> - @slot(slot),
>> - @type_name($t),
>> - @munch_fields($($fields)*,),
>> - @acc(),
>> - );
>> + (|| {
>> + $crate::__init_internal!(make_initializer:
>> + @slot(slot),
>> + @type_name($t),
>> + @munch_fields($($fields)*,),
>> + @acc(),
>> + );
>> + })();
>
> Is it necessary to call this closure? Typechecking of the closure should happen even without calling it.
You are right, I do not need to call it. Will fix.
--
Cheers,
Benno
>
>> }
>> }
>> Ok(__InitOk)
>> --
>> 2.41.0
>
> Cheers,
> Björn
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-24 15:11 ` Björn Roy Baron
@ 2023-06-24 21:14 ` Benno Lossin
2023-06-25 12:56 ` Björn Roy Baron
0 siblings, 1 reply; 33+ messages in thread
From: Benno Lossin @ 2023-06-24 21:14 UTC (permalink / raw)
To: Björn Roy Baron
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On 6/24/23 17:11, Björn Roy Baron wrote:
> On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
>
>> Add the struct update syntax to the init macros, but only for
>> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
>> allows one to omit fields from the initializer, these fields will be
>> initialized with 0x00 set to every byte. Only types that implement the
>> `Zeroable` trait can utilize this.
>>
>> Suggested-by: Asahi Lina <lina@asahilina.net>
>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> ---
>> rust/kernel/init.rs | 16 +++++-
>> rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 128 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
>> index ecf6a4bd0ce4..44bc3e77419a 100644
>> --- a/rust/kernel/init.rs
>> +++ b/rust/kernel/init.rs
>> @@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
>> /// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
>> /// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
>> /// pointer named `this` inside of the initializer.
>> +/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
>> +/// struct, this initializes every field with 0 and then runs all initializers specified in the
>> +/// body. This can only be done if [`Zeroable`] is implemented for the struct.
>> ///
>> /// For instance:
>> ///
>> /// ```rust
>> /// # use kernel::pin_init;
>> -/// # use macros::pin_data;
>> +/// # use macros::{pin_data, Zeroable};
>> /// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
>> /// #[pin_data]
>> +/// #[derive(Zeroable)]
>> /// struct Buf {
>> /// // `ptr` points into `buf`.
>> /// ptr: *mut u8,
>> @@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
>> /// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
>> /// pin: PhantomPinned,
>> /// });
>> +/// pin_init!(Buf {
>> +/// buf: [1; 64],
>> +/// ..Zeroable::zeroed(),
>> +/// });
>> /// ```
>> ///
>> /// [`try_pin_init!`]: kernel::try_pin_init
>> @@ -547,6 +555,7 @@ macro_rules! pin_init {
>> @data(PinData, use_data),
>> @has_data(HasPinData, __pin_data),
>> @construct_closure(pin_init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> }
>> @@ -603,6 +612,7 @@ macro_rules! try_pin_init {
>> @data(PinData, use_data),
>> @has_data(HasPinData, __pin_data),
>> @construct_closure(pin_init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
>> @@ -616,6 +626,7 @@ macro_rules! try_pin_init {
>> @data(PinData, use_data),
>> @has_data(HasPinData, __pin_data),
>> @construct_closure(pin_init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> }
>> @@ -650,6 +661,7 @@ macro_rules! init {
>> @data(InitData, /*no use_data*/),
>> @has_data(HasInitData, __init_data),
>> @construct_closure(init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> }
>> }
>> @@ -700,6 +712,7 @@ macro_rules! try_init {
>> @data(InitData, /*no use_data*/),
>> @has_data(HasInitData, __init_data),
>> @construct_closure(init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
>> @@ -713,6 +726,7 @@ macro_rules! try_init {
>> @data(InitData, /*no use_data*/),
>> @has_data(HasInitData, __init_data),
>> @construct_closure(init_from_closure),
>> + @munch_fields($($fields)*),
>> )
>> };
>> }
>> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
>> index 1e0c4aca055a..5dcb2e513f26 100644
>> --- a/rust/kernel/init/macros.rs
>> +++ b/rust/kernel/init/macros.rs
>> @@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
>> ///
>> /// This macro has multiple internal call configurations, these are always the very first ident:
>> /// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
>> +/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
>> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
>> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
>> /// field has been initialized exactly once.
>> @@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
>> @has_data($has_data:ident, $get_data:ident),
>> // `pin_init_from_closure` or `init_from_closure`.
>> @construct_closure($construct_closure:ident),
>> + @munch_fields(),
>> + ) => {
>> + $crate::__init_internal!(with_update_parsed:
>> + @this($($this)?),
>> + @typ($t $(::<$($generics),*>)? ),
>> + @fields($($fields)*),
>> + @error($err),
>> + @data($data, $($use_data)?),
>> + @has_data($has_data, $get_data),
>> + @construct_closure($construct_closure),
>> + @zeroed(), // nothing means default behavior.
>> + )
>> + };
>> + (
>> + @this($($this:ident)?),
>> + @typ($t:ident $(::<$($generics:ty),*>)?),
>> + @fields($($fields:tt)*),
>> + @error($err:ty),
>> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
>> + // case.
>> + @data($data:ident, $($use_data:ident)?),
>> + // `HasPinData` or `HasInitData`.
>> + @has_data($has_data:ident, $get_data:ident),
>> + // `pin_init_from_closure` or `init_from_closure`.
>> + @construct_closure($construct_closure:ident),
>> + @munch_fields(..Zeroable::zeroed()),
>> + ) => {
>> + $crate::__init_internal!(with_update_parsed:
>> + @this($($this)?),
>> + @typ($t $(::<$($generics),*>)? ),
>> + @fields($($fields)*),
>> + @error($err),
>> + @data($data, $($use_data)?),
>> + @has_data($has_data, $get_data),
>> + @construct_closure($construct_closure),
>> + @zeroed(()), // `()` means zero all fields not mentioned.
>> + )
>> + };
>> + (
>> + @this($($this:ident)?),
>> + @typ($t:ident $(::<$($generics:ty),*>)?),
>> + @fields($($fields:tt)*),
>> + @error($err:ty),
>> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
>> + // case.
>> + @data($data:ident, $($use_data:ident)?),
>> + // `HasPinData` or `HasInitData`.
>> + @has_data($has_data:ident, $get_data:ident),
>> + // `pin_init_from_closure` or `init_from_closure`.
>> + @construct_closure($construct_closure:ident),
>> + @munch_fields($ignore:tt $($rest:tt)*),
>> + ) => {
>> + $crate::__init_internal!(
>> + @this($($this)?),
>> + @typ($t $(::<$($generics),*>)? ),
>> + @fields($($fields)*),
>> + @error($err),
>> + @data($data, $($use_data)?),
>> + @has_data($has_data, $get_data),
>> + @construct_closure($construct_closure),
>> + @munch_fields($($rest)*),
>> + )
>> + };
>> + (with_update_parsed:
>> + @this($($this:ident)?),
>> + @typ($t:ident $(::<$($generics:ty),*>)?),
>> + @fields($($fields:tt)*),
>> + @error($err:ty),
>> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
>> + // case.
>> + @data($data:ident, $($use_data:ident)?),
>> + // `HasPinData` or `HasInitData`.
>> + @has_data($has_data:ident, $get_data:ident),
>> + // `pin_init_from_closure` or `init_from_closure`.
>> + @construct_closure($construct_closure:ident),
>> + @zeroed($($init_zeroed:expr)?),
>> ) => {{
>> // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
>> // type and shadow it later when we insert the arbitrary user code. That way there will be
>> @@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
>> {
>> // Shadow the structure so it cannot be used to return early.
>> struct __InitOk;
>> + // If `$init_zeroed` is present we should zero the slot now and not emit an
>> + // error when fields are missing (since they will be zeroed). We also have to
>> + // check that the type actually implements `Zeroable`.
>> + $(
>> + fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
>
> Maybe call this assert_zeroable?
Sure.
>
>> + // Ensure that the struct is indeed `Zeroable`.
>> + is_zeroable(slot);
>> + // SAFETY: The type implements `Zeroable` by the check above.
>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
>> + $init_zeroed // this will be `()` if set.
>
> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
It is the last expression of a block and since it is `()` it is ok
(adding a ; would also be ok, but it is not necessary).
>
>> + )?
>> // Create the `this` so it can be referenced by the user inside of the
>> // expressions creating the individual fields.
>> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
>> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
>> @data($data:ident),
>> @slot($slot:ident),
>> @guards($($guards:ident,)*),
>> - @munch_fields($(,)?),
>> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
>> ) => {
>> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
>> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
>> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
>> @munch_fields($($rest)*),
>> );
>> };
>> + (make_initializer:
>> + @slot($slot:ident),
>> + @type_name($t:ident),
>> + @munch_fields(..Zeroable::zeroed() $(,)?),
>> + @acc($($acc:tt)*),
>> + ) => {
>> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
>> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
>> + // not been overwritten are thus zero and initialized. We still check that all fields are
>> + // actually accessible by using the struct update syntax ourselves.
>> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
>> + // get the correct type inference here:
>> + unsafe {
>> + let mut zeroed = ::core::mem::zeroed();
>> + // We have to use type inference her to make zeroed have the correct type. This does
>
> *here
Will fix.
--
Cheers,
Benno
>
>> + // not get executed, so it has no effect.
>> + ::core::ptr::write($slot, zeroed);
>> + zeroed = ::core::mem::zeroed();
>> + ::core::ptr::write($slot, $t {
>> + $($acc)*
>> + ..zeroed
>> + });
>> + }
>> + };
>> (make_initializer:
>> @slot($slot:ident),
>> @type_name($t:ident),
>> --
>> 2.41.0
>
> Cheers,
> Björn
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-24 21:14 ` Benno Lossin
@ 2023-06-25 12:56 ` Björn Roy Baron
2023-06-25 13:07 ` Benno Lossin
0 siblings, 1 reply; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-25 12:56 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On Saturday, June 24th, 2023 at 23:14, Benno Lossin <benno.lossin@proton.me> wrote:
> On 6/24/23 17:11, Björn Roy Baron wrote:
> > On Saturday, June 24th, 2023 at 11:25, Benno Lossin <benno.lossin@proton.me> wrote:
> >
> >> Add the struct update syntax to the init macros, but only for
> >> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
> >> allows one to omit fields from the initializer, these fields will be
> >> initialized with 0x00 set to every byte. Only types that implement the
> >> `Zeroable` trait can utilize this.
> >>
> >> Suggested-by: Asahi Lina <lina@asahilina.net>
> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> >> ---
> >> rust/kernel/init.rs | 16 +++++-
> >> rust/kernel/init/macros.rs | 114 ++++++++++++++++++++++++++++++++++++-
> >> 2 files changed, 128 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> >> index ecf6a4bd0ce4..44bc3e77419a 100644
> >> --- a/rust/kernel/init.rs
> >> +++ b/rust/kernel/init.rs
> >> @@ -508,14 +508,18 @@ macro_rules! stack_try_pin_init {
> >> /// - Fields that you want to initialize in-place have to use `<-` instead of `:`.
> >> /// - In front of the initializer you can write `&this in` to have access to a [`NonNull<Self>`]
> >> /// pointer named `this` inside of the initializer.
> >> +/// - Using struct update syntax one can place `..Zeroable::zeroed()` at the very end of the
> >> +/// struct, this initializes every field with 0 and then runs all initializers specified in the
> >> +/// body. This can only be done if [`Zeroable`] is implemented for the struct.
> >> ///
> >> /// For instance:
> >> ///
> >> /// ```rust
> >> /// # use kernel::pin_init;
> >> -/// # use macros::pin_data;
> >> +/// # use macros::{pin_data, Zeroable};
> >> /// # use core::{ptr::addr_of_mut, marker::PhantomPinned};
> >> /// #[pin_data]
> >> +/// #[derive(Zeroable)]
> >> /// struct Buf {
> >> /// // `ptr` points into `buf`.
> >> /// ptr: *mut u8,
> >> @@ -528,6 +532,10 @@ macro_rules! stack_try_pin_init {
> >> /// ptr: unsafe { addr_of_mut!((*this.as_ptr()).buf).cast() },
> >> /// pin: PhantomPinned,
> >> /// });
> >> +/// pin_init!(Buf {
> >> +/// buf: [1; 64],
> >> +/// ..Zeroable::zeroed(),
> >> +/// });
> >> /// ```
> >> ///
> >> /// [`try_pin_init!`]: kernel::try_pin_init
> >> @@ -547,6 +555,7 @@ macro_rules! pin_init {
> >> @data(PinData, use_data),
> >> @has_data(HasPinData, __pin_data),
> >> @construct_closure(pin_init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> }
> >> @@ -603,6 +612,7 @@ macro_rules! try_pin_init {
> >> @data(PinData, use_data),
> >> @has_data(HasPinData, __pin_data),
> >> @construct_closure(pin_init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> >> @@ -616,6 +626,7 @@ macro_rules! try_pin_init {
> >> @data(PinData, use_data),
> >> @has_data(HasPinData, __pin_data),
> >> @construct_closure(pin_init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> }
> >> @@ -650,6 +661,7 @@ macro_rules! init {
> >> @data(InitData, /*no use_data*/),
> >> @has_data(HasInitData, __init_data),
> >> @construct_closure(init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> }
> >> }
> >> @@ -700,6 +712,7 @@ macro_rules! try_init {
> >> @data(InitData, /*no use_data*/),
> >> @has_data(HasInitData, __init_data),
> >> @construct_closure(init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> ($(&$this:ident in)? $t:ident $(::<$($generics:ty),* $(,)?>)? {
> >> @@ -713,6 +726,7 @@ macro_rules! try_init {
> >> @data(InitData, /*no use_data*/),
> >> @has_data(HasInitData, __init_data),
> >> @construct_closure(init_from_closure),
> >> + @munch_fields($($fields)*),
> >> )
> >> };
> >> }
> >> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> >> index 1e0c4aca055a..5dcb2e513f26 100644
> >> --- a/rust/kernel/init/macros.rs
> >> +++ b/rust/kernel/init/macros.rs
> >> @@ -989,6 +989,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> >> ///
> >> /// This macro has multiple internal call configurations, these are always the very first ident:
> >> /// - nothing: this is the base case and called by the `{try_}{pin_}init!` macros.
> >> +/// - `with_update_parsed`: when the `..Zeroable::zeroed()` syntax has been handled.
> >> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> >> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> >> /// field has been initialized exactly once.
> >> @@ -1007,6 +1008,82 @@ macro_rules! __init_internal {
> >> @has_data($has_data:ident, $get_data:ident),
> >> // `pin_init_from_closure` or `init_from_closure`.
> >> @construct_closure($construct_closure:ident),
> >> + @munch_fields(),
> >> + ) => {
> >> + $crate::__init_internal!(with_update_parsed:
> >> + @this($($this)?),
> >> + @typ($t $(::<$($generics),*>)? ),
> >> + @fields($($fields)*),
> >> + @error($err),
> >> + @data($data, $($use_data)?),
> >> + @has_data($has_data, $get_data),
> >> + @construct_closure($construct_closure),
> >> + @zeroed(), // nothing means default behavior.
> >> + )
> >> + };
> >> + (
> >> + @this($($this:ident)?),
> >> + @typ($t:ident $(::<$($generics:ty),*>)?),
> >> + @fields($($fields:tt)*),
> >> + @error($err:ty),
> >> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> >> + // case.
> >> + @data($data:ident, $($use_data:ident)?),
> >> + // `HasPinData` or `HasInitData`.
> >> + @has_data($has_data:ident, $get_data:ident),
> >> + // `pin_init_from_closure` or `init_from_closure`.
> >> + @construct_closure($construct_closure:ident),
> >> + @munch_fields(..Zeroable::zeroed()),
> >> + ) => {
> >> + $crate::__init_internal!(with_update_parsed:
> >> + @this($($this)?),
> >> + @typ($t $(::<$($generics),*>)? ),
> >> + @fields($($fields)*),
> >> + @error($err),
> >> + @data($data, $($use_data)?),
> >> + @has_data($has_data, $get_data),
> >> + @construct_closure($construct_closure),
> >> + @zeroed(()), // `()` means zero all fields not mentioned.
> >> + )
> >> + };
> >> + (
> >> + @this($($this:ident)?),
> >> + @typ($t:ident $(::<$($generics:ty),*>)?),
> >> + @fields($($fields:tt)*),
> >> + @error($err:ty),
> >> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> >> + // case.
> >> + @data($data:ident, $($use_data:ident)?),
> >> + // `HasPinData` or `HasInitData`.
> >> + @has_data($has_data:ident, $get_data:ident),
> >> + // `pin_init_from_closure` or `init_from_closure`.
> >> + @construct_closure($construct_closure:ident),
> >> + @munch_fields($ignore:tt $($rest:tt)*),
> >> + ) => {
> >> + $crate::__init_internal!(
> >> + @this($($this)?),
> >> + @typ($t $(::<$($generics),*>)? ),
> >> + @fields($($fields)*),
> >> + @error($err),
> >> + @data($data, $($use_data)?),
> >> + @has_data($has_data, $get_data),
> >> + @construct_closure($construct_closure),
> >> + @munch_fields($($rest)*),
> >> + )
> >> + };
> >> + (with_update_parsed:
> >> + @this($($this:ident)?),
> >> + @typ($t:ident $(::<$($generics:ty),*>)?),
> >> + @fields($($fields:tt)*),
> >> + @error($err:ty),
> >> + // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> >> + // case.
> >> + @data($data:ident, $($use_data:ident)?),
> >> + // `HasPinData` or `HasInitData`.
> >> + @has_data($has_data:ident, $get_data:ident),
> >> + // `pin_init_from_closure` or `init_from_closure`.
> >> + @construct_closure($construct_closure:ident),
> >> + @zeroed($($init_zeroed:expr)?),
> >> ) => {{
> >> // We do not want to allow arbitrary returns, so we declare this type as the `Ok` return
> >> // type and shadow it later when we insert the arbitrary user code. That way there will be
> >> @@ -1024,6 +1101,17 @@ macro_rules! __init_internal {
> >> {
> >> // Shadow the structure so it cannot be used to return early.
> >> struct __InitOk;
> >> + // If `$init_zeroed` is present we should zero the slot now and not emit an
> >> + // error when fields are missing (since they will be zeroed). We also have to
> >> + // check that the type actually implements `Zeroable`.
> >> + $(
> >> + fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> >
> > Maybe call this assert_zeroable?
>
> Sure.
>
> >
> >> + // Ensure that the struct is indeed `Zeroable`.
> >> + is_zeroable(slot);
> >> + // SAFETY: The type implements `Zeroable` by the check above.
> >> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
> >> + $init_zeroed // this will be `()` if set.
> >
> > How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
>
> It is the last expression of a block and since it is `()` it is ok
> (adding a ; would also be ok, but it is not necessary).
I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
allow variables defined inside this as if they were outside of it. Also I can't reproduce this
behavior with:
macro_rules! foo {
($($a:expr)?) => {
$($a)?
bar();
}
}
fn main() {
foo!(());
}
Is there something I'm missing?
Cheers,
Björn
>
> >
> >> + )?
> >> // Create the `this` so it can be referenced by the user inside of the
> >> // expressions creating the individual fields.
> >> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)?
> >> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> >> @data($data:ident),
> >> @slot($slot:ident),
> >> @guards($($guards:ident,)*),
> >> - @munch_fields($(,)?),
> >> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
> >> ) => {
> >> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> >> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> >> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> >> @munch_fields($($rest)*),
> >> );
> >> };
> >> + (make_initializer:
> >> + @slot($slot:ident),
> >> + @type_name($t:ident),
> >> + @munch_fields(..Zeroable::zeroed() $(,)?),
> >> + @acc($($acc:tt)*),
> >> + ) => {
> >> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> >> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> >> + // not been overwritten are thus zero and initialized. We still check that all fields are
> >> + // actually accessible by using the struct update syntax ourselves.
> >> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> >> + // get the correct type inference here:
> >> + unsafe {
> >> + let mut zeroed = ::core::mem::zeroed();
> >> + // We have to use type inference her to make zeroed have the correct type. This does
> >
> > *here
>
> Will fix.
>
> --
> Cheers,
> Benno
>
> >
> >> + // not get executed, so it has no effect.
> >> + ::core::ptr::write($slot, zeroed);
> >> + zeroed = ::core::mem::zeroed();
> >> + ::core::ptr::write($slot, $t {
> >> + $($acc)*
> >> + ..zeroed
> >> + });
> >> + }
> >> + };
> >> (make_initializer:
> >> @slot($slot:ident),
> >> @type_name($t:ident),
> >> --
> >> 2.41.0
> >
> > Cheers,
> > Björn
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-25 12:56 ` Björn Roy Baron
@ 2023-06-25 13:07 ` Benno Lossin
2023-06-25 14:17 ` Björn Roy Baron
0 siblings, 1 reply; 33+ messages in thread
From: Benno Lossin @ 2023-06-25 13:07 UTC (permalink / raw)
To: Björn Roy Baron
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On 25.06.23 14:56, Björn Roy Baron wrote:
> On Saturday, June 24th, 2023 at 23:14, Benno Lossin <benno.lossin@proton.me> wrote:
>>>> + // Ensure that the struct is indeed `Zeroable`.
>>>> + is_zeroable(slot);
>>>> + // SAFETY: The type implements `Zeroable` by the check above.
>>>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
>>>> + $init_zeroed // this will be `()` if set.
>>>
>>> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
>>
>> It is the last expression of a block and since it is `()` it is ok
>> (adding a ; would also be ok, but it is not necessary).
>
> I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
> allow variables defined inside this as if they were outside of it. Also I can't reproduce this
> behavior with:
>
> macro_rules! foo {
> ($($a:expr)?) => {
> $($a)?
> bar();
> }
> }
>
> fn main() {
> foo!(());
> }
>
> Is there something I'm missing?
>
> Cheers,
> Björn
Not sure what you mean with "allow variables defined inside this
as if they were outside of it". But note that in the macro `$init_zeroed`
is the last expression of a block. Here is a small example:
```
macro_rules! foo {
($($a:expr)?) => {{
$(
bar();
$a
)?
}};
}
fn bar() {}
fn main() {
foo!(());
foo!();
}
```
it expands to this:
```
fn main() {
{
bar();
()
};
{};
}
```
--
Cheers,
Benno
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-25 13:07 ` Benno Lossin
@ 2023-06-25 14:17 ` Björn Roy Baron
2023-06-25 16:46 ` Benno Lossin
0 siblings, 1 reply; 33+ messages in thread
From: Björn Roy Baron @ 2023-06-25 14:17 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On Sunday, June 25th, 2023 at 15:07, Benno Lossin <benno.lossin@proton.me> wrote:
> On 25.06.23 14:56, Björn Roy Baron wrote:
> > On Saturday, June 24th, 2023 at 23:14, Benno Lossin <benno.lossin@proton.me> wrote:
>
> >>>> + // Ensure that the struct is indeed `Zeroable`.
> >>>> + is_zeroable(slot);
> >>>> + // SAFETY: The type implements `Zeroable` by the check above.
> >>>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
> >>>> + $init_zeroed // this will be `()` if set.
> >>>
> >>> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
> >>
> >> It is the last expression of a block and since it is `()` it is ok
> >> (adding a ; would also be ok, but it is not necessary).
> >
> > I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
> > allow variables defined inside this as if they were outside of it. Also I can't reproduce this
> > behavior with:
> >
> > macro_rules! foo {
> > ($($a:expr)?) => {
> > $($a)?
> > bar();
> > }
> > }
> >
> > fn main() {
> > foo!(());
> > }
> >
> > Is there something I'm missing?
> >
> > Cheers,
> > Björn
>
> Not sure what you mean with "allow variables defined inside this
> as if they were outside of it". But note that in the macro `$init_zeroed`
> is the last expression of a block. Here is a small example:
$(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)? comes after
this code in the same block that contains struct __InitOk;. And after that another
$crate::__init_internal!() invocation. That is why I don't get that this is allowed
at all.
>
> ```
> macro_rules! foo {
> ($($a:expr)?) => {{
> $(
> bar();
> $a
> )?
> }};
> }
>
> fn bar() {}
>
> fn main() {
> foo!(());
> foo!();
> }
> ```
>
> it expands to this:
> ```
> fn main() {
> {
> bar();
> ()
> };
> {};
> }
> ```
>
> --
> Cheers,
> Benno
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-25 14:17 ` Björn Roy Baron
@ 2023-06-25 16:46 ` Benno Lossin
0 siblings, 0 replies; 33+ messages in thread
From: Benno Lossin @ 2023-06-25 16:46 UTC (permalink / raw)
To: Björn Roy Baron
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Gary Guo, Alice Ryhl, Andreas Hindborg, rust-for-linux,
linux-kernel, patches, Asahi Lina
On 6/25/23 16:17, Björn Roy Baron wrote:
> On Sunday, June 25th, 2023 at 15:07, Benno Lossin <benno.lossin@proton.me> wrote:
>> On 25.06.23 14:56, Björn Roy Baron wrote:
>>> On Saturday, June 24th, 2023 at 23:14, Benno Lossin <benno.lossin@proton.me> wrote:
>>
>>>>>> + // Ensure that the struct is indeed `Zeroable`.
>>>>>> + is_zeroable(slot);
>>>>>> + // SAFETY: The type implements `Zeroable` by the check above.
>>>>>> + unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
>>>>>> + $init_zeroed // this will be `()` if set.
>>>>>
>>>>> How does this work? Shouldn't there be a ; after $init_zeroed to consume the () value?
>>>>
>>>> It is the last expression of a block and since it is `()` it is ok
>>>> (adding a ; would also be ok, but it is not necessary).
>>>
>>> I'm surprised it is considered the last expression of a block. Unlike with {} using $()? will still
>>> allow variables defined inside this as if they were outside of it. Also I can't reproduce this
>>> behavior with:
>>>
>>> macro_rules! foo {
>>> ($($a:expr)?) => {
>>> $($a)?
>>> bar();
>>> }
>>> }
>>>
>>> fn main() {
>>> foo!(());
>>> }
>>>
>>> Is there something I'm missing?
>>>
>>> Cheers,
>>> Björn
>>
>> Not sure what you mean with "allow variables defined inside this
>> as if they were outside of it". But note that in the macro `$init_zeroed`
>> is the last expression of a block. Here is a small example:
>
> $(let $this = unsafe { ::core::ptr::NonNull::new_unchecked(slot) };)? comes after
> this code in the same block that contains struct __InitOk;. And after that another
> $crate::__init_internal!() invocation. That is why I don't get that this is allowed
> at all.
>
Oh I see the issue now, I thought I wrote
```
$({
fn assert_zeroable<T: Zeroable>(ptr: *mut T) {}
// Ensure that the struct is indeed `Zeroable`.
assert_zeroable(slot);
// SAFETY: The type implements `Zeroable` by the check above.
unsafe { ::core::ptr::write_bytes(slot, 0, 1) };
$init_zeroed // this will be `()` if set.
})?
```
But I forgot the inner `{}`. Good catch!
--
Cheers,
Benno
>>
>> ```
>> macro_rules! foo {
>> ($($a:expr)?) => {{
>> $(
>> bar();
>> $a
>> )?
>> }};
>> }
>>
>> fn bar() {}
>>
>> fn main() {
>> foo!(());
>> foo!();
>> }
>> ```
>>
>> it expands to this:
>> ```
>> fn main() {
>> {
>> bar();
>> ()
>> };
>> {};
>> }
>> ```
>>
>> --
>> Cheers,
>> Benno
>>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/7] rust: add derive macro for `Zeroable`
2023-06-24 9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
2023-06-24 14:55 ` Björn Roy Baron
@ 2023-06-25 20:46 ` Gary Guo
2023-07-03 11:50 ` Alice Ryhl
2 siblings, 0 replies; 33+ messages in thread
From: Gary Guo @ 2023-06-25 20:46 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On Sat, 24 Jun 2023 09:25:03 +0000
Benno Lossin <benno.lossin@proton.me> wrote:
> Add a derive proc-macro for the `Zeroable` trait. The macro supports
> structs where every field implements the `Zeroable` trait. This way
> `unsafe` implementations can be avoided.
>
> The macro is split into two parts:
> - a proc-macro to parse generics into impl and ty generics,
> - a declarative macro that expands to the impl block.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> rust/kernel/init/macros.rs | 28 ++++++++++++++++++++++++++++
> rust/kernel/prelude.rs | 2 +-
> rust/macros/lib.rs | 20 ++++++++++++++++++++
> rust/macros/quote.rs | 6 ++++++
> rust/macros/zeroable.rs | 25 +++++++++++++++++++++++++
> 5 files changed, 80 insertions(+), 1 deletion(-)
> create mode 100644 rust/macros/zeroable.rs
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index fbaebd34f218..e8165ff53a94 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -1213,3 +1213,31 @@ macro_rules! __init_internal {
> );
> };
> }
> +
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __derive_zeroable {
> + (parse_input:
> + @sig(
> + $(#[$($struct_attr:tt)*])*
> + $vis:vis struct $name:ident
> + $(where $($whr:tt)*)?
> + ),
> + @impl_generics($($impl_generics:tt)*),
> + @ty_generics($($ty_generics:tt)*),
> + @body({
> + $(
> + $(#[$($field_attr:tt)*])*
> + $field:ident : $field_ty:ty
> + ),* $(,)?
> + }),
> + ) => {
> + // SAFETY: every field type implements `Zeroable` and padding bytes may be zero.
> + #[automatically_derived]
> + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> + where
> + $($field_ty: $crate::Zeroable,)*
> + $($($whr)*)?
> + {}
> + };
> +}
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index c28587d68ebc..ae21600970b3 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -18,7 +18,7 @@
> pub use alloc::{boxed::Box, vec::Vec};
>
> #[doc(no_inline)]
> -pub use macros::{module, pin_data, pinned_drop, vtable};
> +pub use macros::{module, pin_data, pinned_drop, vtable, Zeroable};
>
> pub use super::build_assert;
>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 3fc74cb4ea19..9f056a5c780a 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -10,6 +10,7 @@
> mod pin_data;
> mod pinned_drop;
> mod vtable;
> +mod zeroable;
>
> use proc_macro::TokenStream;
>
> @@ -246,3 +247,22 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
> pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pinned_drop::pinned_drop(args, input)
> }
> +
> +/// Derives the [`Zeroable`] trait for the given struct.
> +///
> +/// This can only be used for structs where every field implements the [`Zeroable`] trait.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// #[derive(Zeroable)]
> +/// pub struct DriverData {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// }
> +/// ```
> +#[proc_macro_derive(Zeroable)]
> +pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> + zeroable::derive(input)
> +}
> diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
> index dddbb4e6f4cb..b76c198a4ed5 100644
> --- a/rust/macros/quote.rs
> +++ b/rust/macros/quote.rs
> @@ -124,6 +124,12 @@ macro_rules! quote_spanned {
> ));
> quote_spanned!(@proc $v $span $($tt)*);
> };
> + (@proc $v:ident $span:ident ; $($tt:tt)*) => {
> + $v.push(::proc_macro::TokenTree::Punct(
> + ::proc_macro::Punct::new(';', ::proc_macro::Spacing::Alone)
> + ));
> + quote_spanned!(@proc $v $span $($tt)*);
> + };
> (@proc $v:ident $span:ident $id:ident $($tt:tt)*) => {
> $v.push(::proc_macro::TokenTree::Ident(::proc_macro::Ident::new(stringify!($id), $span)));
> quote_spanned!(@proc $v $span $($tt)*);
> diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
> new file mode 100644
> index 000000000000..cddb866c44ef
> --- /dev/null
> +++ b/rust/macros/zeroable.rs
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use crate::helpers::{parse_generics, Generics};
> +use proc_macro::TokenStream;
> +
> +pub(crate) fn derive(input: TokenStream) -> TokenStream {
> + let (
> + Generics {
> + impl_generics,
> + ty_generics,
> + },
> + mut rest,
> + ) = parse_generics(input);
> + // This should be the body of the struct `{...}`.
> + let last = rest.pop();
> + quote! {
> + ::kernel::__derive_zeroable!(
> + parse_input:
> + @sig(#(#rest)*),
> + @impl_generics(#(#impl_generics)*),
> + @ty_generics(#(#ty_generics)*),
> + @body(#last),
> + );
> + }
> +}
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/7] rust: init: make guards in the init macros hygienic
2023-06-24 9:25 ` [PATCH 3/7] rust: init: make guards in the init macros hygienic Benno Lossin
2023-06-24 14:58 ` Björn Roy Baron
@ 2023-06-25 20:54 ` Gary Guo
2023-06-28 11:41 ` Benno Lossin
1 sibling, 1 reply; 33+ messages in thread
From: Gary Guo @ 2023-06-25 20:54 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On Sat, 24 Jun 2023 09:25:10 +0000
Benno Lossin <benno.lossin@proton.me> wrote:
> Use hygienic identifiers for the guards instead of the field names. This
> makes the init macros feel more like normal struct initializers, since
> assigning identifiers with the name of a field does not create
> conflicts.
> Also change the internals of the guards, no need to make the `forget`
> function `unsafe`, since users cannot access the guards anyways. Now the
> guards are carried directly on the stack and have no extra `Cell<bool>`
> field that marks if they have been forgotten or not, instead they are
> just forgotten via `mem::forget`.
The code LGTM, so:
Reviewed-by: Gary Guo <gary@garyguo.net>
Although this will cause the new expansion we have to be no longer
compatible with a totally-proc-macro impl, if we want to do everything
in proc macro in the future.
If we have the paste macro upstream (
https://github.com/nbdd0121/linux/commit/fff00461b0be7fd3ec218dcc428f25886b5ec04a
) then we can replace the `guard` with `paste!([<$field>])` and keep
the expansion identical.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/kernel/init.rs | 1 -
> rust/kernel/init/__internal.rs | 25 +++------------
> rust/kernel/init/macros.rs | 56 ++++++++++++----------------------
> 3 files changed, 23 insertions(+), 59 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index d9a91950cba2..ecf6a4bd0ce4 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -206,7 +206,6 @@
> use alloc::boxed::Box;
> use core::{
> alloc::AllocError,
> - cell::Cell,
> convert::Infallible,
> marker::PhantomData,
> mem::MaybeUninit,
> diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
> index 44751fb62b51..7abd1fb65e41 100644
> --- a/rust/kernel/init/__internal.rs
> +++ b/rust/kernel/init/__internal.rs
> @@ -174,7 +174,6 @@ pub fn init<E>(self: Pin<&mut Self>, init: impl PinInit<T, E>) -> Result<Pin<&mu
> /// Can be forgotten to prevent the drop.
> pub struct DropGuard<T: ?Sized> {
> ptr: *mut T,
> - do_drop: Cell<bool>,
> }
>
> impl<T: ?Sized> DropGuard<T> {
> @@ -190,32 +189,16 @@ impl<T: ?Sized> DropGuard<T> {
> /// - will not be dropped by any other means.
> #[inline]
> pub unsafe fn new(ptr: *mut T) -> Self {
> - Self {
> - ptr,
> - do_drop: Cell::new(true),
> - }
> - }
> -
> - /// Prevents this guard from dropping the supplied pointer.
> - ///
> - /// # Safety
> - ///
> - /// This function is unsafe in order to prevent safe code from forgetting this guard. It should
> - /// only be called by the macros in this module.
> - #[inline]
> - pub unsafe fn forget(&self) {
> - self.do_drop.set(false);
> + Self { ptr }
> }
> }
>
> impl<T: ?Sized> Drop for DropGuard<T> {
> #[inline]
> fn drop(&mut self) {
> - if self.do_drop.get() {
> - // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> - // ensuring that this operation is safe.
> - unsafe { ptr::drop_in_place(self.ptr) }
> - }
> + // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function
> + // ensuring that this operation is safe.
> + unsafe { ptr::drop_in_place(self.ptr) }
> }
> }
>
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index e8165ff53a94..df4281743175 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -992,7 +992,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`.
> /// - `make_initializer`: recursively create the struct initializer that guarantees that every
> /// field has been initialized exactly once.
> -/// - `forget_guards`: recursively forget the drop guards for every field.
> #[doc(hidden)]
> #[macro_export]
> macro_rules! __init_internal {
> @@ -1032,6 +1031,7 @@ macro_rules! __init_internal {
> $crate::__init_internal!(init_slot($($use_data)?):
> @data(data),
> @slot(slot),
> + @guards(),
> @munch_fields($($fields)*,),
> );
> // We use unreachable code to ensure that all fields have been mentioned exactly
> @@ -1046,10 +1046,6 @@ macro_rules! __init_internal {
> @acc(),
> );
> }
> - // Forget all guards, since initialization was a success.
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($fields)*,),
> - );
> }
> Ok(__InitOk)
> }
> @@ -1063,13 +1059,17 @@ macro_rules! __init_internal {
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> @munch_fields($(,)?),
> ) => {
> - // Endpoint of munching, no fields are left.
> + // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> + // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> + $(::core::mem::forget($guards);)*
> };
> (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1080,24 +1080,26 @@ macro_rules! __init_internal {
> // return when an error/panic occurs.
> // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`.
> unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // 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 $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($use_data):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot(): // no use_data, so we use `Init::__init` directly.
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // In-place initialization syntax.
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> ) => {
> @@ -1107,24 +1109,26 @@ macro_rules! __init_internal {
> // SAFETY: `slot` is valid, because we are inside of an initializer closure, we
> // return when an error/panic occurs.
> unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? };
> - // Create the drop guard.
> + // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code.
> + // 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 $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot():
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> (init_slot($($use_data:ident)?):
> @data($data:ident),
> @slot($slot:ident),
> + @guards($($guards:ident,)*),
> // Init by-value.
> @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> ) => {
> @@ -1135,16 +1139,17 @@ macro_rules! __init_internal {
> unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) };
> // Create the drop guard:
> //
> - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten.
> + // 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 $field = &unsafe {
> + let guard = unsafe {
> $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
>
> $crate::__init_internal!(init_slot($($use_data)?):
> @data($data),
> @slot($slot),
> + @guards(guard, $($guards,)*),
> @munch_fields($($rest)*),
> );
> };
> @@ -1189,29 +1194,6 @@ macro_rules! __init_internal {
> @acc($($acc)* $field: ::core::panic!(),),
> );
> };
> - (forget_guards:
> - @munch_fields($(,)?),
> - ) => {
> - // Munching finished.
> - };
> - (forget_guards:
> - @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> - (forget_guards:
> - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> - ) => {
> - unsafe { $crate::init::__internal::DropGuard::forget($field) };
> -
> - $crate::__init_internal!(forget_guards:
> - @munch_fields($($rest)*),
> - );
> - };
> }
>
> #[doc(hidden)]
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 7/7] rust: init: add support for arbitrary paths in init macros
2023-06-24 9:25 ` [PATCH 7/7] rust: init: add support for arbitrary paths in init macros Benno Lossin
2023-06-24 15:20 ` Björn Roy Baron
@ 2023-06-25 21:01 ` Gary Guo
2023-06-28 11:26 ` Benno Lossin
1 sibling, 1 reply; 33+ messages in thread
From: Gary Guo @ 2023-06-25 21:01 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On Sat, 24 Jun 2023 09:25:39 +0000
Benno Lossin <benno.lossin@proton.me> wrote:
> Previously only `ident` and generic types were supported in the
> `{try_}{pin_}init!` macros. This patch allows arbitrary path fragments,
> so for example `Foo::Bar` but also very complex paths such as
> `<Foo as Baz>::Bar::<0, i32>`.
>
> Internally this is accomplished by using `path` fragments. Due to some
> peculiar declarative macro limitations, we have to "forget" certain
> additional parsing information in the token trees. This is achieved by
> the new `retokenize` proc macro. It does not modify the input, but just
> strips this information. For example, if a declarative macro takes
> `$t:path` as its input, it cannot sensibly propagate this to a macro that
> takes `$($p:tt)*` as its input, since the `$t` token will only be
> considered one `tt` token for the second macro. If we first pipe the
> tokens through `retokenize`, then it parses as expected.
I think this "retokenize" macro could also be functionally replaced by
`paste`. Would you mind to apply my paste patch (referenced in a
previous email) and see if it works?
Best,
Gary
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/kernel/init/__internal.rs | 2 ++
> rust/kernel/init/macros.rs | 42 +++++++++++++++++++---------------
> rust/macros/lib.rs | 17 +++++++++++++-
> 3 files changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs
> index 7abd1fb65e41..e36a706a4a1b 100644
> --- a/rust/kernel/init/__internal.rs
> +++ b/rust/kernel/init/__internal.rs
> @@ -9,6 +9,8 @@
>
> use super::*;
>
> +pub use ::macros::retokenize;
> +
> /// See the [nomicon] for what subtyping is. See also [this table].
> ///
> /// [nomicon]: https://doc.rust-lang.org/nomicon/subtyping.html
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 5dcb2e513f26..6a82be675808 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -998,7 +998,7 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*>
> macro_rules! __init_internal {
> (
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1012,7 +1012,7 @@ macro_rules! __init_internal {
> ) => {
> $crate::__init_internal!(with_update_parsed:
> @this($($this)?),
> - @typ($t $(::<$($generics),*>)? ),
> + @typ($t),
> @fields($($fields)*),
> @error($err),
> @data($data, $($use_data)?),
> @@ -1023,7 +1023,7 @@ macro_rules! __init_internal {
> };
> (
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1037,7 +1037,7 @@ macro_rules! __init_internal {
> ) => {
> $crate::__init_internal!(with_update_parsed:
> @this($($this)?),
> - @typ($t $(::<$($generics),*>)? ),
> + @typ($t),
> @fields($($fields)*),
> @error($err),
> @data($data, $($use_data)?),
> @@ -1048,7 +1048,7 @@ macro_rules! __init_internal {
> };
> (
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1062,7 +1062,7 @@ macro_rules! __init_internal {
> ) => {
> $crate::__init_internal!(
> @this($($this)?),
> - @typ($t $(::<$($generics),*>)? ),
> + @typ($t),
> @fields($($fields)*),
> @error($err),
> @data($data, $($use_data)?),
> @@ -1073,7 +1073,7 @@ macro_rules! __init_internal {
> };
> (with_update_parsed:
> @this($($this:ident)?),
> - @typ($t:ident $(::<$($generics:ty),*>)?),
> + @typ($t:path),
> @fields($($fields:tt)*),
> @error($err:ty),
> // Either `PinData` or `InitData`, `$use_data` should only be present in the `PinData`
> @@ -1092,7 +1092,7 @@ macro_rules! __init_internal {
> // Get the data about fields from the supplied type.
> let data = unsafe {
> use $crate::init::__internal::$has_data;
> - $t$(::<$($generics),*>)?::$get_data()
> + $crate::init::__internal::retokenize!($t::$get_data())
> };
> // Ensure that `data` really is of type `$data` and help with type inference:
> let init = $crate::init::__internal::$data::make_closure::<_, __InitOk, $err>(
> @@ -1247,7 +1247,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields(..Zeroable::zeroed() $(,)?),
> @acc($($acc:tt)*),
> ) => {
> @@ -1263,15 +1263,17 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> // not get executed, so it has no effect.
> ::core::ptr::write($slot, zeroed);
> zeroed = ::core::mem::zeroed();
> - ::core::ptr::write($slot, $t {
> - $($acc)*
> - ..zeroed
> - });
> + $crate::init::__internal::retokenize!(
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + ..zeroed
> + });
> + );
> }
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields($(,)?),
> @acc($($acc:tt)*),
> ) => {
> @@ -1279,14 +1281,16 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> // get the correct type inference here:
> unsafe {
> - ::core::ptr::write($slot, $t {
> - $($acc)*
> - });
> + $crate::init::__internal::retokenize!(
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + });
> + );
> }
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields($field:ident <- $val:expr, $($rest:tt)*),
> @acc($($acc:tt)*),
> ) => {
> @@ -1299,7 +1303,7 @@ fn is_zeroable<T: Zeroable>(ptr: *mut T) {}
> };
> (make_initializer:
> @slot($slot:ident),
> - @type_name($t:ident),
> + @type_name($t:path),
> @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*),
> @acc($($acc:tt)*),
> ) => {
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 9f056a5c780a..d329ab622fd4 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -12,7 +12,7 @@
> mod vtable;
> mod zeroable;
>
> -use proc_macro::TokenStream;
> +use proc_macro::{Group, TokenStream, TokenTree};
>
> /// Declares a kernel module.
> ///
> @@ -266,3 +266,18 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> zeroable::derive(input)
> }
> +
> +/// Does not modify the given TokenStream, but removes any declarative macro information.
> +#[proc_macro]
> +pub fn retokenize(input: TokenStream) -> TokenStream {
> + fn id(tt: TokenTree) -> TokenTree {
> + match tt {
> + TokenTree::Group(g) => TokenTree::Group(Group::new(
> + g.delimiter(),
> + g.stream().into_iter().map(id).collect(),
> + )),
> + x => x,
> + }
> + }
> + input.into_iter().map(id).collect()
> +}
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 7/7] rust: init: add support for arbitrary paths in init macros
2023-06-25 21:01 ` Gary Guo
@ 2023-06-28 11:26 ` Benno Lossin
2023-06-28 17:13 ` Gary Guo
0 siblings, 1 reply; 33+ messages in thread
From: Benno Lossin @ 2023-06-28 11:26 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On 25.06.23 23:01, Gary Guo wrote:
> On Sat, 24 Jun 2023 09:25:39 +0000
> Benno Lossin <benno.lossin@proton.me> wrote:
>
>> Previously only `ident` and generic types were supported in the
>> `{try_}{pin_}init!` macros. This patch allows arbitrary path fragments,
>> so for example `Foo::Bar` but also very complex paths such as
>> `<Foo as Baz>::Bar::<0, i32>`.
>>
>> Internally this is accomplished by using `path` fragments. Due to some
>> peculiar declarative macro limitations, we have to "forget" certain
>> additional parsing information in the token trees. This is achieved by
>> the new `retokenize` proc macro. It does not modify the input, but just
>> strips this information. For example, if a declarative macro takes
>> `$t:path` as its input, it cannot sensibly propagate this to a macro that
>> takes `$($p:tt)*` as its input, since the `$t` token will only be
>> considered one `tt` token for the second macro. If we first pipe the
>> tokens through `retokenize`, then it parses as expected.
>
> I think this "retokenize" macro could also be functionally replaced by
> `paste`. Would you mind to apply my paste patch (referenced in a
> previous email) and see if it works?
I tried your patch and it seems to work. I also executed all of the test
in the userspace library and they passed.
The `paste!` code also looks good to me. One thing that I thought was this:
do we want to accept `paste!( [<= foo bar>])`? Because the `<` token has
spacing `Joint`, maybe add a check for that? No idea if that would be a problem.
--
Cheers,
Benno
>
> Best,
> Gary
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/7] rust: init: make guards in the init macros hygienic
2023-06-25 20:54 ` Gary Guo
@ 2023-06-28 11:41 ` Benno Lossin
2023-06-28 16:48 ` Gary Guo
0 siblings, 1 reply; 33+ messages in thread
From: Benno Lossin @ 2023-06-28 11:41 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On 25.06.23 22:54, Gary Guo wrote:
> On Sat, 24 Jun 2023 09:25:10 +0000
> Benno Lossin <benno.lossin@proton.me> wrote:
>
>> Use hygienic identifiers for the guards instead of the field names. This
>> makes the init macros feel more like normal struct initializers, since
>> assigning identifiers with the name of a field does not create
>> conflicts.
>> Also change the internals of the guards, no need to make the `forget`
>> function `unsafe`, since users cannot access the guards anyways. Now the
>> guards are carried directly on the stack and have no extra `Cell<bool>`
>> field that marks if they have been forgotten or not, instead they are
>> just forgotten via `mem::forget`.
>
> The code LGTM, so:
>
> Reviewed-by: Gary Guo <gary@garyguo.net>
>
> Although this will cause the new expansion we have to be no longer
> compatible with a totally-proc-macro impl, if we want to do everything
> in proc macro in the future.
>
> If we have the paste macro upstream (
> https://github.com/nbdd0121/linux/commit/fff00461b0be7fd3ec218dcc428f25886b5ec04a
> ) then we can replace the `guard` with `paste!([<$field>])` and keep
> the expansion identical.
>
I tried it and it seems to work, but I am not sure why the hygiene is
set correctly. Could you maybe explain why this works?
```
$crate::__internal::paste!{
let [<$field>] = unsafe {
$crate::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
};
$crate::__init_internal!(init_slot($use_data):
@data($data),
@slot($slot),
@guards([<$field>], $($guards,)*),
@munch_fields($($rest)*),
);
}
```
i.e. why can't a user access the guard? I think it is because the hygiene of the `[<>]`
is used, but not sure why that works.
--
Cheers,
Benno
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/7] rust: init: make guards in the init macros hygienic
2023-06-28 11:41 ` Benno Lossin
@ 2023-06-28 16:48 ` Gary Guo
0 siblings, 0 replies; 33+ messages in thread
From: Gary Guo @ 2023-06-28 16:48 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On Wed, 28 Jun 2023 11:41:59 +0000
Benno Lossin <benno.lossin@proton.me> wrote:
> On 25.06.23 22:54, Gary Guo wrote:
> > On Sat, 24 Jun 2023 09:25:10 +0000
> > Benno Lossin <benno.lossin@proton.me> wrote:
> >
> >> Use hygienic identifiers for the guards instead of the field names. This
> >> makes the init macros feel more like normal struct initializers, since
> >> assigning identifiers with the name of a field does not create
> >> conflicts.
> >> Also change the internals of the guards, no need to make the `forget`
> >> function `unsafe`, since users cannot access the guards anyways. Now the
> >> guards are carried directly on the stack and have no extra `Cell<bool>`
> >> field that marks if they have been forgotten or not, instead they are
> >> just forgotten via `mem::forget`.
> >
> > The code LGTM, so:
> >
> > Reviewed-by: Gary Guo <gary@garyguo.net>
> >
> > Although this will cause the new expansion we have to be no longer
> > compatible with a totally-proc-macro impl, if we want to do everything
> > in proc macro in the future.
> >
> > If we have the paste macro upstream (
> > https://github.com/nbdd0121/linux/commit/fff00461b0be7fd3ec218dcc428f25886b5ec04a
> > ) then we can replace the `guard` with `paste!([<$field>])` and keep
> > the expansion identical.
> >
>
> I tried it and it seems to work, but I am not sure why the hygiene is
> set correctly. Could you maybe explain why this works?
> ```
> $crate::__internal::paste!{
> let [<$field>] = unsafe {
> $crate::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field))
> };
> $crate::__init_internal!(init_slot($use_data):
> @data($data),
> @slot($slot),
> @guards([<$field>], $($guards,)*),
> @munch_fields($($rest)*),
> );
> }
> ```
>
> i.e. why can't a user access the guard? I think it is because the hygiene of the `[<>]`
> is used, but not sure why that works.
Yes, by default the hygiene of pasted macro is that of the group,
unless explicitly overriden.
Best,
Gary
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 7/7] rust: init: add support for arbitrary paths in init macros
2023-06-28 11:26 ` Benno Lossin
@ 2023-06-28 17:13 ` Gary Guo
0 siblings, 0 replies; 33+ messages in thread
From: Gary Guo @ 2023-06-28 17:13 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Boqun Feng,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On Wed, 28 Jun 2023 11:26:54 +0000
Benno Lossin <benno.lossin@proton.me> wrote:
> On 25.06.23 23:01, Gary Guo wrote:
> > On Sat, 24 Jun 2023 09:25:39 +0000
> > Benno Lossin <benno.lossin@proton.me> wrote:
> >
> >> Previously only `ident` and generic types were supported in the
> >> `{try_}{pin_}init!` macros. This patch allows arbitrary path fragments,
> >> so for example `Foo::Bar` but also very complex paths such as
> >> `<Foo as Baz>::Bar::<0, i32>`.
> >>
> >> Internally this is accomplished by using `path` fragments. Due to some
> >> peculiar declarative macro limitations, we have to "forget" certain
> >> additional parsing information in the token trees. This is achieved by
> >> the new `retokenize` proc macro. It does not modify the input, but just
> >> strips this information. For example, if a declarative macro takes
> >> `$t:path` as its input, it cannot sensibly propagate this to a macro that
> >> takes `$($p:tt)*` as its input, since the `$t` token will only be
> >> considered one `tt` token for the second macro. If we first pipe the
> >> tokens through `retokenize`, then it parses as expected.
> >
> > I think this "retokenize" macro could also be functionally replaced by
> > `paste`. Would you mind to apply my paste patch (referenced in a
> > previous email) and see if it works?
>
> I tried your patch and it seems to work. I also executed all of the test
> in the userspace library and they passed.
> The `paste!` code also looks good to me. One thing that I thought was this:
> do we want to accept `paste!( [<= foo bar>])`? Because the `<` token has
> spacing `Joint`, maybe add a check for that? No idea if that would be a problem.
>
I don't think checking that is necessary, since that wouldn't be
valid Rust anyway?
Best,
Gary
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/7] rust: add derive macro for `Zeroable`
2023-06-24 9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
2023-06-24 14:55 ` Björn Roy Baron
2023-06-25 20:46 ` Gary Guo
@ 2023-07-03 11:50 ` Alice Ryhl
2 siblings, 0 replies; 33+ messages in thread
From: Alice Ryhl @ 2023-07-03 11:50 UTC (permalink / raw)
To: benno.lossin
Cc: alex.gaynor, aliceryhl, bjorn3_gh, boqun.feng, gary, lina,
linux-kernel, nmi, ojeda, patches, rust-for-linux, wedsonaf
Benno Lossin <benno.lossin@proton.me> writes:
> Add a derive proc-macro for the `Zeroable` trait. The macro supports
> structs where every field implements the `Zeroable` trait. This way
> `unsafe` implementations can be avoided.
>
> The macro is split into two parts:
> - a proc-macro to parse generics into impl and ty generics,
> - a declarative macro that expands to the impl block.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-24 9:25 ` [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
2023-06-24 15:11 ` Björn Roy Baron
@ 2023-07-03 11:58 ` Alice Ryhl
2023-07-03 18:15 ` Boqun Feng
2 siblings, 0 replies; 33+ messages in thread
From: Alice Ryhl @ 2023-07-03 11:58 UTC (permalink / raw)
To: benno.lossin
Cc: alex.gaynor, aliceryhl, bjorn3_gh, boqun.feng, gary, lina,
linux-kernel, nmi, ojeda, patches, rust-for-linux, wedsonaf
Benno Lossin <benno.lossin@proton.me> writes:
> Add the struct update syntax to the init macros, but only for
> `..Zeroable::zeroed()`. Adding this at the end of the struct initializer
> allows one to omit fields from the initializer, these fields will be
> initialized with 0x00 set to every byte. Only types that implement the
> `Zeroable` trait can utilize this.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
I'm a bit surprised by how large this change is, but it looks ok.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 6/7] rust: init: Add functions to create array initializers
2023-06-24 9:25 ` [PATCH 6/7] rust: init: Add functions to create array initializers Benno Lossin
2023-06-24 15:17 ` Björn Roy Baron
@ 2023-07-03 12:03 ` Alice Ryhl
1 sibling, 0 replies; 33+ messages in thread
From: Alice Ryhl @ 2023-07-03 12:03 UTC (permalink / raw)
To: benno.lossin
Cc: alex.gaynor, aliceryhl, bjorn3_gh, boqun.feng, gary, lina,
linux-kernel, nmi, ojeda, patches, rust-for-linux, wedsonaf
Benno Lossin <benno.lossin@proton.me> writes:
> Add two functions `pin_init_array_from_fn` and `init_array_from_fn` that
> take a function that generates initializers for `T` from usize, the added
> functions then return an initializer for `[T; N]` where every element is
> initialized by an element returned from the generator function.
>
> Suggested-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> +/// Initializes an array by initializing each element via the provided initializer.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// let array: Box<[usize; 1000_000_000]>= Box::init(init_array_from_fn(|i| i)).unwrap();
> +/// println!("{array:?}");
> +/// ```
> +pub fn init_array_from_fn<I, const N: usize, T, E>(
> + mut make_init: impl FnMut(usize) -> I,
> +) -> impl Init<[T; N], E>
> +where
> + I: Init<T, E>,
> +{
> + let init = move |slot: *mut [T; N]| {
> + let slot = slot.cast::<T>();
> + for i in 0..N {
> + let init = make_init(i);
> + // SAFETY: since 0 <= `i` < N, it is still in bounds of `[T; N]`.
> + let ptr = unsafe { slot.add(i) };
> + // SAFETY: The pointer is derived from `slot` and thus satisfies the `__init`
> + // requirements.
> + match unsafe { init.__init(ptr) } {
> + Ok(()) => {}
> + Err(e) => {
> + // We now free every element that has been initialized before:
> + for j in 0..i {
> + let ptr = unsafe { slot.add(j) };
> + // SAFETY: The value was initialized in a previous iteration of the loop
> + // and since we return `Err` below, the caller will consider the memory at
> + // `slot` as uninitialized.
> + unsafe { ptr::drop_in_place(ptr) };
> + }
The loop can be simplified like this:
ptr::drop_in_place(ptr::slice_from_raw_parts(slot, i));
Yes, this actually works and will run the destructor of each value.
Alice
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-06-24 9:25 ` [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
2023-06-24 15:11 ` Björn Roy Baron
2023-07-03 11:58 ` Alice Ryhl
@ 2023-07-03 18:15 ` Boqun Feng
2023-07-05 17:48 ` Gary Guo
2 siblings, 1 reply; 33+ messages in thread
From: Boqun Feng @ 2023-07-03 18:15 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor, Gary Guo,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On Sat, Jun 24, 2023 at 09:25:19AM +0000, Benno Lossin wrote:
[...]
(this is `init_slot`)
> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> @data($data:ident),
> @slot($slot:ident),
> @guards($($guards:ident,)*),
> - @munch_fields($(,)?),
> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
since you append an unconditional comma ',' to init_slot and
make_initializer when "calling" them in with_update_parsed, shouldn't
this be:
+ @munch_fields($(..Zeroable::zeroed(),)? $(,)?),
, and..
> ) => {
> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> @munch_fields($($rest)*),
> );
> };
> + (make_initializer:
> + @slot($slot:ident),
> + @type_name($t:ident),
> + @munch_fields(..Zeroable::zeroed() $(,)?),
this should be:
+ @munch_fields(..Zeroable::zeroed() , $(,)?),
Otherwise the example before `pin_init!()` wouldn't compile:
/// pin_init!(Buf {
/// buf: [1; 64],
/// ..Zeroable::zeroed(),
/// });
Regards,
Boqun
> + @acc($($acc:tt)*),
> + ) => {
> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> + // not been overwritten are thus zero and initialized. We still check that all fields are
> + // actually accessible by using the struct update syntax ourselves.
> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> + // get the correct type inference here:
> + unsafe {
> + let mut zeroed = ::core::mem::zeroed();
> + // We have to use type inference her to make zeroed have the correct type. This does
> + // not get executed, so it has no effect.
> + ::core::ptr::write($slot, zeroed);
> + zeroed = ::core::mem::zeroed();
> + ::core::ptr::write($slot, $t {
> + $($acc)*
> + ..zeroed
> + });
> + }
> + };
> (make_initializer:
> @slot($slot:ident),
> @type_name($t:ident),
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-07-03 18:15 ` Boqun Feng
@ 2023-07-05 17:48 ` Gary Guo
2023-07-05 21:44 ` Benno Lossin
0 siblings, 1 reply; 33+ messages in thread
From: Gary Guo @ 2023-07-05 17:48 UTC (permalink / raw)
To: Boqun Feng
Cc: Benno Lossin, Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On Mon, 3 Jul 2023 11:15:55 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:
> On Sat, Jun 24, 2023 at 09:25:19AM +0000, Benno Lossin wrote:
> [...]
> (this is `init_slot`)
> > @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
> > @data($data:ident),
> > @slot($slot:ident),
> > @guards($($guards:ident,)*),
> > - @munch_fields($(,)?),
> > + @munch_fields($(..Zeroable::zeroed())? $(,)?),
>
> since you append an unconditional comma ',' to init_slot and
> make_initializer when "calling" them in with_update_parsed, shouldn't
> this be:
>
> + @munch_fields($(..Zeroable::zeroed(),)? $(,)?),
>
> , and..
>
> > ) => {
> > // Endpoint of munching, no fields are left. If execution reaches this point, all fields
> > // have been initialized. Therefore we can now dismiss the guards by forgetting them.
> > @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
> > @munch_fields($($rest)*),
> > );
> > };
> > + (make_initializer:
> > + @slot($slot:ident),
> > + @type_name($t:ident),
> > + @munch_fields(..Zeroable::zeroed() $(,)?),
>
> this should be:
>
> + @munch_fields(..Zeroable::zeroed() , $(,)?),
>
> Otherwise the example before `pin_init!()` wouldn't compile:
>
> /// pin_init!(Buf {
> /// buf: [1; 64],
> /// ..Zeroable::zeroed(),
> /// });
Comma is not allowed after base struct.
>
> Regards,
> Boqun
>
> > + @acc($($acc:tt)*),
> > + ) => {
> > + // Endpoint, nothing more to munch, create the initializer. Since the users specified
> > + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
> > + // not been overwritten are thus zero and initialized. We still check that all fields are
> > + // actually accessible by using the struct update syntax ourselves.
> > + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
> > + // get the correct type inference here:
> > + unsafe {
> > + let mut zeroed = ::core::mem::zeroed();
> > + // We have to use type inference her to make zeroed have the correct type. This does
> > + // not get executed, so it has no effect.
> > + ::core::ptr::write($slot, zeroed);
> > + zeroed = ::core::mem::zeroed();
> > + ::core::ptr::write($slot, $t {
> > + $($acc)*
> > + ..zeroed
> > + });
> > + }
> > + };
> > (make_initializer:
> > @slot($slot:ident),
> > @type_name($t:ident),
> > --
> > 2.41.0
> >
> >
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields
2023-07-05 17:48 ` Gary Guo
@ 2023-07-05 21:44 ` Benno Lossin
0 siblings, 0 replies; 33+ messages in thread
From: Benno Lossin @ 2023-07-05 21:44 UTC (permalink / raw)
To: Gary Guo
Cc: Boqun Feng, Miguel Ojeda, Wedson Almeida Filho, Alex Gaynor,
Björn Roy Baron, Alice Ryhl, Andreas Hindborg,
rust-for-linux, linux-kernel, patches, Asahi Lina
On 05.07.23 19:48, Gary Guo wrote:
> On Mon, 3 Jul 2023 11:15:55 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
>
>> On Sat, Jun 24, 2023 at 09:25:19AM +0000, Benno Lossin wrote:
>> [...]
>> (this is `init_slot`)
>>> @@ -1064,7 +1152,7 @@ macro_rules! __init_internal {
>>> @data($data:ident),
>>> @slot($slot:ident),
>>> @guards($($guards:ident,)*),
>>> - @munch_fields($(,)?),
>>> + @munch_fields($(..Zeroable::zeroed())? $(,)?),
>>
>> since you append an unconditional comma ',' to init_slot and
>> make_initializer when "calling" them in with_update_parsed, shouldn't
>> this be:
>>
>> + @munch_fields($(..Zeroable::zeroed(),)? $(,)?),
>>
>> , and..
>>
>>> ) => {
>>> // Endpoint of munching, no fields are left. If execution reaches this point, all fields
>>> // have been initialized. Therefore we can now dismiss the guards by forgetting them.
>>> @@ -1157,6 +1245,30 @@ macro_rules! __init_internal {
>>> @munch_fields($($rest)*),
>>> );
>>> };
>>> + (make_initializer:
>>> + @slot($slot:ident),
>>> + @type_name($t:ident),
>>> + @munch_fields(..Zeroable::zeroed() $(,)?),
>>
>> this should be:
>>
>> + @munch_fields(..Zeroable::zeroed() , $(,)?),
>>
>> Otherwise the example before `pin_init!()` wouldn't compile:
>>
>> /// pin_init!(Buf {
>> /// buf: [1; 64],
>> /// ..Zeroable::zeroed(),
>> /// });
>
> Comma is not allowed after base struct.
Yes this is a mistake in the example, will fix.
--
Cheers,
Benno
>
>>
>> Regards,
>> Boqun
>>
>>> + @acc($($acc:tt)*),
>>> + ) => {
>>> + // Endpoint, nothing more to munch, create the initializer. Since the users specified
>>> + // `..Zeroable::zeroed()`, the slot will already have been zeroed and all field that have
>>> + // not been overwritten are thus zero and initialized. We still check that all fields are
>>> + // actually accessible by using the struct update syntax ourselves.
>>> + // Since we are in the `if false` branch, this will never get executed. We abuse `slot` to
>>> + // get the correct type inference here:
>>> + unsafe {
>>> + let mut zeroed = ::core::mem::zeroed();
>>> + // We have to use type inference her to make zeroed have the correct type. This does
>>> + // not get executed, so it has no effect.
>>> + ::core::ptr::write($slot, zeroed);
>>> + zeroed = ::core::mem::zeroed();
>>> + ::core::ptr::write($slot, $t {
>>> + $($acc)*
>>> + ..zeroed
>>> + });
>>> + }
>>> + };
>>> (make_initializer:
>>> @slot($slot:ident),
>>> @type_name($t:ident),
>>> --
>>> 2.41.0
>>>
>>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2023-07-05 21:44 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-24 9:24 [PATCH 1/7] rust: init: consolidate init macros Benno Lossin
2023-06-24 9:25 ` [PATCH 2/7] rust: add derive macro for `Zeroable` Benno Lossin
2023-06-24 14:55 ` Björn Roy Baron
2023-06-25 20:46 ` Gary Guo
2023-07-03 11:50 ` Alice Ryhl
2023-06-24 9:25 ` [PATCH 3/7] rust: init: make guards in the init macros hygienic Benno Lossin
2023-06-24 14:58 ` Björn Roy Baron
2023-06-25 20:54 ` Gary Guo
2023-06-28 11:41 ` Benno Lossin
2023-06-28 16:48 ` Gary Guo
2023-06-24 9:25 ` [PATCH 4/7] rust: init: wrap type checking struct initializers in a closure Benno Lossin
2023-06-24 15:03 ` Björn Roy Baron
2023-06-24 21:05 ` Benno Lossin
2023-06-24 9:25 ` [PATCH 5/7] rust: init: add `..Zeroable::zeroed()` syntax for zeroing all missing fields Benno Lossin
2023-06-24 15:11 ` Björn Roy Baron
2023-06-24 21:14 ` Benno Lossin
2023-06-25 12:56 ` Björn Roy Baron
2023-06-25 13:07 ` Benno Lossin
2023-06-25 14:17 ` Björn Roy Baron
2023-06-25 16:46 ` Benno Lossin
2023-07-03 11:58 ` Alice Ryhl
2023-07-03 18:15 ` Boqun Feng
2023-07-05 17:48 ` Gary Guo
2023-07-05 21:44 ` Benno Lossin
2023-06-24 9:25 ` [PATCH 6/7] rust: init: Add functions to create array initializers Benno Lossin
2023-06-24 15:17 ` Björn Roy Baron
2023-07-03 12:03 ` Alice Ryhl
2023-06-24 9:25 ` [PATCH 7/7] rust: init: add support for arbitrary paths in init macros Benno Lossin
2023-06-24 15:20 ` Björn Roy Baron
2023-06-25 21:01 ` Gary Guo
2023-06-28 11:26 ` Benno Lossin
2023-06-28 17:13 ` Gary Guo
2023-06-24 14:49 ` [PATCH 1/7] rust: init: consolidate " Björn Roy Baron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).