[PATCH v2 00/13] kbuild: second round of Clang LTO refactoring

[PATCH v2 00/13] kbuild: second round of Clang LTO refactoring

[Masahiro Yamada]

Masahiro Yamada (13):
  kbuild: move objtool_args back to scripts/Makefile.build
  kbuild: rename __objtool_obj to objtool
  kbuild: store the objtool command in *.cmd files
  kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro
  kbuild: detect objtool update without using .SECONDEXPANSION
  kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules
  kbuild: do not create built-in.a.symversions or lib.a.symversions
  kbuild: build modules in the same way with/without Clang LTO
  kbuild: add cmd_and_savecmd macro
  kbuild: rebuild modules when objtool is updated for CONFIG_LTO_CLANG
  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/Kbuild.include    |   6 +-
 scripts/Makefile.build    | 207 ++++++++++++++++----------------------
 scripts/Makefile.lib      |  27 ++---
 scripts/Makefile.modfinal |   4 +-
 scripts/Makefile.modpost  |   7 +-
 scripts/link-vmlinux.sh   |  31 +++---
 scripts/merge-symvers.pl  |  52 ++++++++++
 scripts/mod/modpost.c     |   6 +-
 8 files changed, 175 insertions(+), 165 deletions(-)
 create mode 100644 scripts/merge-symvers.pl

-- 
2.30.2

[PATCH v2 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 in a separate file.

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

Remove the stale comment too.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
---

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

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3efc984d4c69..17508c0e4358 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -224,6 +224,16 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
 endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 
 ifdef CONFIG_STACK_VALIDATION
+
+objtool_args =								\
+	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
+	$(if $(part-of-module), --module)				\
+	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
+	$(if $(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 cd011f3f6f78..34c4c11c4bc1 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -232,17 +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 $(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 v2 01/13] kbuild: move objtool_args back to scripts/Makefile.build

[Josh Poimboeuf]

On Tue, Aug 31, 2021 at 04:39:52PM +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 in a separate file.
> 
> Get it back to the original place, close to the objtool command.
> 
> Remove the stale comment too.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Reviewed-by: Kees Cook <keescook@chromium.org>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[PATCH v2 02/13] kbuild: rename __objtool_obj to objtool

[Masahiro Yamada]

Rename __objtool_obj to objtool, and move it out of the
'ifndef CONFIG_LTO_CLANG' conditional, so it can be used for
cmd_cc_lto_link_modules as well.

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

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

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 17508c0e4358..e78096cd396b 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -225,6 +225,8 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 
 ifdef CONFIG_STACK_VALIDATION
 
+objtool := $(objtree)/tools/objtool/objtool
+
 objtool_args =								\
 	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
 	$(if $(part-of-module), --module)				\
@@ -236,17 +238,15 @@ 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) $(objtool_args) $@)
 objtool_obj = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	$(__objtool_obj))
+	$(objtool))
 
 endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
@@ -300,8 +300,7 @@ cmd_cc_lto_link_modules =						\
 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 $@
+cmd_cc_lto_link_modules += ; $(objtool) $(objtool_args) --module $@
 endif
 
 $(obj)/%.lto.o: $(obj)/%.o FORCE
-- 
2.30.2

Re: [PATCH v2 02/13] kbuild: rename __objtool_obj to objtool

[Nick Desaulniers]

On Tue, Aug 31, 2021 at 12:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Rename __objtool_obj to objtool, and move it out of the
> 'ifndef CONFIG_LTO_CLANG' conditional, so it can be used for
> cmd_cc_lto_link_modules as well.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

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

> ---
>
>  scripts/Makefile.build | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 17508c0e4358..e78096cd396b 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -225,6 +225,8 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>
>  ifdef CONFIG_STACK_VALIDATION
>
> +objtool := $(objtree)/tools/objtool/objtool
> +
>  objtool_args =                                                         \
>         $(if $(CONFIG_UNWINDER_ORC),orc generate,check)                 \
>         $(if $(part-of-module), --module)                               \
> @@ -236,17 +238,15 @@ 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) $(objtool_args) $@)
>  objtool_obj = $(if $(patsubst y%,, \
>         $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> -       $(__objtool_obj))
> +       $(objtool))
>
>  endif # CONFIG_LTO_CLANG
>  endif # CONFIG_STACK_VALIDATION
> @@ -300,8 +300,7 @@ cmd_cc_lto_link_modules =                                           \
>  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 $@
> +cmd_cc_lto_link_modules += ; $(objtool) $(objtool_args) --module $@
>  endif
>
>  $(obj)/%.lto.o: $(obj)/%.o FORCE
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 02/13] kbuild: rename __objtool_obj to objtool

[Kees Cook]

On Tue, Aug 31, 2021 at 04:39:53PM +0900, Masahiro Yamada wrote:
> Rename __objtool_obj to objtool, and move it out of the
> 'ifndef CONFIG_LTO_CLANG' conditional, so it can be used for
> cmd_cc_lto_link_modules as well.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Yup, seems good.

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

-- 
Kees Cook

Re: [PATCH v2 02/13] kbuild: rename __objtool_obj to objtool

[Josh Poimboeuf]

On Tue, Aug 31, 2021 at 04:39:53PM +0900, Masahiro Yamada wrote:
> Rename __objtool_obj to objtool, and move it out of the
> 'ifndef CONFIG_LTO_CLANG' conditional, so it can be used for
> cmd_cc_lto_link_modules as well.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[PATCH v2 03/13] kbuild: store the objtool command in *.cmd files

[Masahiro Yamada]

objtool_dep includes include/config/{ORC_UNWINDER,STACK_VALIDATION}
so that all the objects are rebuilt when any of CONFIG_ORC_UNWINDER
and CONFIG_STACK_VALIDATION is toggled.

As you can see in 'objtool_args', there are more CONFIG options
that affect the objtool command line.

