On Fri, Mar 01, 2024 at 01:09:27PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 1, 2024 at 2:26 AM Kees Cook <[email protected]> wrote:
> >
> > Convert test_string.c to KUnit so it can be easily run with everything
> > else.
>
> Have you run it?
Yes:
$ ./tools/testing/kunit/kunit.py run string
[09:21:32] Starting KUnit Kernel (1/1)...
[09:21:32] ============================================================
[09:21:32] =================== string (6 subtests) ====================
[09:21:32] [PASSED] test_memset16
[09:21:32] [PASSED] test_memset32
[09:21:32] [PASSED] test_memset64
[09:21:32] [PASSED] test_strchr
[09:21:32] [PASSED] test_strnchr
[09:21:32] [PASSED] test_strspn
[09:21:32] ===================== [PASSED] string ======================
[09:21:32] ============================================================
[09:21:32] Testing complete. Ran 6 tests: passed: 6
[09:21:32] Elapsed time: 11.545s total, 0.001s configuring, 11.327s building,
0.183s running
> ...
>
> > if (i < 256)
> > - return (i << 24) | (j << 16) | k | 0x8000;
> > - return 0;
> > + KUNIT_EXPECT_EQ(test, 0, (i << 24) | (j << 16) | k |
> > 0x8000);
>
> First of all, this special value encodes the problematic patterns, so
> you missed proper messaging.
Yeah, I see now this isn't a test but rather an encoded report. Since
the failures are caught earlier, I can improve those messages instead of
doing an encoded version.
> Second, the returned value has a constant, how do you expect 0 to be
> equal to something (guaranteed not to be 0)?
>
> This needs a good rethink of what you should do in the KUnit approach.
>
> ...
>
> > + KUNIT_EXPECT_EQ(test, 0, (i << 24) | (j << 16) | k |
> > 0x8000);
>
> Ditto.
>
> ...
>
> > + KUNIT_EXPECT_EQ(test, 0, (i << 24) | (j << 16) | k |
> > 0x8000);
>
> Ditto.
>
> ...
>
> > for (i = 0; i < strlen(test_string) + 1; i++) {
> > result = strchr(test_string, test_string[i]);
> > - if (result - test_string != i)
> > - return i + 'a';
> > + KUNIT_ASSERT_EQ(test, result - test_string, i);
>
> In a similar way, all returned values are *special*, you really need
> to think about them before converting to a simple (and sometimes
> wrong) checks)
This encoding is trying to report "i", so I've adjusted the error
reporting in v3.
> I dunno if KUnit has a fault ejection simulation. It should, in order
> to be sure that test cases are fine when they fail.
Yeah, bumping offsets and such produce expected failures.
--
Kees Cook
