lkml.org 
[lkml]   [2021]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools
On Tue, Feb 9, 2021 at 9:26 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Fri, 5 Feb 2021, Daniel Latypov wrote:
>
> > From: Uriel Guajardo <urielguajardo@google.com>
> >
> > Add a kunit_fail_current_test() function to fail the currently running
> > test, if any, with an error message.
> >
> > This is largely intended for dynamic analysis tools like UBSAN and for
> > fakes.
> > E.g. say I had a fake ops struct for testing and I wanted my `free`
> > function to complain if it was called with an invalid argument, or
> > caught a double-free. Most return void and have no normal means of
> > signalling failure (e.g. super_operations, iommu_ops, etc.).
> >
> > Key points:
> > * Always update current->kunit_test so anyone can use it.
> > * commit 83c4e7a0363b ("KUnit: KASAN Integration") only updated it for
> > CONFIG_KASAN=y
> >
> > * Create a new header <kunit/test-bug.h> so non-test code doesn't have
> > to include all of <kunit/test.h> (e.g. lib/ubsan.c)
> >
> > * Forward the file and line number to make it easier to track down
> > failures
> >
>
> Thanks for doing this!
>
> > * Declare it as a function for nice __printf() warnings about mismatched
> > format strings even when KUnit is not enabled.
> >
>
> One thing I _think_ this assumes is that KUnit is builtin;
> don't we need an

Ah, you're correct.
Also going to rename it to have two _ to match other functions used in
macros like __kunit_test_suites_init.

I had been having some recent issues with getting QEMU to work on my
machine so I hadn't tested it before.
Somehow I've finally fixed it and can now say that it works w/
CONFIG_KUNIT=m after making the change

# modprobe kunit
# modprobe kunit-example-test
[ 27.689840] # Subtest: example
[ 27.689994] 1..1
[ 27.692337] # example_simple_test: initializing
[ 27.692862] # example_simple_test:
lib/kunit/kunit-example-test.c:31: example failure message: 42
[ 27.693158] not ok 1 - example_simple_test
[ 27.693654] not ok 1 - example



>
> EXPORT_SYMBOL_GPL(_kunit_fail_current_test);
>
> ?
>
> Without it, if an analysis tool (or indeed if KUnit) is built
> as a module, it won't be possible to use this functionality.
>
> > Example output from kunit_fail_current_test("message"):
> > [15:19:34] [FAILED] example_simple_test
> > [15:19:34] # example_simple_test: initializing
> > [15:19:34] # example_simple_test: lib/kunit/kunit-example-test.c:24: message
> > [15:19:34] not ok 1 - example_simple_test
> >
> > Co-developed-by: Daniel Latypov <dlatypov@google.com>
> > Signed-off-by: Uriel Guajardo <urielguajardo@google.com>
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > ---
> > include/kunit/test-bug.h | 30 ++++++++++++++++++++++++++++++
> > lib/kunit/test.c | 36 ++++++++++++++++++++++++++++++++----
> > 2 files changed, 62 insertions(+), 4 deletions(-)
> > create mode 100644 include/kunit/test-bug.h
> >
> > diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> > new file mode 100644
> > index 000000000000..4963ed52c2df
> > --- /dev/null
> > +++ b/include/kunit/test-bug.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * KUnit API allowing dynamic analysis tools to interact with KUnit tests
> > + *
> > + * Copyright (C) 2020, Google LLC.
>
> nit; might want to update copyright year.
>
> > + * Author: Uriel Guajardo <urielguajardo@google.com>
> > + */
> > +
> > +#ifndef _KUNIT_TEST_BUG_H
> > +#define _KUNIT_TEST_BUG_H
> > +
> > +#define kunit_fail_current_test(fmt, ...) \
> > + _kunit_fail_current_test(__FILE__, __LINE__, fmt, ##__VA_ARGS__)
> > +
> > +#if IS_ENABLED(CONFIG_KUNIT)
> > +
> > +extern __printf(3, 4) void _kunit_fail_current_test(const char *file, int line,
> > + const char *fmt, ...);
> > +
> > +#else
> > +
> > +static __printf(3, 4) void _kunit_fail_current_test(const char *file, int line,
> > + const char *fmt, ...)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +
> > +#endif /* _KUNIT_TEST_BUG_H */
> > diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> > index ec9494e914ef..7b16aae0ccae 100644
> > --- a/lib/kunit/test.c
> > +++ b/lib/kunit/test.c
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include <kunit/test.h>
> > +#include <kunit/test-bug.h>
> > #include <linux/kernel.h>
> > #include <linux/kref.h>
> > #include <linux/sched/debug.h>
> > @@ -16,6 +17,37 @@
> > #include "string-stream.h"
> > #include "try-catch-impl.h"
> >
> > +/*
> > + * Fail the current test and print an error message to the log.
> > + */
> > +void _kunit_fail_current_test(const char *file, int line, const char *fmt, ...)
> > +{
> > + va_list args;
> > + int len;
> > + char *buffer;
> > +
> > + if (!current->kunit_test)
> > + return;
> > +
> > + kunit_set_failure(current->kunit_test);
> > +
> > + /* kunit_err() only accepts literals, so evaluate the args first. */
> > + va_start(args, fmt);
> > + len = vsnprintf(NULL, 0, fmt, args) + 1;
> > + va_end(args);
> > +
> > + buffer = kunit_kmalloc(current->kunit_test, len, GFP_KERNEL);
> > + if (!buffer)
> > + return;
> > +
> > + va_start(args, fmt);
> > + vsnprintf(buffer, len, fmt, args);
> > + va_end(args);
> > +
> > + kunit_err(current->kunit_test, "%s:%d: %s", file, line, buffer);
> > + kunit_kfree(current->kunit_test, buffer);
> > +}
> > +
> > /*
> > * Append formatted message to log, size of which is limited to
> > * KUNIT_LOG_SIZE bytes (including null terminating byte).
> > @@ -273,9 +305,7 @@ static void kunit_try_run_case(void *data)
> > struct kunit_suite *suite = ctx->suite;
> > struct kunit_case *test_case = ctx->test_case;
> >
> > -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
> > current->kunit_test = test;
> > -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
> >
> > /*
> > * kunit_run_case_internal may encounter a fatal error; if it does,
> > @@ -624,9 +654,7 @@ void kunit_cleanup(struct kunit *test)
> > spin_unlock(&test->lock);
> > kunit_remove_resource(test, res);
> > }
> > -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
> > current->kunit_test = NULL;
> > -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
> > }
> > EXPORT_SYMBOL_GPL(kunit_cleanup);
> >
> > --
> > 2.30.0.478.g8a0d178c01-goog
> >
> >

\
 
 \ /
  Last update: 2021-02-10 01:54    [W:0.053 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site