kernel/linux.git/lib/kunit/assert.c, branch v6.6.132

kunit: fix bug in KUNIT_EXPECT_MEMEQ

2023-01-30T21:46:46+00:00

In KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ, add check if one of the inputs is NULL and fail if this is the case. Currently, the kernel crashes if one of the inputs is NULL. Instead, fail the test and add an appropriate error message. Fixes: b8a926bea8b1 ("kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros") This was found by the kernel test robot: https://lore.kernel.org/all/202212191448.D6EDPdOh-lkp@intel.com/ Reported-by: kernel test robot Signed-off-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan

kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros

2022-10-27T08:39:47+00:00

Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp function, such as: KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0); Although this usage produces correct results for the test cases, when the expectation fails, the error message is not very helpful, indicating only the return of the memcmp function. Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size. In case of expectation failure, those macros print the hex dump of the memory blocks, making it easier to debug test failures for memory blocks. That said, the expectation KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0); would translate to the expectation KUNIT_EXPECT_MEMEQ(test, foo, bar, size); Signed-off-by: Maíra Canal Reviewed-by: Daniel Latypov Reviewed-by: Muhammad Usama Anjum Reviewed-by: David Gow Signed-off-by: Shuah Khan

kunit: log numbers in decimal and hex

2022-10-27T08:32:41+00:00

When KUNIT_EXPECT_EQ() or KUNIT_ASSERT_EQ() log a failure, they log the two values being compared, with numerical values logged in decimal. In some cases, decimal output is painful to consume, and hexadecimal output would be more helpful. For example, this is the case for tests I'm currently developing for the arm64 insn encoding/decoding code, where comparing two 32-bit instruction opcodes results in output such as: | # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791 | Expected obj_insn == gen_insn, but | obj_insn == 2332164128 | gen_insn == 1258422304 To make this easier to consume, this patch logs the values in both decimal and hexadecimal: | # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791 | Expected obj_insn == gen_insn, but | obj_insn == 2332164128 (0x8b020020) | gen_insn == 1258422304 (0x4b020020) As can be seen from the example, having hexadecimal makes it significantly easier for a human to spot which specific bits are incorrect. Signed-off-by: Mark Rutland Cc: Brendan Higgins Cc: David Gow Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Acked-by: Daniel Latypov Reviewed-by: David Gow Signed-off-by: Shuah Khan

kunit: factor out str constants from binary assertion structs

2022-01-31T18:55:39+00:00

