Re: [PATCH v3 03/15] kunit: Add test cases for backtrace warning suppression

2024-04-09 Thread Guenter Roeck
On Tue, Apr 09, 2024 at 04:29:42PM +0800, David Gow wrote:
> > +ifeq ($(CCONFIG_KUNIT_SUPPRESS_BACKTRACE),y)
> 
> s/CCONFIG_/CONFIG_/ ?
> 
> 
Odd, I know I tested this (and it still works ;-).
The additional "C" must have slipped in at some point.
Thanks for noticing!

Guenter


Re: [PATCH v3 03/15] kunit: Add test cases for backtrace warning suppression

2024-04-09 Thread David Gow
On Wed, 3 Apr 2024 at 21:19, Guenter Roeck  wrote:
>
> Add unit tests to verify that warning backtrace suppression works.
>
> If backtrace suppression does _not_ work, the unit tests will likely
> trigger unsuppressed backtraces, which should actually help to get
> the affected architectures / platforms fixed.
>
> Tested-by: Linux Kernel Functional Testing 
> Acked-by: Dan Carpenter 
> Reviewed-by: Kees Cook 
> Signed-off-by: Guenter Roeck 
> ---

There's a typo in the Makefile, which stops this from being built at
all. Otherwise, it seems good to me.

-- David

> v2:
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
> v3:
> - Rebased to v6.9-rc2
>
>  lib/kunit/Makefile |   7 +-
>  lib/kunit/backtrace-suppression-test.c | 104 +
>  2 files changed, 109 insertions(+), 2 deletions(-)
>  create mode 100644 lib/kunit/backtrace-suppression-test.c
>
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 545b57c3be48..3eee1bd0ce5e 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -16,10 +16,13 @@ endif
>
>  # KUnit 'hooks' and bug handling are built-in even when KUnit is built
>  # as a module.
> -obj-y +=   hooks.o \
> -   bug.o
> +obj-y +=   hooks.o
> +obj-$(CONFIG_KUNIT_SUPPRESS_BACKTRACE) += bug.o
>
>  obj-$(CONFIG_KUNIT_TEST) +=kunit-test.o
> +ifeq ($(CCONFIG_KUNIT_SUPPRESS_BACKTRACE),y)

s/CCONFIG_/CONFIG_/ ?




