Everywhere we use the assert structs now takes them via const*, as of
commit 7466886b40 ("kunit: take `kunit_assert` as `const`").
So now let's properly declare the structs as const as well.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Context:
Currently this macro's name, KUNIT_ASSERTION conflicts with the name of
an enum whose values are {KUNIT_EXPECTATION, KUNIT_ASSERTION}.
It's hard to think of a better name for the enum, so rename this macro.
It's also a bit strange that the macro might do nothing depending on the
boolean argument `pass`. Why not have callers check themselves?
This patch:
Moves the pass/fail checking into the callers of KUNIT_ASSERTION, so now
we only call it when the check has failed.
Then we rename the macro the _KUNIT_FAILED() to reflect the new
semantics.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Each calll to a KUNIT_EXPECT_*() macro creates a local variable which
contains a struct kunit_assert.
Normally, we'd hope the compiler would be able to optimize this away,
but we've seen cases where it hasn't, see
https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/GbrMNej2BAAJ.
In changes like commit 21957f90b2 ("kunit: split out part of
kunit_assert into a static const"), we've moved more and more parts out
of struct kunit_assert and its children types (kunit_binary_assert).
This patch removes the final field and gets us to:
sizeof(struct kunit_assert) == 0
sizeof(struct kunit_binary_assert) == 24 (on UML x86_64).
This also reduces the amount of macro plumbing going on at the cost of
passing in one more arg to the base KUNIT_ASSERTION macro and
kunit_do_failed_assertion().
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
kunit_kfree() exists to clean up allocations from kunit_kmalloc() and
friends early instead of waiting for this to happen automatically at the
end of the test.
But it can be used on *anything* registered with the kunit resource API.
E.g. the last 2 statements are equivalent:
struct kunit_resource *res = something();
kfree(res->data);
kunit_put_resource(res);
The problem is that there could be multiple resources that point to the
same `data`.
E.g. you can have a named resource acting as a pseudo-global variable in
a test. If you point it to data allocated with kunit_kmalloc(), then
calling `kunit_kfree(ptr)` has the chance to delete either the named
resource or to kfree `ptr`.
Which one it does depends on the order the resources are registered as
kunit_kfree() will delete resources in LIFO order.
So this patch restricts kunit_kfree() to only working on resources
created by kunit_kmalloc(). Calling it is therefore guaranteed to free
the memory, not do anything else.
Note: kunit_resource_instance_match() wasn't used outside of KUnit, so
it should be safe to remove from the public interface. It's also
generally dangerous, as shown above, and shouldn't be used.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Because KUnit test execution is not a guarantee with the kunit.enable
parameter we want to be careful to only taint the kernel when actual
tests run. Calling module_info(test, "Y") for every KUnit module
automatically causes the kernel to be tainted upon module load. Therefore,
we're removing this call and relying on the KUnit framework to taint the
kernel or not.
Signed-off-by: Joe Fradley <joefradley@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
This patch adds the kunit.enable module parameter that will need to be
set to true in addition to KUNIT being enabled for KUnit tests to run.
The default value is true giving backwards compatibility. However, for
the production+testing use case the new config option
KUNIT_DEFAULT_ENABLED can be set to N requiring the tester to opt-in
by passing kunit.enable=1 to the kernel.
Signed-off-by: Joe Fradley <joefradley@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
When replacing KUNIT_BINARY_*_MSG_ASSERTION() macros with
KUNIT_BINARY_INT_ASSERTION(), the assert_type parameter was not always
correctly transferred. Specifically, the following errors were
introduced:
- KUNIT_EXPECT_LE_MSG() uses KUNIT_ASSERTION
- KUNIT_ASSERT_LT_MSG() uses KUNIT_EXPECTATION
- KUNIT_ASSERT_GT_MSG() uses KUNIT_EXPECTATION
A failing KUNIT_EXPECT_LE_MSG() test thus prevents further tests from
running, while failing KUNIT_ASSERT_{LT,GT}_MSG() tests do not prevent
further tests from running. This is contrary to the documentation,
which states that failing KUNIT_EXPECT_* macros allow further tests to
run, while failing KUNIT_ASSERT_* macros should prevent this.
Revert the KUNIT_{ASSERTION,EXPECTATION} switches to fix the behaviour
for the affected macros.
Fixes: 40f39777ce ("kunit: decrease macro layering for integer asserts")
Signed-off-by: Sander Vanheule <sander@svanheule.net>
Reviewed-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
We currently store kunit suites in the .kunit_test_suites ELF section as
a `struct kunit_suite***` (modulo some `const`s).
For every test file, we store a struct kunit_suite** NULL-terminated array.
This adds quite a bit of complexity to the test filtering code in the
executor.
Instead, let's just make the .kunit_test_suites section contain a single
giant array of struct kunit_suite pointers, which can then be directly
manipulated. This array is not NULL-terminated, and so none of the test
filtering code needs to NULL-terminate anything.
Tested-by: Maíra Canal <maira.canal@usp.br>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Co-developed-by: David Gow <davidgow@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Currently, KUnit runs built-in tests and tests loaded from modules
differently. For built-in tests, the kunit_test_suite{,s}() macro adds a
list of suites in the .kunit_test_suites linker section. However, for
kernel modules, a module_init() function is used to run the test suites.
This causes problems if tests are included in a module which already
defines module_init/exit_module functions, as they'll conflict with the
kunit-provided ones.
This change removes the kunit-defined module inits, and instead parses
the kunit tests from their own section in the module. After module init,
we call __kunit_test_suites_init() on the contents of that section,
which prepares and runs the suite.
This essentially unifies the module- and non-module kunit init formats.
Tested-by: Maíra Canal <maira.canal@usp.br>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Fix this kernel-doc warning:
Documentation/dev-tools/kunit/api/test:9: ./include/kunit/test.h:323: WARNING: Inline interpreted text or phrase reference start-string without end-string.
Functions should use func_name() on kernel-doc markups, as
documented at:
Documentation/doc-guide/kernel-doc.rst
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Make KUnit trigger the new TAINT_TEST taint when any KUnit test is run.
Due to KUnit tests not being intended to run on production systems, and
potentially causing problems (or security issues like leaking kernel
addresses), the kernel's state should not be considered safe for
production use after KUnit tests are run.
This both marks KUnit modules as test modules using MODULE_INFO() and
manually taints the kernel when tests are run (which catches builtin
tests).
Acked-by: Luis Chamberlain <mcgrof@kernel.org>
Tested-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Tested-by: Maíra Canal <mairacanal@riseup.net>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
The `kunit_do_failed_assertion` function passes its
`struct kunit_assert` argument to `kunit_fail`. This one,
in turn, calls its `format` field passing the assert again
as a `const` pointer.
Therefore, the whole chain may be made `const`.
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
KUnit's test-managed resources can be created in two ways:
- Using the kunit_add_resource() family of functions, which accept a
struct kunit_resource pointer, typically allocated statically or on
the stack during the test.
- Using the kunit_alloc_resource() family of functions, which allocate a
struct kunit_resource using kzalloc() behind the scenes.
Both of these families of functions accept a 'free' function to be
called when the resource is finally disposed of.
At present, KUnit will kfree() the resource if this 'free' function is
specified, and will not if it is NULL. However, this can lead
kunit_alloc_resource() to leak memory (if no 'free' function is passed
in), or kunit_add_resource() to incorrectly kfree() memory which was
allocated by some other means (on the stack, as part of a larger
allocation, etc), if a 'free' function is provided.
Instead, always kfree() if the resource was allocated with
kunit_alloc_resource(), and never kfree() if it was passed into
kunit_add_resource() by the user. (If the user of kunit_add_resource()
wishes the resource be kfree()ed, they can call kfree() on the resource
from within the 'free' function.
This is implemented by adding a 'should_free' member to
struct kunit_resource and setting it appropriately. To facilitate this,
the various resource add/alloc functions have been refactored somewhat,
making them all call a __kunit_add_resource() helper after setting the
'should_free' member appropriately. In the process, all other functions
have been made static inline functions.
Signed-off-by: David Gow <davidgow@google.com>
Tested-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
KUnit has support for setup/cleanup logic for each test case in a suite.
But it lacks the ability to specify setup/cleanup for the entire suite
itself.
This can be used to do setup that is too expensive or cumbersome to do
for each test.
Or it can be used to do simpler things like log debug information after
the suite completes.
It's a fairly common feature, so the lack of it is noticeable.
Some examples in other frameworks and languages:
* https://docs.python.org/3/library/unittest.html#setupclass-and-teardownclass
* https://google.github.io/googletest/reference/testing.html#Test::SetUpTestSuite
Meta:
This is very similar to this patch here: https://lore.kernel.org/linux-kselftest/20210805043503.20252-3-bvanassche@acm.org/
The changes from that patch:
* pass in `struct kunit *` so users can do stuff like
`kunit_info(suite, "debug message")`
* makes sure the init failure is bubbled up as a failure
* updates kunit-example-test.c to use a suite init
* Updates kunit/usage.rst to mention the new support
* some minor cosmetic things
* use `suite_{init,exit}` instead of `{init/exit}_suite`
* make suite init error message more consistent w/ test init
* etc.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Add support for a new kind of kunit_suite registration macro called
kunit_test_init_section_suite(); this new registration macro allows the
registration of kunit_suites that reference functions marked __init and
data marked __initdata.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Background:
Currently, a reader looking at kunit/test.h will find the file is quite
long, and the first meaty comment is a doc comment about struct
kunit_resource.
Most users will not ever use the KUnit resource API directly.
They'll use kunit_kmalloc() and friends, or decide it's simpler to do
cleanups via labels (it often can be) instead of figuring out how to use
the API.
It's also logically separate from everything else in test.h.
Removing it from the file doesn't cause any compilation errors (since
struct kunit has `struct list_head resources` to store them).
This commit:
Let's move it into a kunit/resource.h file and give it a separate page
in the docs, kunit/api/resource.rst.
We include resource.h at the bottom of test.h since
* don't want to force existing users to add a new include if they use the API
* it accesses `lock` inside `struct kunit` in a inline func
* so we can't just forward declare, and the alternatives require
uninlining the func, adding hepers to lock/unlock, or other more
invasive changes.
Now the first big comment in test.h is about kunit_case, which is a lot
more relevant to what a new user wants to know.
A side effect of this is git blame won't properly track history by
default, users need to run
$ git blame -L ,1 -C17 include/kunit/resource.h
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Today, when we want to check if a pointer is NULL and not ERR we have
two options:
KUNIT_EXPECT_TRUE(test, ptr == NULL);
or
KUNIT_EXPECT_PTR_NE(test, ptr, (struct mystruct *)NULL);
Create a new set of macros that take care of NULL checks.
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Daniel Latypov <dlatypov@google.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
All the operands should be tagged `const`.
We're only assigning them to variables so that we can compare them (e.g.
check if left == right, etc.) and avoid evaluating expressions multiple
times.
There's no need for them to be mutable.
Also rename the helper variable `loc` to `__loc` like we do with
`__assertion` and `__strs` to avoid potential name collisions with user
code.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
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 = <struct kunit_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 = <struct kunit_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 <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
We currently have 2 other versions of KUNIT_INIT_BINARY_ASSERT_STRUCT.
The only differences are that
* the format funcition they pass is different
* the types of left_val/right_val should be different (integral,
pointer, string).
The latter doesn't actually matter since these macros are just plumbing
them along to KUNIT_ASSERTION where they will get type checked.
So combine them all into a single KUNIT_INIT_BINARY_ASSERT_STRUCT that
now also takes the format function as a parameter.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
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 <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Introduce KUNIT_BINARY_PTR_ASSERTION to match KUNIT_BINARY_INT_ASSERTION
and make KUNIT_EXPECT_EQ and KUNIT_EXPECT_PTREQ use these instead of
shared intermediate macros that only remove the need to type "==" or
"!=".
The current macro chain looks like:
KUNIT_EXPECT_EQ_MSG => KUNIT_BINARY_EQ_MSG_ASSERTION => KUNIT_BASE_EQ_MSG_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
KUNIT_EXPECT_PTR_EQ_MSG => KUNIT_BINARY_PTR_EQ_MSG_ASSERTION => KUNIT_BASE_EQ_MSG_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
<ditto for NE and ASSERT>
After this change:
KUNIT_EXPECT_EQ_MSG => KUNIT_BINARY_INT_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
KUNIT_EXPECT_PTR_EQ_MSG => KUNIT_BINARY_PTR_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Introduce a KUNIT_BINARY_INT_ASSERTION for the likes of KUNIT_EXPECT_LT.
This is analagous to KUNIT_BINARY_STR_ASSERTION.
Note: this patch leaves the EQ/NE macros untouched since those share
some intermediate macros for the pointer-based macros.
The current macro chain looks like:
KUNIT_EXPECT_LT_MSG => KUNIT_BASE_LT_MSG_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
KUNIT_EXPECT_GT_MSG => KUNIT_BASE_GT_MSG_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
<ditto for LE, GE, and ASSERT variants>
After this change:
KUNIT_EXPECT_LT_MSG => KUNIT_BINARY_INT_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
KUNIT_EXPECT_GT_MSG => KUNIT_BINARY_INT_ASSERTION => KUNIT_BASE_BINARY_ASSERTION
I.e. we've traded all the unique intermediary macros for a single shared
KUNIT_BINARY_INT_ASSERTION. The only difference is that users of
KUNIT_BINARY_INT_ASSERTION also need to pass the operation (==, <, etc.).
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
The current macro chain looks like:
KUNIT_EXPECT_STREQ => KUNIT_EXPECT_STREQ_MSG => KUNIT_BINARY_STR_EQ_MSG_ASSERTION => KUNIT_BINARY_STR_ASSERTION.
KUNIT_ASSERT_STREQ => KUNIT_ASSERT_STREQ_MSG => KUNIT_BINARY_STR_EQ_MSG_ASSERTION => KUNIT_BINARY_STR_ASSERTION.
<ditto for STR_NE>
After this change:
KUNIT_EXPECT_STREQ => KUNIT_EXPECT_STREQ_MSG => KUNIT_BINARY_STR_ASSERTION.
KUNIT_ASSERT_STREQ => KUNIT_ASSERT_STREQ_MSG => KUNIT_BINARY_STR_ASSERTION.
<ditto for STR_NE>
All the intermediate macro did was pass in "==" or "!=", so it seems
better to just drop them at the cost of a bit more copy-paste.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
We have the intermediate macros for KUNIT_EXPECT_PTR_GT() and friends,
but these macros don't exist.
I can see niche usecases for these macros existing, but since we've been
fine without them for so long, let's drop this dead code.
Users can instead cast the pointers and use the other GT/LT macros.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
