[PATCH 1/8] scripts/kallsyms: remove redundant code for omitting U and N

[PATCH 1/8] scripts/kallsyms: remove redundant code for omitting U and N

[Masahiro Yamada]

The symbol types 'U' and 'N' are already filtered out by the following
line in scripts/mksysmap:

    -e ' [aNUw] '

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index ea1e3d3aaa6b..8148e880f78e 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -178,10 +178,7 @@ static bool is_ignored_symbol(const char *name, char type)
 			return true;
 	}
 
-	if (type == 'U' || type == 'u')
-		return true;
-	/* exclude debugging symbols */
-	if (type == 'N' || type == 'n')
+	if (type == 'u' || type == 'n')
 		return true;
 
 	if (toupper(type) == 'A') {
-- 
2.34.1

[PATCH 2/8] scripts/mksysmap: remove comments described in nm(1)

[Masahiro Yamada]

I do not think we need to repeat what is written in 'man nm'.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mksysmap | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/scripts/mksysmap b/scripts/mksysmap
index 16a08b8ef2f8..697fc6653953 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -10,25 +10,6 @@
 #####
 # Generate System.map (actual filename passed as second argument)
 
-# $NM produces the following output:
-# f0081e80 T alloc_vfsmnt
-
-#   The second row specify the type of the symbol:
-#   A = Absolute
-#   B = Uninitialised data (.bss)
-#   C = Common symbol
-#   D = Initialised data
-#   G = Initialised data for small objects
-#   I = Indirect reference to another symbol
-#   N = Debugging symbol
-#   R = Read only
-#   S = Uninitialised data for small objects
-#   T = Text code symbol
-#   U = Undefined symbol
-#   V = Weak symbol
-#   W = Weak symbol
-#   Corresponding small letters are local symbols
-
 # For System.map filter away:
 #   a - local absolute symbols
 #   U - undefined global symbols
-- 
2.34.1

Re: [PATCH 2/8] scripts/mksysmap: remove comments described in nm(1)

[Nick Desaulniers]

On Wed, Mar 8, 2023 at 3:52 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I do not think we need to repeat what is written in 'man nm'.

I agree, but the paragraphs following the one you removed refer to a,
U, N, w, T, t, and W now with no context that those are the nm
formats.

I'm ok with this patch, but please consider adding a sentence along
the lines of:

The following refers to the symbol type as per nm(1).

>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/mksysmap | 19 -------------------
>  1 file changed, 19 deletions(-)
>
> diff --git a/scripts/mksysmap b/scripts/mksysmap
> index 16a08b8ef2f8..697fc6653953 100755
> --- a/scripts/mksysmap
> +++ b/scripts/mksysmap
> @@ -10,25 +10,6 @@
>  #####
>  # Generate System.map (actual filename passed as second argument)
>
> -# $NM produces the following output:
> -# f0081e80 T alloc_vfsmnt
> -
> -#   The second row specify the type of the symbol:
> -#   A = Absolute
> -#   B = Uninitialised data (.bss)
> -#   C = Common symbol
> -#   D = Initialised data
> -#   G = Initialised data for small objects
> -#   I = Indirect reference to another symbol
> -#   N = Debugging symbol
> -#   R = Read only
> -#   S = Uninitialised data for small objects
> -#   T = Text code symbol
> -#   U = Undefined symbol
> -#   V = Weak symbol
> -#   W = Weak symbol
> -#   Corresponding small letters are local symbols
> -
>  # For System.map filter away:
>  #   a - local absolute symbols
>  #   U - undefined global symbols
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 2/8] scripts/mksysmap: remove comments described in nm(1)

[Masahiro Yamada]

On Sat, Apr 8, 2023 at 3:46 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Mar 8, 2023 at 3:52 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > I do not think we need to repeat what is written in 'man nm'.
>
> I agree, but the paragraphs following the one you removed refer to a,
> U, N, w, T, t, and W now with no context that those are the nm
> formats.
>
> I'm ok with this patch, but please consider adding a sentence along
> the lines of:
>
> The following refers to the symbol type as per nm(1).


OK, I will fold this comment locally.

Thanks.




--
Best Regards
Masahiro Yamada

[PATCH 3/8] scripts/mksysmap: use sed with in-line comments

[Masahiro Yamada]

Move comments close to the code.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mksysmap | 61 +++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/scripts/mksysmap b/scripts/mksysmap
index 697fc6653953..8ea1955e03c6 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -10,32 +10,45 @@
 #####
 # Generate System.map (actual filename passed as second argument)
 
-# For System.map filter away:
-#   a - local absolute symbols
-#   U - undefined global symbols
-#   N - debugging symbols
-#   w - local weak symbols
-
 # readprofile starts reading symbols when _stext is found, and
 # continue until it finds a symbol which is not either of 'T', 't',
 # 'W' or 'w'.
 #
-# Ignored prefixes:
-#  $                    - local symbols for ARM, MIPS, etc.
-#  .L                   - local labels, .LBB,.Ltmpxxx,.L__unnamed_xx,.LASANPC, etc.
-#  __crc_               - modversions
-#  __kstrtab_           - EXPORT_SYMBOL (symbol name)
-#  __kstrtabns_         - EXPORT_SYMBOL (namespace)
+
+${NM} -n ${1} | sed >${2} -e "
+# ---------------------------------------------------------------------------
+# Ignored symbol types
 #
-# Ignored symbols:
-#  L0                   - for LoongArch?
-
-$NM -n $1 | grep -v		\
-	-e ' [aNUw] '		\
-	-e ' \$'		\
-	-e ' \.L'		\
-	-e ' __crc_'		\
-	-e ' __kstrtab_'	\
-	-e ' __kstrtabns_'	\
-	-e ' L0$'		\
-> $2
+
+# a: local absolute symbols
+# N: debugging symbols
+# U: undefined global symbols
+# w: local weak symbols
+/ [aNUw] /d
+
+# ---------------------------------------------------------------------------
+# Ignored prefixes
+#  (do not forget a space before each pattern)
+
+# local symbols for ARM, MIPS, etc.
+/ \$/d
+
+# local labels, .LBB, .Ltmpxxx, .L__unnamed_xx, .LASANPC, etc.
+/ \.L/d
+
+# CRC from modversions
+/ __crc_/d
+
+# EXPORT_SYMBOL (symbol name)
+/ __kstrtab_/d
+
+# EXPORT_SYMBOL (namespace)
+/ __kstrtabns_/d
+
+# ---------------------------------------------------------------------------
+# Ignored symbols (exact match)
+#  (do not forget a space before and '$' after each pattern)
+
+# for LoongArch?
+/ L0$/d
+"
-- 
2.34.1

