Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kunit to check the longest symbol length #956

Draft
wants to merge 1 commit into
base: rust
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/Kconfig.debug
Original file line number Diff line number Diff line change
Expand Up @@ -2621,6 +2621,15 @@ config FORTIFY_KUNIT_TEST
by the str*() and mem*() family of functions. For testing runtime
traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.

config LONGEST_SYM_KUNIT_TEST
tristate "Test the longest symbol possible" if !KUNIT_ALL_TESTS
Copy link

@sulix sulix Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't build as a module:

ERROR: modpost: missing MODULE_LICENSE() in lib/longest_symbol_kunit.o
ERROR: modpost: "kallsyms_lookup_name" [lib/longest_symbol_kunit.ko] undefined!

The easy thing to do would be to make this a bool option, but this is exactly the sort of thing we probably should test as a module, too (maybe that's why the longest+1 name is working: the compiler/linker are fine, but the module tools (e.g. modpost) aren't). Fixing it up to work as a module would lose us some of the simplicity of the KUnit port, though.

Copy link
Author

@sergiocollado sergiocollado Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting ... yes, that may be very well the reason for the longest+1 working. That is good, because I was feeling quite uneasy about it.

I will look into it and make it a module.

Thanks for pointing it out :) 👍

Copy link
Author

@sergiocollado sergiocollado Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sulix I have updated the code, so now it actually compiles and can be executed as a module (as referenced here: https://www.kernel.org/doc/html/latest/dev-tools/kunit/running_tips.html#running-tests-as-modules ).

Incredible as it sounds, the test with the longest symbol +1, when executed as a module, passes. Is not clear to me why. I have to delve into this further.

sergio@ubuntu:/lib/modules/6.1.0-rc1-188160-g3ee25d98b592/kernel/lib$ dmesg -HW &
[1] 3207
sergio@ubuntu:/lib/modules/6.1.0-rc1-188160-g3ee25d98b592/kernel/lib$ sudo modprobe longest_symbol_kunit
[feb27 21:49]     # Subtest: longest-symbol
[  +0,000002]     1..4
[  +0,000075]     ok 1 - test_longest_symbol
[  +0,028325]     ok 2 - test_longest_symbol_kallsyms
[  +0,000237]     ok 3 - test_longest_symbol_plus1
[  +0,012761]     ok 4 - test_longest_symbol_plus1_kallsyms
[  +0,000005] # longest-symbol: pass:4 fail:0 skip:0 total:4
[  +0,000001] # Totals: pass:4 fail:0 skip:0 total:4
[  +0,000001] ok 1 - longest-symbol

running kunit.py:

$ ./tools/testing/kunit/kunit.py run "longest-symbol"
...

[22:34:12] Starting KUnit Kernel (1/1)...
[22:34:12] ============================================================
[22:34:12] =============== longest-symbol (4 subtests) ================
[22:34:12] [PASSED] test_longest_symbol
[22:34:12] [PASSED] test_longest_symbol_kallsyms
[22:34:12] [PASSED] test_longest_symbol_plus1
[22:34:12] [PASSED] test_longest_symbol_plus1_kallsyms
[22:34:12] ================= [PASSED] longest-symbol ==================
[22:34:12] ============================================================
[22:34:12] Testing complete. Ran 4 tests: passed: 4
[22:34:12] Elapsed time: 38.391s total, 0.001s configuring, 38.275s building, 0.093s running

Copy link
Member

@ojeda ojeda Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have taken a look: the test passes unexpectedly because the symbol is simply LONGEST_SYM_PLUS1, not the expected one (very long).

This is due to two issues:

  • A typo in LONGEST_SYM_PLUS1 vs. LONGEST_SYM_NAME_PLUS1.

  • The preprocessor will concatenate before expanding. So you need one level of indirection more, e.g. with the __PASTE macro.

diff --git a/lib/longest_symbol_kunit.c b/lib/longest_symbol_kunit.c
index 575edca77864..10d2e9967647 100644
--- a/lib/longest_symbol_kunit.c
+++ b/lib/longest_symbol_kunit.c
@@ -18,13 +18,13 @@
 #define DDDDI(name) DDDI(n##name##name)
 #define DDDDDI(name) DDDDI(n##name##name)
 
-#define PLUS1(name) name##e
+#define PLUS1(name) __PASTE(name,e)
 
 /*Generate a symbol whose name length is 511 */
 #define LONGEST_SYM_NAME  DDDDDI(g1h2i3j4k5l6m7n)
 
 /*Generate a symbol whose name length is 512 */
-#define LONGEST_SYM_PLUS1 PLUS1(LONGEST_SYM_NAME)
+#define LONGEST_SYM_NAME_PLUS1 PLUS1(LONGEST_SYM_NAME)
 
 int noinline LONGEST_SYM_NAME(void)
 {

With this, the expected symbol is generated:

$ nm lib/longest_symbol_kunit.o | awk 'length($3) > 100 { print length($3), $3 }'
511 snnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7n
512 snnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nnng1h2i3j4k5l6m7ng1h2i3j4k5l6m7nng1h2i3j4k5l6m7ng1h2i3j4k5l6m7ne

You can play with it at https://godbolt.org/z/9ach7Wq63.

Hope that helps!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) Great! thanks a lot for having a look and pointing out the issue! I will fix this 👍

Copy link
Member

@ojeda ojeda Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My pleasure, and apologies for the delay!

depends on KUNIT && KPROBES
default KUNIT_ALL_TESTS
help
Tests the longest symbol possible

If unsure, say N.

config HW_BREAKPOINT_KUNIT_TEST
bool "Test hw_breakpoint constraints accounting" if !KUNIT_ALL_TESTS
depends on HAVE_HW_BREAKPOINT
Expand Down
1 change: 1 addition & 0 deletions lib/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
obj-$(CONFIG_STRSCPY_KUNIT_TEST) += strscpy_kunit.o
obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
obj-$(CONFIG_LONGEST_SYM_KUNIT_TEST) += longest_symbol_kunit.o

obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o

Expand Down
119 changes: 119 additions & 0 deletions lib/longest_symbol_kunit.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// SPDX-License-Identifier: GPL-2.0
/*
* Test the longest symbol length. Execute with:
* ./tools/testing/kunit/kunit.py run longest-symbol
* --arch=x86_64 --kconfig_add CONFIG_KPROBES=y --kconfig_add CONFIG_MODULES=y
*/

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

#include <kunit/test.h>
#include <linux/stringify.h>
#include <linux/kprobes.h>
#include <linux/kallsyms.h>

#define DI(name) s##name##name
#define DDI(name) DI(n##name##name)
#define DDDI(name) DDI(n##name##name)
#define DDDDI(name) DDDI(n##name##name)
#define DDDDDI(name) DDDDI(n##name##name)

#define PLUS1(name) __PASTE(name, e)

/*Generate a symbol whose name length is 511 */
#define LONGEST_SYM_NAME DDDDDI(g1h2i3j4k5l6m7n)

/*Generate a symbol whose name length is 512 */
#define LONGEST_SYM_NAME_PLUS1 PLUS1(LONGEST_SYM_NAME)

noinline int LONGEST_SYM_NAME(void)
{
return 424242;
}

noinline int LONGEST_SYM_NAME_PLUS1(void)
{
return 434343;
}

_Static_assert(sizeof(__stringify(LONGEST_SYM_NAME)) == KSYM_NAME_LEN,
"Incorrect symbol length found. Expected KSYM_NAME_LEN: "
__stringify(KSYM_NAME) ", but found: "
__stringify(sizeof(LONGEST_SYM_NAME)));

static void test_longest_symbol(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, 424242, LONGEST_SYM_NAME());
};

