diff mbox series

[1/3] kunit: add kunit.filter_glob cmdline option to filter suites

Message ID 20210203234116.839819-2-dlatypov@google.com (mailing list archive)
State Superseded
Delegated to: Brendan Higgins
Headers show
Series kunit: support running subsets of test suites from | expand

Commit Message

Daniel Latypov Feb. 3, 2021, 11:41 p.m. UTC 
E.g. specifying this would run suites with "list" in their name.
  kunit.filter_glob=list*

Note: the executor prints out a TAP header that includes the number of
suites we intend to run.
So unless we want to report empty results for filtered-out suites, we
need to do the filtering here in the executor.
It's also probably better in the executor since we most likely don't
want any filtering to apply to tests built as modules.

This code does add a CONFIG_GLOB=y dependency for CONFIG_KUNIT=y.
But the code seems light enough that it shouldn't be an issue.

For now, we only filter on suite names so we don't have to create copies
of the suites themselves, just the array (of arrays) holding them.

The name is rather generic since in the future, we could consider
extending it to a syntax like:
  kunit.filter_glob=<suite_glob>.<test_glob>
E.g. to run all the del list tests
  kunit.filter_glob=list-kunit-test.*del*

But at the moment, it's far easier to manually comment out test cases in
test files as opposed to messing with sets of Kconfig entries to select
specific suites.
So even just doing this makes using kunit far less annoying.

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 lib/kunit/Kconfig    |  1 +
 lib/kunit/executor.c | 91 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 83 insertions(+), 9 deletions(-)

Comments

Dan Carpenter Feb. 4, 2021, 7:11 a.m. UTC | #1 
Hi Daniel,

url:    https://github.com/0day-ci/linux/commits/Daniel-Latypov/kunit-support-running-subsets-of-test-suites-from/20210204-074405
base:   88bb507a74ea7d75fa49edd421eaa710a7d80598
config: x86_64-randconfig-m001-20210202 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
lib/kunit/executor.c:110 kunit_run_all_tests() error: double free of 'suite_set.start'

vim +110 lib/kunit/executor.c

