Re: [PATCH v2 1/3] rust: kunit: add #[should_panic] support

[David Gow <david@davidgow.net>]

Le 02/11/2026 à 22:58, Nicolás Antinori a écrit :
> On Thu Oct 29, 2026 at 1:59 PM -03, Nicolás Antinori via B4 Relay wrote:
>> From: Nicolás Antinori <nico.antinori.7@gmail.com>
>>
>> KUnit tests in Rust are written using user-space like syntax. This patch
>> adds support for the `#[should_panic]` attribute, enabling the user to
>> test conditions that are expected to cause a panic and report the test
>> as successful.
>>
>> Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
> 
> Hello, Sashiko found two issues [1] with this patch:
> 
>> [Severity: High]
>> Will these new selftests crash the kernel or hang when
>> CONFIG_RUST_KUNIT_SELFTEST is enabled under CONFIG_UML=y,
>> CONFIG_PANIC_ON_OOPS=y, or CONFIG_BUG=n?
> 
> This is true, I tested this with:
> 
> ./tools/testing/kunit/kunit.py run \
> 	--make_options LLVM=1 \
> 	--kconfig_add CONFIG_RUST=y \
> 	--kconfig_add CONFIG_RUST_KUNIT_TESTS=y \
> 	--kconfig_add CONFIG_KUNIT=y \
> 	--kconfig_add CONFIG_UML=y \
> 	--arch=um rust_kernel_kunit
> 
> ./tools/testing/kunit/kunit.py run \
> 	--make_options LLVM=1 \
> 	--kconfig_add CONFIG_RUST=y \
> 	--kconfig_add CONFIG_RUST_KUNIT_TESTS=y \
> 	--kconfig_add CONFIG_KUNIT=y \
> 	--kconfig_add CONFIG_PANIC_ON_OOPS=y \
> 	--arch=x86_64 rust_kernel_kunit
> 
> And some other configs using the configs causing problems.
> 
> To solve this I created a new config "RUST_KUNIT_SHOULD_PANIC_ATTRIBUTE"
> that depends on !UML && !PANIC_ON_OOPS && BUG. If the test is annotated
> with `#[should_panic]` and CONFIG_RUST_KUNIT_SHOULD_PANIC_ATTRIBUTE=n,
> the test won't run and won't show up in the summary.
> 

This is failing on UML for me, too. I'd rather have an implementation 
which worked on UML (such as a panic_handler), rather than trying to 
intercept BUG().

If you do want to add a new config option, I'd probably call it 
something like HAVE_KUNIT_RUST_SHOULD_PANIC, and set it in the 
architecture configs, but piggy-backing on the existing 
CONFIG_KUNIT_FAULT_TEST is probably best. And if we do want to support 
non-Rust panics, then this probably shouldn't be a rust-specific option.

The RFC's panic_handler-based implementation worked fine for me on UML, 
so I'd stick with that.

And, of course, UML isn't the only architecture that can't handle faults 
(though it might be the only one Rust supports at the moment). e.g., 
SPARC also would fail here.

>> [Severity: Medium]
>> Does defining macro_rules! assert and macro_rules! assert_eq at module scope
>> in kunit_tests() leak the preceding test's #should_panic value into helper
>> functions or impl blocks defined later in the module?
> 
> This is also true, the solution was redefining the `assert!` and
> `assert_eq!` macros inside the function's body (as Sashiko suggested).
> 
> I'll wait a bit in case there are any feedback on the current series and
> send a v3 with these changes.
> 

As mentioned, I'm not a big fan of including should_panic in the 
assert!() macros, so if we can just do away with that, this should no 
longer be a problem.

> Best Regards,
> Nicolás
> 
> [1] https://sashiko.dev/#/patchset/20261029-rust_kunit_v6_send-v2-0-5ce551efc48e@gmail.com?part=1


Cheers,
-- David