| Message ID | 20210831074004.3195284-4-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 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 >
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>
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)
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>
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 >
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
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(-)