[PATCH 00/13] kbuild: refactoring after Clang LTO

[PATCH 00/13] kbuild: refactoring after Clang LTO

[Masahiro Yamada]

The introduction of Clang LTO, the kbuild code became much
uglier due to CONFIG_LTO_CLANG conditionals.

It is painful to maintain the messed-up code, and to review
code changed on top of that.



Masahiro Yamada (13):
  kbuild: move objtool_args back to scripts/Makefile.build
  gen_compile_commands: extract compiler command from a series of
    commands
  kbuild: detect objtool changes correctly and remove .SECONDEXPANSION
  kbuild: remove unused quiet_cmd_update_lto_symversions
  kbuild: remove stale *.symversions
  kbuild: merge vmlinux_link() between the ordinary link and Clang LTO
  kbuild: do not remove 'linux' link in scripts/link-vmlinux.sh
  kbuild: merge vmlinux_link() between ARCH=um and other architectures
  kbuild: do not create built-in.a.symversions or lib.a.symversions
  kbuild: build modules in the same way with/without Clang LTO
  kbuild: always postpone CRC links for module versioning
  kbuild: merge cmd_modversions_c and cmd_modversions_S
  kbuild: merge cmd_ar_builtin and cmd_ar_module

 scripts/Makefile.build                      | 196 ++++++++------------
 scripts/Makefile.lib                        |  28 +--
 scripts/Makefile.modfinal                   |   4 +-
 scripts/Makefile.modpost                    |   7 +-
 scripts/clang-tools/gen_compile_commands.py |   2 +-
 scripts/link-vmlinux.sh                     | 125 +++++++------
 scripts/mod/modpost.c                       |   6 +-
 scripts/mod/sumversion.c                    |   6 +-
 8 files changed, 164 insertions(+), 210 deletions(-)

-- 
2.30.2

[PATCH 01/13] kbuild: move objtool_args back to scripts/Makefile.build

[Masahiro Yamada]

Commit b1a1a1a09b46 ("kbuild: lto: postpone objtool") moved objtool_args
to Makefile.lib, so the arguments can be used in Makefile.modfinal as
well as Makefile.build.

With commit 2b1d7fc05467 ("kbuild: Fix TRIM_UNUSED_KSYMS with
LTO_CLANG"), module LTO linking came back to scripts/Makefile.build
again.

So, there is no more reason to keep objtool_args separately in
scripts/Makefile.lib.

Get it back to the original place, close to the objtool command.

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

 scripts/Makefile.build | 13 +++++++++++++
 scripts/Makefile.lib   | 12 ------------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index ea549579bfb7..31154e44c251 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -222,6 +222,19 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
 endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 
 ifdef CONFIG_STACK_VALIDATION
+
+# Objtool arguments are also needed for modfinal with LTO, so we define
+# then here to avoid duplication.
+objtool_args =								\
+	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
+	$(if $(part-of-module), --module,)				\
+	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
+	$(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)),		\
+		--no-unreachable,)					\
+	$(if $(CONFIG_RETPOLINE), --retpoline,)				\
+	$(if $(CONFIG_X86_SMAP), --uaccess,)				\
+	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount,)
+
 ifndef CONFIG_LTO_CLANG
 
 __objtool_obj := $(objtree)/tools/objtool/objtool
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index af1c920a585c..34c4c11c4bc1 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -232,18 +232,6 @@ ifeq ($(CONFIG_LTO_CLANG),y)
 mod-prelink-ext := .lto
 endif
 
-# Objtool arguments are also needed for modfinal with LTO, so we define
-# then here to avoid duplication.
-objtool_args =								\
-	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
-	$(if $(part-of-module), --module,)				\
-	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
-	$(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)), 		\
-		--no-unreachable,)					\
-	$(if $(CONFIG_RETPOLINE), --retpoline,)				\
-	$(if $(CONFIG_X86_SMAP), --uaccess,)				\
-	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount,)
-
 # Useful for describing the dependency of composite objects
 # Usage:
 #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
-- 
2.30.2

Re: [PATCH 01/13] kbuild: move objtool_args back to scripts/Makefile.build

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:32AM +0900, Masahiro Yamada wrote:
> Commit b1a1a1a09b46 ("kbuild: lto: postpone objtool") moved objtool_args
> to Makefile.lib, so the arguments can be used in Makefile.modfinal as
> well as Makefile.build.
> 
> With commit 2b1d7fc05467 ("kbuild: Fix TRIM_UNUSED_KSYMS with
> LTO_CLANG"), module LTO linking came back to scripts/Makefile.build
> again.
> 
> So, there is no more reason to keep objtool_args separately in
> scripts/Makefile.lib.
> 
> Get it back to the original place, close to the objtool command.

Ah, yup. Good point. Nice cleanup.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build | 13 +++++++++++++
>  scripts/Makefile.lib   | 12 ------------
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index ea549579bfb7..31154e44c251 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -222,6 +222,19 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
>  endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>  
>  ifdef CONFIG_STACK_VALIDATION
> +
> +# Objtool arguments are also needed for modfinal with LTO, so we define
> +# then here to avoid duplication.
> +objtool_args =								\
> +	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
> +	$(if $(part-of-module), --module,)				\
> +	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
> +	$(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)),		\
> +		--no-unreachable,)					\
> +	$(if $(CONFIG_RETPOLINE), --retpoline,)				\
> +	$(if $(CONFIG_X86_SMAP), --uaccess,)				\
> +	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount,)
> +
>  ifndef CONFIG_LTO_CLANG
>  
>  __objtool_obj := $(objtree)/tools/objtool/objtool
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index af1c920a585c..34c4c11c4bc1 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -232,18 +232,6 @@ ifeq ($(CONFIG_LTO_CLANG),y)
>  mod-prelink-ext := .lto
>  endif
>  
> -# Objtool arguments are also needed for modfinal with LTO, so we define
> -# then here to avoid duplication.
> -objtool_args =								\
> -	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
> -	$(if $(part-of-module), --module,)				\
> -	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
> -	$(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)), 		\
> -		--no-unreachable,)					\
> -	$(if $(CONFIG_RETPOLINE), --retpoline,)				\
> -	$(if $(CONFIG_X86_SMAP), --uaccess,)				\
> -	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount,)
> -
>  # Useful for describing the dependency of composite objects
>  # Usage:
>  #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 02/13] gen_compile_commands: extract compiler command from a series of commands

[Masahiro Yamada]

The current gen_compile_commands.py assumes that objects are always
built by a single command.

It makes sense to support cases where objects are built by a series of
commands:

  cmd_<object> := <command1> ; <command2>

One use-case is <command1> is a compiler command, and <command2> is
an objtool command.

It allows *.cmd files to contain an objtool command so that any change
in it triggers object rebuilds.

If ; appears after the C source file, take the first command.

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

 scripts/clang-tools/gen_compile_commands.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index b7e9ecf16e56..0033eedce003 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -18,7 +18,7 @@ _DEFAULT_OUTPUT = 'compile_commands.json'
 _DEFAULT_LOG_LEVEL = 'WARNING'
 
 _FILENAME_PATTERN = r'^\..*\.cmd$'
-_LINE_PATTERN = r'^cmd_[^ ]*\.o := (.* )([^ ]*\.c)$'
+_LINE_PATTERN = r'^cmd_[^ ]*\.o := (.* )([^ ]*\.c) *(;|$)'
 _VALID_LOG_LEVELS = ['DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL']
 # The tools/ directory adopts a different build system, and produces .cmd
 # files in a different format. Do not support it.
-- 
2.30.2

Re: [PATCH 02/13] gen_compile_commands: extract compiler command from a series of commands

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:33AM +0900, Masahiro Yamada wrote:
> The current gen_compile_commands.py assumes that objects are always
> built by a single command.
> 
> It makes sense to support cases where objects are built by a series of
> commands:
> 
>   cmd_<object> := <command1> ; <command2>
> 
> One use-case is <command1> is a compiler command, and <command2> is
> an objtool command.
> 
> It allows *.cmd files to contain an objtool command so that any change
> in it triggers object rebuilds.
> 
> If ; appears after the C source file, take the first command.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Seems reasonable, given patch 3.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
> 
>  scripts/clang-tools/gen_compile_commands.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index b7e9ecf16e56..0033eedce003 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -18,7 +18,7 @@ _DEFAULT_OUTPUT = 'compile_commands.json'
>  _DEFAULT_LOG_LEVEL = 'WARNING'
>  
>  _FILENAME_PATTERN = r'^\..*\.cmd$'
> -_LINE_PATTERN = r'^cmd_[^ ]*\.o := (.* )([^ ]*\.c)$'
> +_LINE_PATTERN = r'^cmd_[^ ]*\.o := (.* )([^ ]*\.c) *(;|$)'
>  _VALID_LOG_LEVELS = ['DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL']
>  # The tools/ directory adopts a different build system, and produces .cmd
>  # files in a different format. Do not support it.
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 03/13] kbuild: detect objtool changes correctly and remove .SECONDEXPANSION

[Masahiro Yamada]

