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

[Nicolás Antinori]

Hello,

There are two observations by Sashiko in this patch [1]:

> [Severity: Low]
> Does this interception occur too late to prevent spurious error logs?
>
> When a Rust test panics as expected, it triggers a kernel fault caught by
> KUnit with an exit code of -EINTR. However, kunit_try_catch_run()
> unconditionally logs an error message for -EINTR before returning to this
> function
>
> lib/kunit/try-catch.c:kunit_try_catch_run() {
>     ...
>     else if (exit_code == -EINTR) {
>         if (test->last_seen.file)
>             kunit_err(test, "try faulted: last line seen %s:%d\n", ...);
>         else
>             kunit_err(test, "try faulted\n");
>     }
>     ...
> }

Even though the tests marked as `#[should_panic]` are reported as
successful by KUnit, when run with `--raw_output=all` those log errors
appear. Is this a problem? Along with those the Oops message and stack
trace message show up as well. The report summary is clean if no special
logging options are used.

> [Severity: Medium]
> Will this condition correctly handle KUnit assertions inside a
> #[should_panic] test?
>
> The kunit_tests macro overrides assert! with kunit_assert!. If an
> assert! fails, kunit_assert! reports a KUnit assertion failure and
> aborts the thread via __kunit_abort, which results in -EFAULT rather
> than -EINTR.
>
> Since -EFAULT is cleared to 0 in kunit_try_catch_run(), this condition
> checking strictly for -EINTR will fail to intercept the abort. Will this
> cause valid #[should_panic] tests containing an assert! failure to be
> incorrectly marked as KUNIT_FAILURE instead of passing?

This is something I missed when working on the series and is accurate.
The following test:

#[test]
#[should_panic]
fn rust_test_assert_false() {
    assert!(false)
}

Is reported as failed:

[18:31:00] # rust_test_assert_false: ASSERTION FAILED at rust/kernel/kunit.rs:383
[18:31:00] Expected false to be true, but is false
[18:31:00] # rust_test_assert_false.speed: normal
[18:31:00] [FAILED] rust_test_assert_false

But the user-space behavior is to pass the test [2]. 

Does it make sense to leave this behavior as it is for the kernel? If I
am not mistaken tests like those can easily be expressed without the
`#[should_panic]` attribute and negating the condition inside `assert!`.
`assert!` that are not in the test body behave as expected.


To solve those I'd need to check the `rust_should_panic` flag inside in
`kunit_try_catch_run` function. That would require changing its
signature (since the `kunit_try_catch` struct does not contain the
`test_case` struct and the context argument is void*, so I can't safely
cast it to what I need). Other alternative is to create a new function
for cases where `rust_should_panic` is true.

My approach to this was to modify as little as possible in the C side.
I think the current behavior is ok but I want to know your opinion.

Best regards,
Nicolás

[1] https://sashiko.dev/#/patchset/20260930-rust_kunit_v4-v1-0-e3b0d45311dd@xxxxxxxxx?part=1
[2] https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=2b4b29a266271497ea9932068ecdb8ff