[PATCH v2 0/2] kallsyms: Ignore $a/$d symbols in kallsyms for ARM

[PATCH v2 0/2] kallsyms: Ignore $a/$d symbols in kallsyms for ARM

[Lexi Shao]

On ARM machine, $a/$d symbols are used by the compiler to mark the beginning
of code/data part in code section. These symbols are filtered out when
linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but are left on
modules. So there are $a symbols in /proc/kallsyms whose address overlap
with the actual module symbols and can confuse tools such as perf when
resolving kernel symbols.

A sample stacktrace shown by perf script:
c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 $a+0x78 ([test_module]) // $a is shown instead of actual sym name
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

This patch set contains 2 patches to fix such problem:
The 1st patch modifies perf userspace tools to ignore $a/$d symbols from
/proc/kallsyms. So people can use new perf tool to get correct kernel symbol
on arm machines instead of updating kernel image.

The 2nd patch modifies the logic of loading modules to ignore arm mapping
symbols in the first place. Being left out in vmlinux and kernelspace API
(e.g. module_kallsyms_on_each_symbol) means these symbols are not used
anywhere, so it should be safe to remove them from module kallsyms list.

v2:
 - Add 2nd patch as discussed with James Clark, see:
   https://lore.kernel.org/all/c7dfbd17-85fd-b914-b90f-082abc64c9d1@arm.com/

Lexi Shao (2):
  perf symbol: ignore $a/$d symbols for ARM modules
  kallsyms: ignore arm mapping symbols when loading module

 kernel/module.c          | 19 +++++++++++--------
 tools/perf/util/symbol.c |  4 ++++
 2 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.12.3

[PATCH v2 1/2] perf symbol: ignore $a/$d symbols for ARM modules

[Lexi Shao]

On ARM machine, kernel symbols from modules can be resolved to $a
instead of printing the actual symbol name. Ignore symbols starting with
"$" when building kallsyms rbtree.

A sample stacktrace is shown as follows:

c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 $a+0x78 ([test_module])
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

On ARM machine, $a/$d symbols are used by the compiler to mark the
beginning of code/data part in code section. These symbols are filtered
out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
are left on modules. So there are $a symbols in /proc/kallsyms which
share the same addresses with the actual module symbols and confuses perf
when resolving symbols.

After this patch, the module symbol name is printed:

c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
bf4a66d8 test_func+0x78 ([test_module])
c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])

Signed-off-by: Lexi Shao <shaolexi@huawei.com>
Reviewed-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/symbol.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0fc9a5410739..35116aed74eb 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
 	if (!symbol_type__filter(type))
 		return 0;
 
+	/* Ignore local symbols for ARM modules */
+	if (name[0] == '$')
+		return 0;
+
 	/*
 	 * module symbols are not sorted so we add all
 	 * symbols, setting length to 0, and rely on
-- 
2.12.3

[PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module

[Lexi Shao]

Arm modules contains mapping symbols(e.g. $a $d) which are ignored in
module_kallsyms_on_each_symbol. However, these symbols are still
displayed when catting /proc/kallsyms. This confuses tools(e.g. perf)
that resolves kernel symbols with address using information from
/proc/kallsyms. See discussion in Link:
https://lore.kernel.org/all/c7dfbd17-85fd-b914-b90f-082abc64c9d1@arm.com/

Being left out in vmlinux(see scripts/kallsyms.c is_ignored_symbol) and
kernelspace API implies that these symbols are not used in any cases.
So we can ignore them in the first place by not adding them to module
kallsyms.

Signed-off-by: Lexi Shao <shaolexi@huawei.com>
---
 kernel/module.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 5c26a76e800b..b30cbbe144c7 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2662,16 +2662,22 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
 	return '?';
 }
 
-static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
-			unsigned int shnum, unsigned int pcpundx)
+static inline int is_arm_mapping_symbol(const char *str);
+static bool is_core_symbol(const Elf_Sym *src, const struct load_info *info)
 {
 	const Elf_Shdr *sec;
+	const Elf_Shdr *sechdrs = info->sechdrs;
+	unsigned int shnum = info->hdr->e_shnum;
+	unsigned int pcpundx = info->index.pcpu;
 
 	if (src->st_shndx == SHN_UNDEF
 	    || src->st_shndx >= shnum
 	    || !src->st_name)
 		return false;
 
+	if (is_arm_mapping_symbol(&info->strtab[src->st_name]))
+		return false;
+
 #ifdef CONFIG_KALLSYMS_ALL
 	if (src->st_shndx == pcpundx)
 		return true;
@@ -2714,8 +2720,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
 	/* Compute total space required for the core symbols' strtab. */
 	for (ndst = i = 0; i < nsrc; i++) {
 		if (i == 0 || is_livepatch_module(mod) ||
-		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
-				   info->index.pcpu)) {
+		    is_core_symbol(src+i, info)) {
 			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
 			ndst++;
 		}