This reverts commit 8852c5524029 ("kbuild: Fix objtool dependency for
'OBJECT_FILES_NON_STANDARD_<obj> := n'"), and fix the dependency in a
cleaner way.

Using .SECONDEXPANSION is expensive since Makefile.build is parsed
twice every time, and the escaping dollars makes the code unreadable.

Adding include/config/* as dependency is not maintainable either because
objtool_args is dependent on more CONFIG options.

A better fix is to include the objtool command in *.cmd files so any
command change is naturally detected by if_change.

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

 scripts/Makefile.build | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 31154e44c251..3e4cd1439cd4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -155,7 +155,7 @@ $(obj)/%.ll: $(src)/%.c FORCE
 # (See cmd_cc_o_c + relevant part of rule_cc_o_c)
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
-      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
+      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool)
 
 ifdef CONFIG_MODVERSIONS
 # When module versioning is enabled the following steps are executed:
@@ -223,6 +223,8 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 
 ifdef CONFIG_STACK_VALIDATION
 
+objtool := $(objtree)/tools/objtool/objtool
+
 # Objtool arguments are also needed for modfinal with LTO, so we define
 # then here to avoid duplication.
 objtool_args =								\
@@ -237,26 +239,19 @@ objtool_args =								\
 
 ifndef CONFIG_LTO_CLANG
 
-__objtool_obj := $(objtree)/tools/objtool/objtool
-
 # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
 # 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
 # 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file
 cmd_objtool = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	$(__objtool_obj) $(objtool_args) $@)
-objtool_obj = $(if $(patsubst y%,, \
-	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	$(__objtool_obj))
+	; $(objtool) $(objtool_args) $@)
+
+# Rebuild all objects when objtool is updated
+objtool_dep = $(objtool)
 
 endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
 
-# Rebuild all objects when objtool changes, or is enabled/disabled.
-objtool_dep = $(objtool_obj)					\
-	      $(wildcard include/config/ORC_UNWINDER		\
-			 include/config/STACK_VALIDATION)
-
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 cmd_gen_ksymdeps = \
 	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
@@ -270,7 +265,6 @@ define rule_cc_o_c
 	$(call cmd,gen_ksymdeps)
 	$(call cmd,checksrc)
 	$(call cmd,checkdoc)
-	$(call cmd,objtool)
 	$(call cmd,modversions_c)
 	$(call cmd,record_mcount)
 endef
@@ -278,13 +272,11 @@ endef
 define rule_as_o_S
 	$(call cmd_and_fixdep,as_o_S)
 	$(call cmd,gen_ksymdeps)
-	$(call cmd,objtool)
 	$(call cmd,modversions_S)
 endef
 
 # Built-in and composite module parts
-.SECONDEXPANSION:
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
 	$(call if_changed_rule,cc_o_c)
 	$(call cmd,force_checksrc)
 
@@ -367,7 +359,7 @@ $(obj)/%.s: $(src)/%.S FORCE
 	$(call if_changed_dep,cpp_s_S)
 
 quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
-      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
+      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< $(cmd_objtool)
 
 ifdef CONFIG_ASM_MODVERSIONS
 
@@ -386,7 +378,7 @@ cmd_modversions_S =								\
 	fi
 endif
 
-$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.S $(objtool_dep) FORCE
 	$(call if_changed_rule,as_o_S)
 
 targets += $(filter-out $(subdir-builtin), $(real-obj-y))
-- 
2.30.2

Re: [PATCH 03/13] kbuild: detect objtool changes correctly and remove .SECONDEXPANSION

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:34AM +0900, Masahiro Yamada wrote:
> This reverts commit 8852c5524029 ("kbuild: Fix objtool dependency for
> 'OBJECT_FILES_NON_STANDARD_<obj> := n'"), and fix the dependency in a
> cleaner way.
> 
> Using .SECONDEXPANSION is expensive since Makefile.build is parsed
> twice every time, and the escaping dollars makes the code unreadable.
> 
> Adding include/config/* as dependency is not maintainable either because
> objtool_args is dependent on more CONFIG options.
> 
> A better fix is to include the objtool command in *.cmd files so any
> command change is naturally detected by if_change.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 31154e44c251..3e4cd1439cd4 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -155,7 +155,7 @@ $(obj)/%.ll: $(src)/%.c FORCE
>  # (See cmd_cc_o_c + relevant part of rule_cc_o_c)
>  
>  quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
> -      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
> +      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool)
>  
>  ifdef CONFIG_MODVERSIONS
>  # When module versioning is enabled the following steps are executed:
> @@ -223,6 +223,8 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>  
>  ifdef CONFIG_STACK_VALIDATION
>  
> +objtool := $(objtree)/tools/objtool/objtool
> +
>  # Objtool arguments are also needed for modfinal with LTO, so we define
>  # then here to avoid duplication.
>  objtool_args =								\
> @@ -237,26 +239,19 @@ objtool_args =								\
>  
>  ifndef CONFIG_LTO_CLANG
>  
> -__objtool_obj := $(objtree)/tools/objtool/objtool
> -
>  # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
>  # 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
>  # 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file
>  cmd_objtool = $(if $(patsubst y%,, \
>  	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> -	$(__objtool_obj) $(objtool_args) $@)
> -objtool_obj = $(if $(patsubst y%,, \
> -	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> -	$(__objtool_obj))
> +	; $(objtool) $(objtool_args) $@)

This is extremely clever -- pasting commands together. :)

Does this correctly propagate failures in the first half of the command?

For example, now we'd have:
	gcc flags..... -c -o out in ; objtool...

But I think objtool will run even on gcc failure, and "make" won't see
the failure from gcc? I need to go test this.

-Kees

> +
> +# Rebuild all objects when objtool is updated
> +objtool_dep = $(objtool)
>  
>  endif # CONFIG_LTO_CLANG
>  endif # CONFIG_STACK_VALIDATION
>  
> -# Rebuild all objects when objtool changes, or is enabled/disabled.
> -objtool_dep = $(objtool_obj)					\
> -	      $(wildcard include/config/ORC_UNWINDER		\
> -			 include/config/STACK_VALIDATION)
> -
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>  cmd_gen_ksymdeps = \
>  	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
> @@ -270,7 +265,6 @@ define rule_cc_o_c
>  	$(call cmd,gen_ksymdeps)
>  	$(call cmd,checksrc)
>  	$(call cmd,checkdoc)
> -	$(call cmd,objtool)
>  	$(call cmd,modversions_c)
>  	$(call cmd,record_mcount)
>  endef
> @@ -278,13 +272,11 @@ endef
>  define rule_as_o_S
>  	$(call cmd_and_fixdep,as_o_S)
>  	$(call cmd,gen_ksymdeps)
> -	$(call cmd,objtool)
>  	$(call cmd,modversions_S)
>  endef
>  
>  # Built-in and composite module parts
> -.SECONDEXPANSION:
> -$(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE
> +$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>  	$(call if_changed_rule,cc_o_c)
>  	$(call cmd,force_checksrc)
>  
> @@ -367,7 +359,7 @@ $(obj)/%.s: $(src)/%.S FORCE
>  	$(call if_changed_dep,cpp_s_S)
>  
>  quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
> -      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
> +      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< $(cmd_objtool)
>  
>  ifdef CONFIG_ASM_MODVERSIONS
>  
> @@ -386,7 +378,7 @@ cmd_modversions_S =								\
>  	fi
>  endif
>  
> -$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
> +$(obj)/%.o: $(src)/%.S $(objtool_dep) FORCE
>  	$(call if_changed_rule,as_o_S)
>  
>  targets += $(filter-out $(subdir-builtin), $(real-obj-y))
> -- 
> 2.30.2
> 

-- 
Kees Cook

Re: [PATCH 03/13] kbuild: detect objtool changes correctly and remove .SECONDEXPANSION

[Masahiro Yamada]

On Thu, Aug 19, 2021 at 11:55 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Aug 19, 2021 at 09:57:34AM +0900, Masahiro Yamada wrote:
> > This reverts commit 8852c5524029 ("kbuild: Fix objtool dependency for
> > 'OBJECT_FILES_NON_STANDARD_<obj> := n'"), and fix the dependency in a
> > cleaner way.
> >
> > Using .SECONDEXPANSION is expensive since Makefile.build is parsed
> > twice every time, and the escaping dollars makes the code unreadable.
> >
> > Adding include/config/* as dependency is not maintainable either because
> > objtool_args is dependent on more CONFIG options.
> >
> > A better fix is to include the objtool command in *.cmd files so any
> > command change is naturally detected by if_change.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/Makefile.build | 28 ++++++++++------------------
> >  1 file changed, 10 insertions(+), 18 deletions(-)
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 31154e44c251..3e4cd1439cd4 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -155,7 +155,7 @@ $(obj)/%.ll: $(src)/%.c FORCE
> >  # (See cmd_cc_o_c + relevant part of rule_cc_o_c)
> >
> >  quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
> > -      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
> > +      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool)
> >
> >  ifdef CONFIG_MODVERSIONS
> >  # When module versioning is enabled the following steps are executed:
> > @@ -223,6 +223,8 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> >
> >  ifdef CONFIG_STACK_VALIDATION
> >
> > +objtool := $(objtree)/tools/objtool/objtool
> > +
> >  # Objtool arguments are also needed for modfinal with LTO, so we define
> >  # then here to avoid duplication.
> >  objtool_args =                                                               \
> > @@ -237,26 +239,19 @@ objtool_args =                                                          \
> >
> >  ifndef CONFIG_LTO_CLANG
> >
> > -__objtool_obj := $(objtree)/tools/objtool/objtool
> > -
> >  # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
> >  # 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
> >  # 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file
> >  cmd_objtool = $(if $(patsubst y%,, \
> >       $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> > -     $(__objtool_obj) $(objtool_args) $@)
> > -objtool_obj = $(if $(patsubst y%,, \
> > -     $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> > -     $(__objtool_obj))
> > +     ; $(objtool) $(objtool_args) $@)
>
> This is extremely clever -- pasting commands together. :)
>
> Does this correctly propagate failures in the first half of the command?


Yes.
Any failure bails out immediately because the -e option is set.


See

  cmd = @set -e; $(echo-cmd) $(cmd_$(1))

in scripts/Kbuild.include




>
> For example, now we'd have:
>         gcc flags..... -c -o out in ; objtool...
>
> But I think objtool will run even on gcc failure, and "make" won't see
> the failure from gcc? I need to go test this.
>
> -Kees
>
> > +
> > +# Rebuild all objects when objtool is updated
> > +objtool_dep = $(objtool)
> >
> >  endif # CONFIG_LTO_CLANG
> >  endif # CONFIG_STACK_VALIDATION
> >
> > -# Rebuild all objects when objtool changes, or is enabled/disabled.
> > -objtool_dep = $(objtool_obj)                                 \
> > -           $(wildcard include/config/ORC_UNWINDER            \
> > -                      include/config/STACK_VALIDATION)
> > -
> >  ifdef CONFIG_TRIM_UNUSED_KSYMS
> >  cmd_gen_ksymdeps = \
> >       $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
> > @@ -270,7 +265,6 @@ define rule_cc_o_c
> >       $(call cmd,gen_ksymdeps)
> >       $(call cmd,checksrc)
> >       $(call cmd,checkdoc)
> > -     $(call cmd,objtool)
> >       $(call cmd,modversions_c)
> >       $(call cmd,record_mcount)
> >  endef
> > @@ -278,13 +272,11 @@ endef
> >  define rule_as_o_S
> >       $(call cmd_and_fixdep,as_o_S)
> >       $(call cmd,gen_ksymdeps)
> > -     $(call cmd,objtool)
> >       $(call cmd,modversions_S)
> >  endef
> >
> >  # Built-in and composite module parts
> > -.SECONDEXPANSION:
> > -$(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE
> > +$(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
> >       $(call if_changed_rule,cc_o_c)
> >       $(call cmd,force_checksrc)
> >
> > @@ -367,7 +359,7 @@ $(obj)/%.s: $(src)/%.S FORCE
> >       $(call if_changed_dep,cpp_s_S)
> >
> >  quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
> > -      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
> > +      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< $(cmd_objtool)
> >
> >  ifdef CONFIG_ASM_MODVERSIONS
> >
> > @@ -386,7 +378,7 @@ cmd_modversions_S =                                                               \
> >       fi
> >  endif
> >
> > -$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
> > +$(obj)/%.o: $(src)/%.S $(objtool_dep) FORCE
> >       $(call if_changed_rule,as_o_S)
> >
> >  targets += $(filter-out $(subdir-builtin), $(real-obj-y))
> > --
> > 2.30.2
> >
>
> --
> Kees Cook



-- 
Best Regards
Masahiro Yamada

[PATCH 04/13] kbuild: remove unused quiet_cmd_update_lto_symversions

[Masahiro Yamada]

This is not used anywhere.

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

 scripts/Makefile.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3e4cd1439cd4..279363266455 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -411,7 +411,6 @@ $(subdir-builtin): $(obj)/%/built-in.a: $(obj)/% ;
 $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
 
 # combine symversions for later processing
-quiet_cmd_update_lto_symversions = SYMVER  $@
 ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
       cmd_update_lto_symversions =					\
 	rm -f $@.symversions						\
-- 
2.30.2

Re: [PATCH 04/13] kbuild: remove unused quiet_cmd_update_lto_symversions

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:35AM +0900, Masahiro Yamada wrote:
> This is not used anywhere.

Ah-ha, I see: cmd_update_lto_symversions is never used through a
$(call cmd, ...) invocation.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3e4cd1439cd4..279363266455 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -411,7 +411,6 @@ $(subdir-builtin): $(obj)/%/built-in.a: $(obj)/% ;
>  $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
>  
>  # combine symversions for later processing
> -quiet_cmd_update_lto_symversions = SYMVER  $@
>  ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
>        cmd_update_lto_symversions =					\
>  	rm -f $@.symversions						\
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 05/13] kbuild: remove stale *.symversions

[Masahiro Yamada]

cmd_update_lto_symversions merges all the existing *.symversions, but
some of them might be stale.

If the last EXPORT_SYMBOL is removed from a C file, the *.symversions
file is not deleted or updated. It contains stale CRCs, which will be
used for linking the vmlinux or modules.

It is not a big deal when the EXPORT_SYMBOL is really removed. However,
when the EXPORT_SYMBOL is moved to another file, the same __crc_<symbol>
will appear twice in the merged *.symversions, possibly with different
CRCs if the function argument is changed at the same time. It would
cause potential breakage of module versioning.

If no EXPORT_SYMBOL is found, let's remove *.symversions explicitly.

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

 scripts/Makefile.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 279363266455..585dae34746a 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -177,6 +177,8 @@ cmd_modversions_c =								\
 	if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then			\
 		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
 		    > $@.symversions;						\
+	else									\
+		rm -f $@.symversions;						\
 	fi;
 else
 cmd_modversions_c =								\
-- 
2.30.2

Re: [PATCH 05/13] kbuild: remove stale *.symversions

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:36AM +0900, Masahiro Yamada wrote:
> cmd_update_lto_symversions merges all the existing *.symversions, but
> some of them might be stale.
> 
> If the last EXPORT_SYMBOL is removed from a C file, the *.symversions
> file is not deleted or updated. It contains stale CRCs, which will be
> used for linking the vmlinux or modules.
> 
> It is not a big deal when the EXPORT_SYMBOL is really removed. However,
> when the EXPORT_SYMBOL is moved to another file, the same __crc_<symbol>
> will appear twice in the merged *.symversions, possibly with different
> CRCs if the function argument is changed at the same time. It would
> cause potential breakage of module versioning.
> 
> If no EXPORT_SYMBOL is found, let's remove *.symversions explicitly.

Ah, and this was an issue for non-LTO builds too? Regardless, nice
catch.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 279363266455..585dae34746a 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -177,6 +177,8 @@ cmd_modversions_c =								\
>  	if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then			\
>  		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
>  		    > $@.symversions;						\
> +	else									\
> +		rm -f $@.symversions;						\
>  	fi;
>  else
>  cmd_modversions_c =								\
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 06/13] kbuild: merge vmlinux_link() between the ordinary link and Clang LTO

[Masahiro Yamada]

When Clang LTO is enabled, vmlinux_link() reuses vmlinux.o instead of
linking ${KBUILD_VMLINUX_OBJS} and ${KBUILD_VMLINUX_LIBS} again.

That is the only difference here, so merge the similar code.

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

 scripts/link-vmlinux.sh | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 36ef7b37fc5d..a6c4d0bce3ba 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -154,12 +154,23 @@ vmlinux_link()
 	local objects
 	local strip_debug
 	local map_option
+	local objs
+	local libs
 
 	info LD ${output}
 
 	# skip output file argument
 	shift
 
+	if [ -n "${CONFIG_LTO_CLANG}" ]; then
+		# Use vmlinux.o instead of performing the slow LTO link again.
+		objs=vmlinux.o
+		libs=
+	else
+		objs="${KBUILD_VMLINUX_OBJS}"
+		libs="${KBUILD_VMLINUX_LIBS}"
+	fi
+
 	# The kallsyms linking does not need debug symbols included.
 	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
 		strip_debug=-Wl,--strip-debug
@@ -170,22 +181,9 @@ vmlinux_link()
 	fi
 
 	if [ "${SRCARCH}" != "um" ]; then
-		if [ -n "${CONFIG_LTO_CLANG}" ]; then
-			# Use vmlinux.o instead of performing the slow LTO
-			# link again.
-			objects="--whole-archive		\
-				vmlinux.o 			\
-				--no-whole-archive		\
-				${@}"
-		else
-			objects="--whole-archive		\
-				${KBUILD_VMLINUX_OBJS}		\
-				--no-whole-archive		\
-				--start-group			\
-				${KBUILD_VMLINUX_LIBS}		\
-				--end-group			\
-				${@}"
-		fi
+		objects="--whole-archive ${objs} --no-whole-archive	\
+			 --start-group ${libs} --end-group		\
+			 $@"
 
 		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
 			${strip_debug#-Wl,}			\
-- 
2.30.2

Re: [PATCH 06/13] kbuild: merge vmlinux_link() between the ordinary link and Clang LTO

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:37AM +0900, Masahiro Yamada wrote:
> When Clang LTO is enabled, vmlinux_link() reuses vmlinux.o instead of
> linking ${KBUILD_VMLINUX_OBJS} and ${KBUILD_VMLINUX_LIBS} again.
> 
> That is the only difference here, so merge the similar code.

Oh excellent! I had tried to get this merged before and was not happy
with anything I constructed. This is much cleaner. Nice! (I think I
didn't realize there could be an empty --start-group/--end-group with
no side-effects.)

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/link-vmlinux.sh | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 36ef7b37fc5d..a6c4d0bce3ba 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -154,12 +154,23 @@ vmlinux_link()
>  	local objects
>  	local strip_debug
>  	local map_option
> +	local objs
> +	local libs
>  
>  	info LD ${output}
>  
>  	# skip output file argument
>  	shift
>  
> +	if [ -n "${CONFIG_LTO_CLANG}" ]; then
> +		# Use vmlinux.o instead of performing the slow LTO link again.
> +		objs=vmlinux.o
> +		libs=
> +	else
> +		objs="${KBUILD_VMLINUX_OBJS}"
> +		libs="${KBUILD_VMLINUX_LIBS}"
> +	fi
> +
>  	# The kallsyms linking does not need debug symbols included.
>  	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
>  		strip_debug=-Wl,--strip-debug
> @@ -170,22 +181,9 @@ vmlinux_link()
>  	fi
>  
>  	if [ "${SRCARCH}" != "um" ]; then
> -		if [ -n "${CONFIG_LTO_CLANG}" ]; then
> -			# Use vmlinux.o instead of performing the slow LTO
> -			# link again.
> -			objects="--whole-archive		\
> -				vmlinux.o 			\
> -				--no-whole-archive		\
> -				${@}"
> -		else
> -			objects="--whole-archive		\
> -				${KBUILD_VMLINUX_OBJS}		\
> -				--no-whole-archive		\
> -				--start-group			\
> -				${KBUILD_VMLINUX_LIBS}		\
> -				--end-group			\
> -				${@}"
> -		fi
> +		objects="--whole-archive ${objs} --no-whole-archive	\
> +			 --start-group ${libs} --end-group		\
> +			 $@"
>  
>  		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
>  			${strip_debug#-Wl,}			\
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 07/13] kbuild: do not remove 'linux' link in scripts/link-vmlinux.sh

[Masahiro Yamada]

arch/um/Makefile passes the -f option to the ln command:

    $(Q)ln -f $< $@

So, the hard link is always re-created, and the old one is removed
anyway.

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

 scripts/link-vmlinux.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index a6c4d0bce3ba..7b9c62e4d54a 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -206,7 +206,6 @@ vmlinux_link()
 			-Wl,-T,${lds}				\
 			${objects}				\
 			-lutil -lrt -lpthread
-		rm -f linux
 	fi
 }
 
-- 
2.30.2

Re: [PATCH 07/13] kbuild: do not remove 'linux' link in scripts/link-vmlinux.sh

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:38AM +0900, Masahiro Yamada wrote:
> arch/um/Makefile passes the -f option to the ln command:
> 
>     $(Q)ln -f $< $@
> 
> So, the hard link is always re-created, and the old one is removed
> anyway.

Hah, yeah, that's a bit of ARCH=um redundancy.

Reviewed-by: Kees Cook <keescook@chromium.org>

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/link-vmlinux.sh | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index a6c4d0bce3ba..7b9c62e4d54a 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -206,7 +206,6 @@ vmlinux_link()
>  			-Wl,-T,${lds}				\
>  			${objects}				\
>  			-lutil -lrt -lpthread
> -		rm -f linux
>  	fi
>  }
>  
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 08/13] kbuild: merge vmlinux_link() between ARCH=um and other architectures

[Masahiro Yamada]

For ARCH=um, ${CC} is used as the linker driver. Hence, the linker
options are prefixed with -Wl, .

Merge the similar code.

I replaced the -T option with the long option --script= so that it
works well with/without ${wl}.

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

 scripts/link-vmlinux.sh | 56 +++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 7b9c62e4d54a..d74cee5c4326 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -149,13 +149,12 @@ objtool_link()
 # ${2}, ${3}, ... - optional extra .o files
 vmlinux_link()
 {
-	local lds="${objtree}/${KBUILD_LDS}"
 	local output=${1}
-	local objects
-	local strip_debug
-	local map_option
 	local objs
 	local libs
+	local ld
+	local ldflags
+	local ldlibs
 
 	info LD ${output}
 
@@ -171,42 +170,33 @@ vmlinux_link()
 		libs="${KBUILD_VMLINUX_LIBS}"
 	fi
 
+	if [ "${SRCARCH}" = "um" ]; then
+		wl=-Wl,
+		ld="${CC}"
+		ldflags="${CFLAGS_vmlinux}"
+		ldlibs="-lutil -lrt -lpthread"
+	else
+		wl=
+		ld="${LD}"
+		ldflags="${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}"
+		ldlibs=
+	fi
+
+	ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
+
 	# The kallsyms linking does not need debug symbols included.
 	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
-		strip_debug=-Wl,--strip-debug
+		ldflags="${ldflags} ${wl}--strip-debug"
 	fi
 
 	if [ -n "${CONFIG_VMLINUX_MAP}" ]; then
-		map_option="-Map=${output}.map"
+		ldflags="${ldflags} ${wl}-Map=${output}.map"
 	fi
 
-	if [ "${SRCARCH}" != "um" ]; then
-		objects="--whole-archive ${objs} --no-whole-archive	\
-			 --start-group ${libs} --end-group		\
-			 $@"
-
-		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
-			${strip_debug#-Wl,}			\
-			-o ${output}				\
-			${map_option}				\
-			-T ${lds} ${objects}
-	else
-		objects="-Wl,--whole-archive			\
-			${KBUILD_VMLINUX_OBJS}			\
-			-Wl,--no-whole-archive			\
-			-Wl,--start-group			\
-			${KBUILD_VMLINUX_LIBS}			\
-			-Wl,--end-group				\
-			${@}"
-
-		${CC} ${CFLAGS_vmlinux}				\
-			${strip_debug}				\
-			-o ${output}				\
-			${map_option:+-Wl,${map_option}}	\
-			-Wl,-T,${lds}				\
-			${objects}				\
-			-lutil -lrt -lpthread
-	fi
+	${ld} ${ldflags} -o ${output}					\
+		${wl}--whole-archive ${objs} ${wl}--no-whole-archive	\
+		${wl}--start-group ${libs} ${wl}--end-group		\
+		$@ ${ldlibs}
 }
 
 # generate .BTF typeinfo from DWARF debuginfo
-- 
2.30.2

Re: [PATCH 08/13] kbuild: merge vmlinux_link() between ARCH=um and other architectures

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:39AM +0900, Masahiro Yamada wrote:
> For ARCH=um, ${CC} is used as the linker driver. Hence, the linker
> options are prefixed with -Wl, .
> 
> Merge the similar code.
> 
> I replaced the -T option with the long option --script= so that it
> works well with/without ${wl}.

And same for this one. So much better!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/link-vmlinux.sh | 56 +++++++++++++++++------------------------
>  1 file changed, 23 insertions(+), 33 deletions(-)
> 
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 7b9c62e4d54a..d74cee5c4326 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -149,13 +149,12 @@ objtool_link()
>  # ${2}, ${3}, ... - optional extra .o files
>  vmlinux_link()
>  {
> -	local lds="${objtree}/${KBUILD_LDS}"
>  	local output=${1}
> -	local objects
> -	local strip_debug
> -	local map_option
>  	local objs
>  	local libs
> +	local ld
> +	local ldflags
> +	local ldlibs
>  
>  	info LD ${output}
>  
> @@ -171,42 +170,33 @@ vmlinux_link()
>  		libs="${KBUILD_VMLINUX_LIBS}"
>  	fi
>  
> +	if [ "${SRCARCH}" = "um" ]; then
> +		wl=-Wl,
> +		ld="${CC}"
> +		ldflags="${CFLAGS_vmlinux}"
> +		ldlibs="-lutil -lrt -lpthread"
> +	else
> +		wl=
> +		ld="${LD}"
> +		ldflags="${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}"
> +		ldlibs=
> +	fi
> +
> +	ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
> +
>  	# The kallsyms linking does not need debug symbols included.
>  	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> -		strip_debug=-Wl,--strip-debug
> +		ldflags="${ldflags} ${wl}--strip-debug"
>  	fi
>  
>  	if [ -n "${CONFIG_VMLINUX_MAP}" ]; then
> -		map_option="-Map=${output}.map"
> +		ldflags="${ldflags} ${wl}-Map=${output}.map"
>  	fi
>  
> -	if [ "${SRCARCH}" != "um" ]; then
> -		objects="--whole-archive ${objs} --no-whole-archive	\
> -			 --start-group ${libs} --end-group		\
> -			 $@"
> -
> -		${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux}	\
> -			${strip_debug#-Wl,}			\
> -			-o ${output}				\
> -			${map_option}				\
> -			-T ${lds} ${objects}
> -	else
> -		objects="-Wl,--whole-archive			\
> -			${KBUILD_VMLINUX_OBJS}			\
> -			-Wl,--no-whole-archive			\
> -			-Wl,--start-group			\
> -			${KBUILD_VMLINUX_LIBS}			\
> -			-Wl,--end-group				\
> -			${@}"
> -
> -		${CC} ${CFLAGS_vmlinux}				\
> -			${strip_debug}				\
> -			-o ${output}				\
> -			${map_option:+-Wl,${map_option}}	\
> -			-Wl,-T,${lds}				\
> -			${objects}				\
> -			-lutil -lrt -lpthread
> -	fi
> +	${ld} ${ldflags} -o ${output}					\
> +		${wl}--whole-archive ${objs} ${wl}--no-whole-archive	\
> +		${wl}--start-group ${libs} ${wl}--end-group		\
> +		$@ ${ldlibs}
>  }
>  
>  # generate .BTF typeinfo from DWARF debuginfo
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 09/13] kbuild: do not create built-in.a.symversions or lib.a.symversions

[Masahiro Yamada]

Merge all *.o.symversions in scripts/link-vmlinux.sh instead of
merging them in the unit of built-in.a or lib.a.

This is a preparation for further code cleanups.

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

 scripts/Makefile.build  | 10 ++--------
 scripts/link-vmlinux.sh | 22 ++++++++++++++++++----
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 585dae34746a..37d6f6da34d6 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -430,11 +430,8 @@ endif
 quiet_cmd_ar_builtin = AR      $@
       cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
 
-quiet_cmd_ar_and_symver = AR      $@
-      cmd_ar_and_symver = $(cmd_update_lto_symversions); $(cmd_ar_builtin)
-
 $(obj)/built-in.a: $(real-obj-y) FORCE
-	$(call if_changed,ar_and_symver)
+	$(call if_changed,ar_builtin)
 
 #
 # Rule to create modules.order file
@@ -454,11 +451,8 @@ $(obj)/modules.order: $(obj-m) FORCE
 #
 # Rule to compile a set of .o files into one .a file (with symbol table)
 #
-quiet_cmd_ar_lib = AR      $@
-      cmd_ar_lib = $(cmd_update_lto_symversions); $(cmd_ar)
-
 $(obj)/lib.a: $(lib-y) FORCE
-	$(call if_changed,ar_lib)
+	$(call if_changed,ar)
 
 # NOTE:
 # Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d74cee5c4326..17976609c2d8 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -52,6 +52,13 @@ gen_initcalls()
 		> .tmp_initcalls.lds
 }
 
+append_symversion()
+{
+	if [ -f ${1}.symversions ]; then
+		cat ${1}.symversions >> .tmp_symversions.lds
+	fi
+}
+
 # If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
 # .tmp_symversions.lds
 gen_symversions()
@@ -59,10 +66,17 @@ gen_symversions()
 	info GEN .tmp_symversions.lds
 	rm -f .tmp_symversions.lds
 
-	for o in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
-		if [ -f ${o}.symversions ]; then
-			cat ${o}.symversions >> .tmp_symversions.lds
-		fi
+	for a in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
+		case $a in
+		*.a)
+			for o in $(${AR} t ${a}); do
+				append_symversion ${o}
+			done
+			;;
+		*)
+			append_symversion ${a}
+			;;
+		esac
 	done
 }
 
-- 
2.30.2

Re: [PATCH 09/13] kbuild: do not create built-in.a.symversions or lib.a.symversions

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:40AM +0900, Masahiro Yamada wrote:
> Merge all *.o.symversions in scripts/link-vmlinux.sh instead of
> merging them in the unit of built-in.a or lib.a.
> 
> This is a preparation for further code cleanups.

Looks good, though I wonder about this becoming serialized during the
link phase rather than doing the work per-target. I mean, it always had
to collect them all during the link phase (with "cat"), but before it
wasn't running $(AR) serially to do it.

I'll ponder how this might be made a little more parallel. But for now:

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build  | 10 ++--------
>  scripts/link-vmlinux.sh | 22 ++++++++++++++++++----
>  2 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 585dae34746a..37d6f6da34d6 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -430,11 +430,8 @@ endif
>  quiet_cmd_ar_builtin = AR      $@
>        cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
>  
> -quiet_cmd_ar_and_symver = AR      $@
> -      cmd_ar_and_symver = $(cmd_update_lto_symversions); $(cmd_ar_builtin)
> -
>  $(obj)/built-in.a: $(real-obj-y) FORCE
> -	$(call if_changed,ar_and_symver)
> +	$(call if_changed,ar_builtin)
>  
>  #
>  # Rule to create modules.order file
> @@ -454,11 +451,8 @@ $(obj)/modules.order: $(obj-m) FORCE
>  #
>  # Rule to compile a set of .o files into one .a file (with symbol table)
>  #
> -quiet_cmd_ar_lib = AR      $@
> -      cmd_ar_lib = $(cmd_update_lto_symversions); $(cmd_ar)
> -
>  $(obj)/lib.a: $(lib-y) FORCE
> -	$(call if_changed,ar_lib)
> +	$(call if_changed,ar)
>  
>  # NOTE:
>  # Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index d74cee5c4326..17976609c2d8 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -52,6 +52,13 @@ gen_initcalls()
>  		> .tmp_initcalls.lds
>  }
>  
> +append_symversion()
> +{
> +	if [ -f ${1}.symversions ]; then
> +		cat ${1}.symversions >> .tmp_symversions.lds
> +	fi
> +}
> +
>  # If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
>  # .tmp_symversions.lds
>  gen_symversions()
> @@ -59,10 +66,17 @@ gen_symversions()
>  	info GEN .tmp_symversions.lds
>  	rm -f .tmp_symversions.lds
>  
> -	for o in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
> -		if [ -f ${o}.symversions ]; then
> -			cat ${o}.symversions >> .tmp_symversions.lds
> -		fi
> +	for a in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
> +		case $a in
> +		*.a)
> +			for o in $(${AR} t ${a}); do
> +				append_symversion ${o}
> +			done
> +			;;
> +		*)
> +			append_symversion ${a}
> +			;;
> +		esac
>  	done
>  }
>  
> -- 
> 2.30.2
> 

-- 
Kees Cook

Re: [PATCH 09/13] kbuild: do not create built-in.a.symversions or lib.a.symversions

[Masahiro Yamada]

On Thu, Aug 19, 2021 at 3:41 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Aug 19, 2021 at 09:57:40AM +0900, Masahiro Yamada wrote:
> > Merge all *.o.symversions in scripts/link-vmlinux.sh instead of
> > merging them in the unit of built-in.a or lib.a.
> >
> > This is a preparation for further code cleanups.
>
> Looks good, though I wonder about this becoming serialized during the
> link phase rather than doing the work per-target. I mean, it always had
> to collect them all during the link phase (with "cat"), but before it
> wasn't running $(AR) serially to do it.
>
> I'll ponder how this might be made a little more parallel. But for now:
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> -Kees
>


I measured the cost of merging all the *.symversions.

For a typical use-case
(x86_64 defconfig + CONFIG_LTO_CLANG_THIN + CONFIG_MODVERSIONS),
my shell script took about 0.40 msec
for merging all the individual *.symversions files.

Most of the cost of 0.40 msec came from the 'cat' command.
The 'cat' command is kind of slow when you concatenate
a large number of files.

I implemented the equivalent functionality with a perl script,
which worked in only 0.04 msec.

I think 0.04 msec should be acceptable cost because
this commit eliminates all the intermediate built-in.a.symversions
and lib.a.symversions, saving disk space.

I also tried allyesconfig + CONFIG_LTO_CLANG_THIN + CONFIG_MODVERSIONS
(the heaviest load), but the result is similar.

This is because most of EXPORT_SYMBOL's come from the core part of
the kernel, and enabling drivers as built-in does not give much impact, I think.

So, I will plan to submit v2 with perl implementation.


The detailed test code is as follows:








masahiro@oscar:~/workspace/linux-kbuild$ cat scripts/merge-symvers.sh
#!/bin/sh

append_symversion()
{
        if [ -f ${1}.symversions ]; then
                cat ${1}.symversions >> .tmp_symversions.lds
        fi
}

# If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
# .tmp_symversions.lds
gen_symversions()
{
        rm -f .tmp_symversions.lds

        for a in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
                case $a in
                *.a)
                        for o in $(${AR} t ${a}); do
                                append_symversion ${o}
                        done
                        ;;
                *)
                        append_symversion ${a}
                        ;;
                esac
        done
}

gen_symversions



masahiro@oscar:~/workspace/linux-kbuild$ cat scripts/merge-symvers.pl
#!/usr/bin/env perl
# SPDX-License-Identifier: GPL-2.0-only

use autodie;
use strict;
use warnings;
use Getopt::Long 'GetOptions';

my $ar;
my $output;

GetOptions(
        'a|ar=s' => \$ar,
        'o|output=s'  => \$output,
);

# Collect all objects
my @objects;

foreach (@ARGV) {
        if (/\.o$/) {
                # Some objects (head-y) are linked to vmlinux directly.
                push(@objects, $_);
        } elsif (/\.a$/) {
                # Most of built-in objects are contained in built-in.a or lib.a.
                # Use 'ar -t' to get the list of the contained objects.
                $_ = `$ar -t $_`;
                push(@objects, split(/\n/));
        } else {
                die "$_: unknown file type\n";
        }
}

open(my $out_fh, '>', "$output");

foreach (@objects) {
        # The symbol CRCs for foo/bar/baz.o is output to
foo/bar/baz.o.symversions
        s/(.*)/$1.symversions/;

        if (! -e $_) {
                # .symversions does not exist if the object does not contain
                # EXPORT_SYMBOL at all. Skip it.
                next;
        }

        open(my $in_fh, '<', "$_");
        # Concatenate all the existing *.symversions files.
        print $out_fh do { local $/; <$in_fh> };
        close $in_fh;
}

close $out_fh;



masahiro@oscar:~/workspace/linux-kbuild$ git diff
diff --git a/Makefile b/Makefile
index 3ef3685b7e4a..5b8fe617769a 100644
--- a/Makefile
+++ b/Makefile
@@ -1175,6 +1175,14 @@ vmlinux: scripts/link-vmlinux.sh
autoksyms_recursive $(vmlinux-deps) FORCE

 targets := vmlinux

+PHONY += merge-symvers-by-shell merge-symvers-by-perl
+
+merge-symvers-by-shell:
+       time sh scripts/merge-symvers.sh
+
+merge-symvers-by-perl:
+       time perl scripts/merge-symvers.pl -a $(AR) -o
.tmp_symversions.lds $(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)
+
 # The actual objects are generated when descending,
 # make sure no implicit rule kicks in
 $(sort $(vmlinux-deps) $(subdir-modorder)): descend ;




masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1 defconfig
*** Default configuration is based on 'x86_64_defconfig'
#
# configuration written to .config
#
masahiro@oscar:~/workspace/linux-kbuild$ ./scripts/config  -d
LTO_NONE -e LTO_CLANG_THIN -e MODVERSIONS
masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1  -s -j24
arch/x86/entry/vdso/Makefile:135: FORCE prerequisite is missing
arch/x86/entry/vdso/Makefile:135: FORCE prerequisite is missing


masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-shell
time sh scripts/merge-symvers.sh
0.40user 0.08system 0:00.47elapsed 101%CPU (0avgtext+0avgdata 7156maxresident)k
0inputs+896outputs (0major+90678minor)pagefaults 0swaps


masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-perl
time perl scripts/merge-symvers.pl -a llvm-ar -o .tmp_symversions.lds
arch/x86/kernel/head_64.o arch/x86/kernel/head64.o
arch/x86/kernel/ebda.o arch/x86/kernel/platform-quirks.o
init/built-in.a usr/built-in.a arch/x86/built-in.a kernel/built-in.a
certs/built-in.a mm/built-in.a fs/built-in.a ipc/built-in.a
security/built-in.a crypto/built-in.a block/built-in.a lib/built-in.a
arch/x86/lib/built-in.a  lib/lib.a  arch/x86/lib/lib.a
drivers/built-in.a sound/built-in.a net/built-in.a virt/built-in.a
arch/x86/pci/built-in.a arch/x86/power/built-in.a
0.04user 0.02system 0:00.06elapsed 101%CPU (0avgtext+0avgdata 10100maxresident)k
0inputs+896outputs (0major+8590minor)pagefaults 0swaps


masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   allyesconfig
#
# configuration written to .config
#
masahiro@oscar:~/workspace/linux-kbuild$ ./scripts/config  -d
LTO_NONE -e LTO_CLANG_THIN
masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1  -s -j24
  [ snip ]

masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-shell
time sh scripts/merge-symvers.sh
0.41user 0.09system 0:00.50elapsed 101%CPU (0avgtext+0avgdata 7172maxresident)k
0inputs+896outputs (0major+91425minor)pagefaults 0swaps

masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-perl
time perl scripts/merge-symvers.pl -a llvm-ar -o .tmp_symversions.lds
arch/x86/kernel/head_64.o arch/x86/kernel/head64.o
arch/x86/kernel/ebda.o arch/x86/kernel/platform-quirks.o
init/built-in.a usr/built-in.a arch/x86/built-in.a kernel/built-in.a
certs/built-in.a mm/built-in.a fs/built-in.a ipc/built-in.a
security/built-in.a crypto/built-in.a block/built-in.a lib/built-in.a
arch/x86/lib/built-in.a  lib/lib.a  arch/x86/lib/lib.a
drivers/built-in.a sound/built-in.a samples/built-in.a net/built-in.a
virt/built-in.a arch/x86/pci/built-in.a arch/x86/power/built-in.a
arch/x86/video/built-in.a
0.08user 0.02system 0:00.11elapsed 100%CPU (0avgtext+0avgdata 15984maxresident)k
0inputs+896outputs (0major+11506minor)pagefaults 0swaps





-- 
Best Regards
Masahiro Yamada

[PATCH 10/13] kbuild: build modules in the same way with/without Clang LTO

[Masahiro Yamada]

When Clang LTO is enabled, additional intermediate files *.lto.o are
created because LLVM bitcode must be converted to ELF before modpost.

For non-LTO builds:

         $(LD)             $(LD)
  objects ---> <modname>.o -----> <modname>.ko
                             |
          <modname>.mod.o ---/

For Clang LTO builds:

         $(AR)            $(LD)                 $(LD)
  objects ---> <modname>.o ---> <modname>.lto.o -----> <modname>.ko
                                                  |
                                <modname>.mod.o --/

Since the Clang LTO introduction, ugly CONFIG_LTO_CLANG conditionals
are sprinkled everywhere in the kbuild code.

Another confusion for Clang LTO builds is, <modname>.o is an archive
that contains LLVM bitcode files. The suffix should have been .a
instead of .o

To clean up the code, unify the build process of modules, as follows:

         $(AR)            $(LD)                     $(LD)
  objects ---> <modname>.a ---> <modname>.prelink.o -----> <modname>.ko
                                                      |
                                <modname>.mod.o ------/

Here, 'objects' are either ELF or LLVM bitcode. <modname>.a is an archive,
<modname>.prelink.o is ELF.

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

 scripts/Makefile.build    | 103 ++++++++++++++++++--------------------
 scripts/Makefile.lib      |  11 ++--
 scripts/Makefile.modfinal |   4 +-
 scripts/Makefile.modpost  |   7 +--
 scripts/mod/modpost.c     |   6 +--
 scripts/mod/sumversion.c  |   6 +--
 6 files changed, 61 insertions(+), 76 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 37d6f6da34d6..957addea830b 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -88,9 +88,7 @@ endif
 
 targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
 
-ifdef CONFIG_LTO_CLANG
-targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
-endif
+targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
 
 ifdef need-modorder
 targets-for-modules += $(obj)/modules.order
@@ -282,33 +280,12 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
 	$(call if_changed_rule,cc_o_c)
 	$(call cmd,force_checksrc)
 
-ifdef CONFIG_LTO_CLANG
-# Module .o files may contain LLVM bitcode, compile them into native code
-# before ELF processing
-quiet_cmd_cc_lto_link_modules = LTO [M] $@
-cmd_cc_lto_link_modules =						\
-	$(LD) $(ld_flags) -r -o $@					\
-		$(shell [ -s $(@:.lto.o=.o.symversions) ] &&		\
-			echo -T $(@:.lto.o=.o.symversions))		\
-		--whole-archive $(filter-out FORCE,$^)
-
-ifdef CONFIG_STACK_VALIDATION
-# objtool was skipped for LLVM bitcode, run it now that we have compiled
-# modules into native code
-cmd_cc_lto_link_modules += ;						\
-	$(objtree)/tools/objtool/objtool $(objtool_args) --module $@
-endif
-
-$(obj)/%.lto.o: $(obj)/%.o FORCE
-	$(call if_changed,cc_lto_link_modules)
-endif
-
 cmd_mod = { \
 	echo $(if $($*-objs)$($*-y)$($*-m), $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)), $(@:.mod=.o)); \
 	$(undefined_syms) echo; \
 	} > $@
 
-$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
+$(obj)/%.mod: $(obj)/%.prelink.o FORCE
 	$(call if_changed,mod)
 
 quiet_cmd_cc_lst_c = MKLST   $@
@@ -412,17 +389,6 @@ $(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/scripts/asn1_compiler
 $(subdir-builtin): $(obj)/%/built-in.a: $(obj)/% ;
 $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
 
-# combine symversions for later processing
-ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
-      cmd_update_lto_symversions =					\
-	rm -f $@.symversions						\
-	$(foreach n, $(filter-out FORCE,$^),				\
-		$(if $(shell test -s $(n).symversions && echo y),	\
-			; cat $(n).symversions >> $@.symversions))
-else
-      cmd_update_lto_symversions = echo >/dev/null
-endif
-
 #
 # Rule to compile a set of .o files into one .a file (without symbol table)
 #
@@ -442,10 +408,10 @@ $(obj)/built-in.a: $(real-obj-y) FORCE
 # modules.order unless contained modules are updated.
 
 cmd_modules_order = { $(foreach m, $(real-prereqs), \
-	$(if $(filter %/modules.order, $m), cat $m, echo $(patsubst %.o,%.ko,$m));) :; } \
+	$(if $(filter %/modules.order, $m), cat $m, echo $(patsubst %.a,%.ko,$m));) :; } \
 	| $(AWK) '!x[$$0]++' - > $@
 
-$(obj)/modules.order: $(obj-m) FORCE
+$(obj)/modules.order: $(modules) FORCE
 	$(call if_changed,modules_order)
 
 #
@@ -454,26 +420,55 @@ $(obj)/modules.order: $(obj-m) FORCE
 $(obj)/lib.a: $(lib-y) FORCE
 	$(call if_changed,ar)
 
-# NOTE:
-# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
-# module is turned into a multi object module, $^ will contain header file
-# dependencies recorded in the .*.cmd file.
+#
+# Rule to prelink modules
+#
+
+ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
+
+cmd_merge_symver =					\
+	rm -f $@;					\
+	touch $@;					\
+	for o in $$($(AR) t $<); do			\
+		if [ -s $${o}.symversions ]; then	\
+			cat $${o}.symversions >> $@;	\
+		fi;					\
+	done
+
+$(obj)/%.prelink.symversions: $(obj)/%.a FORCE
+	$(call if_changed,merge_symver)
+
+$(obj)/%.prelink.o: ld_flags += --script=$(filter %.symversions,$^)
+module-symver = $(obj)/%.prelink.symversions
+
+endif
+
+quiet_cmd_ld_o_a = LD [M]  $@
+      cmd_ld_o_a = $(LD) $(ld_flags) -r -o $@ --whole-archive $<
+
+$(obj)/%.prelink.o: $(obj)/%.a $(module-symver) FORCE
+	$(call if_changed,ld_o_a)
+
 ifdef CONFIG_LTO_CLANG
-quiet_cmd_link_multi-m = AR [M]  $@
-cmd_link_multi-m =						\
-	$(cmd_update_lto_symversions);				\
-	rm -f $@; 						\
-	$(AR) cDPrsT $@ $(filter %.o,$^)
-else
-quiet_cmd_link_multi-m = LD [M]  $@
-      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
+ifdef CONFIG_STACK_VALIDATION
+# objtool was skipped for LLVM bitcode, run it now that we have compiled
+# modules into native code
+cmd_ld_o_a += ; $(objtool) $(objtool_args) --module $@
 endif
+endif
+
+quiet_cmd_ar_module = AR [M]  $@
+      cmd_ar_module = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
+
+$(modules-single): %.a: %.o FORCE
+	$(call if_changed,ar_module)
+
+$(modules-multi): FORCE
+	$(call if_changed,ar_module)
+$(call multi_depend, $(modules-multi), .a, -objs -y -m)
 
-$(multi-obj-m): FORCE
-	$(call if_changed,link_multi-m)
-$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
+targets += $(modules-single) $(modules-multi)
 
-targets += $(multi-obj-m)
 targets := $(filter-out $(PHONY), $(targets))
 
 # Add intermediate targets:
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 34c4c11c4bc1..f604d2d01cad 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -106,6 +106,10 @@ multi-dtb-y	:= $(addprefix $(obj)/, $(multi-dtb-y))
 real-dtb-y	:= $(addprefix $(obj)/, $(real-dtb-y))
 subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
 
+modules		:= $(patsubst %.o, %.a, $(obj-m))
+modules-multi	:= $(patsubst %.o, %.a, $(multi-obj-m))
+modules-single	:= $(filter-out $(modules-multi), $(filter %.a, $(modules)))
+
 # Finds the multi-part object the current object will be linked into.
 # If the object belongs to two or more multi-part objects, list them all.
 modname-multi = $(sort $(foreach m,$(multi-obj-ym),\
@@ -225,13 +229,6 @@ dtc_cpp_flags  = -Wp,-MMD,$(depfile).pre.tmp -nostdinc                    \
 		 $(addprefix -I,$(DTC_INCLUDE))                          \
 		 -undef -D__DTS__
 
-ifeq ($(CONFIG_LTO_CLANG),y)
-# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we
-# need to run LTO to compile them into native code (.lto.o) before further
-# processing.
-mod-prelink-ext := .lto
-endif
-
 # Useful for describing the dependency of composite objects
 # Usage:
 #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index ff805777431c..1b6401f53662 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -9,7 +9,7 @@ __modfinal:
 include include/config/auto.conf
 include $(srctree)/scripts/Kbuild.include
 
-# for c_flags and mod-prelink-ext
+# for c_flags
 include $(srctree)/scripts/Makefile.lib
 
 # find all modules listed in modules.order
@@ -55,7 +55,7 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
 
 
 # Re-generate module BTFs if either module's .ko or vmlinux changed
-$(modules): %.ko: %$(mod-prelink-ext).o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
+$(modules): %.ko: %.prelink.o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
 	+$(call if_changed_except,ld_ko_o,vmlinux)
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 	+$(if $(newer-prereqs),$(call cmd,btf_ko))
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index eef56d629799..11883b31c615 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -41,9 +41,6 @@ __modpost:
 include include/config/auto.conf
 include $(srctree)/scripts/Kbuild.include
 
-# for mod-prelink-ext
-include $(srctree)/scripts/Makefile.lib
-
 MODPOST = scripts/mod/modpost								\
 	$(if $(CONFIG_MODVERSIONS),-m)							\
 	$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)					\
@@ -128,9 +125,9 @@ endif
 # Read out modules.order to pass in modpost.
 # Otherwise, allmodconfig would fail with "Argument list too long".
 quiet_cmd_modpost = MODPOST $@
-      cmd_modpost = sed 's/\.ko$$/$(mod-prelink-ext)\.o/' $< | $(MODPOST) -T -
+      cmd_modpost = sed 's/ko$$/prelink.o/' $< | $(MODPOST) -T -
 
-$(output-symdump): $(MODORDER) $(input-symdump) $(modules:.ko=$(mod-prelink-ext).o) FORCE
+$(output-symdump): $(MODORDER) $(input-symdump) $(modules:ko=prelink.o) FORCE
 	$(call if_changed,modpost)
 
 targets += $(output-symdump)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 270a7df898e2..8c63c52af88d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1991,9 +1991,9 @@ static void read_symbols(const char *modname)
 		/* strip trailing .o */
 		tmp = NOFAIL(strdup(modname));
 		tmp[strlen(tmp) - 2] = '\0';
-		/* strip trailing .lto */
-		if (strends(tmp, ".lto"))
-			tmp[strlen(tmp) - 4] = '\0';
+		/* strip trailing .prelink */
+		if (strends(tmp, ".prelink"))
+			tmp[strlen(tmp) - 8] = '\0';
 		mod = new_module(tmp);
 		free(tmp);
 	}
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 760e6baa7eda..8ea0f7b23c63 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -391,14 +391,10 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
 	struct md4_ctx md;
 	char *fname;
 	char filelist[PATH_MAX + 1];
