[PATCH] [RFC] kunit: move binary assertion out of line

From: Arnd Bergmann
Date: Fri Jan 10 2020 - 08:44:04 EST


In combination with the structleak gcc plugin, kunit can lead to excessive
stack usage when each assertion adds another structure to the stack from
of the calling function:

base/test/property-entry-test.c:99:1: error: the frame size of 3032 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]

As most assertions are binary, change those over to a direct function
call that does not have this problem. This can probably be improved
further, I just went for a straightforward conversion, but a function
call with 12 fixed arguments plus varargs it not great either.

Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Cc: Brendan Higgins <brendanhiggins@xxxxxxxxxx>
Cc: Stephen Boyd <sboyd@xxxxxxxxxx>
Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
Cc: linux-kselftest@xxxxxxxxxxxxxxx
Cc: kunit-dev@xxxxxxxxxxxxxxxx
Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
---
I think improving the compiler or the plugin to not force the
allocation of these structs would be better, as it avoids the
whack-a-mole game of fixing up each time it hits us, but this
may not be possible using the current plugin infrastructure.
---
include/kunit/test.h | 155 ++++++++++++-------------------------------
lib/kunit/assert.c | 8 +--
lib/kunit/test.c | 42 ++++++++++++
3 files changed, 90 insertions(+), 115 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index dba48304b3bd..76eadd4a8b77 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -391,6 +391,16 @@ void kunit_do_assertion(struct kunit *test,
bool pass,
const char *fmt, ...);

