2023-03-31 09:25:43

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 0/3] Modify is_arm_mapping_symbol() related code

Based on "MODULE SUPPORT" tree:
git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next

Tiezhu Yang (3):
module: Sync code of is_arm_mapping_symbol()
module: Move is_arm_mapping_symbol() to module_symbol.h
module: Ignore L0 and rename is_arm_mapping_symbol()

include/linux/module_symbol.h | 17 +++++++++++++++++
kernel/module/kallsyms.c | 15 ++-------------
scripts/mod/modpost.c | 12 +++---------
3 files changed, 22 insertions(+), 22 deletions(-)
create mode 100644 include/linux/module_symbol.h

--
2.1.0


2023-03-31 09:26:02

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 3/3] module: Ignore L0 and rename is_arm_mapping_symbol()

The L0 symbol is generated when build module on LoongArch, ignore it in
modpost and when looking at module symbols, otherwise we can not see the
expected call trace.

Now is_arm_mapping_symbol() is not only for ARM, in order to reflect the
reality, rename is_arm_mapping_symbol() to is_mapping_symbol().

This is related with commit c17a2538704f ("mksysmap: Fix the mismatch of
'L0' symbols in System.map").

(1) Simple test case

[loongson@linux hello]$ cat hello.c
#include <linux/init.h>
#include <linux/module.h>
#include <linux/printk.h>

static void test_func(void)
{
pr_info("This is a test\n");
dump_stack();
}

static int __init hello_init(void)
{
pr_warn("Hello, world\n");
test_func();

return 0;
}

static void __exit hello_exit(void)
{
pr_warn("Goodbye\n");
}

module_init(hello_init);
module_exit(hello_exit);
MODULE_LICENSE("GPL");
[loongson@linux hello]$ cat Makefile
obj-m:=hello.o

ccflags-y += -g -Og

all:
make -C /lib/modules/$(shell uname -r)/build/ M=$(PWD) modules
clean:
make -C /lib/modules/$(shell uname -r)/build/ M=$(PWD) clean

(2) Test environment

system: LoongArch CLFS 5.5
https://github.com/sunhaiyong1978/CLFS-for-LoongArch/releases/tag/5.0
It needs to update grub to avoid booting error "invalid magic number".

kernel: 6.3-rc1 with loongson3_defconfig + CONFIG_DYNAMIC_FTRACE=y

(3) Test result

Without this patch:

[root@linux hello]# insmod hello.ko
[root@linux hello]# dmesg
...
Hello, world
This is a test
...
Call Trace:
[<9000000000223728>] show_stack+0x68/0x18c
[<90000000013374cc>] dump_stack_lvl+0x60/0x88
[<ffff800002050028>] L0\x01+0x20/0x2c [hello]
[<ffff800002058028>] L0\x01+0x20/0x30 [hello]
[<900000000022097c>] do_one_initcall+0x88/0x288
[<90000000002df890>] do_init_module+0x54/0x200
[<90000000002e1e18>] __do_sys_finit_module+0xc4/0x114
[<90000000013382e8>] do_syscall+0x7c/0x94
[<9000000000221e3c>] handle_syscall+0xbc/0x158

With this patch:

[root@linux hello]# insmod hello.ko
[root@linux hello]# dmesg
...
Hello, world
This is a test
...
Call Trace:
[<9000000000223728>] show_stack+0x68/0x18c
[<90000000013374cc>] dump_stack_lvl+0x60/0x88
[<ffff800002050028>] test_func+0x28/0x34 [hello]
[<ffff800002058028>] hello_init+0x28/0x38 [hello]
[<900000000022097c>] do_one_initcall+0x88/0x288
[<90000000002df890>] do_init_module+0x54/0x200
[<90000000002e1e18>] __do_sys_finit_module+0xc4/0x114
[<90000000013382e8>] do_syscall+0x7c/0x94
[<9000000000221e3c>] handle_syscall+0xbc/0x158

Signed-off-by: Tiezhu Yang <[email protected]>
---
include/linux/module_symbol.h | 4 +++-
kernel/module/kallsyms.c | 2 +-
scripts/mod/modpost.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h
index 9fa4173..7ace7ba 100644
--- a/include/linux/module_symbol.h
+++ b/include/linux/module_symbol.h
@@ -3,10 +3,12 @@
#define _LINUX_MODULE_SYMBOL_H

/* This ignores the intensely annoying "mapping symbols" found in ELF files. */
-static inline int is_arm_mapping_symbol(const char *str)
+static inline int is_mapping_symbol(const char *str)
{
if (str[0] == '.' && str[1] == 'L')
return true;
+ if (str[0] == 'L' && str[1] == '0')
+ return true;
return str[0] == '$' &&
(str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x')
&& (str[2] == '\0' || str[2] == '.');
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 5de3207..d8e426a 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -289,7 +289,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
* and inserted at a whim.
*/
if (*kallsyms_symbol_name(kallsyms, i) == '\0' ||
- is_arm_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
+ is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
continue;

if (thisval <= addr && thisval > bestval) {
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 7241db8..5cddf76 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1115,7 +1115,7 @@ static int secref_whitelist(const struct sectioncheck *mismatch,

/*
* If there's no name there, ignore it; likewise, ignore it if it's
- * one of the magic symbols emitted used by current ARM tools.
+ * one of the magic symbols emitted used by current tools.
*
* Otherwise if find_symbols_between() returns those symbols, they'll
* fail the whitelist tests and cause lots of false alarms ... fixable
@@ -1128,7 +1128,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)

if (!name || !strlen(name))
return 0;
- return !is_arm_mapping_symbol(name);
+ return !is_mapping_symbol(name);
}

/**
--
2.1.0

2023-03-31 09:26:45

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH 2/3] module: Move is_arm_mapping_symbol() to module_symbol.h

In order to avoid duplicated code, move is_arm_mapping_symbol() to
include/linux/module_symbol.h, then remove is_arm_mapping_symbol()
in the other places.

Signed-off-by: Tiezhu Yang <[email protected]>
---
include/linux/module_symbol.h | 15 +++++++++++++++
kernel/module/kallsyms.c | 14 +-------------
scripts/mod/modpost.c | 10 +---------
3 files changed, 17 insertions(+), 22 deletions(-)
create mode 100644 include/linux/module_symbol.h

diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h
new file mode 100644
index 0000000..9fa4173
--- /dev/null
+++ b/include/linux/module_symbol.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _LINUX_MODULE_SYMBOL_H
+#define _LINUX_MODULE_SYMBOL_H
+
+/* This ignores the intensely annoying "mapping symbols" found in ELF files. */
+static inline int is_arm_mapping_symbol(const char *str)
+{
+ if (str[0] == '.' && str[1] == 'L')
+ return true;
+ return str[0] == '$' &&
+ (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x')
+ && (str[2] == '\0' || str[2] == '.');
+}
+
+#endif /* _LINUX_MODULE_SYMBOL_H */
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index a9045fe..5de3207 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -6,6 +6,7 @@
*/

#include <linux/module.h>
+#include <linux/module_symbol.h>
#include <linux/kallsyms.h>
#include <linux/buildid.h>
#include <linux/bsearch.h>
@@ -243,19 +244,6 @@ void init_build_id(struct module *mod, const struct load_info *info)
}
#endif

-/*
- * This ignores the intensely annoying "mapping symbols" found
- * in ARM ELF files: $a, $t and $d.
- */
-static inline int is_arm_mapping_symbol(const char *str)
-{
- if (str[0] == '.' && str[1] == 'L')
- return true;
- return str[0] == '$' &&
- (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x')
- && (str[2] == '\0' || str[2] == '.');
-}
-
static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms, unsigned int symnum)
{
return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 79a27cc..7241db8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -22,6 +22,7 @@
#include <errno.h>
#include "modpost.h"
#include "../../include/linux/license.h"
+#include "../../include/linux/module_symbol.h"

/* Are we using CONFIG_MODVERSIONS? */
static bool modversions;
@@ -1112,15 +1113,6 @@ static int secref_whitelist(const struct sectioncheck *mismatch,
return 1;
}

-static inline int is_arm_mapping_symbol(const char *str)
-{
- if (str[0] == '.' && str[1] == 'L')
- return true;
- return str[0] == '$' &&
- (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x')
- && (str[2] == '\0' || str[2] == '.');
-}
-
/*
* If there's no name there, ignore it; likewise, ignore it if it's
* one of the magic symbols emitted used by current ARM tools.
--
2.1.0

2023-04-04 11:06:11

by Youling Tang

[permalink] [raw]

2023-04-12 07:42:50

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH 0/3] Modify is_arm_mapping_symbol() related code

Hi Luis,

Are you OK with this change?
Any comments will be much appreciated.

https://lore.kernel.org/lkml/[email protected]/

Thanks,
Tiezhu

2023-04-14 00:17:23

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH 0/3] Modify is_arm_mapping_symbol() related code

On Wed, Apr 12, 2023 at 03:37:27PM +0800, Tiezhu Yang wrote:
> Hi Luis,
>
> Are you OK with this change?
> Any comments will be much appreciated.
>
> https://lore.kernel.org/lkml/[email protected]/
>

Yes applied to modules-next and pushed, thanks!

Luis