@@ -2778,8 +2783,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
 	for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
 		mod->kallsyms->typetab[i] = elf_type(src + i, info);
 		if (i == 0 || is_livepatch_module(mod) ||
-		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
-				   info->index.pcpu)) {
+		    is_core_symbol(src+i, info)) {
 			mod->core_kallsyms.typetab[ndst] =
 			    mod->kallsyms->typetab[i];
 			dst[ndst] = src[i];
@@ -4246,8 +4250,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
 		 * We ignore unnamed symbols: they're uninformative
 		 * and inserted at a whim.
 		 */
-		if (*kallsyms_symbol_name(kallsyms, i) == '\0'
-		    || is_arm_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
+		if (*kallsyms_symbol_name(kallsyms, i) == '\0')
 			continue;
 
 		if (thisval <= addr && thisval > bestval) {
-- 
2.12.3

Re: [PATCH v2 2/2] kallsyms: ignore arm mapping symbols when loading module

[James Clark]

On 29/10/2021 07:50, Lexi Shao wrote:
> Arm modules contains mapping symbols(e.g. $a $d) which are ignored in
> module_kallsyms_on_each_symbol. However, these symbols are still
> displayed when catting /proc/kallsyms. This confuses tools(e.g. perf)
> that resolves kernel symbols with address using information from
> /proc/kallsyms. See discussion in Link:
> https://lore.kernel.org/all/c7dfbd17-85fd-b914-b90f-082abc64c9d1@arm.com/
> 
> Being left out in vmlinux(see scripts/kallsyms.c is_ignored_symbol) and
> kernelspace API implies that these symbols are not used in any cases.
> So we can ignore them in the first place by not adding them to module
> kallsyms.
> 
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>


I tested this and it has removed the $ symbols from kallsyms where I saw
them before.

Reviewed-by: James Clark <james.clark@arm.com>

> ---
>  kernel/module.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 5c26a76e800b..b30cbbe144c7 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2662,16 +2662,22 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
>  	return '?';
>  }
>  
> -static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
> -			unsigned int shnum, unsigned int pcpundx)
> +static inline int is_arm_mapping_symbol(const char *str);
> +static bool is_core_symbol(const Elf_Sym *src, const struct load_info *info)
>  {
>  	const Elf_Shdr *sec;
> +	const Elf_Shdr *sechdrs = info->sechdrs;
> +	unsigned int shnum = info->hdr->e_shnum;
> +	unsigned int pcpundx = info->index.pcpu;
>  
>  	if (src->st_shndx == SHN_UNDEF
>  	    || src->st_shndx >= shnum
>  	    || !src->st_name)
>  		return false;
>  
> +	if (is_arm_mapping_symbol(&info->strtab[src->st_name]))
> +		return false;
> +
>  #ifdef CONFIG_KALLSYMS_ALL
>  	if (src->st_shndx == pcpundx)
>  		return true;
> @@ -2714,8 +2720,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
>  	/* Compute total space required for the core symbols' strtab. */
>  	for (ndst = i = 0; i < nsrc; i++) {
>  		if (i == 0 || is_livepatch_module(mod) ||
> -		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
> -				   info->index.pcpu)) {
> +		    is_core_symbol(src+i, info)) {
>  			strtab_size += strlen(&info->strtab[src[i].st_name])+1;
>  			ndst++;
>  		}
> @@ -2778,8 +2783,7 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
>  	for (ndst = i = 0; i < mod->kallsyms->num_symtab; i++) {
>  		mod->kallsyms->typetab[i] = elf_type(src + i, info);
>  		if (i == 0 || is_livepatch_module(mod) ||
> -		    is_core_symbol(src+i, info->sechdrs, info->hdr->e_shnum,
> -				   info->index.pcpu)) {
> +		    is_core_symbol(src+i, info)) {
>  			mod->core_kallsyms.typetab[ndst] =
>  			    mod->kallsyms->typetab[i];
>  			dst[ndst] = src[i];
> @@ -4246,8 +4250,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
>  		 * We ignore unnamed symbols: they're uninformative
>  		 * and inserted at a whim.
>  		 */
> -		if (*kallsyms_symbol_name(kallsyms, i) == '\0'
> -		    || is_arm_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
> +		if (*kallsyms_symbol_name(kallsyms, i) == '\0')
>  			continue;
>  
>  		if (thisval <= addr && thisval > bestval) {
>