| Message ID | 20210831074004.3195284-6-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: second round of Clang LTO refactoring | expand |
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>
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 >
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
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
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>
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))
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(-)