[PATCH] lib/tests: Make RANDSTRUCT_KUNIT_TEST depend on RANDSTRUCT

Geert Uytterhoeven posted 1 patch 6 months, 2 weeks ago
lib/Kconfig.debug            | 4 ++--
lib/tests/randstruct_kunit.c | 9 ---------
2 files changed, 2 insertions(+), 11 deletions(-)
[PATCH] lib/tests: Make RANDSTRUCT_KUNIT_TEST depend on RANDSTRUCT
Posted by Geert Uytterhoeven 6 months, 2 weeks ago
When CONFIG_RANDSTRUCT is not enabled, all randstruct tests are skipped.
Move this logic from run-time to config-time, to avoid people building
and running tests that do not do anything.

Fixes: b370f7eacdcfe1dd ("lib/tests: Add randstruct KUnit test")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
FTR, after adding "select HAVE_GCC_PLUGINS" to arch/m68k/Kconfig and
installing gcc-13-plugin-dev-m68k-linux-gnu, I could enable
CONFIG_RANDSTRUCT_FULL and run the tests (all passed!), so probably I
should send a patch to select HAVE_GCC_PLUGINS?
---
 lib/Kconfig.debug            | 4 ++--
 lib/tests/randstruct_kunit.c | 9 ---------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a8b4febad716a4be..407f2ed7fcb3e94c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2895,10 +2895,10 @@ config OVERFLOW_KUNIT_TEST
 config RANDSTRUCT_KUNIT_TEST
 	tristate "Test randstruct structure layout randomization at runtime" if !KUNIT_ALL_TESTS
 	depends on KUNIT
+	depends on RANDSTRUCT
 	default KUNIT_ALL_TESTS
 	help
-	  Builds unit tests for the checking CONFIG_RANDSTRUCT=y, which
-	  randomizes structure layouts.
+	  Builds unit tests for checking structure layout randomization.
 
 config STACKINIT_KUNIT_TEST
 	tristate "Test level of stack variable initialization" if !KUNIT_ALL_TESTS
diff --git a/lib/tests/randstruct_kunit.c b/lib/tests/randstruct_kunit.c
index f3a2d63c4cfbe7dc..2c95eca76d2411bc 100644
--- a/lib/tests/randstruct_kunit.c
+++ b/lib/tests/randstruct_kunit.c
@@ -305,14 +305,6 @@ static void randstruct_initializers(struct kunit *test)
 #undef init_members
 }
 
-static int randstruct_test_init(struct kunit *test)
-{
-	if (!IS_ENABLED(CONFIG_RANDSTRUCT))
-		kunit_skip(test, "Not built with CONFIG_RANDSTRUCT=y");
-
-	return 0;
-}
-
 static struct kunit_case randstruct_test_cases[] = {
 	KUNIT_CASE(randstruct_layout_same),
 	KUNIT_CASE(randstruct_layout_mixed),
@@ -324,7 +316,6 @@ static struct kunit_case randstruct_test_cases[] = {
 
 static struct kunit_suite randstruct_test_suite = {
 	.name = "randstruct",
-	.init = randstruct_test_init,
 	.test_cases = randstruct_test_cases,
 };
 
-- 
2.43.0
Re: [PATCH] lib/tests: Make RANDSTRUCT_KUNIT_TEST depend on RANDSTRUCT
Posted by Kees Cook 6 months, 2 weeks ago
On Mon, Jun 02, 2025 at 04:49:51PM +0200, Geert Uytterhoeven wrote:
> When CONFIG_RANDSTRUCT is not enabled, all randstruct tests are skipped.
> Move this logic from run-time to config-time, to avoid people building
> and running tests that do not do anything.

I don't like doing this because it means that looking at CI output means
I can't tell if the test was not built or if the config was not
included. I want to always have the test available, but skip the test if
the config is missing.

-Kees

-- 
Kees Cook
Re: [PATCH] lib/tests: Make RANDSTRUCT_KUNIT_TEST depend on RANDSTRUCT
Posted by Geert Uytterhoeven 6 months, 2 weeks ago
Hi Kees,

On Mon, 2 Jun 2025 at 20:08, Kees Cook <kees@kernel.org> wrote:
> On Mon, Jun 02, 2025 at 04:49:51PM +0200, Geert Uytterhoeven wrote:
> > When CONFIG_RANDSTRUCT is not enabled, all randstruct tests are skipped.
> > Move this logic from run-time to config-time, to avoid people building
> > and running tests that do not do anything.
>
> I don't like doing this because it means that looking at CI output means
> I can't tell if the test was not built or if the config was not
> included. I want to always have the test available, but skip the test if
> the config is missing.

So should we drop all dependencies from tests?
Do you want Zorro bus, NuBus, ... tests (assumed we have them) to
be built on all platforms, and "run" on all CIs?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] lib/tests: Make RANDSTRUCT_KUNIT_TEST depend on RANDSTRUCT
Posted by Kees Cook 6 months, 2 weeks ago
On Mon, Jun 02, 2025 at 08:14:08PM +0200, Geert Uytterhoeven wrote:
> Hi Kees,
> 
> On Mon, 2 Jun 2025 at 20:08, Kees Cook <kees@kernel.org> wrote:
> > On Mon, Jun 02, 2025 at 04:49:51PM +0200, Geert Uytterhoeven wrote:
> > > When CONFIG_RANDSTRUCT is not enabled, all randstruct tests are skipped.
> > > Move this logic from run-time to config-time, to avoid people building
> > > and running tests that do not do anything.
> >
> > I don't like doing this because it means that looking at CI output means
> > I can't tell if the test was not built or if the config was not
> > included. I want to always have the test available, but skip the test if
> > the config is missing.
> 
> So should we drop all dependencies from tests?
> Do you want Zorro bus, NuBus, ... tests (assumed we have them) to
> be built on all platforms, and "run" on all CIs?

I can't speak for those authors, but I think they are pretty different
from treewide instrumentation tests. As a consumer of CI output, I would
like to be able to always see this kind of test output. Sometimes it's
Kconfig settings (like here) and sometimes it's architectural capabilities
(as seen in LKDTM tests). If the test is always run and explains why
it has been skipped if it is skipped, I always get actionable details
without needing to do a round-trip with the CI runner, spend time
locating .config files, etc, etc.

-- 
Kees Cook