From patchwork Wed Jul 19 14:20:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benno Lossin X-Patchwork-Id: 122660 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c923:0:b0:3e4:2afc:c1 with SMTP id j3csp2485160vqt; Wed, 19 Jul 2023 07:39:21 -0700 (PDT) X-Google-Smtp-Source: APBJJlHtnDceaYi4gCNllUszRemkVBRbPLW+7uIUmrPSpGfmcK/biID3PTkyUmoJewrch6ihZvAf X-Received: by 2002:aa7:d659:0:b0:51d:a488:3b3d with SMTP id v25-20020aa7d659000000b0051da4883b3dmr2541458edr.35.1689777560674; Wed, 19 Jul 2023 07:39:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689777560; cv=none; d=google.com; s=arc-20160816; b=f9x6asxl+RXfDWiimlwmBhbeSOtsuqqr/l9r+7QdSDG3G+uqyWjio2hRUbo+QPZ1S+ hRky8WhBrWOpl1D8a9sluQ8BFlw7pzXBLcAX3a8RXDH0isCTeySr1sF/lfVluHGxaT8R 7tsAQ/EzR9a344ROaPdOjSlRfVLnZtISSxP8ztSJA9VHuP7RCUIC8xaMGFuAwnKPFW5o 84Okj5WdQvqLeH1tOKg9UdGRGXB5zg3IlbKjnqjuQaPpMHHfDgn86RFlBreje8diOi1Q KYeyYK86fA2uYFoebAFx5CEZuQlttT/G5Hwv+KtffYGVNEhWcORbRqxXdktHtaDZe1t1 59ag== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :feedback-id:references:in-reply-to:message-id:subject:cc:from:to :dkim-signature:date; bh=SS2q1NjexBrjmn/WRxMHKEqapQDfI+FDXAjjD6I2TmQ=; fh=ahHPrvxY+ty2HIuIPh5QXslVmwB1wHeJjzGZ+GeRryw=; b=s7TZualFv6i6zmYaNuweDO38oe/A7bRPvRJnnnYDNMbl6nBIEDI4LpJv/BD1lFkFqH k8/inM12Qw1IZB1G7/vQ9Cwyg4V0wJ4tKlInhRrKtbrHQCXoy0R4i1p+obmLxJjACRei ZAChpMLrb2j2PRbzm+sdBRxv5301YeNAxXjlOM7j4MrZ3d6yuPOJoWuW/QUQw3jIJnt1 A0afrOuEPKpF0y3DVHTNFzVJvfHCM1PubSqi5/kwmHUiaBXBrBFRdHk5ttBG/weOXFpx Lp0fCoOjoLFWQmQ2/sGcvkkCaH4B/BpllaIiCjOXKNs/tfWnQJTLL9uaewkF7EbXSUgn UrBA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@proton.me header.s=protonmail header.b=f2ZPvplH; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=proton.me Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id j1-20020a50ed01000000b00521a8adfdebsi2890447eds.601.2023.07.19.07.38.56; Wed, 19 Jul 2023 07:39:20 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@proton.me header.s=protonmail header.b=f2ZPvplH; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=proton.me Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231524AbjGSOVA (ORCPT + 99 others); Wed, 19 Jul 2023 10:21:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50902 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231529AbjGSOUy (ORCPT ); Wed, 19 Jul 2023 10:20:54 -0400 Received: from mail-40133.protonmail.ch (mail-40133.protonmail.ch [185.70.40.133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6AECC1723 for ; Wed, 19 Jul 2023 07:20:47 -0700 (PDT) Date: Wed, 19 Jul 2023 14:20:29 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1689776445; x=1690035645; bh=SS2q1NjexBrjmn/WRxMHKEqapQDfI+FDXAjjD6I2TmQ=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=f2ZPvplHFjBE4cxEKAJzhCiZ/I6AMsGvCfpV0CraUr/DUOtlkmGHVBMUNVdsG21uN 06Uf3E+t/OWxWPJ39n5iiHmZnwbL3DUZBzq0l2HmpgdsZtoF1jdbHG9vjj/L4+aTn1 gyPQ89zBWcalOz/YnDYGDXAJfcGB9wWQtb+8fXLIxECrwauIAf3tI42HH/dCLnxCen TdnSw83mmoMgYWlDdCtFtCOShyRKMEJVAmjlxjRKdUT9q9JM0+KKhWSHUVnyaPLERR YN8lgxVhQo95K5knFCpwaMRFUo9JAU3eb2LcR2mjGL77HW+qo6QVh/ojTvZSiQP5Xn UR6I7O4dj6Psw== To: Miguel Ojeda , Wedson Almeida Filho , Alex Gaynor From: Benno Lossin Cc: Boqun Feng , Gary Guo , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Alice Ryhl , Andreas Hindborg , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Asahi Lina Subject: [PATCH v2 03/12] rust: init: make guards in the init macros hygienic Message-ID: <20230719141918.543938-4-benno.lossin@proton.me> In-Reply-To: <20230719141918.543938-1-benno.lossin@proton.me> References: <20230719141918.543938-1-benno.lossin@proton.me> Feedback-ID: 71780778:user:proton MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771860195887142034 X-GMAIL-MSGID: 1771860195887142034 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` field that marks if they have been forgotten or not, instead they are just forgotten via `mem::forget`. Suggested-by: Asahi Lina Signed-off-by: Benno Lossin Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl --- v1 -> v2: * use Gary's `paste!` macro to create the guard hygiene rust/kernel/init.rs | 1 - rust/kernel/init/__internal.rs | 25 ++----- rust/kernel/init/macros.rs | 116 +++++++++++++++------------------ 3 files changed, 56 insertions(+), 86 deletions(-) diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index d431d0b153a2..0120674b451e 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -216,7 +216,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(self: Pin<&mut Self>, init: impl PinInit) -> Result { ptr: *mut T, - do_drop: Cell, } impl DropGuard { @@ -190,32 +189,16 @@ impl DropGuard { /// - 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 Drop for DropGuard { #[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 c50429173fc7..c5f977f52d0c 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,28 @@ 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. - // - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. + // Create the drop guard: // - // 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)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for $field. + ::kernel::macros::paste! { + // 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)*), - ); + $crate::__init_internal!(init_slot($use_data): + @data($data), + @slot($slot), + @guards([<$field>], $($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 +1111,28 @@ 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. - // - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. + // Create the drop guard: // - // 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)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for $field. + ::kernel::macros::paste! { + // 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)*), - ); + $crate::__init_internal!(init_slot(): + @data($data), + @slot($slot), + @guards([<$field>], $($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,18 +1143,21 @@ 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. - // - // 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)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for $field. + ::kernel::macros::paste! { + // 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)*), - ); + $crate::__init_internal!(init_slot($($use_data)?): + @data($data), + @slot($slot), + @guards([<$field>], $($guards,)*), + @munch_fields($($rest)*), + ); + } }; (make_initializer: @slot($slot:ident), @@ -1189,29 +1200,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)]