Re: [PATCH 3/8] scripts/mksysmap: use sed with in-line comments

[Nick Desaulniers]

On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Move comments close to the code.

Consider adding to the commit message why you switch from grep to sed;
that's currently unclear.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Orthogonal to this patch, don't .L prefixed local symbols not have
entries in the symbol table? If they're not printed with nm, why
filter them out (since they're impossible).

>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/mksysmap | 61 +++++++++++++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 24 deletions(-)
>
> diff --git a/scripts/mksysmap b/scripts/mksysmap
> index 697fc6653953..8ea1955e03c6 100755
> --- a/scripts/mksysmap
> +++ b/scripts/mksysmap
> @@ -10,32 +10,45 @@
>  #####
>  # Generate System.map (actual filename passed as second argument)
>
> -# For System.map filter away:
> -#   a - local absolute symbols
> -#   U - undefined global symbols
> -#   N - debugging symbols
> -#   w - local weak symbols
> -
>  # readprofile starts reading symbols when _stext is found, and
>  # continue until it finds a symbol which is not either of 'T', 't',
>  # 'W' or 'w'.
>  #
> -# Ignored prefixes:
> -#  $                    - local symbols for ARM, MIPS, etc.
> -#  .L                   - local labels, .LBB,.Ltmpxxx,.L__unnamed_xx,.LASANPC, etc.
> -#  __crc_               - modversions
> -#  __kstrtab_           - EXPORT_SYMBOL (symbol name)
> -#  __kstrtabns_         - EXPORT_SYMBOL (namespace)
> +
> +${NM} -n ${1} | sed >${2} -e "
> +# ---------------------------------------------------------------------------
> +# Ignored symbol types
>  #
> -# Ignored symbols:
> -#  L0                   - for LoongArch?
> -
> -$NM -n $1 | grep -v            \
> -       -e ' [aNUw] '           \
> -       -e ' \$'                \
> -       -e ' \.L'               \
> -       -e ' __crc_'            \
> -       -e ' __kstrtab_'        \
> -       -e ' __kstrtabns_'      \
> -       -e ' L0$'               \
> -> $2
> +
> +# a: local absolute symbols
> +# N: debugging symbols
> +# U: undefined global symbols
> +# w: local weak symbols
> +/ [aNUw] /d
> +
> +# ---------------------------------------------------------------------------
> +# Ignored prefixes
> +#  (do not forget a space before each pattern)
> +
> +# local symbols for ARM, MIPS, etc.
> +/ \$/d
> +
> +# local labels, .LBB, .Ltmpxxx, .L__unnamed_xx, .LASANPC, etc.
> +/ \.L/d
> +
> +# CRC from modversions
> +/ __crc_/d
> +
> +# EXPORT_SYMBOL (symbol name)
> +/ __kstrtab_/d
> +
> +# EXPORT_SYMBOL (namespace)
> +/ __kstrtabns_/d
> +
> +# ---------------------------------------------------------------------------
> +# Ignored symbols (exact match)
> +#  (do not forget a space before and '$' after each pattern)
> +
> +# for LoongArch?
> +/ L0$/d
> +"
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/8] scripts/mksysmap: use sed with in-line comments

[Nick Desaulniers]

On Fri, Apr 7, 2023 at 11:59 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Move comments close to the code.
>
> Consider adding to the commit message why you switch from grep to sed;
> that's currently unclear.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Also, perhaps scripts/mksysmap could just be replaced with a sed
input-file? Then
scripts/link-vmlinux.sh would invoke nm and pipe it into that sed script?

>
> Orthogonal to this patch, don't .L prefixed local symbols not have
> entries in the symbol table? If they're not printed with nm, why
> filter them out (since they're impossible).
>
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/mksysmap | 61 +++++++++++++++++++++++++++++-------------------
> >  1 file changed, 37 insertions(+), 24 deletions(-)
> >
> > diff --git a/scripts/mksysmap b/scripts/mksysmap
> > index 697fc6653953..8ea1955e03c6 100755
> > --- a/scripts/mksysmap
> > +++ b/scripts/mksysmap
> > @@ -10,32 +10,45 @@
> >  #####
> >  # Generate System.map (actual filename passed as second argument)
> >
> > -# For System.map filter away:
> > -#   a - local absolute symbols
> > -#   U - undefined global symbols
> > -#   N - debugging symbols
> > -#   w - local weak symbols
> > -
> >  # readprofile starts reading symbols when _stext is found, and
> >  # continue until it finds a symbol which is not either of 'T', 't',
> >  # 'W' or 'w'.
> >  #
> > -# Ignored prefixes:
> > -#  $                    - local symbols for ARM, MIPS, etc.
> > -#  .L                   - local labels, .LBB,.Ltmpxxx,.L__unnamed_xx,.LASANPC, etc.
> > -#  __crc_               - modversions
> > -#  __kstrtab_           - EXPORT_SYMBOL (symbol name)
> > -#  __kstrtabns_         - EXPORT_SYMBOL (namespace)
> > +
> > +${NM} -n ${1} | sed >${2} -e "
> > +# ---------------------------------------------------------------------------
> > +# Ignored symbol types
> >  #
> > -# Ignored symbols:
> > -#  L0                   - for LoongArch?
> > -
> > -$NM -n $1 | grep -v            \
> > -       -e ' [aNUw] '           \
> > -       -e ' \$'                \
> > -       -e ' \.L'               \
> > -       -e ' __crc_'            \
> > -       -e ' __kstrtab_'        \
> > -       -e ' __kstrtabns_'      \
> > -       -e ' L0$'               \
> > -> $2
> > +
> > +# a: local absolute symbols
> > +# N: debugging symbols
> > +# U: undefined global symbols
> > +# w: local weak symbols
> > +/ [aNUw] /d
> > +
> > +# ---------------------------------------------------------------------------
> > +# Ignored prefixes
> > +#  (do not forget a space before each pattern)
> > +
> > +# local symbols for ARM, MIPS, etc.
> > +/ \$/d
> > +
> > +# local labels, .LBB, .Ltmpxxx, .L__unnamed_xx, .LASANPC, etc.
> > +/ \.L/d
> > +
> > +# CRC from modversions
> > +/ __crc_/d
> > +
> > +# EXPORT_SYMBOL (symbol name)
> > +/ __kstrtab_/d
> > +
> > +# EXPORT_SYMBOL (namespace)
> > +/ __kstrtabns_/d
> > +
> > +# ---------------------------------------------------------------------------
> > +# Ignored symbols (exact match)
> > +#  (do not forget a space before and '$' after each pattern)
> > +
> > +# for LoongArch?
> > +/ L0$/d
> > +"
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/8] scripts/mksysmap: use sed with in-line comments

[Masahiro Yamada]

On Sat, Apr 8, 2023 at 4:01 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Fri, Apr 7, 2023 at 11:59 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > Move comments close to the code.
> >
> > Consider adding to the commit message why you switch from grep to sed;
> > that's currently unclear.
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Also, perhaps scripts/mksysmap could just be replaced with a sed
> input-file? Then
> scripts/link-vmlinux.sh would invoke nm and pipe it into that sed script?



I still need shell.


The last line of the next commit 4/8 uses
shell code.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/8] scripts/mksysmap: use sed with in-line comments

[Masahiro Yamada]

On Sat, Apr 8, 2023 at 4:00 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Move comments close to the code.
>
> Consider adding to the commit message why you switch from grep to sed;
> that's currently unclear.



I just thought "Move comments close to the code"
explained my motivation.,


I want to insert in-line comments.
Something like the following.
Apparently, it does not work.


$NM -n $1 | grep -v             \
         # comment1
        -e ' [aNUw] '           \
         # comment2
        -e ' \$'                \
         # comment3
        -e ' \.L'               \
         # comment4
        -e ' __crc_'            \
         # comment5
        -e ' __kstrtab_'        \
         # comment6
        -e ' __kstrtabns_'      \
         # comment7
        -e ' L0$'               \
> $2






> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Orthogonal to this patch, don't .L prefixed local symbols not have
> entries in the symbol table? If they're not printed with nm, why
> filter them out (since they're impossible).


Sorry, I could not understand your question, but
you may get something from d4c858643263cfde13f7d937eaff95c2ed87cdf1
(you reviewed it)




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/8] scripts/mksysmap: use sed with in-line comments

[Masahiro Yamada]

On Sat, Apr 8, 2023 at 11:29 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sat, Apr 8, 2023 at 4:00 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > Move comments close to the code.
> >
> > Consider adding to the commit message why you switch from grep to sed;
> > that's currently unclear.
>
>
>
> I just thought "Move comments close to the code"
> explained my motivation.,
>
>
> I want to insert in-line comments.
> Something like the following.
> Apparently, it does not work.
>
>
> $NM -n $1 | grep -v             \
>          # comment1
>         -e ' [aNUw] '           \
>          # comment2
>         -e ' \$'                \
>          # comment3
>         -e ' \.L'               \
>          # comment4
>         -e ' __crc_'            \
>          # comment5
>         -e ' __kstrtab_'        \
>          # comment6
>         -e ' __kstrtabns_'      \
>          # comment7
>         -e ' L0$'               \
> > $2
>
>
>

Anyway, I will rephase the commit description.

"""
It is not feasible to insert comments in a multi-line shell command.
Use sed, and move comments close to the code.
"""



--
Best Regards
Masahiro Yamada

[PATCH 4/8] scripts/kallsyms: exclude symbols generated by itself dynamically

[Masahiro Yamada]

Drop the symbols generated by scripts/kallsyms itself automatically
instead of maintaining the symbol list manually.

Pass the kallsyms object from the previous kallsyms step (if it exists)
as the third parameter of scripts/mksysmap, which will weed out the
generated symbols from the input to the next kallsyms step.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c      | 17 -----------------
 scripts/link-vmlinux.sh |  6 +++---
 scripts/mksysmap        | 11 ++++++++++-
 3 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 8148e880f78e..e572fda6fe42 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -104,23 +104,6 @@ static bool is_ignored_symbol(const char *name, char type)
 {
 	/* Symbol names that exactly match to the following are ignored.*/
 	static const char * const ignored_symbols[] = {
-		/*
-		 * Symbols which vary between passes. Passes 1 and 2 must have
-		 * identical symbol lists. The kallsyms_* symbols below are
-		 * only added after pass 1, they would be included in pass 2
-		 * when --all-symbols is specified so exclude them to get a
-		 * stable symbol list.
-		 */
-		"kallsyms_addresses",
-		"kallsyms_offsets",
-		"kallsyms_relative_base",
-		"kallsyms_num_syms",
-		"kallsyms_names",
-		"kallsyms_markers",
-		"kallsyms_token_table",
-		"kallsyms_token_index",
-		"kallsyms_seqs_of_names",
-		/* Exclude linker generated symbols which vary between passes */
 		"_SDA_BASE_",		/* ppc */
 		"_SDA2_BASE_",		/* ppc */
 		NULL
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 32e573943cf0..679eb4653b16 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -174,7 +174,7 @@ kallsyms_step()
 	kallsyms_S=${kallsyms_vmlinux}.S
 
 	vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o}
-	mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms
+	mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms ${kallsymso_prev}
 	kallsyms ${kallsyms_vmlinux}.syms ${kallsyms_S}
 
 	info AS ${kallsyms_S}
@@ -188,7 +188,7 @@ kallsyms_step()
 mksysmap()
 {
 	info NM ${2}
-	${CONFIG_SHELL} "${srctree}/scripts/mksysmap" ${1} ${2}
+	${CONFIG_SHELL} "${srctree}/scripts/mksysmap" ${1} ${2} ${3}
 }
 
 sorttable()
@@ -277,7 +277,7 @@ if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then
 	${RESOLVE_BTFIDS} vmlinux
 fi
 
-mksysmap vmlinux System.map
+mksysmap vmlinux System.map ${kallsymso}
 
 if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then
 	info SORTTAB vmlinux
diff --git a/scripts/mksysmap b/scripts/mksysmap
index 8ea1955e03c6..1efd61ee0bac 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -4,7 +4,7 @@
 # tools to retrieve the actual addresses of symbols in the kernel.
 #
 # Usage
-# mksysmap vmlinux System.map
+# mksysmap vmlinux System.map [exclude]
 
 
 #####
@@ -51,4 +51,13 @@ ${NM} -n ${1} | sed >${2} -e "
 
 # for LoongArch?
 / L0$/d
+
+# ---------------------------------------------------------------------------
+# Ignored kallsyms symbols
+#
+# If the 3rd parameter exists, symbols from it will be omitted from the output.
+# This makes kallsyms have the identical symbol lists in the step 1 and 2.
+# Without this, the step2 would get new symbols generated by scripts/kallsyms.c
+# when CONFIG_KALLSYMS_ALL is enabled. That might require one more pass.
+$(if [ $# -ge 3 ]; then ${NM} ${3} | sed -n '/ U /!s:.* \([^ ]*\)$:/ \1$/d:p'; fi)
 "
-- 
2.34.1

Re: [PATCH 4/8] scripts/kallsyms: exclude symbols generated by itself dynamically

[Nick Desaulniers]

On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Drop the symbols generated by scripts/kallsyms itself automatically
> instead of maintaining the symbol list manually.
>
> Pass the kallsyms object from the previous kallsyms step (if it exists)
> as the third parameter of scripts/mksysmap, which will weed out the
> generated symbols from the input to the next kallsyms step.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/kallsyms.c      | 17 -----------------
>  scripts/link-vmlinux.sh |  6 +++---
>  scripts/mksysmap        | 11 ++++++++++-
>  3 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 8148e880f78e..e572fda6fe42 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -104,23 +104,6 @@ static bool is_ignored_symbol(const char *name, char type)
>  {
>         /* Symbol names that exactly match to the following are ignored.*/
>         static const char * const ignored_symbols[] = {
> -               /*
> -                * Symbols which vary between passes. Passes 1 and 2 must have
> -                * identical symbol lists. The kallsyms_* symbols below are
> -                * only added after pass 1, they would be included in pass 2
> -                * when --all-symbols is specified so exclude them to get a
> -                * stable symbol list.
> -                */
> -               "kallsyms_addresses",
> -               "kallsyms_offsets",
> -               "kallsyms_relative_base",
> -               "kallsyms_num_syms",
> -               "kallsyms_names",
> -               "kallsyms_markers",
> -               "kallsyms_token_table",
> -               "kallsyms_token_index",
> -               "kallsyms_seqs_of_names",
> -               /* Exclude linker generated symbols which vary between passes */

^ Do we want to retain this comment for the below two symbols?

>                 "_SDA_BASE_",           /* ppc */
>                 "_SDA2_BASE_",          /* ppc */
>                 NULL
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 32e573943cf0..679eb4653b16 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -174,7 +174,7 @@ kallsyms_step()
>         kallsyms_S=${kallsyms_vmlinux}.S
>
>         vmlinux_link ${kallsyms_vmlinux} "${kallsymso_prev}" ${btf_vmlinux_bin_o}
> -       mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms
> +       mksysmap ${kallsyms_vmlinux} ${kallsyms_vmlinux}.syms ${kallsymso_prev}
>         kallsyms ${kallsyms_vmlinux}.syms ${kallsyms_S}
>
>         info AS ${kallsyms_S}
> @@ -188,7 +188,7 @@ kallsyms_step()
>  mksysmap()
>  {
>         info NM ${2}
> -       ${CONFIG_SHELL} "${srctree}/scripts/mksysmap" ${1} ${2}
> +       ${CONFIG_SHELL} "${srctree}/scripts/mksysmap" ${1} ${2} ${3}
>  }
>
>  sorttable()
> @@ -277,7 +277,7 @@ if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then
>         ${RESOLVE_BTFIDS} vmlinux
>  fi
>
> -mksysmap vmlinux System.map
> +mksysmap vmlinux System.map ${kallsymso}
>
>  if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then
>         info SORTTAB vmlinux
> diff --git a/scripts/mksysmap b/scripts/mksysmap
> index 8ea1955e03c6..1efd61ee0bac 100755
> --- a/scripts/mksysmap
> +++ b/scripts/mksysmap
> @@ -4,7 +4,7 @@
>  # tools to retrieve the actual addresses of symbols in the kernel.
>  #
>  # Usage
> -# mksysmap vmlinux System.map
> +# mksysmap vmlinux System.map [exclude]
>
>
>  #####
> @@ -51,4 +51,13 @@ ${NM} -n ${1} | sed >${2} -e "
>
>  # for LoongArch?
>  / L0$/d
> +
> +# ---------------------------------------------------------------------------
> +# Ignored kallsyms symbols
> +#
> +# If the 3rd parameter exists, symbols from it will be omitted from the output.
> +# This makes kallsyms have the identical symbol lists in the step 1 and 2.
> +# Without this, the step2 would get new symbols generated by scripts/kallsyms.c
> +# when CONFIG_KALLSYMS_ALL is enabled. That might require one more pass.
> +$(if [ $# -ge 3 ]; then ${NM} ${3} | sed -n '/ U /!s:.* \([^ ]*\)$:/ \1$/d:p'; fi)
>  "
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 4/8] scripts/kallsyms: exclude symbols generated by itself dynamically

[Masahiro Yamada]

On Sat, Apr 8, 2023 at 5:13 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Drop the symbols generated by scripts/kallsyms itself automatically
> > instead of maintaining the symbol list manually.
> >
> > Pass the kallsyms object from the previous kallsyms step (if it exists)
> > as the third parameter of scripts/mksysmap, which will weed out the
> > generated symbols from the input to the next kallsyms step.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/kallsyms.c      | 17 -----------------
> >  scripts/link-vmlinux.sh |  6 +++---
> >  scripts/mksysmap        | 11 ++++++++++-
> >  3 files changed, 13 insertions(+), 21 deletions(-)
> >
> > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> > index 8148e880f78e..e572fda6fe42 100644
> > --- a/scripts/kallsyms.c
> > +++ b/scripts/kallsyms.c
> > @@ -104,23 +104,6 @@ static bool is_ignored_symbol(const char *name, char type)
> >  {
> >         /* Symbol names that exactly match to the following are ignored.*/
> >         static const char * const ignored_symbols[] = {
> > -               /*
> > -                * Symbols which vary between passes. Passes 1 and 2 must have
> > -                * identical symbol lists. The kallsyms_* symbols below are
> > -                * only added after pass 1, they would be included in pass 2
> > -                * when --all-symbols is specified so exclude them to get a
> > -                * stable symbol list.
> > -                */
> > -               "kallsyms_addresses",
> > -               "kallsyms_offsets",
> > -               "kallsyms_relative_base",
> > -               "kallsyms_num_syms",
> > -               "kallsyms_names",
> > -               "kallsyms_markers",
> > -               "kallsyms_token_table",
> > -               "kallsyms_token_index",
> > -               "kallsyms_seqs_of_names",
> > -               /* Exclude linker generated symbols which vary between passes */
>
> ^ Do we want to retain this comment for the below two symbols?



Correct.

I will keep the comment, but it will be removed
in the next commit, 5/8 because all the ignored patterns
will go away anyway.

Thanks.




>
> >                 "_SDA_BASE_",           /* ppc */
> >                 "_SDA2_BASE_",          /* ppc */
> >                 NULL



--
Best Regards
Masahiro Yamada

[PATCH 5/8] scripts/kallsyms: move compiler-generated symbol patterns to mksysmap

[Masahiro Yamada]

scripts/kallsyms.c maintains compiler-generated symbols, but we end up
with something similar in scripts/mksysmap to avoid the "Inconsistent
kallsyms data" error. For example, commit c17a2538704f ("mksysmap: Fix
the mismatch of 'L0' symbols in System.map").

They were separately maintained prior to commit 94ff2f63d6a3 ("kbuild:
reuse mksysmap output for kallsyms").

Now that scripts/kallsyms.c parses the output of scripts/mksysmap,
it makes more sense to collect all the ignored patterns to mksysmap.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c | 59 ----------------------------------------------
 scripts/mksysmap   | 43 +++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 59 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index e572fda6fe42..97d514c0fc8f 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -102,65 +102,6 @@ static char *sym_name(const struct sym_entry *s)
 
 static bool is_ignored_symbol(const char *name, char type)
 {
-	/* Symbol names that exactly match to the following are ignored.*/
-	static const char * const ignored_symbols[] = {
-		"_SDA_BASE_",		/* ppc */
-		"_SDA2_BASE_",		/* ppc */
-		NULL
-	};
-
-	/* Symbol names that begin with the following are ignored.*/
-	static const char * const ignored_prefixes[] = {
-		"__efistub_",		/* arm64 EFI stub namespace */
-		"__kvm_nvhe_$",		/* arm64 local symbols in non-VHE KVM namespace */
-		"__kvm_nvhe_.L",	/* arm64 local symbols in non-VHE KVM namespace */
-		"__AArch64ADRPThunk_",	/* arm64 lld */
-		"__ARMV5PILongThunk_",	/* arm lld */
-		"__ARMV7PILongThunk_",
-		"__ThumbV7PILongThunk_",
-		"__LA25Thunk_",		/* mips lld */
-		"__microLA25Thunk_",
-		"__kcfi_typeid_",	/* CFI type identifiers */
-		NULL
-	};
-
-	/* Symbol names that end with the following are ignored.*/
-	static const char * const ignored_suffixes[] = {
-		"_from_arm",		/* arm */
-		"_from_thumb",		/* arm */
-		"_veneer",		/* arm */
-		NULL
-	};
-
-	/* Symbol names that contain the following are ignored.*/
-	static const char * const ignored_matches[] = {
-		".long_branch.",	/* ppc stub */
-		".plt_branch.",		/* ppc stub */
-		NULL
-	};
-
-	const char * const *p;
-
-	for (p = ignored_symbols; *p; p++)
-		if (!strcmp(name, *p))
-			return true;
-
-	for (p = ignored_prefixes; *p; p++)
-		if (!strncmp(name, *p, strlen(*p)))
-			return true;
-
-	for (p = ignored_suffixes; *p; p++) {
-		int l = strlen(name) - strlen(*p);
-
-		if (l >= 0 && !strcmp(name + l, *p))
-			return true;
-	}
-
-	for (p = ignored_matches; *p; p++) {
-		if (strstr(name, *p))
-			return true;
-	}
-
 	if (type == 'u' || type == 'n')
 		return true;
 
diff --git a/scripts/mksysmap b/scripts/mksysmap
index 1efd61ee0bac..d8ad6ff69320 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -36,6 +36,28 @@ ${NM} -n ${1} | sed >${2} -e "
 # local labels, .LBB, .Ltmpxxx, .L__unnamed_xx, .LASANPC, etc.
 / \.L/d
 
+# arm64 EFI stub namespace
+/ __efistub_/d
+
+# arm64 local symbols in non-VHE KVM namespace
+/ __kvm_nvhe_\$/d
+/ __kvm_nvhe_\.L/d
+
+# arm64 lld
+/ __AArch64ADRPThunk_/d
+
+# arm lld
+/ __ARMV5PILongThunk_/d
+/ __ARMV7PILongThunk_/d
+/ __ThumbV7PILongThunk_/d
+
+# mips lld
+/ __LA25Thunk_/d
+/ __microLA25Thunk_/d
+
+# CFI type identifiers
+/ __kcfi_typeid_/d
+
 # CRC from modversions
 / __crc_/d
 
@@ -45,6 +67,15 @@ ${NM} -n ${1} | sed >${2} -e "
 # EXPORT_SYMBOL (namespace)
 / __kstrtabns_/d
 
+# ---------------------------------------------------------------------------
+# Ignored suffixes
+#  (do not forget '$' after each pattern)
+
+# arm
+/_from_arm$/d
+/_from_thumb$/d
+/_veneer$/d
+
 # ---------------------------------------------------------------------------
 # Ignored symbols (exact match)
 #  (do not forget a space before and '$' after each pattern)
@@ -52,6 +83,18 @@ ${NM} -n ${1} | sed >${2} -e "
 # for LoongArch?
 / L0$/d
 
+# ppc
+/ _SDA_BASE_$/d
+/ _SDA2_BASE_$/d
+
+# ---------------------------------------------------------------------------
+# Ignored patterns
+#  (symbols that contain the pattern are ignored)
+
+# ppc stub
+/\.long_branch\./d
+/\.plt_branch\./d
+
 # ---------------------------------------------------------------------------
 # Ignored kallsyms symbols
 #
-- 
2.34.1

Re: [PATCH 5/8] scripts/kallsyms: move compiler-generated symbol patterns to mksysmap

[Nick Desaulniers]

On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> scripts/kallsyms.c maintains compiler-generated symbols, but we end up
> with something similar in scripts/mksysmap to avoid the "Inconsistent
> kallsyms data" error. For example, commit c17a2538704f ("mksysmap: Fix
> the mismatch of 'L0' symbols in System.map").
>
> They were separately maintained prior to commit 94ff2f63d6a3 ("kbuild:
> reuse mksysmap output for kallsyms").
>
> Now that scripts/kallsyms.c parses the output of scripts/mksysmap,
> it makes more sense to collect all the ignored patterns to mksysmap.

Cool, thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/kallsyms.c | 59 ----------------------------------------------
>  scripts/mksysmap   | 43 +++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 59 deletions(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index e572fda6fe42..97d514c0fc8f 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -102,65 +102,6 @@ static char *sym_name(const struct sym_entry *s)
>
>  static bool is_ignored_symbol(const char *name, char type)
>  {
> -       /* Symbol names that exactly match to the following are ignored.*/
> -       static const char * const ignored_symbols[] = {
> -               "_SDA_BASE_",           /* ppc */
> -               "_SDA2_BASE_",          /* ppc */
> -               NULL
> -       };
> -
> -       /* Symbol names that begin with the following are ignored.*/
> -       static const char * const ignored_prefixes[] = {
> -               "__efistub_",           /* arm64 EFI stub namespace */
> -               "__kvm_nvhe_$",         /* arm64 local symbols in non-VHE KVM namespace */
> -               "__kvm_nvhe_.L",        /* arm64 local symbols in non-VHE KVM namespace */
> -               "__AArch64ADRPThunk_",  /* arm64 lld */
> -               "__ARMV5PILongThunk_",  /* arm lld */
> -               "__ARMV7PILongThunk_",
> -               "__ThumbV7PILongThunk_",
> -               "__LA25Thunk_",         /* mips lld */
> -               "__microLA25Thunk_",
> -               "__kcfi_typeid_",       /* CFI type identifiers */
> -               NULL
> -       };
> -
> -       /* Symbol names that end with the following are ignored.*/
> -       static const char * const ignored_suffixes[] = {
> -               "_from_arm",            /* arm */
> -               "_from_thumb",          /* arm */
> -               "_veneer",              /* arm */
> -               NULL
> -       };
> -
> -       /* Symbol names that contain the following are ignored.*/
> -       static const char * const ignored_matches[] = {
> -               ".long_branch.",        /* ppc stub */
> -               ".plt_branch.",         /* ppc stub */
> -               NULL
> -       };
> -
> -       const char * const *p;
> -
> -       for (p = ignored_symbols; *p; p++)
> -               if (!strcmp(name, *p))
> -                       return true;
> -
> -       for (p = ignored_prefixes; *p; p++)
> -               if (!strncmp(name, *p, strlen(*p)))
> -                       return true;
> -
> -       for (p = ignored_suffixes; *p; p++) {
> -               int l = strlen(name) - strlen(*p);
> -
> -               if (l >= 0 && !strcmp(name + l, *p))
> -                       return true;
> -       }
> -
> -       for (p = ignored_matches; *p; p++) {
> -               if (strstr(name, *p))
> -                       return true;
> -       }
> -
>         if (type == 'u' || type == 'n')
>                 return true;
>
> diff --git a/scripts/mksysmap b/scripts/mksysmap
> index 1efd61ee0bac..d8ad6ff69320 100755
> --- a/scripts/mksysmap
> +++ b/scripts/mksysmap
> @@ -36,6 +36,28 @@ ${NM} -n ${1} | sed >${2} -e "
>  # local labels, .LBB, .Ltmpxxx, .L__unnamed_xx, .LASANPC, etc.
>  / \.L/d
>
> +# arm64 EFI stub namespace
> +/ __efistub_/d
> +
> +# arm64 local symbols in non-VHE KVM namespace
> +/ __kvm_nvhe_\$/d
> +/ __kvm_nvhe_\.L/d
> +
> +# arm64 lld
> +/ __AArch64ADRPThunk_/d
> +
> +# arm lld
> +/ __ARMV5PILongThunk_/d
> +/ __ARMV7PILongThunk_/d
> +/ __ThumbV7PILongThunk_/d
> +
> +# mips lld
> +/ __LA25Thunk_/d
> +/ __microLA25Thunk_/d
> +
> +# CFI type identifiers
> +/ __kcfi_typeid_/d
> +
>  # CRC from modversions
>  / __crc_/d
>
> @@ -45,6 +67,15 @@ ${NM} -n ${1} | sed >${2} -e "
>  # EXPORT_SYMBOL (namespace)
>  / __kstrtabns_/d
>
> +# ---------------------------------------------------------------------------
> +# Ignored suffixes
> +#  (do not forget '$' after each pattern)
> +
> +# arm
> +/_from_arm$/d
> +/_from_thumb$/d
> +/_veneer$/d
> +
>  # ---------------------------------------------------------------------------
>  # Ignored symbols (exact match)
>  #  (do not forget a space before and '$' after each pattern)
> @@ -52,6 +83,18 @@ ${NM} -n ${1} | sed >${2} -e "
>  # for LoongArch?
>  / L0$/d
>
> +# ppc
> +/ _SDA_BASE_$/d
> +/ _SDA2_BASE_$/d
> +
> +# ---------------------------------------------------------------------------
> +# Ignored patterns
> +#  (symbols that contain the pattern are ignored)
> +
> +# ppc stub
> +/\.long_branch\./d
> +/\.plt_branch\./d
> +
>  # ---------------------------------------------------------------------------
>  # Ignored kallsyms symbols
>  #
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

[PATCH 6/8] scripts/kallsyms: change the output order

[Masahiro Yamada]

Currently, this tool outputs symbol data in the following order.

 (1) kallsyms_addressed / kallsyms_offsets
 (2) kallsyms_relative_base
 (3) kallsyms_num_syms
 (4) kallsyms_names
 (5) kallsyms_markers
 (6) kallsyms_seq_of_names
 (7) kallsyms_token_table
 (8) kallsyms_token_index

This commit changes the order as follows:

 (1) kallsyms_num_syms
 (2) kallsyms_names
 (3) kallsyms_markers
 (4) kallsyms_token_table
 (5) kallsyms_token_index
 (6) kallsyms_addressed / kallsyms_offsets
 (7) kallsyms_relative_base
 (8) kallsyms_seq_of_names

The motivation is to decrease the number of function calls to
expand_symbol() and cleanup_symbol_name().

The compressed names are only required for writing 'kallsyms_names'.
If you do this first, we can restore the original symbol names.
You do not need to repeat the same operation over again.

The actual refactoring will happen in the next commit.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c | 118 ++++++++++++++++++++++-----------------------
 1 file changed, 59 insertions(+), 59 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 97d514c0fc8f..5996f1e61bcf 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -412,56 +412,6 @@ static void write_src(void)
 
 	printf("\t.section .rodata, \"a\"\n");
 
-	if (!base_relative)
-		output_label("kallsyms_addresses");
-	else
-		output_label("kallsyms_offsets");
-
-	for (i = 0; i < table_cnt; i++) {
-		if (base_relative) {
-			/*
-			 * Use the offset relative to the lowest value
-			 * encountered of all relative symbols, and emit
-			 * non-relocatable fixed offsets that will be fixed
-			 * up at runtime.
-			 */
-
-			long long offset;
-			int overflow;
-
-			if (!absolute_percpu) {
-				offset = table[i]->addr - relative_base;
-				overflow = (offset < 0 || offset > UINT_MAX);
-			} else if (symbol_absolute(table[i])) {
-				offset = table[i]->addr;
-				overflow = (offset < 0 || offset > INT_MAX);
-			} else {
-				offset = relative_base - table[i]->addr - 1;
-				overflow = (offset < INT_MIN || offset >= 0);
-			}
-			if (overflow) {
-				fprintf(stderr, "kallsyms failure: "
-					"%s symbol value %#llx out of range in relative mode\n",
-					symbol_absolute(table[i]) ? "absolute" : "relative",
-					table[i]->addr);
-				exit(EXIT_FAILURE);
-			}
-			expand_symbol(table[i]->sym, table[i]->len, buf);
-			printf("\t.long\t%#x	/* %s */\n", (int)offset, buf);
-		} else if (!symbol_absolute(table[i])) {
-			output_address(table[i]->addr);
-		} else {
-			printf("\tPTR\t%#llx\n", table[i]->addr);
-		}
-	}
-	printf("\n");
-
-	if (base_relative) {
-		output_label("kallsyms_relative_base");
-		output_address(relative_base);
-		printf("\n");
-	}
-
 	output_label("kallsyms_num_syms");
 	printf("\t.long\t%u\n", table_cnt);
 	printf("\n");
@@ -521,15 +471,6 @@ static void write_src(void)
 
 	free(markers);
 
-	sort_symbols_by_name();
-	output_label("kallsyms_seqs_of_names");
-	for (i = 0; i < table_cnt; i++)
-		printf("\t.byte 0x%02x, 0x%02x, 0x%02x\n",
-			(unsigned char)(table[i]->seq >> 16),
-			(unsigned char)(table[i]->seq >> 8),
-			(unsigned char)(table[i]->seq >> 0));
-	printf("\n");
-
 	output_label("kallsyms_token_table");
 	off = 0;
 	for (i = 0; i < 256; i++) {
@@ -544,6 +485,65 @@ static void write_src(void)
 	for (i = 0; i < 256; i++)
 		printf("\t.short\t%d\n", best_idx[i]);
 	printf("\n");
+
+	if (!base_relative)
+		output_label("kallsyms_addresses");
+	else
+		output_label("kallsyms_offsets");
+
+	for (i = 0; i < table_cnt; i++) {
+		if (base_relative) {
+			/*
+			 * Use the offset relative to the lowest value
+			 * encountered of all relative symbols, and emit
+			 * non-relocatable fixed offsets that will be fixed
+			 * up at runtime.
+			 */
+
+			long long offset;
+			int overflow;
+
+			if (!absolute_percpu) {
+				offset = table[i]->addr - relative_base;
+				overflow = (offset < 0 || offset > UINT_MAX);
+			} else if (symbol_absolute(table[i])) {
+				offset = table[i]->addr;
+				overflow = (offset < 0 || offset > INT_MAX);
+			} else {
+				offset = relative_base - table[i]->addr - 1;
+				overflow = (offset < INT_MIN || offset >= 0);
+			}
+			if (overflow) {
+				fprintf(stderr, "kallsyms failure: "
+					"%s symbol value %#llx out of range in relative mode\n",
+					symbol_absolute(table[i]) ? "absolute" : "relative",
+					table[i]->addr);
+				exit(EXIT_FAILURE);
+			}
+			expand_symbol(table[i]->sym, table[i]->len, buf);
+			printf("\t.long\t%#x	/* %s */\n", (int)offset, buf);
+		} else if (!symbol_absolute(table[i])) {
+			output_address(table[i]->addr);
+		} else {
+			printf("\tPTR\t%#llx\n", table[i]->addr);
+		}
+	}
+	printf("\n");
+
+	if (base_relative) {
+		output_label("kallsyms_relative_base");
+		output_address(relative_base);
+		printf("\n");
+	}
+
+	sort_symbols_by_name();
+	output_label("kallsyms_seqs_of_names");
+	for (i = 0; i < table_cnt; i++)
+		printf("\t.byte 0x%02x, 0x%02x, 0x%02x\n",
+			(unsigned char)(table[i]->seq >> 16),
+			(unsigned char)(table[i]->seq >> 8),
+			(unsigned char)(table[i]->seq >> 0));
+	printf("\n");
 }
 
 
-- 
2.34.1

[PATCH 7/8] scripts/kallsyms: decrease expand_symbol() / cleanup_symbol_name() calls

[Masahiro Yamada]

Currently, expand_symbol() is called many times to get the uncompressed
symbol names for sorting, and also for adding comments.

With the output order shuffled in the previous commit, the symbol data
are now written in the following order:

 (1) kallsyms_num_syms
 (2) kallsyms_names                         <-- need compressed names
 (3) kallsyms_markers
 (4) kallsyms_token_table
 (5) kallsyms_token_index
 (6) kallsyms_addressed / kallsyms_offsets  <-- need uncompressed names (for commenting)
 (7) kallsyms_relative_base
 (8) kallsyms_seq_of_names                  <-- need uncompressed names (for sorting)

The compressed names are only needed by (2).

Call expand_symbol() between (2) and (3) to restore the original symbol
names. This requires just one expand_symbol() call for each symbol.

Call cleanup_symbol_name() between (7) and (8) instead of during sorting.
It is allowed to overwrite the ->sym field because (8) just outputs the
index instead of the name of each symbol. Again, this requires just one
cleanup_symbol_name() call for each symbol.

This refactoring makes it ~30% faster.

[Before]

  $ time scripts/kallsyms --all-symbols --absolute-percpu --base-relative \
    .tmp_vmlinux.kallsyms2.syms >/dev/null

  real    0m1.027s
  user    0m1.010s
  sys     0m0.016s

  $ time scripts/kallsyms --all-symbols --absolute-percpu --base-relative \
    .tmp_vmlinux.kallsyms2.syms >/dev/null

  real    0m0.717s
  user    0m0.717s
  sys     0m0.000s

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 5996f1e61bcf..937900823fa8 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -335,19 +335,10 @@ static int symbol_absolute(const struct sym_entry *s)
 	return s->percpu_absolute;
 }
 
-static char * s_name(char *buf)
-{
-	/* Skip the symbol type */
-	return buf + 1;
-}
-
 static void cleanup_symbol_name(char *s)
 {
 	char *p;
 
-	if (!lto_clang)
-		return;
-
 	/*
 	 * ASCII[.]   = 2e
 	 * ASCII[0-9] = 30,39
@@ -366,16 +357,10 @@ static void cleanup_symbol_name(char *s)
 static int compare_names(const void *a, const void *b)
 {
 	int ret;
-	char sa_namebuf[KSYM_NAME_LEN];
-	char sb_namebuf[KSYM_NAME_LEN];
 	const struct sym_entry *sa = *(const struct sym_entry **)a;
 	const struct sym_entry *sb = *(const struct sym_entry **)b;
 
-	expand_symbol(sa->sym, sa->len, sa_namebuf);
-	expand_symbol(sb->sym, sb->len, sb_namebuf);
-	cleanup_symbol_name(s_name(sa_namebuf));
-	cleanup_symbol_name(s_name(sb_namebuf));
-	ret = strcmp(s_name(sa_namebuf), s_name(sb_namebuf));
+	ret = strcmp(sym_name(sa), sym_name(sb));
 	if (!ret) {
 		if (sa->addr > sb->addr)
 			return 1;
@@ -464,6 +449,15 @@ static void write_src(void)
 	}
 	printf("\n");
 
+	/*
+	 * Now that we wrote out the compressed symbol names, restore the
+	 * original names, which are needed in some of the later steps.
+	 */
+	for (i = 0; i < table_cnt; i++) {
+		expand_symbol(table[i]->sym, table[i]->len, buf);
+		strcpy((char *)table[i]->sym, buf);
+	}
+
 	output_label("kallsyms_markers");
 	for (i = 0; i < ((table_cnt + 255) >> 8); i++)
 		printf("\t.long\t%u\n", markers[i]);
@@ -520,8 +514,7 @@ static void write_src(void)
 					table[i]->addr);
 				exit(EXIT_FAILURE);
 			}
-			expand_symbol(table[i]->sym, table[i]->len, buf);
-			printf("\t.long\t%#x	/* %s */\n", (int)offset, buf);
+			printf("\t.long\t%#x	/* %s */\n", (int)offset, table[i]->sym);
 		} else if (!symbol_absolute(table[i])) {
 			output_address(table[i]->addr);
 		} else {
@@ -536,6 +529,10 @@ static void write_src(void)
 		printf("\n");
 	}
 
+	if (lto_clang)
+		for (i = 0; i < table_cnt; i++)
+			cleanup_symbol_name((char *)table[i]->sym);
+
 	sort_symbols_by_name();
 	output_label("kallsyms_seqs_of_names");
 	for (i = 0; i < table_cnt; i++)
-- 
2.34.1

[PATCH 8/8] scripts/kallsyms: update the usage in the comment block

[Masahiro Yamada]

Commit 010a0aad39fc ("kallsyms: Correctly sequence symbols when
CONFIG_LTO_CLANG=y") added --lto-clang, and updated the usage()
function, but not the comment. Update it in the same way.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kallsyms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 937900823fa8..0d2db41177b2 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -6,7 +6,7 @@
  * of the GNU General Public License, incorporated herein by reference.
  *
  * Usage: kallsyms [--all-symbols] [--absolute-percpu]
- *                         [--base-relative] in.map > out.S
+ *                         [--base-relative] [--lto-clang] in.map > out.S
  *
  *      Table compression uses all the unused char codes on the symbols and
  *  maps these to the most used substrings (tokens). For instance, it might
-- 
2.34.1

Re: [PATCH 8/8] scripts/kallsyms: update the usage in the comment block

[Nick Desaulniers]

On Wed, Mar 8, 2023 at 3:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Commit 010a0aad39fc ("kallsyms: Correctly sequence symbols when
> CONFIG_LTO_CLANG=y") added --lto-clang, and updated the usage()
> function, but not the comment. Update it in the same way.

I'm surprised to not recall that commit.  Was our mailing list cc'ed?
Looking at https://lore.kernel.org/lkml/20221102084921.1615-4-thunder.leizhen@huawei.com/,
it looks like "no."

Zhen, Luis, please make sure to use scripts/get_maintainer.pl when
sending patches.  You can put Cc: below the fold of the commit message
in the patch file even when sending a series and git send-email will
use those just for that commit.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/kallsyms.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 937900823fa8..0d2db41177b2 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -6,7 +6,7 @@
>   * of the GNU General Public License, incorporated herein by reference.
>   *
>   * Usage: kallsyms [--all-symbols] [--absolute-percpu]
> - *                         [--base-relative] in.map > out.S
> + *                         [--base-relative] [--lto-clang] in.map > out.S
>   *
>   *      Table compression uses all the unused char codes on the symbols and
>   *  maps these to the most used substrings (tokens). For instance, it might
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/8] scripts/kallsyms: remove redundant code for omitting U and N

[Nick Desaulniers]

On Wed, Mar 08, 2023 at 08:52:36PM +0900, Masahiro Yamada wrote:
> The symbol types 'U' and 'N' are already filtered out by the following
> line in scripts/mksysmap:
> 
>     -e ' [aNUw] '
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
> 
>  scripts/kallsyms.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index ea1e3d3aaa6b..8148e880f78e 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -178,10 +178,7 @@ static bool is_ignored_symbol(const char *name, char type)
>  			return true;
>  	}
>  
> -	if (type == 'U' || type == 'u')
> -		return true;
> -	/* exclude debugging symbols */
> -	if (type == 'N' || type == 'n')
> +	if (type == 'u' || type == 'n')
>  		return true;
>  
>  	if (toupper(type) == 'A') {
> -- 
> 2.34.1
> 
>