[PATCH v2 0/2] kunit: fail tests on UBSAN errors

[PATCH v2 0/2] kunit: fail tests on UBSAN errors

[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

[PATCH v2 1/2] kunit: support failure from dynamic analysis tools

[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

[PATCH v2 2/2] kunit: ubsan integration

[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

Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools

[Alan Maguire]

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
> 
> 

Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools

[Daniel Latypov]

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
> >
> >

Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools

[Alan Maguire]

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>

Re: [PATCH v2 1/2] kunit: support failure from dynamic analysis tools

[Daniel Latypov]

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!