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: <CAJ-ks9kARZT5TMrfXvAqmGdsMD+EgdaY061tyhm_n4oerFLOfA@mail.gmail.com>
Date: Sun, 4 May 2025 13:41:30 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Miguel Ojeda <ojeda@...nel.org>
Cc: Brendan Higgins <brendan.higgins@...ux.dev>, David Gow <davidgow@...gle.com>, 
	Alex Gaynor <alex.gaynor@...il.com>, Rae Moar <rmoar@...gle.com>, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH 1/7] rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s

On Fri, May 2, 2025 at 5:53 PM Miguel Ojeda <ojeda@...nel.org> wrote:
>
> The KUnit `#[test]` support that landed recently is very basic and does
> not map the `assert*!` macros into KUnit like the doctests do, so they
> panic at the moment.
>
> Thus implement the custom mapping in a similar way to doctests, reusing
> the infrastructure there.
>
> In Rust 1.88.0, the `file()` method in `Span` may be stable [1]. However,
> it was changed recently (from `SourceFile`), so we need to do something
> different in previous versions. Thus create a helper for it and use it
> to get the path.
>
> With this, a failing test suite like:
>
>     #[kunit_tests(my_test_suite)]
>     mod tests {
>         use super::*;
>
>         #[test]
>         fn my_first_test() {
>             assert_eq!(42, 43);
>         }
>
>         #[test]
>         fn my_second_test() {
>             assert!(42 >= 43);
>         }
>     }
>
> will properly map back to KUnit, printing something like:
>
>     [    1.924325]     KTAP version 1
>     [    1.924421]     # Subtest: my_test_suite
>     [    1.924506]     # speed: normal
>     [    1.924525]     1..2
>     [    1.926385]     # my_first_test: ASSERTION FAILED at rust/kernel/lib.rs:251
>     [    1.926385]     Expected 42 == 43 to be true, but is false
>     [    1.928026]     # my_first_test.speed: normal
>     [    1.928075]     not ok 1 my_first_test
>     [    1.928723]     # my_second_test: ASSERTION FAILED at rust/kernel/lib.rs:256
>     [    1.928723]     Expected 42 >= 43 to be true, but is false
>     [    1.929834]     # my_second_test.speed: normal
>     [    1.929868]     not ok 2 my_second_test
>     [    1.930032] # my_test_suite: pass:0 fail:2 skip:0 total:2
>     [    1.930153] # Totals: pass:0 fail:2 skip:0 total
>
> Link: https://github.com/rust-lang/rust/pull/140514 [1]
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
> ---
>  init/Kconfig           |  3 +++
>  rust/Makefile          |  3 ++-
>  rust/kernel/kunit.rs   |  1 -
>  rust/macros/helpers.rs | 16 ++++++++++++++++
>  rust/macros/kunit.rs   | 28 +++++++++++++++++++++++++++-
>  rust/macros/lib.rs     |  4 ++++
>  6 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6..5f442c64c47b 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>  config RUSTC_HAS_COERCE_POINTEE
>         def_bool RUSTC_VERSION >= 108400
>
> +config RUSTC_HAS_SPAN_FILE
> +       def_bool RUSTC_VERSION >= 108800
> +
>  config PAHOLE_VERSION
>         int
>         default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/Makefile b/rust/Makefile
> index 3aca903a7d08..075b38a24997 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -402,7 +402,8 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
>                 -Clink-args='$(call escsq,$(KBUILD_PROCMACROLDFLAGS))' \
>                 --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \
>                 --crate-type proc-macro \
> -               --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) $<
> +               --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) \
> +               @$(objtree)/include/generated/rustc_cfg $<
>
>  # Procedural macros can only be used with the `rustc` that compiled it.
>  $(obj)/$(libmacros_name): $(src)/macros/lib.rs FORCE
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 1604fb6a5b1b..2659895d4c5d 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -323,7 +323,6 @@ mod tests {
>
>      #[test]
>      fn rust_test_kunit_example_test() {
> -        #![expect(clippy::eq_op)]

How come this vanished?

>          assert_eq!(1 + 1, 2);
>      }
>
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index a3ee27e29a6f..57c3b0f0c194 100644
> --- a/rust/macros/helpers.rs
> +++ b/rust/macros/helpers.rs
> @@ -86,3 +86,19 @@ pub(crate) fn function_name(input: TokenStream) -> Option<Ident> {
>      }
>      None
>  }
> +
> +pub(crate) fn file() -> String {
> +    #[cfg(not(CONFIG_RUSTC_HAS_SPAN_FILE))]
> +    {
> +        proc_macro::Span::call_site()
> +            .source_file()
> +            .path()
> +            .to_string_lossy()
> +            .into_owned()
> +    }
> +
> +    #[cfg(CONFIG_RUSTC_HAS_SPAN_FILE)]
> +    {
> +        proc_macro::Span::call_site().file()
> +    }
> +}
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> index 4f553ecf40c0..eb4f2afdbe43 100644
> --- a/rust/macros/kunit.rs
> +++ b/rust/macros/kunit.rs
> @@ -101,6 +101,8 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>      // ```
>      let mut kunit_macros = "".to_owned();
>      let mut test_cases = "".to_owned();
> +    let mut assert_macros = "".to_owned();

nit: why not String::new() for all these?

> +    let path = crate::helpers::file();
>      for test in &tests {
>          let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
>          let kunit_wrapper = format!(
> @@ -114,6 +116,27 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>              test, kunit_wrapper_fn_name
>          )
>          .unwrap();
> +        writeln!(
> +            assert_macros,
> +            r#"
> +/// Overrides the usual [`assert!`] macro with one that calls KUnit instead.
> +#[allow(unused)]
> +macro_rules! assert {{
> +    ($cond:expr $(,)?) => {{{{
> +        kernel::kunit_assert!("{test}", "{path}", 0, $cond);
> +    }}}}
> +}}
> +
> +/// Overrides the usual [`assert_eq!`] macro with one that calls KUnit instead.
> +#[allow(unused)]
> +macro_rules! assert_eq {{
> +    ($left:expr, $right:expr $(,)?) => {{{{
> +        kernel::kunit_assert_eq!("{test}", "{path}", 0, $left, $right);
> +    }}}}
> +}}
> +        "#
> +        )
> +        .unwrap();
>      }
>
>      writeln!(kunit_macros).unwrap();
> @@ -152,7 +175,10 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
>          }
>      }
>
> -    let mut new_body = TokenStream::from_iter(new_body);
> +    let body = new_body;
> +    let mut new_body = TokenStream::new();
> +    new_body.extend::<TokenStream>(assert_macros.parse().unwrap());
> +    new_body.extend(body);

Could we do this (pushing `assert_macros`) before the block above to
avoid this body/new_body name juggling?

>      new_body.extend::<TokenStream>(kunit_macros.parse().unwrap());
>
>      tokens.push(TokenTree::Group(Group::new(Delimiter::Brace, new_body)));
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 9acaa68c974e..8bd7906276be 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -6,6 +6,10 @@
>  // and thus add a dependency on `include/config/RUSTC_VERSION_TEXT`, which is
>  // touched by Kconfig when the version string from the compiler changes.
>
> +// TODO: check that when Rust 1.88.0 is released, this would be enough:
> +// #![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
> +#![feature(proc_macro_span)]
> +
>  #[macro_use]
>  mod quote;
>  mod concat_idents;
> --
> 2.49.0
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