8c0d884986ba22 Brendan Higgins 2020-08-04   96  int kunit_run_all_tests(void)
aac35468ca20a3 Alan Maguire    2020-08-04   97  {
aac35468ca20a3 Alan Maguire    2020-08-04   98  	struct kunit_suite * const * const *suites;
aac35468ca20a3 Alan Maguire    2020-08-04   99  
d5554dd78a454b Daniel Latypov  2021-02-03  100  	struct suite_set suite_set = kunit_filter_suites();
45dcbb6f5ef78b Brendan Higgins 2020-08-04  101  
d5554dd78a454b Daniel Latypov  2021-02-03  102  	kunit_print_tap_header(&suite_set);
d5554dd78a454b Daniel Latypov  2021-02-03  103  
d5554dd78a454b Daniel Latypov  2021-02-03  104  	for (suites = suite_set.start; suites < suite_set.end; suites++)
aac35468ca20a3 Alan Maguire    2020-08-04  105  		__kunit_test_suites_init(*suites);
aac35468ca20a3 Alan Maguire    2020-08-04  106  
d5554dd78a454b Daniel Latypov  2021-02-03  107  	if (filter_glob) { /* a copy was made of each array */
d5554dd78a454b Daniel Latypov  2021-02-03  108  		for (suites = suite_set.start; suites < suite_set.end; suites++)
                                                                     ^^^^^^^^^^^^^^^^^^^^^^^^
This will free "suite_set.start" will in the first iteration through the
loop

d5554dd78a454b Daniel Latypov  2021-02-03  109  			kfree(suites);
d5554dd78a454b Daniel Latypov  2021-02-03 @110  		kfree(suite_set.start);
                                                                      ^^^^^^^^^^^^^^^
and then double free it.

d5554dd78a454b Daniel Latypov  2021-02-03  111  	}
d5554dd78a454b Daniel Latypov  2021-02-03  112  
aac35468ca20a3 Alan Maguire    2020-08-04  113  	return 0;
aac35468ca20a3 Alan Maguire    2020-08-04  114  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Daniel Latypov Feb. 4, 2021, 5:30 p.m. UTC | #2 
On Wed, Feb 3, 2021 at 11:13 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hi Daniel,
>
> url:    https://github.com/0day-ci/linux/commits/Daniel-Latypov/kunit-support-running-subsets-of-test-suites-from/20210204-074405
> base:   88bb507a74ea7d75fa49edd421eaa710a7d80598
> config: x86_64-randconfig-m001-20210202 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> lib/kunit/executor.c:110 kunit_run_all_tests() error: double free of 'suite_set.start'
>
> vim +110 lib/kunit/executor.c
>
> 8c0d884986ba22 Brendan Higgins 2020-08-04   96  int kunit_run_all_tests(void)
> aac35468ca20a3 Alan Maguire    2020-08-04   97  {
> aac35468ca20a3 Alan Maguire    2020-08-04   98          struct kunit_suite * const * const *suites;
> aac35468ca20a3 Alan Maguire    2020-08-04   99
> d5554dd78a454b Daniel Latypov  2021-02-03  100          struct suite_set suite_set = kunit_filter_suites();
> 45dcbb6f5ef78b Brendan Higgins 2020-08-04  101
> d5554dd78a454b Daniel Latypov  2021-02-03  102          kunit_print_tap_header(&suite_set);
> d5554dd78a454b Daniel Latypov  2021-02-03  103
> d5554dd78a454b Daniel Latypov  2021-02-03  104          for (suites = suite_set.start; suites < suite_set.end; suites++)
> aac35468ca20a3 Alan Maguire    2020-08-04  105                  __kunit_test_suites_init(*suites);
> aac35468ca20a3 Alan Maguire    2020-08-04  106
> d5554dd78a454b Daniel Latypov  2021-02-03  107          if (filter_glob) { /* a copy was made of each array */
> d5554dd78a454b Daniel Latypov  2021-02-03  108                  for (suites = suite_set.start; suites < suite_set.end; suites++)
>                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^
> This will free "suite_set.start" will in the first iteration through the
> loop

Ah, the loop is supposed to contain `kfree(*suites)`.
I'll fix the patch and resend.

I'm not familiar with conventions but it feels like adding Reported-by
on the amended patch would almost imply the report suggested the need
for the ability to filter suites.
So I'll add an informal attribution in the cover letter.

Thanks!

>
> d5554dd78a454b Daniel Latypov  2021-02-03  109                          kfree(suites);
> d5554dd78a454b Daniel Latypov  2021-02-03 @110                  kfree(suite_set.start);
>                                                                       ^^^^^^^^^^^^^^^
> and then double free it.
>
> d5554dd78a454b Daniel Latypov  2021-02-03  111          }
> d5554dd78a454b Daniel Latypov  2021-02-03  112
> aac35468ca20a3 Alan Maguire    2020-08-04  113          return 0;
> aac35468ca20a3 Alan Maguire    2020-08-04  114  }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Dan Carpenter Feb. 8, 2021, 11:12 a.m. UTC | #3 
On Thu, Feb 04, 2021 at 09:30:43AM -0800, Daniel Latypov wrote:
> On Wed, Feb 3, 2021 at 11:13 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > Hi Daniel,
> >
> > url:    https://github.com/0day-ci/linux/commits/Daniel-Latypov/kunit-support-running-subsets-of-test-suites-from/20210204-074405 
> > base:   88bb507a74ea7d75fa49edd421eaa710a7d80598
> > config: x86_64-randconfig-m001-20210202 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > smatch warnings:
> > lib/kunit/executor.c:110 kunit_run_all_tests() error: double free of 'suite_set.start'
> >
> > vim +110 lib/kunit/executor.c
> >
> > 8c0d884986ba22 Brendan Higgins 2020-08-04   96  int kunit_run_all_tests(void)
> > aac35468ca20a3 Alan Maguire    2020-08-04   97  {
> > aac35468ca20a3 Alan Maguire    2020-08-04   98          struct kunit_suite * const * const *suites;
> > aac35468ca20a3 Alan Maguire    2020-08-04   99
> > d5554dd78a454b Daniel Latypov  2021-02-03  100          struct suite_set suite_set = kunit_filter_suites();
> > 45dcbb6f5ef78b Brendan Higgins 2020-08-04  101
> > d5554dd78a454b Daniel Latypov  2021-02-03  102          kunit_print_tap_header(&suite_set);
> > d5554dd78a454b Daniel Latypov  2021-02-03  103
> > d5554dd78a454b Daniel Latypov  2021-02-03  104          for (suites = suite_set.start; suites < suite_set.end; suites++)
> > aac35468ca20a3 Alan Maguire    2020-08-04  105                  __kunit_test_suites_init(*suites);
> > aac35468ca20a3 Alan Maguire    2020-08-04  106
> > d5554dd78a454b Daniel Latypov  2021-02-03  107          if (filter_glob) { /* a copy was made of each array */
> > d5554dd78a454b Daniel Latypov  2021-02-03  108                  for (suites = suite_set.start; suites < suite_set.end; suites++)
> >                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^
> > This will free "suite_set.start" will in the first iteration through the
> > loop
> 
> Ah, the loop is supposed to contain `kfree(*suites)`.
> I'll fix the patch and resend.
> 
> I'm not familiar with conventions but it feels like adding Reported-by
> on the amended patch would almost imply the report suggested the need
> for the ability to filter suites.
> So I'll add an informal attribution in the cover letter.
> 
> Thanks!

These emails are autogenerated by the kbuild bot and I just look them
over and hit forward.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index 00909e6a2443..0b5dfb001bac 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -4,6 +4,7 @@ 
 
 menuconfig KUNIT
 	tristate "KUnit - Enable support for unit tests"
+	select GLOB if KUNIT=y
 	help
 	  Enables support for kernel unit tests (KUnit), a lightweight unit
 	  testing and mocking framework for the Linux kernel. These tests are
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index a95742a4ece7..25a891285816 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -1,6 +1,8 @@ 
 // SPDX-License-Identifier: GPL-2.0
 
 #include <kunit/test.h>
+#include <linux/glob.h>
+#include <linux/moduleparam.h>
 
 /*
  * These symbols point to the .kunit_test_suites section and are defined in
@@ -11,14 +13,79 @@  extern struct kunit_suite * const * const __kunit_suites_end[];
 
 #if IS_BUILTIN(CONFIG_KUNIT)
 
-static void kunit_print_tap_header(void)
+static char *filter_glob;
+module_param(filter_glob, charp, 0);
+
+static struct kunit_suite * const *
+kunit_filter_subsuite(struct kunit_suite * const * const subsuite)
+{
+	int i, n = 0;
+	struct kunit_suite **filtered;
+
+	n = 0;
+	for (i = 0; subsuite[i] != NULL; ++i) {
+		if (glob_match(filter_glob, subsuite[i]->name))
+			++n;
+	}
+
+	if (n == 0)
+		return NULL;
+
+	filtered = kmalloc_array(n + 1, sizeof(*filtered), GFP_KERNEL);
+	if (!filtered)
+		return NULL;
+
+	n = 0;
+	for (i = 0; subsuite[i] != NULL; ++i) {
+		if (glob_match(filter_glob, subsuite[i]->name))
+			filtered[n++] = subsuite[i];
+	}
+	filtered[n] = NULL;
+
+	return filtered;
+}
+
+struct suite_set {
+	struct kunit_suite * const * const *start;
+	struct kunit_suite * const * const *end;
+};
+
+static struct suite_set kunit_filter_suites(void)
+{
+	int i;
+	struct kunit_suite * const **copy, * const *filtered_subsuite;
+	struct suite_set filtered;
+
+	const size_t max = __kunit_suites_end - __kunit_suites_start;
+
+	if (!filter_glob) {
+		filtered.start = __kunit_suites_start;
+		filtered.end = __kunit_suites_end;
+		return filtered;
+	}
+
+	copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL);
+	filtered.start = copy;
+	if (!copy) { /* won't be able to run anything, return an empty set */
+		filtered.end = copy;
+		return filtered;
+	}
+
+	for (i = 0; i < max; ++i) {
+		filtered_subsuite = kunit_filter_subsuite(__kunit_suites_start[i]);
+		if (filtered_subsuite)
+			*copy++ = filtered_subsuite;
+	}
+	filtered.end = copy;
+	return filtered;
+}
+
+static void kunit_print_tap_header(struct suite_set *suite_set)
 {
 	struct kunit_suite * const * const *suites, * const *subsuite;
 	int num_of_suites = 0;
 
-	for (suites = __kunit_suites_start;
-	     suites < __kunit_suites_end;
-	     suites++)
+	for (suites = suite_set->start; suites < suite_set->end; suites++)
 		for (subsuite = *suites; *subsuite != NULL; subsuite++)
 			num_of_suites++;
 
@@ -30,12 +97,18 @@  int kunit_run_all_tests(void)
 {
 	struct kunit_suite * const * const *suites;
 
-	kunit_print_tap_header();
+	struct suite_set suite_set = kunit_filter_suites();
+
+	kunit_print_tap_header(&suite_set);
+
+	for (suites = suite_set.start; suites < suite_set.end; suites++)
+		__kunit_test_suites_init(*suites);
 
-	for (suites = __kunit_suites_start;
-	     suites < __kunit_suites_end;
-	     suites++)
-			__kunit_test_suites_init(*suites);
+	if (filter_glob) { /* a copy was made of each array */
+		for (suites = suite_set.start; suites < suite_set.end; suites++)
+			kfree(suites);
+		kfree(suite_set.start);
+	}
 
 	return 0;
 }