> +obj-$(CONFIG_KUNIT_TEST) +=backtrace-suppression-test.o
> +endif
>
>  # string-stream-test compiles built-in only.
>  ifeq ($(CONFIG_KUNIT_TEST),y)
> diff --git a/lib/kunit/backtrace-suppression-test.c 
> b/lib/kunit/backtrace-suppression-test.c
> new file mode 100644
> index ..47c619283802
> --- /dev/null
> +++ b/lib/kunit/backtrace-suppression-test.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for suppressing warning tracebacks
> + *
> + * Copyright (C) 2024, Guenter Roeck
> + * Author: Guenter Roeck 
> + */
> +
> +#include 
> +#include 
> +
> +static void backtrace_suppression_test_warn_direct(struct kunit *test)
> +{
> +   DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
> +
> +   START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
> +   WARN(1, "This backtrace should be suppressed");
> +   END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
> +
> +   KUNIT_EXPECT_EQ(test, 
> SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_direct), 1);
> +}
> +
> +static void trigger_backtrace_warn(void)
> +{
> +   WARN(1, "This backtrace should be suppressed");
> +}
> +
> +static void backtrace_suppression_test_warn_indirect(struct kunit *test)
> +{
> +   DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +
> +   START_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +   trigger_backtrace_warn();
> +   END_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +
> +   KUNIT_EXPECT_EQ(test, 
> SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn), 1);
> +}
> +
> +static void backtrace_suppression_test_warn_multi(struct kunit *test)
> +{
> +   DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +   DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
> +
> +   START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
> +   START_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +   WARN(1, "This backtrace should be suppressed");
> +   trigger_backtrace_warn();
> +   END_SUPPRESSED_WARNING(trigger_backtrace_warn);
> +   END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
> +
> +   KUNIT_EXPECT_EQ(test, 
> SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_multi), 1);
> +   KUNIT_EXPECT_EQ(test, 
> SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn), 1);
> +}
> +
> +static void backtrace_suppression_test_warn_on_direct(struct kunit *test)
> +{
> +   DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
> +
> +   if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE) && 
> !IS_ENABLED(CONFIG_KALLSYMS))
> +   kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE or 
> CONFIG_KALLSYMS");
> +
> +   START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
> +   WARN_ON(1);
> +   END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
> +
> +   KUNIT_EXPECT_EQ(test,
> +   
> SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_on_direct), 1);
> +}
> +
> +static void trigger_backtrace_warn_on(void)
> +{
> +   WARN_ON(1);
> +}
> +
> +static void backtrace_suppression_test_warn_on_indirect(struct kunit *test)
> +{
> +   DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn_on);
> +
> +   if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE))
> +   kunit_skip(test, "r

[PATCH v3 03/15] kunit: Add test cases for backtrace warning suppression

2024-04-03 Thread Guenter Roeck
Add unit tests to verify that warning backtrace suppression works.

If backtrace suppression does _not_ work, the unit tests will likely
trigger unsuppressed backtraces, which should actually help to get
the affected architectures / platforms fixed.

Tested-by: Linux Kernel Functional Testing 
Acked-by: Dan Carpenter 
Reviewed-by: Kees Cook 
Signed-off-by: Guenter Roeck 
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2

 lib/kunit/Makefile |   7 +-
 lib/kunit/backtrace-suppression-test.c | 104 +
 2 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 lib/kunit/backtrace-suppression-test.c

diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 545b57c3be48..3eee1bd0ce5e 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -16,10 +16,13 @@ endif
 
 # KUnit 'hooks' and bug handling are built-in even when KUnit is built
 # as a module.
-obj-y +=   hooks.o \
-   bug.o
+obj-y +=   hooks.o
+obj-$(CONFIG_KUNIT_SUPPRESS_BACKTRACE) += bug.o
 
 obj-$(CONFIG_KUNIT_TEST) +=kunit-test.o
+ifeq ($(CCONFIG_KUNIT_SUPPRESS_BACKTRACE),y)
+obj-$(CONFIG_KUNIT_TEST) +=backtrace-suppression-test.o
+endif
 
 # string-stream-test compiles built-in only.
 ifeq ($(CONFIG_KUNIT_TEST),y)
diff --git a/lib/kunit/backtrace-suppression-test.c 
b/lib/kunit/backtrace-suppression-test.c
new file mode 100644
index ..47c619283802
--- /dev/null
+++ b/lib/kunit/backtrace-suppression-test.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for suppressing warning tracebacks
+ *
+ * Copyright (C) 2024, Guenter Roeck
+ * Author: Guenter Roeck 
+ */
+
+#include 
+#include 
+
+static void backtrace_suppression_test_warn_direct(struct kunit *test)
+{
+   DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
+
+   START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
+   WARN(1, "This backtrace should be suppressed");
+   END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
+
+   KUNIT_EXPECT_EQ(test, 
SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_direct), 1);
+}
+
+static void trigger_backtrace_warn(void)
+{
+   WARN(1, "This backtrace should be suppressed");
+}
+
+static void backtrace_suppression_test_warn_indirect(struct kunit *test)
+{
+   DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
+
+   START_SUPPRESSED_WARNING(trigger_backtrace_warn);
+   trigger_backtrace_warn();
+   END_SUPPRESSED_WARNING(trigger_backtrace_warn);
+
+   KUNIT_EXPECT_EQ(test, SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn), 
1);
+}
+
+static void backtrace_suppression_test_warn_multi(struct kunit *test)
+{
+   DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
+   DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
+
+   START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
+   START_SUPPRESSED_WARNING(trigger_backtrace_warn);
+   WARN(1, "This backtrace should be suppressed");
+   trigger_backtrace_warn();
+   END_SUPPRESSED_WARNING(trigger_backtrace_warn);
+   END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
+
+   KUNIT_EXPECT_EQ(test, 
SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_multi), 1);
+   KUNIT_EXPECT_EQ(test, SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn), 
1);
+}
+
+static void backtrace_suppression_test_warn_on_direct(struct kunit *test)
+{
+   DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
+
+   if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE) && 
!IS_ENABLED(CONFIG_KALLSYMS))
+   kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE or 
CONFIG_KALLSYMS");
+
+   START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
+   WARN_ON(1);
+   END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
+
+   KUNIT_EXPECT_EQ(test,
+   
SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_on_direct), 1);
+}
+
+static void trigger_backtrace_warn_on(void)
+{
+   WARN_ON(1);
+}
+
+static void backtrace_suppression_test_warn_on_indirect(struct kunit *test)
+{
+   DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn_on);
+
+   if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE))
+   kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE");
+
+   START_SUPPRESSED_WARNING(trigger_backtrace_warn_on);
+   trigger_backtrace_warn_on();
+   END_SUPPRESSED_WARNING(trigger_backtrace_warn_on);
+
+   KUNIT_EXPECT_EQ(test, 
SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn_on), 1);
+}
+
+static struct kunit_case backtrace_suppression_test_cases[] = {
+   KUNIT_CASE(backtrace_suppression_test_warn_direct),
+   KUNIT_CASE(backtrace_suppression_test_wa