linux-kernel - Re: [PATCH] rust: add `assert

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DAGNOQOY3B3X.MQD616P04I3U@kernel.org>
Date: Sun, 08 Jun 2025 00:31:58 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Christian Schrefl" <chrisi.schrefl@...il.com>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>
Cc: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH] rust: add `assert_sync` function

On Sat Jun 7, 2025 at 9:20 PM CEST, Christian Schrefl wrote:
> On 07.06.25 8:11 PM, Benno Lossin wrote:
>> On Sat Jun 7, 2025 at 5:54 PM CEST, Christian Schrefl wrote:
>>> On 07.06.25 5:42 PM, Benno Lossin wrote:
>>>> On Sat Jun 7, 2025 at 3:02 PM CEST, Christian Schrefl wrote:
>>>>> - Add `assert_send` as well.
>>>>
>>>> Sounds like a good idea.
>>>
>>> Should I already add this in V2 for this series?
>> 
>> If you want to then sure, but we can also wait until we have a use-case.
>> Also, let's finish the discussion about the macro idea below.
>> 
>>>>> +///     assert_sync::<i32>(); // Succeeds because `i32` is Sync
>>>>> +///     // assert_sync::<NotThreadSafe>(); // Fails because `NotThreadSafe` is not `Sync`.
>>>>
>>>> Can you split this into two examples and mark the failing one with
>>>> `compile_fail`?
>>>
>>> I've tried it with `compile_fail` and it didn't work, I think
>>> that's not supported in (kernel) doc tests yet. 
>> 
>> Hmm, I thought that this worked... @Miguel any idea?
>> 
>>>> We also could provide a macro similar to [1].
>>>>
>>>> [1]: https://docs.rs/static_assertions/latest/static_assertions/
>>>
>>> You mean the `assert_impl_*!` macros?
>> 
>> Yes, but the others might also be useful from time to time.
>> >> That might make sense, with macros we would not need to write
>>> a const block to ensure its not executed at runtime (although
>>> it's probably optimized out anyways).
>> 
>> It 100% will be optimized out.
>> 
>>> It would also mean that we won't need a assert for every Trait, which
>>> seems nice. So a macro sounds pretty good to me.
>> 
>> It depends, the macro impl needs to define its own function, which might
>> be inefficient if one uses it a lot. But there is no way to be generic
>> over traits, so there is no other way.
>> 
>> Let's see what the others think.
>
> The error messages in the macro are slightly worse:
> error[E0277]: `*mut ()` cannot be shared between threads safely
>    --> rust/kernel/compile_assert.rs:40:18
>     |
> 40  | assert_impl_all!(NotThreadSafe: Sync); // Fails because `NotThreadSafe` is not `Sync`
>     |                  ^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
>     |
>     = help: within `PhantomData<*mut ()>`, the trait `Sync` is not implemented for `*mut ()`, which is required by `PhantomData<*mut ()>: Sync`
> note: required because it appears within the type `PhantomData<*mut ()>`
>    --> /home/chrisi/.rustup/toolchains/1.78-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/marker.rs:740:12
>     |
> 740 | pub struct PhantomData<T: ?Sized>;
>     |            ^^^^^^^^^^^
> note: required by a bound in `assert_impl`
>    --> rust/kernel/compile_assert.rs:34:48
>     |
> 34  |             const fn assert_impl<T: ?Sized $(+ $trait)+>() {}
>     |                                                ^^^^^^ required by this bound in `assert_impl`
> ...
> 40  | assert_impl_all!(NotThreadSafe: Sync); // Fails because `NotThreadSafe` is not `Sync`
>     | ------------------------------------- in this macro invocation
>     = note: this error originates in the macro `assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)
>
> error: aborting due to 1 previous error
>
> compared to the function:
>
> error[E0277]: `*mut ()` cannot be shared between threads safely
>    --> rust/kernel/compile_assert.rs:28:31
>     |
> 28  | const _: () = { assert_sync::<NotThreadSafe>() };
>     |                               ^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
>     |
>     = help: within `PhantomData<*mut ()>`, the trait `Sync` is not implemented for `*mut ()`, which is required by `PhantomData<*mut ()>: Sync`
> note: required because it appears within the type `PhantomData<*mut ()>`
>    --> /home/chrisi/.rustup/toolchains/1.78-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/marker.rs:740:12
>     |
> 740 | pub struct PhantomData<T: ?Sized>;
>     |            ^^^^^^^^^^^
> note: required by a bound in `assert_sync`
>    --> rust/kernel/compile_assert.rs:26:38
>     |
> 26  | pub const fn assert_sync<T: ?Sized + Sync>() {}
>     |                                      ^^^^ required by this bound in `assert_sync`
>
> I guess I'll keep it as a function for now.

Can we improve this by using a proc-macro instead and manipulating the
span? I honestly don't think the error is too bad.

---
Cheers,
Benno