+void kunit_do_binary_assertion(struct kunit *test, bool pass,
+ enum kunit_assert_type type,
+ int line, const char *file,
+ const char *operation,
+ const char *left_text, long long left_value,
+ const char *right_text, long long right_value,
+ void (*format)(const struct kunit_assert *assert,
+ struct string_stream *stream),
+ const char *fmt, ...);
+
#define KUNIT_ASSERTION(test, pass, assert_class, INITIALIZER, fmt, ...) do { \
struct assert_class __assertion = INITIALIZER; \
kunit_do_assertion(test, \
@@ -491,19 +501,32 @@ do { \
typeof(left) __left = (left); \
typeof(right) __right = (right); \
((void)__typecheck(__left, __right)); \
- \
- KUNIT_ASSERTION(test, \
- __left op __right, \
- assert_class, \
- ASSERT_CLASS_INIT(test, \
- assert_type, \
- #op, \
- #left, \
- __left, \
- #right, \
- __right), \
- fmt, \
- ##__VA_ARGS__); \
+ kunit_do_binary_assertion(test, left op right, assert_type, \
+ __LINE__, __FILE__, #op, #left, __left, \
+ #right, __right, \
+ kunit_binary_assert_format, \
+ fmt, ##__VA_ARGS__); \
+} while (0)
+
+#define KUNIT_BASE_POINTER_ASSERTION(test, \
+ assert_class, \
+ ASSERT_CLASS_INIT, \
+ assert_type, \
+ left, \
+ op, \
+ right, \
+ fmt, \
+ ...) \
+do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ ((void)__typecheck(__left, __right)); \
+ kunit_do_binary_assertion(test, left op right, assert_type, \
+ __LINE__, __FILE__, #op, \
+ #left, (uintptr_t)__left, \
+ #right, (uintptr_t)__right, \
+ kunit_binary_ptr_assert_format, \
+ fmt, ##__VA_ARGS__); \
} while (0)

#define KUNIT_BASE_EQ_MSG_ASSERTION(test, \
@@ -625,12 +648,11 @@ do { \
right, \
fmt, \
...) \
- KUNIT_BASE_EQ_MSG_ASSERTION(test, \
- kunit_binary_ptr_assert, \
- KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT, \
+ KUNIT_BASE_POINTER_ASSERTION(test, \
+ assert_class, \
+ ASSERT_CLASS_INIT, \
assert_type, \
- left, \
- right, \
+ left, ==, right, \
fmt, \
##__VA_ARGS__)

@@ -664,12 +686,11 @@ do { \
right, \
fmt, \
...) \
- KUNIT_BASE_NE_MSG_ASSERTION(test, \
- kunit_binary_ptr_assert, \
- KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT, \
+ KUNIT_BASE_POINTER_ASSERTION(test, \
+ assert_class, \
+ ASSERT_CLASS_INIT, \
assert_type, \
- left, \
- right, \
+ left, !=, right, \
fmt, \
##__VA_ARGS__)

@@ -697,28 +718,6 @@ do { \
right, \
NULL)

-#define KUNIT_BINARY_PTR_LT_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ...) \
- KUNIT_BASE_LT_MSG_ASSERTION(test, \
- kunit_binary_ptr_assert, \
- KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ##__VA_ARGS__)
-
-#define KUNIT_BINARY_PTR_LT_ASSERTION(test, assert_type, left, right) \
- KUNIT_BINARY_PTR_LT_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- NULL)
-
#define KUNIT_BINARY_LE_MSG_ASSERTION(test, assert_type, left, right, fmt, ...)\
KUNIT_BASE_LE_MSG_ASSERTION(test, \
kunit_binary_assert, \
@@ -736,28 +735,6 @@ do { \
right, \
NULL)

-#define KUNIT_BINARY_PTR_LE_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ...) \
- KUNIT_BASE_LE_MSG_ASSERTION(test, \
- kunit_binary_ptr_assert, \
- KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ##__VA_ARGS__)
-
-#define KUNIT_BINARY_PTR_LE_ASSERTION(test, assert_type, left, right) \
- KUNIT_BINARY_PTR_LE_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- NULL)
-
#define KUNIT_BINARY_GT_MSG_ASSERTION(test, assert_type, left, right, fmt, ...)\
KUNIT_BASE_GT_MSG_ASSERTION(test, \
kunit_binary_assert, \
@@ -775,28 +752,6 @@ do { \
right, \
NULL)

-#define KUNIT_BINARY_PTR_GT_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ...) \
- KUNIT_BASE_GT_MSG_ASSERTION(test, \
- kunit_binary_ptr_assert, \
- KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ##__VA_ARGS__)
-
-#define KUNIT_BINARY_PTR_GT_ASSERTION(test, assert_type, left, right) \
- KUNIT_BINARY_PTR_GT_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- NULL)
-
#define KUNIT_BINARY_GE_MSG_ASSERTION(test, assert_type, left, right, fmt, ...)\
KUNIT_BASE_GE_MSG_ASSERTION(test, \
kunit_binary_assert, \
@@ -814,28 +769,6 @@ do { \
right, \
NULL)

-#define KUNIT_BINARY_PTR_GE_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ...) \
- KUNIT_BASE_GE_MSG_ASSERTION(test, \
- kunit_binary_ptr_assert, \
- KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT, \
- assert_type, \
- left, \
- right, \
- fmt, \
- ##__VA_ARGS__)
-
-#define KUNIT_BINARY_PTR_GE_ASSERTION(test, assert_type, left, right) \
- KUNIT_BINARY_PTR_GE_MSG_ASSERTION(test, \
- assert_type, \
- left, \
- right, \
- NULL)
-
#define KUNIT_BINARY_STR_ASSERTION(test, \
assert_type, \
left, \
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index 86013d4cf891..abb6c70de925 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -101,8 +101,8 @@ void kunit_binary_assert_format(const struct kunit_assert *assert,
void kunit_binary_ptr_assert_format(const struct kunit_assert *assert,
struct string_stream *stream)
{
- struct kunit_binary_ptr_assert *binary_assert = container_of(
- assert, struct kunit_binary_ptr_assert, assert);
+ struct kunit_binary_assert *binary_assert = container_of(
+ assert, struct kunit_binary_assert, assert);

kunit_base_assert_format(assert, stream);
string_stream_add(stream,
@@ -112,10 +112,10 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert,
binary_assert->right_text);
string_stream_add(stream, "\t\t%s == %pK\n",
binary_assert->left_text,
- binary_assert->left_value);
+ (void *)(uintptr_t)binary_assert->left_value);
string_stream_add(stream, "\t\t%s == %pK",
binary_assert->right_text,
- binary_assert->right_value);
+ (void *)(uintptr_t)binary_assert->right_value);
kunit_assert_print_msg(assert, stream);
}

diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index c83c0fa59cbd..937f3fbe5ecc 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -172,6 +172,48 @@ void kunit_do_assertion(struct kunit *test,
kunit_abort(test);
}

+void kunit_do_binary_assertion(struct kunit *test, bool pass,
+ enum kunit_assert_type type,
+ int line, const char *file,
+ const char *operation,
+ const char *left_text, long long left_value,
+ const char *right_text, long long right_value,
+ void (*format)(const struct kunit_assert *assert,
+ struct string_stream *stream),
+ const char *fmt, ...)
+{
+ va_list args;
+ struct kunit_binary_assert assert = {
+ .assert = {
+ .test = test,
+ .type = type,
+ .file = file,
+ .line = line,
+ .message.fmt = fmt,
+ .format = format,
+ },
+ .operation = operation,
+ .left_text = left_text,
+ .left_value = left_value,
+ .right_text = right_text,
+ .right_value = right_value,
+ };
+
+ if (pass)
+ return;
+
+ va_start(args, fmt);
+
+ assert.assert.message.va = &args;
+
+ kunit_fail(test, &assert.assert);
+
+ va_end(args);
+
+ if (type == KUNIT_ASSERTION)
+ kunit_abort(test);
+}
+
void kunit_init_test(struct kunit *test, const char *name)
{
spin_lock_init(&test->lock);
--
2.20.0