static void test_longest_symbol_kallsyms(struct kunit *test)
{
unsigned long (*kallsyms_lookup_name)(const char *name);
static int (*longest_sym)(void);

struct kprobe kp = {
.symbol_name = "kallsyms_lookup_name",
};

if (register_kprobe(&kp) < 0) {
pr_info("%s: kprobe not registered\n", __func__);
kunit_warn(test, "test_longest_symbol kallsyms: kprobe not registered\n");
KUNIT_ASSERT_TRUE(test, register_kprobe(&kp) < 0);
KUNIT_FAIL(test, "test_longest_symbol kallsysms: kprobe not registered\n");
return;
}

kunit_warn(test, "test_longest_symbol kallsyms: kprobe registered\n");
kallsyms_lookup_name = (unsigned long (*)(const char *name))kp.addr;
unregister_kprobe(&kp);

longest_sym =
(void *) kallsyms_lookup_name(__stringify(LONGEST_SYM_NAME));
KUNIT_EXPECT_EQ(test, 424242, longest_sym());
};

static void test_longest_symbol_plus1(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, 434343, LONGEST_SYM_NAME_PLUS1());
};

static void test_longest_symbol_plus1_kallsyms(struct kunit *test)
{
unsigned long (*kallsyms_lookup_name)(const char *name);
static int (*longest_sym_plus1)(void);

struct kprobe kp = {
.symbol_name = "kallsyms_lookup_name",
};

if (register_kprobe(&kp) < 0) {
pr_info("%s: kprobe not registered\n", __func__);
KUNIT_ASSERT_TRUE(test, register_kprobe(&kp) < 0);
KUNIT_FAIL(test, "test_longest_symbol kallsysms: kprobe not registered\n");
return;
}

kunit_warn(test, "test_longest_symbol_plus1 kallsyms: kprobe registered\n");
kallsyms_lookup_name = (unsigned long (*)(const char *name))kp.addr;
unregister_kprobe(&kp);

longest_sym_plus1 =
(void *) kallsyms_lookup_name(__stringify(LONGEST_SYM_NAME_PLUS1));
KUNIT_EXPECT_EQ(test, NULL, longest_sym_plus1);
};

static struct kunit_case longest_symbol_test_cases[] = {
KUNIT_CASE(test_longest_symbol),
KUNIT_CASE(test_longest_symbol_kallsyms),
KUNIT_CASE(test_longest_symbol_plus1),
KUNIT_CASE(test_longest_symbol_plus1_kallsyms),
{}
};

static struct kunit_suite longest_symbol_test_suite = {
.name = "longest-symbol",
.test_cases = longest_symbol_test_cases,
};
kunit_test_suite(longest_symbol_test_suite);

MODULE_LICENSE("GPL");