Adding more and more include/config/* is ugly and unmaintainable.

Another issue is that non-standard objects are needlessly rebuilt.
Objects specified as OBJECT_FILES_NON_STANDARD is not processed by
objtool, but they are rebuilt anyway when CONFIG_ORC_UNWINDER or
CONFIG_STACK_VALIDATION is toggled. This is not a big deal, but
better to fix.

A cleaner and more precise 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 | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index e78096cd396b..021ae0146913 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:
@@ -243,7 +243,7 @@ ifndef CONFIG_LTO_CLANG
 # '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) $(objtool_args) $@)
+	; $(objtool) $(objtool_args) $@)
 objtool_obj = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
 	$(objtool))
@@ -251,10 +251,8 @@ objtool_obj = $(if $(patsubst y%,, \
 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)
+# Rebuild all objects when objtool changes
+objtool_dep = $(objtool_obj)
 
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 cmd_gen_ksymdeps = \
@@ -269,7 +267,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
@@ -277,7 +274,6 @@ 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
 
@@ -365,7 +361,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
 
-- 
2.30.2

Re: [PATCH v2 03/13] kbuild: store the objtool command in *.cmd files

[Nick Desaulniers]

On Tue, Aug 31, 2021 at 12:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> objtool_dep includes include/config/{ORC_UNWINDER,STACK_VALIDATION}
> so that all the objects are rebuilt when any of CONFIG_ORC_UNWINDER
> and CONFIG_STACK_VALIDATION is toggled.
>
> As you can see in 'objtool_args', there are more CONFIG options
> that affect the objtool command line.
>
> Adding more and more include/config/* is ugly and unmaintainable.
>
> Another issue is that non-standard objects are needlessly rebuilt.
> Objects specified as OBJECT_FILES_NON_STANDARD is not processed by
> objtool, but they are rebuilt anyway when CONFIG_ORC_UNWINDER or
> CONFIG_STACK_VALIDATION is toggled. This is not a big deal, but
> better to fix.
>
> A cleaner and more precise 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>

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

> ---
>
>  scripts/Makefile.build | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index e78096cd396b..021ae0146913 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:
> @@ -243,7 +243,7 @@ ifndef CONFIG_LTO_CLANG
>  # '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) $(objtool_args) $@)
> +       ; $(objtool) $(objtool_args) $@)
>  objtool_obj = $(if $(patsubst y%,, \
>         $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
>         $(objtool))
> @@ -251,10 +251,8 @@ objtool_obj = $(if $(patsubst y%,, \
>  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)
> +# Rebuild all objects when objtool changes
> +objtool_dep = $(objtool_obj)
>
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>  cmd_gen_ksymdeps = \
> @@ -269,7 +267,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
> @@ -277,7 +274,6 @@ 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
>
> @@ -365,7 +361,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
>
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 03/13] kbuild: store the objtool command in *.cmd files

[Kees Cook]

On Tue, Aug 31, 2021 at 04:39:54PM +0900, Masahiro Yamada wrote:
> objtool_dep includes include/config/{ORC_UNWINDER,STACK_VALIDATION}
> so that all the objects are rebuilt when any of CONFIG_ORC_UNWINDER
> and CONFIG_STACK_VALIDATION is toggled.
> 
> As you can see in 'objtool_args', there are more CONFIG options
> that affect the objtool command line.
> 
> Adding more and more include/config/* is ugly and unmaintainable.
> 
> Another issue is that non-standard objects are needlessly rebuilt.
> Objects specified as OBJECT_FILES_NON_STANDARD is not processed by
> objtool, but they are rebuilt anyway when CONFIG_ORC_UNWINDER or
> CONFIG_STACK_VALIDATION is toggled. This is not a big deal, but
> better to fix.
> 
> A cleaner and more precise 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>

Yeah, nice solution for this.

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

-- 
Kees Cook

Re: [PATCH v2 03/13] kbuild: store the objtool command in *.cmd files

[Josh Poimboeuf]

On Tue, Aug 31, 2021 at 04:39:54PM +0900, Masahiro Yamada wrote:
> objtool_dep includes include/config/{ORC_UNWINDER,STACK_VALIDATION}
> so that all the objects are rebuilt when any of CONFIG_ORC_UNWINDER
> and CONFIG_STACK_VALIDATION is toggled.
> 
> As you can see in 'objtool_args', there are more CONFIG options
> that affect the objtool command line.
> 
> Adding more and more include/config/* is ugly and unmaintainable.
> 
> Another issue is that non-standard objects are needlessly rebuilt.
> Objects specified as OBJECT_FILES_NON_STANDARD is not processed by
> objtool, but they are rebuilt anyway when CONFIG_ORC_UNWINDER or
> CONFIG_STACK_VALIDATION is toggled. This is not a big deal, but
> better to fix.
> 
> A cleaner and more precise fix is to include the objtool command in
> *.cmd files so any command change is naturally detected by if_change.

Nice improvement, thanks!

s/CONFIG_ORC_UNWINDER/CONFIG_UNWINDER_ORC/g

And yes, this means the original ORC unwinder dependency didn't
work:

> -objtool_dep = $(objtool_obj)					\
> -	      $(wildcard include/config/ORC_UNWINDER		\
> -			 include/config/STACK_VALIDATION)


-- 
Josh

Re: [PATCH v2 03/13] kbuild: store the objtool command in *.cmd files

[Josh Poimboeuf]

On Sat, Sep 04, 2021 at 11:04:37AM -0700, Josh Poimboeuf wrote:
> On Tue, Aug 31, 2021 at 04:39:54PM +0900, Masahiro Yamada wrote:
> > objtool_dep includes include/config/{ORC_UNWINDER,STACK_VALIDATION}
> > so that all the objects are rebuilt when any of CONFIG_ORC_UNWINDER
> > and CONFIG_STACK_VALIDATION is toggled.
> > 
> > As you can see in 'objtool_args', there are more CONFIG options
> > that affect the objtool command line.
> > 
> > Adding more and more include/config/* is ugly and unmaintainable.
> > 
> > Another issue is that non-standard objects are needlessly rebuilt.
> > Objects specified as OBJECT_FILES_NON_STANDARD is not processed by
> > objtool, but they are rebuilt anyway when CONFIG_ORC_UNWINDER or
> > CONFIG_STACK_VALIDATION is toggled. This is not a big deal, but
> > better to fix.
> > 
> > A cleaner and more precise fix is to include the objtool command in
> > *.cmd files so any command change is naturally detected by if_change.
> 
> Nice improvement, thanks!
> 
> s/CONFIG_ORC_UNWINDER/CONFIG_UNWINDER_ORC/g
> 
> And yes, this means the original ORC unwinder dependency didn't
> work:
> 
> > -objtool_dep = $(objtool_obj)					\
> > -	      $(wildcard include/config/ORC_UNWINDER		\
> > -			 include/config/STACK_VALIDATION)

With the typos fixed, and this dependency bug mentioned in the commit
log:

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

Re: [PATCH v2 03/13] kbuild: store the objtool command in *.cmd files

[Masahiro Yamada]

On Sun, Sep 5, 2021 at 3:45 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Sat, Sep 04, 2021 at 11:04:37AM -0700, Josh Poimboeuf wrote:
> > On Tue, Aug 31, 2021 at 04:39:54PM +0900, Masahiro Yamada wrote:
> > > objtool_dep includes include/config/{ORC_UNWINDER,STACK_VALIDATION}
> > > so that all the objects are rebuilt when any of CONFIG_ORC_UNWINDER
> > > and CONFIG_STACK_VALIDATION is toggled.
> > >
> > > As you can see in 'objtool_args', there are more CONFIG options
> > > that affect the objtool command line.
> > >
> > > Adding more and more include/config/* is ugly and unmaintainable.
> > >
> > > Another issue is that non-standard objects are needlessly rebuilt.
> > > Objects specified as OBJECT_FILES_NON_STANDARD is not processed by
> > > objtool, but they are rebuilt anyway when CONFIG_ORC_UNWINDER or
> > > CONFIG_STACK_VALIDATION is toggled. This is not a big deal, but
> > > better to fix.
> > >
> > > A cleaner and more precise fix is to include the objtool command in
> > > *.cmd files so any command change is naturally detected by if_change.
> >
> > Nice improvement, thanks!
> >
> > s/CONFIG_ORC_UNWINDER/CONFIG_UNWINDER_ORC/g
> >
> > And yes, this means the original ORC unwinder dependency didn't
> > work:
> >
> > > -objtool_dep = $(objtool_obj)                                       \
> > > -         $(wildcard include/config/ORC_UNWINDER            \
> > > -                    include/config/STACK_VALIDATION)
>
> With the typos fixed, and this dependency bug mentioned in the commit
> log:


Ah, nice catch.



commit 11af847446ed0d131cf24d16a7ef3d5ea7a49554
missed to adjust the dependency part.

I will update the commit log
and mention this breakage.







> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
>
> --
> Josh
>


-- 
Best Regards
Masahiro Yamada

[PATCH v2 04/13] kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro

[Masahiro Yamada]

The OBJECT_FILES_NON_STANDARD check is quite long.

Factor it out into a new macro, objtool-enabled, to not repeat it.

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

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

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 021ae0146913..720a86642f48 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -241,12 +241,12 @@ ifndef CONFIG_LTO_CLANG
 # '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) $(objtool_args) $@)
-objtool_obj = $(if $(patsubst y%,, \
-	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	$(objtool))
+
+objtool-enabled = $(if $(filter-out y%, \
+	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
+
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
+objtool_obj = $(if $(objtool-enabled), $(objtool))
 
 endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
-- 
2.30.2

Re: [PATCH v2 04/13] kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro

[Nick Desaulniers]

On Tue, Aug 31, 2021 at 12:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The OBJECT_FILES_NON_STANDARD check is quite long.
>
> Factor it out into a new macro, objtool-enabled, to not repeat it.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

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

> ---
>
>  scripts/Makefile.build | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 021ae0146913..720a86642f48 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -241,12 +241,12 @@ ifndef CONFIG_LTO_CLANG
>  # '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) $(objtool_args) $@)
> -objtool_obj = $(if $(patsubst y%,, \
> -       $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> -       $(objtool))
> +
> +objtool-enabled = $(if $(filter-out y%, \
> +       $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
> +
> +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> +objtool_obj = $(if $(objtool-enabled), $(objtool))
>
>  endif # CONFIG_LTO_CLANG
>  endif # CONFIG_STACK_VALIDATION
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 04/13] kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro

[Kees Cook]

On Tue, Aug 31, 2021 at 04:39:55PM +0900, Masahiro Yamada wrote:
> The OBJECT_FILES_NON_STANDARD check is quite long.
> 
> Factor it out into a new macro, objtool-enabled, to not repeat it.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Looks right to me.

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

-- 
Kees Cook

Re: [PATCH v2 04/13] kbuild: factor out OBJECT_FILES_NON_STANDARD check into a macro

[Josh Poimboeuf]

On Tue, Aug 31, 2021 at 04:39:55PM +0900, Masahiro Yamada wrote:
> The OBJECT_FILES_NON_STANDARD check is quite long.
> 
> Factor it out into a new macro, objtool-enabled, to not repeat it.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[PATCH v2 05/13] kbuild: detect objtool update without using .SECONDEXPANSION

[Masahiro Yamada]

Redo commit 8852c5524029 ("kbuild: Fix objtool dependency for
'OBJECT_FILES_NON_STANDARD_<obj> := n'") to add the objtool
dependency in a cleaner way.

Using .SECONDEXPANSION ends up with unreadable code due to escaped
dollars. Also, it is not efficient because the second half of
Makefile.build is parsed twice every time.

Append the objtool dependency to the *.cmd files at the build time.

This is what fixdep and gen_ksymdeps.sh already do. So, following the
same pattern seems a natural solution.

This allows us to drop $$(objtool_deps) entirely.

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

 scripts/Makefile.build | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 720a86642f48..21b55f37a23f 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -246,14 +246,11 @@ objtool-enabled = $(if $(filter-out y%, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
 
 cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
-objtool_obj = $(if $(objtool-enabled), $(objtool))
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
 
 endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
 
-# Rebuild all objects when objtool changes
-objtool_dep = $(objtool_obj)
-
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 cmd_gen_ksymdeps = \
 	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
@@ -267,6 +264,7 @@ define rule_cc_o_c
 	$(call cmd,gen_ksymdeps)
 	$(call cmd,checksrc)
 	$(call cmd,checkdoc)
+	$(call cmd,gen_objtooldep)
 	$(call cmd,modversions_c)
 	$(call cmd,record_mcount)
 endef
@@ -274,12 +272,12 @@ endef
 define rule_as_o_S
 	$(call cmd_and_fixdep,as_o_S)
 	$(call cmd,gen_ksymdeps)
+	$(call cmd,gen_objtooldep)
 	$(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) FORCE
 	$(call if_changed_rule,cc_o_c)
 	$(call cmd,force_checksrc)
 
@@ -380,7 +378,7 @@ cmd_modversions_S =								\
 	fi
 endif
 
-$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.S FORCE
 	$(call if_changed_rule,as_o_S)
 
 targets += $(filter-out $(subdir-builtin), $(real-obj-y))
-- 
2.30.2

Re: [PATCH v2 05/13] kbuild: detect objtool update without using .SECONDEXPANSION

[Kees Cook]

On Tue, Aug 31, 2021 at 04:39:56PM +0900, Masahiro Yamada wrote:
> Redo commit 8852c5524029 ("kbuild: Fix objtool dependency for
> 'OBJECT_FILES_NON_STANDARD_<obj> := n'") to add the objtool
> dependency in a cleaner way.
> 
> Using .SECONDEXPANSION ends up with unreadable code due to escaped
> dollars. Also, it is not efficient because the second half of
> Makefile.build is parsed twice every time.
> 
> Append the objtool dependency to the *.cmd files at the build time.
> 
> This is what fixdep and gen_ksymdeps.sh already do. So, following the
> same pattern seems a natural solution.
> 
> This allows us to drop $$(objtool_deps) entirely.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Okay, so IIUC, this means objtool (and args) now ends up in the .cmd
file instead of in the Makefile dep rules? That seems reasonable.

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

-- 
Kees Cook

Re: [PATCH v2 05/13] kbuild: detect objtool update without using .SECONDEXPANSION

[Masahiro Yamada]

On Wed, Sep 1, 2021 at 2:32 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Aug 31, 2021 at 04:39:56PM +0900, Masahiro Yamada wrote:
> > Redo commit 8852c5524029 ("kbuild: Fix objtool dependency for
> > 'OBJECT_FILES_NON_STANDARD_<obj> := n'") to add the objtool
> > dependency in a cleaner way.
> >
> > Using .SECONDEXPANSION ends up with unreadable code due to escaped
> > dollars. Also, it is not efficient because the second half of
> > Makefile.build is parsed twice every time.
> >
> > Append the objtool dependency to the *.cmd files at the build time.
> >
> > This is what fixdep and gen_ksymdeps.sh already do. So, following the
> > same pattern seems a natural solution.
> >
> > This allows us to drop $$(objtool_deps) entirely.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> Okay, so IIUC, this means objtool (and args) now ends up in the .cmd
> file instead of in the Makefile dep rules? That seems reasonable.


Yes.

For example, after 'make defconfig all',
you can see it at the bottom line of *.cmd files.


$ tail -n 5  kernel/.smp.o.cmd
kernel/smp.o: $(deps_kernel/smp.o)

$(deps_kernel/smp.o):

kernel/smp.o: $(wildcard ./tools/objtool/objtool)





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



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 05/13] kbuild: detect objtool update without using .SECONDEXPANSION

[Nick Desaulniers]

On Tue, Aug 31, 2021 at 12:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Redo commit 8852c5524029 ("kbuild: Fix objtool dependency for
> 'OBJECT_FILES_NON_STANDARD_<obj> := n'") to add the objtool
> dependency in a cleaner way.
>
> Using .SECONDEXPANSION ends up with unreadable code due to escaped
> dollars. Also, it is not efficient because the second half of
> Makefile.build is parsed twice every time.
>
> Append the objtool dependency to the *.cmd files at the build time.
>
> This is what fixdep and gen_ksymdeps.sh already do. So, following the
> same pattern seems a natural solution.
>
> This allows us to drop $$(objtool_deps) entirely.

s/objtool_deps/objtool_dep/

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

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

You and Josh should be cc'ing each other explicitly on these kind of changes.

> ---
>
>  scripts/Makefile.build | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 720a86642f48..21b55f37a23f 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -246,14 +246,11 @@ objtool-enabled = $(if $(filter-out y%, \
>         $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
>
>  cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> -objtool_obj = $(if $(objtool-enabled), $(objtool))
> +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
>
>  endif # CONFIG_LTO_CLANG
>  endif # CONFIG_STACK_VALIDATION
>
> -# Rebuild all objects when objtool changes
> -objtool_dep = $(objtool_obj)
> -
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>  cmd_gen_ksymdeps = \
>         $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
> @@ -267,6 +264,7 @@ define rule_cc_o_c
>         $(call cmd,gen_ksymdeps)
>         $(call cmd,checksrc)
>         $(call cmd,checkdoc)
> +       $(call cmd,gen_objtooldep)
>         $(call cmd,modversions_c)
>         $(call cmd,record_mcount)
>  endef
> @@ -274,12 +272,12 @@ endef
>  define rule_as_o_S
>         $(call cmd_and_fixdep,as_o_S)
>         $(call cmd,gen_ksymdeps)
> +       $(call cmd,gen_objtooldep)
>         $(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) FORCE
>         $(call if_changed_rule,cc_o_c)
>         $(call cmd,force_checksrc)
>
> @@ -380,7 +378,7 @@ cmd_modversions_S =                                                         \
>         fi
>  endif
>
> -$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
> +$(obj)/%.o: $(src)/%.S FORCE
>         $(call if_changed_rule,as_o_S)
>
>  targets += $(filter-out $(subdir-builtin), $(real-obj-y))
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 05/13] kbuild: detect objtool update without using .SECONDEXPANSION

[Masahiro Yamada]

On Wed, Sep 1, 2021 at 2:33 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Tue, Aug 31, 2021 at 12:40 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Redo commit 8852c5524029 ("kbuild: Fix objtool dependency for
> > 'OBJECT_FILES_NON_STANDARD_<obj> := n'") to add the objtool
> > dependency in a cleaner way.
> >
> > Using .SECONDEXPANSION ends up with unreadable code due to escaped
> > dollars. Also, it is not efficient because the second half of
> > Makefile.build is parsed twice every time.
> >
> > Append the objtool dependency to the *.cmd files at the build time.
> >
> > This is what fixdep and gen_ksymdeps.sh already do. So, following the
> > same pattern seems a natural solution.
> >
> > This allows us to drop $$(objtool_deps) entirely.
>
> s/objtool_deps/objtool_dep/
>
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> You and Josh should be cc'ing each other explicitly on these kind of changes.
>


FWIW, this is the entire patch set if Josh is interested:

https://patchwork.kernel.org/project/linux-kbuild/list/?series=539621


--
Best Regards
Masahiro Yamada

Re: [PATCH v2 05/13] kbuild: detect objtool update without using .SECONDEXPANSION

[Josh Poimboeuf]

On Tue, Aug 31, 2021 at 04:39:56PM +0900, Masahiro Yamada wrote:
> Redo commit 8852c5524029 ("kbuild: Fix objtool dependency for
> 'OBJECT_FILES_NON_STANDARD_<obj> := n'") to add the objtool
> dependency in a cleaner way.
> 
> Using .SECONDEXPANSION ends up with unreadable code due to escaped
> dollars. Also, it is not efficient because the second half of
> Makefile.build is parsed twice every time.
> 
> Append the objtool dependency to the *.cmd files at the build time.
> 
> This is what fixdep and gen_ksymdeps.sh already do. So, following the
> same pattern seems a natural solution.
> 
> This allows us to drop $$(objtool_deps) entirely.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[PATCH v2 06/13] kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules

[Masahiro Yamada]

For CONFIG_LTO_CLANG=y, the objtool processing is not possible at the
compilation, hence postponed by the link time.

Reuse $(cmd_objtool) for CONFIG_LTO_CLANG=y by defining objtool-enabled
properly.

For CONFIG_LTO_CLANG=y:

  objtool-enabled is off for %.o compilation
  objtool-enabled is on  for %.lto link

For CONFIG_LTO_CLANG=n:

  objtool-enabled is on for %.o compilation
      (but, it depends on OBJECT_FILE_NON_STANDARD)

Set part-of-module := y for %.lto.o to avoid repeating --module.

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

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

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 21b55f37a23f..afc906cd7256 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -236,20 +236,26 @@ objtool_args =								\
 	$(if $(CONFIG_X86_SMAP), --uaccess)				\
 	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
 
-ifndef CONFIG_LTO_CLANG
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+
+endif # CONFIG_STACK_VALIDATION
+
+ifdef CONFIG_LTO_CLANG
+
+# Skip objtool for LLVM bitcode
+$(obj)/%o: objtool-enabled :=
+
+else
 
 # '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
 
-objtool-enabled = $(if $(filter-out y%, \
+$(obj)/%o: objtool-enabled = $(if $(filter-out y%, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
 
-cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
-cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
-
-endif # CONFIG_LTO_CLANG
-endif # CONFIG_STACK_VALIDATION
+endif
 
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 cmd_gen_ksymdeps = \
@@ -289,13 +295,13 @@ 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,$^)
+		--whole-archive $(filter-out FORCE,$^)			\
+		$(cmd_objtool)
 
-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 += ; $(objtool) $(objtool_args) --module $@
-endif
+$(obj)/%.lto.o: objtool-enabled = y
+$(obj)/%.lto.o: part-of-module := y
 
 $(obj)/%.lto.o: $(obj)/%.o FORCE
 	$(call if_changed,cc_lto_link_modules)
-- 
2.30.2

Re: [PATCH v2 06/13] kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules

[Kees Cook]

On Tue, Aug 31, 2021 at 04:39:57PM +0900, Masahiro Yamada wrote:
> For CONFIG_LTO_CLANG=y, the objtool processing is not possible at the
> compilation, hence postponed by the link time.
> 
> Reuse $(cmd_objtool) for CONFIG_LTO_CLANG=y by defining objtool-enabled
> properly.
> 
> For CONFIG_LTO_CLANG=y:
> 
>   objtool-enabled is off for %.o compilation
>   objtool-enabled is on  for %.lto link
> 
> For CONFIG_LTO_CLANG=n:
> 
>   objtool-enabled is on for %.o compilation
>       (but, it depends on OBJECT_FILE_NON_STANDARD)
> 
> Set part-of-module := y for %.lto.o to avoid repeating --module.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 21b55f37a23f..afc906cd7256 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -236,20 +236,26 @@ objtool_args =								\
>  	$(if $(CONFIG_X86_SMAP), --uaccess)				\
>  	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
>  
> -ifndef CONFIG_LTO_CLANG
> +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> +
> +endif # CONFIG_STACK_VALIDATION
> +
> +ifdef CONFIG_LTO_CLANG
> +
> +# Skip objtool for LLVM bitcode
> +$(obj)/%o: objtool-enabled :=

Is this intentionally "%o" instead of "%.o"? (And it later overridden by
the "%.lto.o" rule?

> +
> +else
>  
>  # '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
>  
> -objtool-enabled = $(if $(filter-out y%, \
> +$(obj)/%o: objtool-enabled = $(if $(filter-out y%, \
>  	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
>  
> -cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> -cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> -
> -endif # CONFIG_LTO_CLANG
> -endif # CONFIG_STACK_VALIDATION
> +endif
>  
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>  cmd_gen_ksymdeps = \
> @@ -289,13 +295,13 @@ 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,$^)
> +		--whole-archive $(filter-out FORCE,$^)			\
> +		$(cmd_objtool)
>  
> -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 += ; $(objtool) $(objtool_args) --module $@
> -endif
> +$(obj)/%.lto.o: objtool-enabled = y
> +$(obj)/%.lto.o: part-of-module := y
>  
>  $(obj)/%.lto.o: $(obj)/%.o FORCE
>  	$(call if_changed,cc_lto_link_modules)
> -- 
> 2.30.2
> 

-- 
Kees Cook

Re: [PATCH v2 06/13] kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules

[Masahiro Yamada]

On Wed, Sep 1, 2021 at 2:35 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Aug 31, 2021 at 04:39:57PM +0900, Masahiro Yamada wrote:
> > For CONFIG_LTO_CLANG=y, the objtool processing is not possible at the
> > compilation, hence postponed by the link time.
> >
> > Reuse $(cmd_objtool) for CONFIG_LTO_CLANG=y by defining objtool-enabled
> > properly.
> >
> > For CONFIG_LTO_CLANG=y:
> >
> >   objtool-enabled is off for %.o compilation
> >   objtool-enabled is on  for %.lto link
> >
> > For CONFIG_LTO_CLANG=n:
> >
> >   objtool-enabled is on for %.o compilation
> >       (but, it depends on OBJECT_FILE_NON_STANDARD)
> >
> > Set part-of-module := y for %.lto.o to avoid repeating --module.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/Makefile.build | 28 +++++++++++++++++-----------
> >  1 file changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 21b55f37a23f..afc906cd7256 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -236,20 +236,26 @@ objtool_args =                                                          \
> >       $(if $(CONFIG_X86_SMAP), --uaccess)                             \
> >       $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
> >
> > -ifndef CONFIG_LTO_CLANG
> > +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> > +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> > +
> > +endif # CONFIG_STACK_VALIDATION
> > +
> > +ifdef CONFIG_LTO_CLANG
> > +
> > +# Skip objtool for LLVM bitcode
> > +$(obj)/%o: objtool-enabled :=
>
> Is this intentionally "%o" instead of "%.o"?

Good catch.

No, it is not intentional.

I will fix "%o" to "%.o"


> (And it later overridden by the "%.lto.o" rule?

No, opposite.

While building %.lto.o, we want to set objtool-enabled.
But, we want to cancel it for %.o




-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 06/13] kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules

[Kees Cook]

On Fri, Sep 03, 2021 at 09:39:14AM +0900, Masahiro Yamada wrote:
> On Wed, Sep 1, 2021 at 2:35 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Tue, Aug 31, 2021 at 04:39:57PM +0900, Masahiro Yamada wrote:
> > > For CONFIG_LTO_CLANG=y, the objtool processing is not possible at the
> > > compilation, hence postponed by the link time.
> > >
> > > Reuse $(cmd_objtool) for CONFIG_LTO_CLANG=y by defining objtool-enabled
> > > properly.
> > >
> > > For CONFIG_LTO_CLANG=y:
> > >
> > >   objtool-enabled is off for %.o compilation
> > >   objtool-enabled is on  for %.lto link
> > >
> > > For CONFIG_LTO_CLANG=n:
> > >
> > >   objtool-enabled is on for %.o compilation
> > >       (but, it depends on OBJECT_FILE_NON_STANDARD)
> > >
> > > Set part-of-module := y for %.lto.o to avoid repeating --module.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > >  scripts/Makefile.build | 28 +++++++++++++++++-----------
> > >  1 file changed, 17 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > index 21b55f37a23f..afc906cd7256 100644
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -236,20 +236,26 @@ objtool_args =                                                          \
> > >       $(if $(CONFIG_X86_SMAP), --uaccess)                             \
> > >       $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
> > >
> > > -ifndef CONFIG_LTO_CLANG
> > > +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> > > +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> > > +
> > > +endif # CONFIG_STACK_VALIDATION
> > > +
> > > +ifdef CONFIG_LTO_CLANG
> > > +
> > > +# Skip objtool for LLVM bitcode
> > > +$(obj)/%o: objtool-enabled :=
> >
> > Is this intentionally "%o" instead of "%.o"?
> 
> Good catch.
> 
> No, it is not intentional.
> 
> I will fix "%o" to "%.o"

Ah-ha, okay, excellent. :) With that:

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

Thanks!

-Kees

> 
> 
> > (And it later overridden by the "%.lto.o" rule?
> 
> No, opposite.
> 
> While building %.lto.o, we want to set objtool-enabled.
> But, we want to cancel it for %.o
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada
> 
> -- 
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAK7LNATkducKiw8%3D%3Du4477JGfyb5vnvbp2gM2s9ndZ_8owXfeg%40mail.gmail.com.

-- 
Kees Cook

Re: [PATCH v2 06/13] kbuild: reuse $(cmd_objtool) for cmd_cc_lto_link_modules

[Josh Poimboeuf]

On Tue, Aug 31, 2021 at 04:39:57PM +0900, Masahiro Yamada wrote:
> For CONFIG_LTO_CLANG=y, the objtool processing is not possible at the
> compilation, hence postponed by the link time.
> 
> Reuse $(cmd_objtool) for CONFIG_LTO_CLANG=y by defining objtool-enabled
> properly.
> 
> For CONFIG_LTO_CLANG=y:
> 
>   objtool-enabled is off for %.o compilation
>   objtool-enabled is on  for %.lto link
> 
> For CONFIG_LTO_CLANG=n:
> 
>   objtool-enabled is on for %.o compilation
>       (but, it depends on OBJECT_FILE_NON_STANDARD)
> 
> Set part-of-module := y for %.lto.o to avoid repeating --module.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

With Kees' suggested fix:

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[PATCH v2 07/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
incrementally merging them in the unit of built-in.a or lib.a.

This is a preparation for further code cleanups.

The initial patch version was implemented in a shell script, but it
was slow due to the slowness of the 'cat' command [1]. This version
was implemented in Perl.

[1]: https://lore.kernel.org/lkml/CAK7LNATyNAu6sa-TT9JXy=BXr5d2Q5K-sp-mVXXtJDuJyi6_bA@mail.gmail.com/

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

 scripts/Makefile.build   | 10 ++------
 scripts/link-vmlinux.sh  |  9 ++-----
 scripts/merge-symvers.pl | 52 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 15 deletions(-)
 create mode 100644 scripts/merge-symvers.pl

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index afc906cd7256..3ad1b1227371 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -434,11 +434,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
@@ -458,11 +455,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..0cc6a03f2cb1 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -57,13 +57,8 @@ gen_initcalls()
 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
-	done
+	${PERL} scripts/merge-symvers.pl -a ${AR} -o .tmp_symversions.lds \
+		${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}
 }
 
 # Link of vmlinux.o used for section mismatch analysis
diff --git a/scripts/merge-symvers.pl b/scripts/merge-symvers.pl
new file mode 100644
index 000000000000..0bd092d24eff
--- /dev/null
+++ b/scripts/merge-symvers.pl
@@ -0,0 +1,52 @@
+#!/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;
-- 
2.30.2

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

[Kees Cook]

On Tue, Aug 31, 2021 at 04:39:58PM +0900, Masahiro Yamada wrote:
> Merge all *.o.symversions in scripts/link-vmlinux.sh instead of
> incrementally merging them in the unit of built-in.a or lib.a.
> 
> This is a preparation for further code cleanups.
> 
> The initial patch version was implemented in a shell script, but it
> was slow due to the slowness of the 'cat' command [1]. This version
> was implemented in Perl.
> 
> [1]: https://lore.kernel.org/lkml/CAK7LNATyNAu6sa-TT9JXy=BXr5d2Q5K-sp-mVXXtJDuJyi6_bA@mail.gmail.com/
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

My earlier questions about speed here were actually with regard to
non-LTO builds. But it sounds like it's not a problem there either.

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


> ---
> 
>  scripts/Makefile.build   | 10 ++------
>  scripts/link-vmlinux.sh  |  9 ++-----
>  scripts/merge-symvers.pl | 52 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 15 deletions(-)
>  create mode 100644 scripts/merge-symvers.pl
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index afc906cd7256..3ad1b1227371 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -434,11 +434,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
> @@ -458,11 +455,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..0cc6a03f2cb1 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -57,13 +57,8 @@ gen_initcalls()
>  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
> -	done
> +	${PERL} scripts/merge-symvers.pl -a ${AR} -o .tmp_symversions.lds \
> +		${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}
>  }
>  
>  # Link of vmlinux.o used for section mismatch analysis
> diff --git a/scripts/merge-symvers.pl b/scripts/merge-symvers.pl
> new file mode 100644
> index 000000000000..0bd092d24eff
> --- /dev/null
> +++ b/scripts/merge-symvers.pl
> @@ -0,0 +1,52 @@
> +#!/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;
> -- 
> 2.30.2
> 

-- 
Kees Cook

[PATCH v2 08/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, Kbuild code is complicated due to
CONFIG_LTO_CLANG conditionals sprinkled everywhere.

Another confusion for Clang LTO builds is, <modname>.o is an archive
that contains LLVM bitcode files. The suffix should be .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    | 100 +++++++++++++++++---------------------
 scripts/Makefile.lib      |  11 ++---
 scripts/Makefile.modfinal |   4 +-
 scripts/Makefile.modpost  |   7 +--
 scripts/mod/modpost.c     |   6 +--
 5 files changed, 56 insertions(+), 72 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3ad1b1227371..cdc09e9080ca 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
@@ -243,9 +241,12 @@ endif # CONFIG_STACK_VALIDATION
 
 ifdef CONFIG_LTO_CLANG
 
-# Skip objtool for LLVM bitcode
+# Skip objtool LLVM bitcode
 $(obj)/%o: objtool-enabled :=
 
+# Run objtool now that we have compiled modules into native code
+$(obj)/%.prelink.o: objtool-enabled := y
+
 else
 
 # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
@@ -255,6 +256,8 @@ else
 $(obj)/%o: objtool-enabled = $(if $(filter-out y%, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
 
+$(obj)/%.prelink.o: objtool-enabled :=
+
 endif
 
 ifdef CONFIG_TRIM_UNUSED_KSYMS
@@ -287,32 +290,12 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) 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,$^)			\
-		$(cmd_objtool)
-
-# objtool was skipped for LLVM bitcode, run it now that we have compiled
-# modules into native code
-$(obj)/%.lto.o: objtool-enabled = y
-$(obj)/%.lto.o: part-of-module := y
-
-$(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   $@
@@ -416,17 +399,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)
 #
@@ -446,10 +418,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)
 
 #
@@ -458,26 +430,44 @@ $(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.
-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,$^)
+#
+# Rule to prelink modules
+#
+
+ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
+
+cmd_merge_symver = $(PERL) scripts/merge-symvers.pl -a $(AR) -o $@ $<
+
+$(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
+
 endif
 
-$(multi-obj-m): FORCE
-	$(call if_changed,link_multi-m)
-$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
+quiet_cmd_ld_o_a = LD [M]  $@
+      cmd_ld_o_a = $(LD) $(ld_flags) -r -o $@ --whole-archive $< $(cmd_objtool)
+
+$(obj)/%.prelink.o: part-of-module := y
+
+$(obj)/%.prelink.o: $(obj)/%.a $(module-symver) FORCE
+	$(call if_changed,ld_o_a)
+
+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)
+
+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..5074922db82d 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	:= $(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.
 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 a26139aa57fd..56cd9b7a5dd0 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2000,9 +2000,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);
 	}
-- 
2.30.2

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

[Kees Cook]

On Tue, Aug 31, 2021 at 04:39:59PM +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, Kbuild code is complicated due to
> CONFIG_LTO_CLANG conditionals sprinkled everywhere.
> 
> Another confusion for Clang LTO builds is, <modname>.o is an archive
> that contains LLVM bitcode files. The suffix should be .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.

This is a good diagram and helps me understand what's happening here. Do
you think there's a place for it somewhere in the kbuild documentation?

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

My question about speed changes also applies to this, since there's now
a new step for non-LTO builds. I think you said it wasn't a meaningful
change in speed, but I think it'd be worth mentioning performance
changes in this commit message.

> ---
> 
>  scripts/Makefile.build    | 100 +++++++++++++++++---------------------
>  scripts/Makefile.lib      |  11 ++---
>  scripts/Makefile.modfinal |   4 +-
>  scripts/Makefile.modpost  |   7 +--
>  scripts/mod/modpost.c     |   6 +--
>  5 files changed, 56 insertions(+), 72 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3ad1b1227371..cdc09e9080ca 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
> @@ -243,9 +241,12 @@ endif # CONFIG_STACK_VALIDATION
>  
>  ifdef CONFIG_LTO_CLANG
>  
> -# Skip objtool for LLVM bitcode
> +# Skip objtool LLVM bitcode

Nit: needless comment change?

>  $(obj)/%o: objtool-enabled :=
>  
> +# Run objtool now that we have compiled modules into native code
> +$(obj)/%.prelink.o: objtool-enabled := y
> +
>  else
>  
>  # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
> @@ -255,6 +256,8 @@ else
>  $(obj)/%o: objtool-enabled = $(if $(filter-out y%, \
>  	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
>  
> +$(obj)/%.prelink.o: objtool-enabled :=
> +
>  endif
>  
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
> @@ -287,32 +290,12 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) 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,$^)			\
> -		$(cmd_objtool)
> -
> -# objtool was skipped for LLVM bitcode, run it now that we have compiled
> -# modules into native code
> -$(obj)/%.lto.o: objtool-enabled = y
> -$(obj)/%.lto.o: part-of-module := y
> -
> -$(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   $@
> @@ -416,17 +399,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)
>  #
> @@ -446,10 +418,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)
>  
>  #
> @@ -458,26 +430,44 @@ $(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.
> -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,$^)
> +#
> +# Rule to prelink modules
> +#
> +
> +ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
> +
> +cmd_merge_symver = $(PERL) scripts/merge-symvers.pl -a $(AR) -o $@ $<
> +
> +$(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
> +
>  endif
>  
> -$(multi-obj-m): FORCE
> -	$(call if_changed,link_multi-m)
> -$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
> +quiet_cmd_ld_o_a = LD [M]  $@
> +      cmd_ld_o_a = $(LD) $(ld_flags) -r -o $@ --whole-archive $< $(cmd_objtool)
> +
> +$(obj)/%.prelink.o: part-of-module := y
> +
> +$(obj)/%.prelink.o: $(obj)/%.a $(module-symver) FORCE
> +	$(call if_changed,ld_o_a)
> +
> +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)
> +
> +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..5074922db82d 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	:= $(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.
>  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 a26139aa57fd..56cd9b7a5dd0 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2000,9 +2000,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);
>  	}
> -- 
> 2.30.2
> 

Otherwise, looks good!

-- 
Kees Cook

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

[Nick Desaulniers]

On Tue, Aug 31, 2021 at 12:40 AM Masahiro Yamada <masahiroy@kernel.org> 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 --/

Is it worth modifying the diagram to note that objects in non-LTO
builds are <objects>.o, while for LTO builds, they are <objects>.bc?
If we're not using the .bc file suffix, can we?

>
> Since the Clang LTO introduction, Kbuild code is complicated due to
> CONFIG_LTO_CLANG conditionals sprinkled everywhere.
>
> Another confusion for Clang LTO builds is, <modname>.o is an archive
> that contains LLVM bitcode files. The suffix should be .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 ------/

And here, too.

>
> 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    | 100 +++++++++++++++++---------------------
>  scripts/Makefile.lib      |  11 ++---
>  scripts/Makefile.modfinal |   4 +-
>  scripts/Makefile.modpost  |   7 +--
>  scripts/mod/modpost.c     |   6 +--
>  5 files changed, 56 insertions(+), 72 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3ad1b1227371..cdc09e9080ca 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
> @@ -243,9 +241,12 @@ endif # CONFIG_STACK_VALIDATION
>
>  ifdef CONFIG_LTO_CLANG
>
> -# Skip objtool for LLVM bitcode
> +# Skip objtool LLVM bitcode

I agree with Kees here; drop this comment change.

>  $(obj)/%o: objtool-enabled :=
>
> +# Run objtool now that we have compiled modules into native code
> +$(obj)/%.prelink.o: objtool-enabled := y
> +
>  else
>
>  # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
> @@ -255,6 +256,8 @@ else
>  $(obj)/%o: objtool-enabled = $(if $(filter-out y%, \
>         $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
>
> +$(obj)/%.prelink.o: objtool-enabled :=

Can we use the canonical .bc file suffix for LLVM bitcode, rather than
.prelink.o?

> +
>  endif
>
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
> @@ -287,32 +290,12 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) 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,$^)                  \
> -               $(cmd_objtool)
> -
> -# objtool was skipped for LLVM bitcode, run it now that we have compiled
> -# modules into native code
> -$(obj)/%.lto.o: objtool-enabled = y
> -$(obj)/%.lto.o: part-of-module := y
> -
> -$(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   $@
> @@ -416,17 +399,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)
>  #
> @@ -446,10 +418,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)
>
>  #
> @@ -458,26 +430,44 @@ $(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.
> -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,$^)
> +#
> +# Rule to prelink modules
> +#
> +
> +ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y)
> +
> +cmd_merge_symver = $(PERL) scripts/merge-symvers.pl -a $(AR) -o $@ $<
> +
> +$(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
> +
>  endif
>
> -$(multi-obj-m): FORCE
> -       $(call if_changed,link_multi-m)
> -$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
> +quiet_cmd_ld_o_a = LD [M]  $@
> +      cmd_ld_o_a = $(LD) $(ld_flags) -r -o $@ --whole-archive $< $(cmd_objtool)
> +
> +$(obj)/%.prelink.o: part-of-module := y
> +
> +$(obj)/%.prelink.o: $(obj)/%.a $(module-symver) FORCE
> +       $(call if_changed,ld_o_a)
> +
> +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)
> +
> +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..5074922db82d 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  := $(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.
>  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 a26139aa57fd..56cd9b7a5dd0 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2000,9 +2000,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);
>         }
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers

[PATCH v2 09/13] kbuild: add cmd_and_savecmd macro

[Masahiro Yamada]

Separate out the command execution part of if_changed, as we did
for if_changed_dep.

This allows us to reuse it in if_changed_rule.

  define rule_foo
          $(call cmd_and_savecmd,foo)
          $(call cmd,bar)
  endef

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

 scripts/Kbuild.include | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index cdec22088423..d09fc14205a0 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -138,9 +138,11 @@ check-FORCE = $(if $(filter FORCE, $^),,$(warning FORCE prerequisite is missing)
 if-changed-cond = $(newer-prereqs)$(cmd-check)$(check-FORCE)
 
 # Execute command if command has changed or prerequisite(s) are updated.
-if_changed = $(if $(if-changed-cond),                                        \
+if_changed = $(if $(if-changed-cond),$(cmd_and_savecmd),@:)
+
+cmd_and_savecmd =                                                            \
 	$(cmd);                                                              \
-	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
 
 # Execute the command and also postprocess generated .d dependencies file.
 if_changed_dep = $(if $(if-changed-cond),$(cmd_and_fixdep),@:)
-- 
2.30.2

Re: [PATCH v2 09/13] kbuild: add cmd_and_savecmd macro

[Kees Cook]

On Tue, Aug 31, 2021 at 04:40:00PM +0900, Masahiro Yamada wrote:
> Separate out the command execution part of if_changed, as we did
> for if_changed_dep.
> 
> This allows us to reuse it in if_changed_rule.
> 
>   define rule_foo
>           $(call cmd_and_savecmd,foo)
>           $(call cmd,bar)
>   endef
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

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

-- 
Kees Cook

[PATCH v2 10/13] kbuild: rebuild modules when objtool is updated for CONFIG_LTO_CLANG

[Masahiro Yamada]

We rebuilt objects when objtool was updated, but only for non LTO
builds.

For CONFIG_LTO_CLANG, the objtool step is postponed by the link time,
and nothing happens even if objtool is updated.

Add the proper objtool dependency to the pre-modpost module link
for CONFIG_LTO_CLANG.

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

 scripts/Makefile.build | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index cdc09e9080ca..b94dfc87b7fa 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -451,10 +451,15 @@ endif
 quiet_cmd_ld_o_a = LD [M]  $@
       cmd_ld_o_a = $(LD) $(ld_flags) -r -o $@ --whole-archive $< $(cmd_objtool)
 
+define rule_ld_o_a
+	$(call cmd_and_savecmd,ld_o_a)
+	$(call cmd,gen_objtooldep)
+endef
+
 $(obj)/%.prelink.o: part-of-module := y
 
 $(obj)/%.prelink.o: $(obj)/%.a $(module-symver) FORCE
-	$(call if_changed,ld_o_a)
+	$(call if_changed_rule,ld_o_a)
 
 quiet_cmd_ar_module = AR [M]  $@
       cmd_ar_module = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
-- 
2.30.2

Re: [PATCH v2 10/13] kbuild: rebuild modules when objtool is updated for CONFIG_LTO_CLANG

[Kees Cook]

On Tue, Aug 31, 2021 at 04:40:01PM +0900, Masahiro Yamada wrote:
> We rebuilt objects when objtool was updated, but only for non LTO
> builds.
> 
> For CONFIG_LTO_CLANG, the objtool step is postponed by the link time,
> and nothing happens even if objtool is updated.
> 
> Add the proper objtool dependency to the pre-modpost module link
> for CONFIG_LTO_CLANG.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

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

-- 
Kees Cook

Re: [PATCH v2 10/13] kbuild: rebuild modules when objtool is updated for CONFIG_LTO_CLANG

[Josh Poimboeuf]

On Tue, Aug 31, 2021 at 04:40:01PM +0900, Masahiro Yamada wrote:
> We rebuilt objects when objtool was updated, but only for non LTO
> builds.
> 
> For CONFIG_LTO_CLANG, the objtool step is postponed by the link time,
> and nothing happens even if objtool is updated.
> 
> Add the proper objtool dependency to the pre-modpost module link
> for CONFIG_LTO_CLANG.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Nice!

Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[PATCH v2 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, and unify the module versioning code.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.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 b94dfc87b7fa..50a6765c9a14 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
@@ -358,12 +341,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
 
@@ -434,7 +414,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 = $(PERL) scripts/merge-symvers.pl -a $(AR) -o $@ $<
 
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 0cc6a03f2cb1..366af3a9d039 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -52,8 +52,7 @@ gen_initcalls()
 		> .tmp_initcalls.lds
 }
 
-# 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
@@ -75,14 +74,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
@@ -179,6 +177,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"
@@ -332,6 +334,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 v2 11/13] kbuild: always postpone CRC links for module versioning

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on kbuild/for-next]
[also build test ERROR on next-20210901]
[cannot apply to linux/master linus/master v5.14]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-second-round-of-Clang-LTO-refactoring/20210831-200741
base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/b74c890622cee1713210b7995932a66cb6ad52b7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Masahiro-Yamada/kbuild-second-round-of-Clang-LTO-refactoring/20210831-200741
        git checkout b74c890622cee1713210b7995932a66cb6ad52b7
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> Can't open perl script "scripts/merge-symvers.pl": No such file or directory

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 70215 bytes --]

[PATCH v2 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>
Reviewed-by: Kees Cook <keescook@chromium.org>
---

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

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 50a6765c9a14..4d12f83389ce 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
@@ -337,14 +340,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 FORCE
-- 
2.30.2

[PATCH v2 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>
Reviewed-by: Kees Cook <keescook@chromium.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 4d12f83389ce..0000b3988464 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -380,11 +380,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
@@ -438,14 +435,11 @@ $(obj)/%.prelink.o: part-of-module := y
 $(obj)/%.prelink.o: $(obj)/%.a $(module-symver) FORCE
 	$(call if_changed_rule,ld_o_a)
 
-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 5074922db82d..cc6ff7ffb39d 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