-	int postfix_len = 1;
-
-	if (strends(modname, ".lto.o"))
-		postfix_len = 5;
 
 	/* objects for a module are listed in the first line of *.mod file. */
 	snprintf(filelist, sizeof(filelist), "%.*smod",
-		 (int)strlen(modname) - postfix_len, modname);
+		 (int)(strlen(modname) - strlen("prelink.o")), modname);
 
 	buf = read_text_file(filelist);
 
-- 
2.30.2

Re: [PATCH 10/13] kbuild: build modules in the same way with/without Clang LTO

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:41AM +0900, Masahiro Yamada wrote:
> When Clang LTO is enabled, additional intermediate files *.lto.o are
> created because LLVM bitcode must be converted to ELF before modpost.
> 
> For non-LTO builds:
> 
>          $(LD)             $(LD)
>   objects ---> <modname>.o -----> <modname>.ko
>                              |
>           <modname>.mod.o ---/
> 
> For Clang LTO builds:
> 
>          $(AR)            $(LD)                 $(LD)
>   objects ---> <modname>.o ---> <modname>.lto.o -----> <modname>.ko
>                                                   |
>                                 <modname>.mod.o --/
> 
> Since the Clang LTO introduction, ugly CONFIG_LTO_CLANG conditionals
> are sprinkled everywhere in the kbuild code.
> 
> Another confusion for Clang LTO builds is, <modname>.o is an archive
> that contains LLVM bitcode files. The suffix should have been .a
> instead of .o
> 
> To clean up the code, unify the build process of modules, as follows:
> 
>          $(AR)            $(LD)                     $(LD)
>   objects ---> <modname>.a ---> <modname>.prelink.o -----> <modname>.ko
>                                                       |
>                                 <modname>.mod.o ------/
> 
> Here, 'objects' are either ELF or LLVM bitcode. <modname>.a is an archive,
> <modname>.prelink.o is ELF.