If the compiler doesn't optimize them away, each kunit assertion (use of KUNIT_EXPECT_EQ, etc.) can use 88 bytes of stack space in the worst and most common case. This has led to compiler warnings and a suggestion from Linus to move data from the structs into static const's where possible [1]. This builds upon [2] which did so for the base struct kunit_assert type. That only reduced sizeof(struct kunit_binary_assert) from 88 to 64. Given these are by far the most commonly used asserts, this patch factors out the textual representations of the operands and comparator into another static const, saving 16 more bytes. In detail, KUNIT_EXPECT_EQ(test, 2 + 2, 5) yields the following struct (struct kunit_binary_assert) { .assert = , .operation = "==", .left_text = "2 + 2", .left_value = 4, .right_text = "5", .right_value = 5, } After this change static const struct kunit_binary_assert_text __text = { .operation = "==", .left_text = "2 + 2", .right_text = "5", }; (struct kunit_binary_assert) { .assert = , .text = &__text, .left_value = 4, .right_value = 5, } This also DRYs the code a bit more since these str fields were repeated for the string and pointer versions of kunit_binary_assert. Note: we could name the kunit_binary_assert_text fields left/right instead of left_text/right_text. But that would require changing the macros a bit since they have args called "left" and "right" which would be substituted in `.left = #left` as `.2 + 2 = \"2 + 2\"`. [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ [2] https://lore.kernel.org/linux-kselftest/20220113165931.451305-6-dlatypov@google.com/ Signed-off-by: Daniel Latypov Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan

kunit: remove va_format from kunit_assert

2022-01-31T18:55:27+00:00

The concern is that having a lot of redundant fields in kunit_assert can blow up stack usage if the compiler doesn't optimize them away [1]. The comment on this field implies that it was meant to be initialized when the expect/assert was declared, but this only happens when we run kunit_do_failed_assertion(). We don't need to access it outside of that function, so move it out of the struct and make it a local variable there. This change also takes the chance to reduce the number of macros by inlining the now simplified KUNIT_INIT_ASSERT_STRUCT() macro. [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ Signed-off-by: Daniel Latypov Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan

kunit: split out part of kunit_assert into a static const

2022-01-25T19:49:59+00:00

This is per Linus's suggestion in [1]. The issue there is that every KUNIT_EXPECT/KUNIT_ASSERT puts a kunit_assert object onto the stack. Normally we rely on compilers to elide this, but when that doesn't work out, this blows up the stack usage of kunit test functions. We can move some data off the stack by making it static. This change introduces a new `struct kunit_loc` to hold the file and line number and then just passing assert_type (EXPECT or ASSERT) as an argument. In [1], it was suggested to also move out the format string as well, but users could theoretically craft a format string at runtime, so we can't. This change leaves a copy of `assert_type` in kunit_assert for now because cleaning up all the macros to not pass it around is a bit more involved. Here's an example of the expanded code for KUNIT_FAIL(): if (__builtin_expect(!!(!(false)), 0)) { static const struct kunit_loc loc = { .file = ... }; struct kunit_fail_assert __assertion = { .assert = { .type ... }; kunit_do_failed_assertion(test, &loc, KUNIT_EXPECTATION, &__assertion.assert, ...); }; [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ Signed-off-by: Daniel Latypov Suggested-by: Linus Torvalds Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan

kunit: factor out kunit_base_assert_format() call into kunit_fail()

2022-01-25T19:49:53+00:00

We call this function first thing for all the assertion `format()` functions. This is the part that prints the file and line number and assertion type (EXPECTATION, ASSERTION). Having it as part of the format functions lets us have the flexibility to not print that information (or print it differently) for new assertion types, but I think this we don't need that. And in the future, we'd like to consider factoring that data (file, line#, type) out of the kunit_assert struct and into a `static` variable, as Linus suggested [1], so we'd need to extract it anyways. [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ Signed-off-by: Daniel Latypov Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan

kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals

2021-04-02T20:14:16+00:00

Before: > Expected str == "world", but > str == hello > "world" == world After: > Expected str == "world", but > str == "hello" Note: like the literal ellision for integers, this doesn't handle the case of KUNIT_EXPECT_STREQ(test, "hello", "world") since we don't expect it to realistically happen in checked in tests. (If you really wanted a test to fail, KUNIT_FAIL("msg") exists) In that case, you'd get: > Expected "hello" == "world", but Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan

kunit: Match parenthesis alignment to improve code readability

2021-03-23T16:10:58+00:00

Tidy up code by fixing the following checkpatch warnings: CHECK: Alignment should match open parenthesis CHECK: Lines should not end with a '(' Signed-off-by: Lucas Stankus Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan

kunit: don't show `1 == 1` in failed assertion messages

2021-02-08T22:43:34+00:00

Currently, given something (fairly dystopian) like > KUNIT_EXPECT_EQ(test, 2 + 2, 5) KUnit will prints a failure message like this. > Expected 2 + 2 == 5, but > 2 + 2 == 4 > 5 == 5 With this patch, the output just becomes > Expected 2 + 2 == 5, but > 2 + 2 == 4 This patch is slightly hacky, but it's quite common* to compare an expression to a literal integer value, so this can make KUnit less chatty in many cases. (This patch also fixes variants like KUNIT_EXPECT_GT, LE, et al.). It also allocates an additional string briefly, but given this only happens on test failures, it doesn't seem too bad a tradeoff. Also, in most cases it'll realize the lengths are unequal and bail out before the allocation. We could save the result of the formatted string to avoid wasting this extra work, but it felt cleaner to leave it as-is. Edge case: for something silly and unrealistic like > KUNIT_EXPECT_EQ(test, 4, 5); It'll generate this message with a trailing "but" > Expected 4 == 5, but > It didn't feel worth adding a check up-front to see if both sides are literals to handle this better. *A quick grep suggests 100+ comparisons to an integer literal as the right hand side. Signed-off-by: Daniel Latypov Tested-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan