* [PATCH v2 0/2] kunit: fail tests on UBSAN errors @ 2021-02-05 23:53 Daniel Latypov 2021-02-05 23:53 ` [PATCH v2 1/2] kunit: support failure from dynamic analysis tools Daniel Latypov 2021-02-05 23:53 ` [PATCH v2 2/2] kunit: ubsan integration Daniel Latypov 0 siblings, 2 replies; 7+ messages in thread From: Daniel Latypov @ 2021-02-05 23:53 UTC (permalink / raw) To: brendanhiggins, davidgow Cc: alan.maguire, linux-kernel, kunit-dev, linux-kselftest, skhan, Daniel Latypov v1 by Uriel is here: [1]. Since it's been a while, I've dropped the Reviewed-By's. It depended on commit 83c4e7a0363b ("KUnit: KASAN Integration") which hadn't been merged yet, so that caused some kerfuffle with applying them previously and the series was reverted. This revives the series but makes the kunit_fail_current_test() function take a format string and logs the file and line number of the failing code, addressing Alan Maguire's comments on the previous version. As a result, the patch that makes UBSAN errors was tweaked slightly to include an error message. [1] https://lore.kernel.org/linux-kselftest/20200806174326.3577537-1-urielguajardojr@gmail.com/ Uriel Guajardo (2): kunit: support failure from dynamic analysis tools kunit: ubsan integration include/kunit/test-bug.h | 30 ++++++++++++++++++++++++++++++ lib/kunit/test.c | 36 ++++++++++++++++++++++++++++++++---- lib/ubsan.c | 3 +++ 3 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 include/kunit/test-bug.h base-commit: 1e0d27fce010b0a4a9e595506b6ede75934c31be -- 2.30.0.478.g8a0d178c01-goog ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] kunit: support failure from dynamic analysis tools 2021-02-05 23:53 [PATCH v2 0/2] kunit: fail tests on UBSAN errors Daniel Latypov @ 2021-02-05 23:53 ` Daniel Latypov 2021-02-09 17:25 ` Alan Maguire 2021-02-05 23:53 ` [PATCH v2 2/2] kunit: ubsan integration Daniel Latypov 1 sibling, 1 reply; 7+ messages in thread From: Daniel Latypov @ 2021-02-05 23:53 UTC (permalink / raw) To: brendanhiggins, davidgow Cc: alan.maguire, linux-kernel, kunit-dev, linux-kselftest, skhan, Uriel Guajardo, Daniel Latypov 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 * Declare it as a function for nice __printf() warnings about mismatched format strings even when KUnit is not enabled. 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. + * 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools 2021-02-05 23:53 ` [PATCH v2 1/2] kunit: support failure from dynamic analysis tools Daniel Latypov @ 2021-02-09 17:25 ` Alan Maguire 2021-02-09 22:07 ` Daniel Latypov 0 siblings, 1 reply; 7+ messages in thread From: Alan Maguire @ 2021-02-09 17:25 UTC (permalink / raw) To: Daniel Latypov Cc: brendanhiggins, davidgow, alan.maguire, linux-kernel, kunit-dev, linux-kselftest, skhan, Uriel Guajardo 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 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools 2021-02-09 17:25 ` Alan Maguire @ 2021-02-09 22:07 ` Daniel Latypov 2021-02-09 22:12 ` Alan Maguire 0 siblings, 1 reply; 7+ messages in thread From: Daniel Latypov @ 2021-02-09 22:07 UTC (permalink / raw) To: Alan Maguire Cc: Brendan Higgins, David Gow, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Uriel Guajardo 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 > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools 2021-02-09 22:07 ` Daniel Latypov @ 2021-02-09 22:12 ` Alan Maguire 2021-02-09 22:21 ` Daniel Latypov 0 siblings, 1 reply; 7+ messages in thread From: Alan Maguire @ 2021-02-09 22:12 UTC (permalink / raw) To: Daniel Latypov Cc: Alan Maguire, Brendan Higgins, David Gow, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Uriel Guajardo On Tue, 9 Feb 2021, Daniel Latypov wrote: > 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. > Great! If you're sending out an updated version with these changes, feel free to add Reviewed-by: Alan Maguire <alan.maguire@oracle.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools 2021-02-09 22:12 ` Alan Maguire @ 2021-02-09 22:21 ` Daniel Latypov 0 siblings, 0 replies; 7+ messages in thread From: Daniel Latypov @ 2021-02-09 22:21 UTC (permalink / raw) To: Alan Maguire Cc: Brendan Higgins, David Gow, Linux Kernel Mailing List, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Uriel Guajardo On Tue, Feb 9, 2021 at 2:12 PM Alan Maguire <alan.maguire@oracle.com> wrote: > > On Tue, 9 Feb 2021, Daniel Latypov wrote: > > > 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. > > > > Great! If you're sending out an updated version with these > changes, feel free to add > > Reviewed-by: Alan Maguire <alan.maguire@oracle.com> Oops, there was a race in sending v3 and seeing this in my inbox. If you could reply to the v3 that'd be great. I've already amended the commit locally. Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] kunit: ubsan integration 2021-02-05 23:53 [PATCH v2 0/2] kunit: fail tests on UBSAN errors Daniel Latypov 2021-02-05 23:53 ` [PATCH v2 1/2] kunit: support failure from dynamic analysis tools Daniel Latypov @ 2021-02-05 23:53 ` Daniel Latypov 1 sibling, 0 replies; 7+ messages in thread From: Daniel Latypov @ 2021-02-05 23:53 UTC (permalink / raw) To: brendanhiggins, davidgow Cc: alan.maguire, linux-kernel, kunit-dev, linux-kselftest, skhan, Uriel Guajardo, Daniel Latypov From: Uriel Guajardo <urielguajardo@google.com> Integrates UBSAN into the KUnit testing framework. It fails KUnit tests whenever it reports undefined behavior. When CONFIG_KUNIT=n, nothing is printed or even formatted, so this has no behavioral impact outside of tests. kunit_fail_current_test() effectively does a pr_err() as well, so there's some slight duplication, but it also ensures an error is recorded in the debugfs entry for the running KUnit test. Print a shorter version of the message to make it less spammy. Co-developed-by: Daniel Latypov <dlatypov@google.com> Signed-off-by: Uriel Guajardo <urielguajardo@google.com> Signed-off-by: Daniel Latypov <dlatypov@google.com> --- lib/ubsan.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/ubsan.c b/lib/ubsan.c index bec38c64d6a6..1ec7d6f1fe63 100644 --- a/lib/ubsan.c +++ b/lib/ubsan.c @@ -14,6 +14,7 @@ #include <linux/types.h> #include <linux/sched.h> #include <linux/uaccess.h> +#include <kunit/test-bug.h> #include "ubsan.h" @@ -141,6 +142,8 @@ static void ubsan_prologue(struct source_location *loc, const char *reason) "========================================\n"); pr_err("UBSAN: %s in %s:%d:%d\n", reason, loc->file_name, loc->line & LINE_MASK, loc->column & COLUMN_MASK); + + kunit_fail_current_test("%s in %s", reason, loc->file_name); } static void ubsan_epilogue(void) -- 2.30.0.478.g8a0d178c01-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-10 1:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-05 23:53 [PATCH v2 0/2] kunit: fail tests on UBSAN errors Daniel Latypov 2021-02-05 23:53 ` [PATCH v2 1/2] kunit: support failure from dynamic analysis tools Daniel Latypov 2021-02-09 17:25 ` Alan Maguire 2021-02-09 22:07 ` Daniel Latypov 2021-02-09 22:12 ` Alan Maguire 2021-02-09 22:21 ` Daniel Latypov 2021-02-05 23:53 ` [PATCH v2 2/2] kunit: ubsan integration Daniel Latypov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.