I like this design, but I do see that it has a small but measurable
impact on build times:

allmodconfig build, GCC:

make -j72 allmodconfig
make -j72 -s clean && time make -j72

    kbuild/for-next:
        6m16.140s
        6m19.742s
        6m15.848s

    +this-series:
        6m22.742s
        6m20.589s
        6m19.911s

Thought with not so many modules, it's within the noise:

defconfig build, GCC:

make -j72 defconfig
make -j72 -s clean && time make -j72

    kbuild/for-next:
        0m41.579s
        0m41.214s
        0m41.370s

    +series:
        0m41.423s
        0m41.434s
        0m41.384s


However, I do see that even LTO builds are slightly slower now, so
perhaps the above numbers aren't due to the added $(AR) step:

allmodconfig + Clang ThinLTO:

make -j72 LLVM=1 LLVM_IAS=1 allmodconfig
./scripts/config -d GCOV_KERNEL -d KASAN -d LTO_NONE -e LTO_CLANG_THIN
make -j72 LLVM=1 LLVM_IAS=1 olddefconfig
make -j72 -s LLVM=1 LLVM_IAS=1 clean && time make -j72 LLVM=1 LLVM_IAS=1

    kbuild/for-next:
        9m53.927s
        9m45.874s
        9m47.722s

    +series:
        9m58.395s
        9m53.201s
        9m56.387s


I haven't been able to isolate where the changes in build times are
coming from (nor have I done link-phase-only timings -- I realize those
are really the most important).

I did notice some warnings from this patch, though, in the
$(modules-single) target:

scripts/Makefile.build:434: target 'drivers/scsi/libiscsi.a' given more than once in the same rule
scripts/Makefile.build:434: target 'drivers/atm/suni.a' given more than once in the same rule

(And I saw the new "FORCE prerequisite is missing" warnings, but those
are in kbuild/for-next and not new for this series.)

Anyway, this is a great clean-up; thank you very much for finding the
time for it! I'll keep poking at it tomorrow.

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build    | 103 ++++++++++++++++++--------------------
>  scripts/Makefile.lib      |  11 ++--
>  scripts/Makefile.modfinal |   4 +-
>  scripts/Makefile.modpost  |   7 +--
>  scripts/mod/modpost.c     |   6 +--
>  scripts/mod/sumversion.c  |   6 +--
>  6 files changed, 61 insertions(+), 76 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 37d6f6da34d6..957addea830b 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -88,9 +88,7 @@ endif
>  
>  targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
>  
> -ifdef CONFIG_LTO_CLANG
> -targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
> -endif
> +targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
>  
>  ifdef need-modorder
>  targets-for-modules += $(obj)/modules.order
> @@ -282,33 +280,12 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>  	$(call if_changed_rule,cc_o_c)
>  	$(call cmd,force_checksrc)
>  
> -ifdef CONFIG_LTO_CLANG
> -# Module .o files may contain LLVM bitcode, compile them into native code
> -# before ELF processing
> -quiet_cmd_cc_lto_link_modules = LTO [M] $@
> -cmd_cc_lto_link_modules =						\
> -	$(LD) $(ld_flags) -r -o $@					\
> -		$(shell [ -s $(@:.lto.o=.o.symversions) ] &&		\
> -			echo -T $(@:.lto.o=.o.symversions))		\
> -		--whole-archive $(filter-out FORCE,$^)
> -
> -ifdef CONFIG_STACK_VALIDATION
> -# objtool was skipped for LLVM bitcode, run it now that we have compiled
> -# modules into native code
> -cmd_cc_lto_link_modules += ;						\
> -	$(objtree)/tools/objtool/objtool $(objtool_args) --module $@
> -endif
> -
> -$(obj)/%.lto.o: $(obj)/%.o FORCE
> -	$(call if_changed,cc_lto_link_modules)
> -endif
> -
>  cmd_mod = { \
>  	echo $(if $($*-objs)$($*-y)$($*-m), $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)), $(@:.mod=.o)); \
>  	$(undefined_syms) echo; \
>  	} > $@
>  
> -$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
> +$(obj)/%.mod: $(obj)/%.prelink.o FORCE
>  	$(call if_changed,mod)
>  
>  quiet_cmd_cc_lst_c = MKLST   $@
> @@ -412,17 +389,6 @@ $(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/scripts/asn1_compiler
>  $(subdir-builtin): $(obj)/%/built-in.a: $(obj)/% ;
>  $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
>  
> -# combine symversions for later processing
> -ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
> -      cmd_update_lto_symversions =					\
> -	rm -f $@.symversions						\
> -	$(foreach n, $(filter-out FORCE,$^),				\
> -		$(if $(shell test -s $(n).symversions && echo y),	\
> -			; cat $(n).symversions >> $@.symversions))
> -else
> -      cmd_update_lto_symversions = echo >/dev/null
> -endif
> -
>  #
>  # Rule to compile a set of .o files into one .a file (without symbol table)
>  #
> @@ -442,10 +408,10 @@ $(obj)/built-in.a: $(real-obj-y) FORCE
>  # modules.order unless contained modules are updated.
>  
>  cmd_modules_order = { $(foreach m, $(real-prereqs), \
> -	$(if $(filter %/modules.order, $m), cat $m, echo $(patsubst %.o,%.ko,$m));) :; } \
> +	$(if $(filter %/modules.order, $m), cat $m, echo $(patsubst %.a,%.ko,$m));) :; } \
>  	| $(AWK) '!x[$$0]++' - > $@
>  
> -$(obj)/modules.order: $(obj-m) FORCE
> +$(obj)/modules.order: $(modules) FORCE
>  	$(call if_changed,modules_order)
>  
>  #
> @@ -454,26 +420,55 @@ $(obj)/modules.order: $(obj-m) FORCE
>  $(obj)/lib.a: $(lib-y) FORCE
>  	$(call if_changed,ar)
>  
> -# NOTE:
> -# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
> -# module is turned into a multi object module, $^ will contain header file
> -# dependencies recorded in the .*.cmd file.
> +#
> +# Rule to prelink modules
> +#
> +
> +ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
> +
> +cmd_merge_symver =					\
> +	rm -f $@;					\
> +	touch $@;					\
> +	for o in $$($(AR) t $<); do			\
> +		if [ -s $${o}.symversions ]; then	\
> +			cat $${o}.symversions >> $@;	\
> +		fi;					\
> +	done
> +
> +$(obj)/%.prelink.symversions: $(obj)/%.a FORCE
> +	$(call if_changed,merge_symver)
> +
> +$(obj)/%.prelink.o: ld_flags += --script=$(filter %.symversions,$^)
> +module-symver = $(obj)/%.prelink.symversions
> +
> +endif
> +
> +quiet_cmd_ld_o_a = LD [M]  $@
> +      cmd_ld_o_a = $(LD) $(ld_flags) -r -o $@ --whole-archive $<
> +
> +$(obj)/%.prelink.o: $(obj)/%.a $(module-symver) FORCE
> +	$(call if_changed,ld_o_a)
> +
>  ifdef CONFIG_LTO_CLANG
> -quiet_cmd_link_multi-m = AR [M]  $@
> -cmd_link_multi-m =						\
> -	$(cmd_update_lto_symversions);				\
> -	rm -f $@; 						\
> -	$(AR) cDPrsT $@ $(filter %.o,$^)
> -else
> -quiet_cmd_link_multi-m = LD [M]  $@
> -      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
> +ifdef CONFIG_STACK_VALIDATION
> +# objtool was skipped for LLVM bitcode, run it now that we have compiled
> +# modules into native code
> +cmd_ld_o_a += ; $(objtool) $(objtool_args) --module $@
>  endif
> +endif
> +
> +quiet_cmd_ar_module = AR [M]  $@
> +      cmd_ar_module = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
> +
> +$(modules-single): %.a: %.o FORCE
> +	$(call if_changed,ar_module)
> +
> +$(modules-multi): FORCE
> +	$(call if_changed,ar_module)
> +$(call multi_depend, $(modules-multi), .a, -objs -y -m)
>  
> -$(multi-obj-m): FORCE
> -	$(call if_changed,link_multi-m)
> -$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
> +targets += $(modules-single) $(modules-multi)
>  
> -targets += $(multi-obj-m)
>  targets := $(filter-out $(PHONY), $(targets))
>  
>  # Add intermediate targets:
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 34c4c11c4bc1..f604d2d01cad 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -106,6 +106,10 @@ multi-dtb-y	:= $(addprefix $(obj)/, $(multi-dtb-y))
>  real-dtb-y	:= $(addprefix $(obj)/, $(real-dtb-y))
>  subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
>  
> +modules		:= $(patsubst %.o, %.a, $(obj-m))
> +modules-multi	:= $(patsubst %.o, %.a, $(multi-obj-m))
> +modules-single	:= $(filter-out $(modules-multi), $(filter %.a, $(modules)))
> +
>  # Finds the multi-part object the current object will be linked into.
>  # If the object belongs to two or more multi-part objects, list them all.
>  modname-multi = $(sort $(foreach m,$(multi-obj-ym),\
> @@ -225,13 +229,6 @@ dtc_cpp_flags  = -Wp,-MMD,$(depfile).pre.tmp -nostdinc                    \
>  		 $(addprefix -I,$(DTC_INCLUDE))                          \
>  		 -undef -D__DTS__
>  
> -ifeq ($(CONFIG_LTO_CLANG),y)
> -# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we
> -# need to run LTO to compile them into native code (.lto.o) before further
> -# processing.
> -mod-prelink-ext := .lto
> -endif
> -
>  # Useful for describing the dependency of composite objects
>  # Usage:
>  #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index ff805777431c..1b6401f53662 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -9,7 +9,7 @@ __modfinal:
>  include include/config/auto.conf
>  include $(srctree)/scripts/Kbuild.include
>  
> -# for c_flags and mod-prelink-ext
> +# for c_flags
>  include $(srctree)/scripts/Makefile.lib
>  
>  # find all modules listed in modules.order
> @@ -55,7 +55,7 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
>  
>  
>  # Re-generate module BTFs if either module's .ko or vmlinux changed
> -$(modules): %.ko: %$(mod-prelink-ext).o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
> +$(modules): %.ko: %.prelink.o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
>  	+$(call if_changed_except,ld_ko_o,vmlinux)
>  ifdef CONFIG_DEBUG_INFO_BTF_MODULES
>  	+$(if $(newer-prereqs),$(call cmd,btf_ko))
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index eef56d629799..11883b31c615 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -41,9 +41,6 @@ __modpost:
>  include include/config/auto.conf
>  include $(srctree)/scripts/Kbuild.include
>  
> -# for mod-prelink-ext
> -include $(srctree)/scripts/Makefile.lib
> -
>  MODPOST = scripts/mod/modpost								\
>  	$(if $(CONFIG_MODVERSIONS),-m)							\
>  	$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)					\
> @@ -128,9 +125,9 @@ endif
>  # Read out modules.order to pass in modpost.
>  # Otherwise, allmodconfig would fail with "Argument list too long".
>  quiet_cmd_modpost = MODPOST $@
> -      cmd_modpost = sed 's/\.ko$$/$(mod-prelink-ext)\.o/' $< | $(MODPOST) -T -
> +      cmd_modpost = sed 's/ko$$/prelink.o/' $< | $(MODPOST) -T -
>  
> -$(output-symdump): $(MODORDER) $(input-symdump) $(modules:.ko=$(mod-prelink-ext).o) FORCE
> +$(output-symdump): $(MODORDER) $(input-symdump) $(modules:ko=prelink.o) FORCE
>  	$(call if_changed,modpost)
>  
>  targets += $(output-symdump)
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 270a7df898e2..8c63c52af88d 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1991,9 +1991,9 @@ static void read_symbols(const char *modname)
>  		/* strip trailing .o */
>  		tmp = NOFAIL(strdup(modname));
>  		tmp[strlen(tmp) - 2] = '\0';
> -		/* strip trailing .lto */
> -		if (strends(tmp, ".lto"))
> -			tmp[strlen(tmp) - 4] = '\0';
> +		/* strip trailing .prelink */
> +		if (strends(tmp, ".prelink"))
> +			tmp[strlen(tmp) - 8] = '\0';
>  		mod = new_module(tmp);
>  		free(tmp);
>  	}
> diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
> index 760e6baa7eda..8ea0f7b23c63 100644
> --- a/scripts/mod/sumversion.c
> +++ b/scripts/mod/sumversion.c
> @@ -391,14 +391,10 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
>  	struct md4_ctx md;
>  	char *fname;
>  	char filelist[PATH_MAX + 1];
> -	int postfix_len = 1;
> -
> -	if (strends(modname, ".lto.o"))
> -		postfix_len = 5;
>  
>  	/* objects for a module are listed in the first line of *.mod file. */
>  	snprintf(filelist, sizeof(filelist), "%.*smod",
> -		 (int)strlen(modname) - postfix_len, modname);
> +		 (int)(strlen(modname) - strlen("prelink.o")), modname);
>  
>  	buf = read_text_file(filelist);
>  
> -- 
> 2.30.2
> 

-- 
Kees Cook

Re: [PATCH 10/13] kbuild: build modules in the same way with/without Clang LTO

[Masahiro Yamada]

On Thu, Aug 19, 2021 at 3:59 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Aug 19, 2021 at 09:57:41AM +0900, Masahiro Yamada wrote:
> > When Clang LTO is enabled, additional intermediate files *.lto.o are
> > created because LLVM bitcode must be converted to ELF before modpost.
> >
> > For non-LTO builds:
> >
> >          $(LD)             $(LD)
> >   objects ---> <modname>.o -----> <modname>.ko
> >                              |
> >           <modname>.mod.o ---/
> >
> > For Clang LTO builds:
> >
> >          $(AR)            $(LD)                 $(LD)
> >   objects ---> <modname>.o ---> <modname>.lto.o -----> <modname>.ko
> >                                                   |
> >                                 <modname>.mod.o --/
> >
> > Since the Clang LTO introduction, ugly CONFIG_LTO_CLANG conditionals
> > are sprinkled everywhere in the kbuild code.
> >
> > Another confusion for Clang LTO builds is, <modname>.o is an archive
> > that contains LLVM bitcode files. The suffix should have been .a
> > instead of .o
> >
> > To clean up the code, unify the build process of modules, as follows:
> >
> >          $(AR)            $(LD)                     $(LD)
> >   objects ---> <modname>.a ---> <modname>.prelink.o -----> <modname>.ko
> >                                                       |
> >                                 <modname>.mod.o ------/
> >
> > Here, 'objects' are either ELF or LLVM bitcode. <modname>.a is an archive,
> > <modname>.prelink.o is ELF.
>
> I like this design, but I do see that it has a small but measurable
> impact on build times:
>
> allmodconfig build, GCC:
>
> make -j72 allmodconfig
> make -j72 -s clean && time make -j72
>
>     kbuild/for-next:
>         6m16.140s
>         6m19.742s
>         6m15.848s
>
>     +this-series:
>         6m22.742s
>         6m20.589s
>         6m19.911s
>
> Thought with not so many modules, it's within the noise:
>
> defconfig build, GCC:
>
> make -j72 defconfig
> make -j72 -s clean && time make -j72
>
>     kbuild/for-next:
>         0m41.579s
>         0m41.214s
>         0m41.370s
>
>     +series:
>         0m41.423s
>         0m41.434s
>         0m41.384s
>
>
> However, I do see that even LTO builds are slightly slower now, so
> perhaps the above numbers aren't due to the added $(AR) step:
>
> allmodconfig + Clang ThinLTO:
>
> make -j72 LLVM=1 LLVM_IAS=1 allmodconfig
> ./scripts/config -d GCOV_KERNEL -d KASAN -d LTO_NONE -e LTO_CLANG_THIN
> make -j72 LLVM=1 LLVM_IAS=1 olddefconfig
> make -j72 -s LLVM=1 LLVM_IAS=1 clean && time make -j72 LLVM=1 LLVM_IAS=1
>
>     kbuild/for-next:
>         9m53.927s
>         9m45.874s
>         9m47.722s
>
>     +series:
>         9m58.395s
>         9m53.201s
>         9m56.387s



I have not tested this closely, but
perhaps this might be the cost of $(AR) t $<)

In Sami's implementation, *.symversions are merged
by shell command.
Presumably, it runs faster than llvm-ar.
Instead, it has a risk of Argument list too long
as reported in [1].

[1] https://lore.kernel.org/lkml/20210614094948.30023-1-lecopzer.chen@mediatek.com/


Anyway, when I find a time,
I will look into some bench mark.




>
>
> I haven't been able to isolate where the changes in build times are
> coming from (nor have I done link-phase-only timings -- I realize those
> are really the most important).
>
> I did notice some warnings from this patch, though, in the
> $(modules-single) target:
>
> scripts/Makefile.build:434: target 'drivers/scsi/libiscsi.a' given more than once in the same rule
> scripts/Makefile.build:434: target 'drivers/atm/suni.a' given more than once in the same rule


Ah, right.

I also noticed needless rebuilds of prelink.symversions.

In v2, I will fix as follows:


index 957addea830b..cf6b79dff5f9 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -438,6 +438,8 @@ cmd_merge_symver =                                  \
 $(obj)/%.prelink.symversions: $(obj)/%.a FORCE
        $(call if_changed,merge_symver)

+targets += $(patsubst %.a, %.prelink.symversions, $(modules))
+
 $(obj)/%.prelink.o: ld_flags += --script=$(filter %.symversions,$^)
 module-symver = $(obj)/%.prelink.symversions

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index f604d2d01cad..5074922db82d 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -107,8 +107,8 @@ real-dtb-y  := $(addprefix $(obj)/, $(real-dtb-y))
 subdir-ym      := $(addprefix $(obj)/,$(subdir-ym))

 modules                := $(patsubst %.o, %.a, $(obj-m))
-modules-multi  := $(patsubst %.o, %.a, $(multi-obj-m))
-modules-single := $(filter-out $(modules-multi), $(filter %.a, $(modules)))
+modules-multi  := $(sort $(patsubst %.o, %.a, $(multi-obj-m)))
+modules-single := $(sort $(filter-out $(modules-multi), $(filter %.a,
$(modules))))

 # Finds the multi-part object the current object will be linked into.
 # If the object belongs to two or more multi-part objects, list them all.








-- 
Best Regards
Masahiro Yamada

[PATCH 11/13] kbuild: always postpone CRC links for module versioning

[Masahiro Yamada]

When CONFIG_MODVERSIONS=y, the CRCs of EXPORT_SYMBOL are linked into
*.o files in-place.

It is impossible for Clang LTO because *.o files are not ELF, but LLVM
bitcode. The CRCs are stored in separate *.symversions files, and then
supplied to the modpost link.

Let's do so for CONFIG_LTO_CLANG=n is possible, and unify the module
versioning code.

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

 scripts/Makefile.build  | 32 ++++++--------------------------
 scripts/link-vmlinux.sh | 22 ++++++++++++++--------
 2 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 957addea830b..874e84a1f3fc 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -158,17 +158,12 @@ quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 ifdef CONFIG_MODVERSIONS
 # When module versioning is enabled the following steps are executed:
 # o compile a <file>.o from <file>.c
-# o if <file>.o doesn't contain a __ksymtab version, i.e. does
-#   not export symbols, it's done.
+# o if <file>.o doesn't contain __ksymtab* symbols, i.e. does
+#   not export symbols, create an empty *.symversions
 # o otherwise, we calculate symbol versions using the good old
 #   genksyms on the preprocessed source and postprocess them in a way
 #   that they are usable as a linker script
-# o generate .tmp_<file>.o from <file>.o using the linker to
-#   replace the unresolved symbols __crc_exported_symbol with
-#   the actual value of the checksum generated by genksyms
-# o remove .tmp_<file>.o to <file>.o
 
-ifdef CONFIG_LTO_CLANG
 # Generate .o.symversions files for each .o with exported symbols, and link these
 # to the kernel and/or modules at the end.
 cmd_modversions_c =								\
@@ -178,18 +173,6 @@ cmd_modversions_c =								\
 	else									\
 		rm -f $@.symversions;						\
 	fi;
-else
-cmd_modversions_c =								\
-	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
-		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
-		    > $(@D)/.tmp_$(@F:.o=.ver);					\
-										\
-		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
-			-T $(@D)/.tmp_$(@F:.o=.ver);				\
-		mv -f $(@D)/.tmp_$(@F) $@;					\
-		rm -f $(@D)/.tmp_$(@F:.o=.ver);					\
-	fi
-endif
 endif
 
 ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
@@ -348,12 +331,9 @@ ifdef CONFIG_ASM_MODVERSIONS
 cmd_modversions_S =								\
 	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
 		$(call cmd_gensymtypes_S,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
-		    > $(@D)/.tmp_$(@F:.o=.ver);					\
-										\
-		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
-			-T $(@D)/.tmp_$(@F:.o=.ver);				\
-		mv -f $(@D)/.tmp_$(@F) $@;					\
-		rm -f $(@D)/.tmp_$(@F:.o=.ver);					\
+		    > $@.symversions;						\
+	else									\
+		rm -rf $@.symversions;						\
 	fi
 endif
 
@@ -424,7 +404,7 @@ $(obj)/lib.a: $(lib-y) FORCE
 # Rule to prelink modules
 #
 
-ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
+ifdef CONFIG_MODVERSIONS
 
 cmd_merge_symver =					\
 	rm -f $@;					\
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 17976609c2d8..66ced6ff8e65 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -59,8 +59,7 @@ append_symversion()
 	fi
 }
 
-# If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
-# .tmp_symversions.lds
+# Collect generated symbol versions into .tmp_symversions.lds
 gen_symversions()
 {
 	info GEN .tmp_symversions.lds
@@ -94,14 +93,13 @@ modpost_link()
 		${KBUILD_VMLINUX_LIBS}				\
 		--end-group"
 
+	if [ -n "${CONFIG_MODVERSIONS}" ]; then
+		lds="${lds} -T .tmp_symversions.lds"
+	fi
+
 	if [ -n "${CONFIG_LTO_CLANG}" ]; then
 		gen_initcalls
-		lds="-T .tmp_initcalls.lds"
-
-		if [ -n "${CONFIG_MODVERSIONS}" ]; then
-			gen_symversions
-			lds="${lds} -T .tmp_symversions.lds"
-		fi
+		lds="${lds} -T .tmp_initcalls.lds"
 
 		# This might take a while, so indicate that we're doing
 		# an LTO link
@@ -198,6 +196,10 @@ vmlinux_link()
 
 	ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
 
+	if [ -n "${CONFIG_MODVERSIONS}" ]; then
+		ldflags="${ldflags} ${wl}--script=.tmp_symversions.lds"
+	fi
+
 	# The kallsyms linking does not need debug symbols included.
 	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
 		ldflags="${ldflags} ${wl}--strip-debug"
@@ -351,6 +353,10 @@ fi;
 # final build of init/
 ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1
 
+if [ -n "${CONFIG_MODVERSIONS}" ]; then
+	gen_symversions
+fi
+
 #link vmlinux.o
 modpost_link vmlinux.o
 objtool_link vmlinux.o
-- 
2.30.2

Re: [PATCH 11/13] kbuild: always postpone CRC links for module versioning

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:42AM +0900, Masahiro Yamada wrote:
> When CONFIG_MODVERSIONS=y, the CRCs of EXPORT_SYMBOL are linked into
> *.o files in-place.
> 
> It is impossible for Clang LTO because *.o files are not ELF, but LLVM
> bitcode. The CRCs are stored in separate *.symversions files, and then
> supplied to the modpost link.
> 
> Let's do so for CONFIG_LTO_CLANG=n is possible, and unify the module
> versioning code.

I worry about this also now being "unparallelized", but it is a nice
cleanup.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build  | 32 ++++++--------------------------
>  scripts/link-vmlinux.sh | 22 ++++++++++++++--------
>  2 files changed, 20 insertions(+), 34 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 957addea830b..874e84a1f3fc 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -158,17 +158,12 @@ quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
>  ifdef CONFIG_MODVERSIONS
>  # When module versioning is enabled the following steps are executed:
>  # o compile a <file>.o from <file>.c
> -# o if <file>.o doesn't contain a __ksymtab version, i.e. does
> -#   not export symbols, it's done.
> +# o if <file>.o doesn't contain __ksymtab* symbols, i.e. does
> +#   not export symbols, create an empty *.symversions
>  # o otherwise, we calculate symbol versions using the good old
>  #   genksyms on the preprocessed source and postprocess them in a way
>  #   that they are usable as a linker script
> -# o generate .tmp_<file>.o from <file>.o using the linker to
> -#   replace the unresolved symbols __crc_exported_symbol with
> -#   the actual value of the checksum generated by genksyms
> -# o remove .tmp_<file>.o to <file>.o
>  
> -ifdef CONFIG_LTO_CLANG
>  # Generate .o.symversions files for each .o with exported symbols, and link these
>  # to the kernel and/or modules at the end.
>  cmd_modversions_c =								\
> @@ -178,18 +173,6 @@ cmd_modversions_c =								\
>  	else									\
>  		rm -f $@.symversions;						\
>  	fi;
> -else
> -cmd_modversions_c =								\
> -	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
> -		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
> -		    > $(@D)/.tmp_$(@F:.o=.ver);					\
> -										\
> -		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
> -			-T $(@D)/.tmp_$(@F:.o=.ver);				\
> -		mv -f $(@D)/.tmp_$(@F) $@;					\
> -		rm -f $(@D)/.tmp_$(@F:.o=.ver);					\
> -	fi
> -endif
>  endif
>  
>  ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> @@ -348,12 +331,9 @@ ifdef CONFIG_ASM_MODVERSIONS
>  cmd_modversions_S =								\
>  	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
>  		$(call cmd_gensymtypes_S,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
> -		    > $(@D)/.tmp_$(@F:.o=.ver);					\
> -										\
> -		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
> -			-T $(@D)/.tmp_$(@F:.o=.ver);				\
> -		mv -f $(@D)/.tmp_$(@F) $@;					\
> -		rm -f $(@D)/.tmp_$(@F:.o=.ver);					\
> +		    > $@.symversions;						\
> +	else									\
> +		rm -rf $@.symversions;						\
>  	fi
>  endif
>  
> @@ -424,7 +404,7 @@ $(obj)/lib.a: $(lib-y) FORCE
>  # Rule to prelink modules
>  #
>  
> -ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
> +ifdef CONFIG_MODVERSIONS
>  
>  cmd_merge_symver =					\
>  	rm -f $@;					\
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 17976609c2d8..66ced6ff8e65 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -59,8 +59,7 @@ append_symversion()
>  	fi
>  }
>  
> -# If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
> -# .tmp_symversions.lds
> +# Collect generated symbol versions into .tmp_symversions.lds
>  gen_symversions()
>  {
>  	info GEN .tmp_symversions.lds
> @@ -94,14 +93,13 @@ modpost_link()
>  		${KBUILD_VMLINUX_LIBS}				\
>  		--end-group"
>  
> +	if [ -n "${CONFIG_MODVERSIONS}" ]; then
> +		lds="${lds} -T .tmp_symversions.lds"
> +	fi
> +
>  	if [ -n "${CONFIG_LTO_CLANG}" ]; then
>  		gen_initcalls
> -		lds="-T .tmp_initcalls.lds"
> -
> -		if [ -n "${CONFIG_MODVERSIONS}" ]; then
> -			gen_symversions
> -			lds="${lds} -T .tmp_symversions.lds"
> -		fi
> +		lds="${lds} -T .tmp_initcalls.lds"
>  
>  		# This might take a while, so indicate that we're doing
>  		# an LTO link
> @@ -198,6 +196,10 @@ vmlinux_link()
>  
>  	ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
>  
> +	if [ -n "${CONFIG_MODVERSIONS}" ]; then
> +		ldflags="${ldflags} ${wl}--script=.tmp_symversions.lds"
> +	fi
> +
>  	# The kallsyms linking does not need debug symbols included.
>  	if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
>  		ldflags="${ldflags} ${wl}--strip-debug"
> @@ -351,6 +353,10 @@ fi;
>  # final build of init/
>  ${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1
>  
> +if [ -n "${CONFIG_MODVERSIONS}" ]; then
> +	gen_symversions
> +fi
> +
>  #link vmlinux.o
>  modpost_link vmlinux.o
>  objtool_link vmlinux.o
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 12/13] kbuild: merge cmd_modversions_c and cmd_modversions_S

[Masahiro Yamada]

Now cmd_modversions_c and cmd_modversions_S are similar.

The latter uses $(OBJDUMP) -h, but it can be replaced with $(NM).

$(NM) works for both ELF and LLVM bitcode (if $(NM) is llvm-nm).

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

 scripts/Makefile.build | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 874e84a1f3fc..97392c26ebd7 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -166,13 +166,16 @@ ifdef CONFIG_MODVERSIONS
 
 # Generate .o.symversions files for each .o with exported symbols, and link these
 # to the kernel and/or modules at the end.
-cmd_modversions_c =								\
+cmd_modversions =								\
 	if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then			\
-		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
+		$(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
 		    > $@.symversions;						\
 	else									\
 		rm -f $@.symversions;						\
 	fi;
+
+cmd_modversions_c = $(call cmd_modversions,c)
+
 endif
 
 ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
@@ -327,14 +330,8 @@ ifdef CONFIG_ASM_MODVERSIONS
 
 # versioning matches the C process described above, with difference that
 # we parse asm-prototypes.h C header to get function definitions.
+cmd_modversions_S = $(call cmd_modversions,S)
 
-cmd_modversions_S =								\
-	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
-		$(call cmd_gensymtypes_S,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
-		    > $@.symversions;						\
-	else									\
-		rm -rf $@.symversions;						\
-	fi
 endif
 
 $(obj)/%.o: $(src)/%.S $(objtool_dep) FORCE
-- 
2.30.2

Re: [PATCH 12/13] kbuild: merge cmd_modversions_c and cmd_modversions_S

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:43AM +0900, Masahiro Yamada wrote:
> Now cmd_modversions_c and cmd_modversions_S are similar.
> 
> The latter uses $(OBJDUMP) -h, but it can be replaced with $(NM).
> 
> $(NM) works for both ELF and LLVM bitcode (if $(NM) is llvm-nm).
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Ah yeah. That's nice consolidation.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
> 
>  scripts/Makefile.build | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 874e84a1f3fc..97392c26ebd7 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -166,13 +166,16 @@ ifdef CONFIG_MODVERSIONS
>  
>  # Generate .o.symversions files for each .o with exported symbols, and link these
>  # to the kernel and/or modules at the end.
> -cmd_modversions_c =								\
> +cmd_modversions =								\
>  	if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then			\
> -		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
> +		$(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
>  		    > $@.symversions;						\
>  	else									\
>  		rm -f $@.symversions;						\
>  	fi;
> +
> +cmd_modversions_c = $(call cmd_modversions,c)
> +
>  endif
>  
>  ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> @@ -327,14 +330,8 @@ ifdef CONFIG_ASM_MODVERSIONS
>  
>  # versioning matches the C process described above, with difference that
>  # we parse asm-prototypes.h C header to get function definitions.
> +cmd_modversions_S = $(call cmd_modversions,S)
>  
> -cmd_modversions_S =								\
> -	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
> -		$(call cmd_gensymtypes_S,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
> -		    > $@.symversions;						\
> -	else									\
> -		rm -rf $@.symversions;						\
> -	fi
>  endif
>  
>  $(obj)/%.o: $(src)/%.S $(objtool_dep) FORCE
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH 13/13] kbuild: merge cmd_ar_builtin and cmd_ar_module

[Masahiro Yamada]

The difference between cmd_ar_builtin and cmd_ar_module is the
'[M]' in the short log.

Merge them into cmd_ar_thin.

$(quiet_modtag) is expanded to '[M]' when it is merging module objects.

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

 scripts/Makefile.build | 12 +++---------
 scripts/Makefile.lib   |  5 ++++-
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 97392c26ebd7..17ce37c3eceb 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -370,11 +370,8 @@ $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
 # Rule to compile a set of .o files into one .a file (without symbol table)
 #
 
-quiet_cmd_ar_builtin = AR      $@
-      cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
-
 $(obj)/built-in.a: $(real-obj-y) FORCE
-	$(call if_changed,ar_builtin)
+	$(call if_changed,ar_thin)
 
 #
 # Rule to create modules.order file
@@ -434,14 +431,11 @@ cmd_ld_o_a += ; $(objtool) $(objtool_args) --module $@
 endif
 endif
 
-quiet_cmd_ar_module = AR [M]  $@
-      cmd_ar_module = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
-
 $(modules-single): %.a: %.o FORCE
-	$(call if_changed,ar_module)
+	$(call if_changed,ar_thin)
 
 $(modules-multi): FORCE
-	$(call if_changed,ar_module)
+	$(call if_changed,ar_thin)
 $(call multi_depend, $(modules-multi), .a, -objs -y -m)
 
 targets += $(modules-single) $(modules-multi)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index f604d2d01cad..bd71a7d6fbc1 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -198,7 +198,7 @@ _cpp_flags += -I $(srctree)/$(src) -I $(objtree)/$(obj)
 endif
 endif
 
-part-of-module = $(if $(filter $(basename $@).o, $(real-obj-m)),y)
+part-of-module = $(if $(filter $(basename $@).o, $(real-obj-m) $(obj-m)),y)
 quiet_modtag = $(if $(part-of-module),[M],   )
 
 modkern_cflags =                                          \
@@ -276,6 +276,9 @@ quiet_cmd_ld = LD      $@
 quiet_cmd_ar = AR      $@
       cmd_ar = rm -f $@; $(AR) cDPrsT $@ $(real-prereqs)
 
+quiet_cmd_ar_thin = AR $(quiet_modtag)  $@
+      cmd_ar_thin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
+
 # Objcopy
 # ---------------------------------------------------------------------------
 
-- 
2.30.2

Re: [PATCH 13/13] kbuild: merge cmd_ar_builtin and cmd_ar_module

[Kees Cook]

On Thu, Aug 19, 2021 at 09:57:44AM +0900, Masahiro Yamada wrote:
> The difference between cmd_ar_builtin and cmd_ar_module is the
> '[M]' in the short log.
> 
> Merge them into cmd_ar_thin.
> 
> $(quiet_modtag) is expanded to '[M]' when it is merging module objects.

Yup, good good!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build | 12 +++---------
>  scripts/Makefile.lib   |  5 ++++-
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 97392c26ebd7..17ce37c3eceb 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -370,11 +370,8 @@ $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
>  # Rule to compile a set of .o files into one .a file (without symbol table)
>  #
>  
> -quiet_cmd_ar_builtin = AR      $@
> -      cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
> -
>  $(obj)/built-in.a: $(real-obj-y) FORCE
> -	$(call if_changed,ar_builtin)
> +	$(call if_changed,ar_thin)
>  
>  #
>  # Rule to create modules.order file
> @@ -434,14 +431,11 @@ cmd_ld_o_a += ; $(objtool) $(objtool_args) --module $@
>  endif
>  endif
>  
> -quiet_cmd_ar_module = AR [M]  $@
> -      cmd_ar_module = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
> -
>  $(modules-single): %.a: %.o FORCE
> -	$(call if_changed,ar_module)
> +	$(call if_changed,ar_thin)
>  
>  $(modules-multi): FORCE
> -	$(call if_changed,ar_module)
> +	$(call if_changed,ar_thin)
>  $(call multi_depend, $(modules-multi), .a, -objs -y -m)
>  
>  targets += $(modules-single) $(modules-multi)
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index f604d2d01cad..bd71a7d6fbc1 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -198,7 +198,7 @@ _cpp_flags += -I $(srctree)/$(src) -I $(objtree)/$(obj)
>  endif
>  endif
>  
> -part-of-module = $(if $(filter $(basename $@).o, $(real-obj-m)),y)
> +part-of-module = $(if $(filter $(basename $@).o, $(real-obj-m) $(obj-m)),y)
>  quiet_modtag = $(if $(part-of-module),[M],   )
>  
>  modkern_cflags =                                          \
> @@ -276,6 +276,9 @@ quiet_cmd_ld = LD      $@
>  quiet_cmd_ar = AR      $@
>        cmd_ar = rm -f $@; $(AR) cDPrsT $@ $(real-prereqs)
>  
> +quiet_cmd_ar_thin = AR $(quiet_modtag)  $@
> +      cmd_ar_thin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
> +
>  # Objcopy
>  # ---------------------------------------------------------------------------
>  
> -- 
> 2.30.2
> 

-- 
Kees Cook

Re: [PATCH 00/13] kbuild: refactoring after Clang LTO

[Masahiro Yamada]

On Thu, Aug 19, 2021 at 9:58 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>
> The introduction of Clang LTO, the kbuild code became much
> uglier due to CONFIG_LTO_CLANG conditionals.
>
> It is painful to maintain the messed-up code, and to review
> code changed on top of that.
>
>
>
> Masahiro Yamada (13):
>   kbuild: move objtool_args back to scripts/Makefile.build
>   gen_compile_commands: extract compiler command from a series of
>     commands
>   kbuild: detect objtool changes correctly and remove .SECONDEXPANSION
>   kbuild: remove unused quiet_cmd_update_lto_symversions
>   kbuild: remove stale *.symversions
>   kbuild: merge vmlinux_link() between the ordinary link and Clang LTO
>   kbuild: do not remove 'linux' link in scripts/link-vmlinux.sh
>   kbuild: merge vmlinux_link() between ARCH=um and other architectures
>   kbuild: do not create built-in.a.symversions or lib.a.symversions
>   kbuild: build modules in the same way with/without Clang LTO
>   kbuild: always postpone CRC links for module versioning
>   kbuild: merge cmd_modversions_c and cmd_modversions_S
>   kbuild: merge cmd_ar_builtin and cmd_ar_module
>


Patch 01-08 applied.

I will take some time for the rest.


-- 
Best Regards
Masahiro Yamada