* [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro @ 2025-07-16 4:59 Ritvik Gupta 2025-07-21 21:51 ` Miguel Ojeda 0 siblings, 1 reply; 5+ messages in thread From: Ritvik Gupta @ 2025-07-16 4:59 UTC (permalink / raw) To: ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr Cc: rust-for-linux, skhan Introduce a new `safety` module containing `unsafe_precondition_assert!` macro. It is a wrapper around `debug_assert!`, intended for validating pre-conditions of unsafe code blocks and functions. When `CONFIG_RUST_DEBUG_ASSERTIONS` flag is enabled, this macro performs runtime checks to ensure that the pre-conditions for unsafe blocks hold. In release builds, the macro is a no-op. Suggested-by: Miguel Ojeda <ojeda@kernel.org> Link: https://github.com/Rust-for-Linux/linux/issues/1162 Link: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/.60unsafe_precondition_assert.60.20macro/with/528457452 Signed-off-by: Ritvik Gupta <ritvikfoss@gmail.com> --- rust/kernel/lib.rs | 1 + rust/kernel/safety.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 rust/kernel/safety.rs diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index f61ac6f81f5d..a242f993f89b 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -101,6 +101,7 @@ pub mod print; pub mod rbtree; pub mod revocable; +pub mod safety; pub mod security; pub mod seq_file; pub mod sizes; diff --git a/rust/kernel/safety.rs b/rust/kernel/safety.rs new file mode 100644 index 000000000000..a115faa52539 --- /dev/null +++ b/rust/kernel/safety.rs @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! This module contains the kernel APIs for verifying invariants +//! required by the unsafe code. + +/// Checks that preconditions of an unsafe code are followed. +/// +/// The check is enabled at runtime if debug assertions (`CONFIG_RUST_DEBUG_ASSERTIONS`) +/// are enabled. In release builds, this macro is no-op. +/// +/// # Examples +/// +/// ``` +/// // SAFETY: The caller ensures the size and alignment +/// unsafe fn transmute_array<const N: usize, T: Copy, U: Copy>(input: [T; N]) -> [U; N] { +/// unsafe_precondition_assert!( +/// core::mem::size_of::<T>() == core::mem::size_of::<U>(), +/// "src and dst must have the same size" +/// ); +/// +/// unsafe_precondition_assert!( +/// core::mem::align_of::<T>() >= core::mem::align_of::<U>(), +/// "src alignment must be compatible with dst alignment" +/// ); +/// +/// core::mem::transmute_copy(&input) +/// } +/// ``` +/// +/// # Panics +/// +/// This will invoke the [`panic!`] macro if the provided expression cannot be evaluated +/// to true at runtime. +#[macro_export] +macro_rules! unsafe_precondition_assert { + ($($arg:tt)*) => { + if cfg!(debug_assertions) { + crate::pr_err!("unsafe precondition(s) violated"); + ::core::assert!($($arg)*); + } + }; +} base-commit: 8ecb65b7b68ea48350833ba59c1257718e859768 -- 2.50.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro 2025-07-16 4:59 [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro Ritvik Gupta @ 2025-07-21 21:51 ` Miguel Ojeda 2025-07-22 11:44 ` Ritvik Gupta 2025-07-22 12:13 ` Ritvik Gupta 0 siblings, 2 replies; 5+ messages in thread From: Miguel Ojeda @ 2025-07-21 21:51 UTC (permalink / raw) To: Ritvik Gupta Cc: ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr, rust-for-linux, skhan On Wed, Jul 16, 2025 at 6:58 AM Ritvik Gupta <ritvikfoss@gmail.com> wrote: > > macro. It is a wrapper around `debug_assert!`, intended for validating > pre-conditions of unsafe code blocks and functions. It isn't really a wrapper, since it doesn't call it, no? > +//! This module contains the kernel APIs for verifying invariants > +//! required by the unsafe code. We normally call these preconditions, and normally I think "unsafe code" is associated with unsafe blocks, rather than e.g. `unsafe fn`. In addition, we may want to use this module for more things, so I would probably just say something more general, e.g. "Safety-related APIs" or similar. Probably Benno can chime in here. > +/// Checks that preconditions of an unsafe code are followed. Same as above -- I would say unsafe function (if we really want to use this only for that). > +/// are enabled. In release builds, this macro is no-op. I would say "Otherwise, ...". Same in the commit message. > +/// // SAFETY: The caller ensures the size and alignment This should be a `# Safety` section, not a `SAFETY` comment. Please see how we usually write those in other files. > macro_rules! unsafe_precondition_assert { I wonder if we should call it `safety_precondition_assert` -- although I like the current name since it matches better the (unstable) Rust standard library one. > + if cfg!(debug_assertions) { > + crate::pr_err!("unsafe precondition(s) violated"); > + ::core::assert!($($arg)*); > + } Doesn't this print an error every time debug assertions are enabled, whether or not the expression is true? Also, shouldn't that be `$crate`? This leads me to believe the patch wasn't tested -- please test patches before submitting them! (Or, if it couldn't be tested for some reason, please say so in the message) Finally, could we actually wrap/forward the call to `debug_assert!`? If we want to add a custom message, could we match the parameters so that we can then prefix it? Thanks for the patch! Cheers, Miguel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro 2025-07-21 21:51 ` Miguel Ojeda @ 2025-07-22 11:44 ` Ritvik Gupta 2025-07-22 12:13 ` Ritvik Gupta 1 sibling, 0 replies; 5+ messages in thread From: Ritvik Gupta @ 2025-07-22 11:44 UTC (permalink / raw) To: Miguel Ojeda Cc: ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr, rust-for-linux, skhan On Mon, Jul 21, 2025 at 11:51:31PM +0200, Miguel Ojeda wrote: > On Wed, Jul 16, 2025 at 6:58 AM Ritvik Gupta <ritvikfoss@gmail.com> wrote: > > > > macro. It is a wrapper around `debug_assert!`, intended for validating > > pre-conditions of unsafe code blocks and functions. > > It isn't really a wrapper, since it doesn't call it, no? It was intended to work the same way as `debug_assert!` [1] with 'unsafe precondition(s) violated' message, if assertion fails. > > +//! This module contains the kernel APIs for verifying invariants > > +//! required by the unsafe code. > > We normally call these preconditions, and normally I think "unsafe > code" is associated with unsafe blocks, rather than e.g. `unsafe fn`. > > In addition, we may want to use this module for more things, so I > would probably just say something more general, e.g. "Safety-related > APIs" or similar. Probably Benno can chime in here. > > > +/// Checks that preconditions of an unsafe code are followed. > > Same as above -- I would say unsafe function (if we really want to use > this only for that). > > > +/// are enabled. In release builds, this macro is no-op. > > I would say "Otherwise, ...". Same in the commit message. > > > +/// // SAFETY: The caller ensures the size and alignment > > This should be a `# Safety` section, not a `SAFETY` comment. Please > see how we usually write those in other files. I'll fix the docs. Yes, @Benno's input would be super helpful as well :) > > + if cfg!(debug_assertions) { > > + crate::pr_err!("unsafe precondition(s) violated"); > > + ::core::assert!($($arg)*); > > + } > > Doesn't this print an error every time debug assertions are enabled, > whether or not the expression is true? You're right! I messed up the testing part (explained later). > Also, shouldn't that be `$crate`? I think, in this case, the compiler automatically resolved it to correct path. Nevertheless, I will use `$crate`. > This leads me to believe the patch wasn't tested -- please test > patches before submitting them! (Or, if it couldn't be tested for some > reason, please say so in the message) The code did compile, but during testing I only verified the failing case and overlooked the passing one. > Finally, could we actually wrap/forward the call to `debug_assert!`? > If we want to add a custom message, could we match the parameters so > that we can then prefix it? Yes, I'll do this way in v2. Thanks for the feedback :) [1]: https://doc.rust-lang.org/src/core/macros/mod.rs.html#306-312 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro 2025-07-21 21:51 ` Miguel Ojeda 2025-07-22 11:44 ` Ritvik Gupta @ 2025-07-22 12:13 ` Ritvik Gupta 2025-07-22 12:31 ` Miguel Ojeda 1 sibling, 1 reply; 5+ messages in thread From: Ritvik Gupta @ 2025-07-22 12:13 UTC (permalink / raw) To: Miguel Ojeda Cc: ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr, rust-for-linux, skhan Just to be clear for v2, the caller should be able to call this way? ``` // Allowed unsafe_precondition_assert!(condition, "message"); unsafe_precondition_assert!(condition, "formatted message: {} {} {}", arg1, arg2, argn); // Not allowed? unsafe_precondition_assert!(condition) ``` When the assertion fails, this will be the output: ``` precondition(s) violated: message passed by caller ``` ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro 2025-07-22 12:13 ` Ritvik Gupta @ 2025-07-22 12:31 ` Miguel Ojeda 0 siblings, 0 replies; 5+ messages in thread From: Miguel Ojeda @ 2025-07-22 12:31 UTC (permalink / raw) To: Ritvik Gupta Cc: ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, dakr, rust-for-linux, skhan On Tue, Jul 22, 2025 at 2:12 PM Ritvik Gupta <ritvikfoss@gmail.com> wrote: > > the caller should be able to call this way? > > ``` > // Allowed > unsafe_precondition_assert!(condition, "message"); > unsafe_precondition_assert!(condition, "formatted message: {} {} {}", arg1, arg2, argn); > > // Not allowed? > unsafe_precondition_assert!(condition) > ``` I think allowing the second one would align it with other macros, which is an advantage, and makes it a bit simpler to use for obvious cases. On the other hand, `assert_unsafe_precondition!` upstream doesn't allow it (but it has different syntax anyway), and we may want to always have a proper message in these cases anyway. So I think either way is OK. I would perhaps lean to being consistent with our other assertion macros, especially if it makes it simpler. Thanks! Cheers, Miguel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-22 12:32 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-16 4:59 [PATCH] rust: kernel: introduce `unsafe_precondition_assert!` macro Ritvik Gupta 2025-07-21 21:51 ` Miguel Ojeda 2025-07-22 11:44 ` Ritvik Gupta 2025-07-22 12:13 ` Ritvik Gupta 2025-07-22 12:31 ` Miguel Ojeda
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).