| Message ID | 20210816180519.1021311-1-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] kbuild: Fix TRIM_UNUSED_KSYMS with LTO_CLANG | expand |
On Tue, Aug 17, 2021 at 3:05 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > With CONFIG_LTO_CLANG, we currently link modules into native > code just before modpost, which means with TRIM_UNUSED_KSYMS > enabled, we still look at the LLVM bitcode in the .o files when > generating the list of used symbols. As the bitcode doesn't > yet have calls to compiler intrinsics and llvm-nm doesn't see > function references that only exist in function-level inline > assembly, we currently need a whitelist for TRIM_UNUSED_KSYMS to > work with LTO. > > This change moves module LTO linking to happen earlier, and > thus avoids the issue with LLVM bitcode and TRIM_UNUSED_KSYMS > entirely, allowing us to also drop the whitelist from > gen_autoksyms.sh. > > Link: https://github.com/ClangBuiltLinux/linux/issues/1369 > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > Reviewed-by: Alexander Lobakin <alobakin@pm.me> > Tested-by: Alexander Lobakin <alobakin@pm.me> > --- > Changes in v4: > - Added .lto.o to targets to actually fix the use of if_changed. > - Replaced an unnecessary mod-prelink-ext with .lto. > > Changes in v3: > - Added missing FORCE. > > Changes in v2: > - Fixed a couple of typos. > - Fixed objtool arguments for .lto.o to always include --module. > > --- > scripts/Makefile.build | 28 +++++++++++++++++++++++++++- > scripts/Makefile.lib | 7 +++++++ > scripts/Makefile.modfinal | 21 ++------------------- > scripts/Makefile.modpost | 22 +++------------------- > scripts/gen_autoksyms.sh | 12 ------------ > 5 files changed, 39 insertions(+), 51 deletions(-) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 02197cb8e3a7..a6f43afd09bb 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -88,6 +88,10 @@ 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 > + > ifdef need-modorder > targets-for-modules += $(obj)/modules.order > endif > @@ -271,12 +275,34 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE > $(call if_changed_rule,cc_o_c) > $(call cmd,force_checksrc) > > +ifdef CONFIG_LTO_CLANG > +# Module .o files may contain LLVM bitcode, compile them into native code > +# before ELF processing > +quiet_cmd_cc_lto_link_modules = LTO [M] $@ > +cmd_cc_lto_link_modules = \ > + $(LD) $(ld_flags) -r -o $@ \ > + $(shell [ -s $(@:.lto.o=.o.symversions) ] && \ > + echo -T $(@:.lto.o=.o.symversions)) \ > + --whole-archive $(filter-out FORCE,$^) > + > +ifdef CONFIG_STACK_VALIDATION > +# objtool was skipped for LLVM bitcode, run it now that we have compiled > +# modules into native code > +cmd_cc_lto_link_modules += ; \ > + $(objtree)/tools/objtool/objtool $(objtool_args) --module \ > + $(@:.ko=.lto.o) What is this "$(@:.ko=.lto.o)" doing ? The target is already suffixed with .lto.o so $(@:.ko=.lto.o) should be the same as $@ Shall I fix it locally unless I find more questionable code? > +endif > + > +$(obj)/%.lto.o: $(obj)/%.o FORCE > + $(call if_changed,cc_lto_link_modules) > +endif > +
On Tue, Aug 17, 2021 at 7:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Tue, Aug 17, 2021 at 3:05 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > > > With CONFIG_LTO_CLANG, we currently link modules into native > > code just before modpost, which means with TRIM_UNUSED_KSYMS > > enabled, we still look at the LLVM bitcode in the .o files when > > generating the list of used symbols. As the bitcode doesn't > > yet have calls to compiler intrinsics and llvm-nm doesn't see > > function references that only exist in function-level inline > > assembly, we currently need a whitelist for TRIM_UNUSED_KSYMS to > > work with LTO. > > > > This change moves module LTO linking to happen earlier, and > > thus avoids the issue with LLVM bitcode and TRIM_UNUSED_KSYMS > > entirely, allowing us to also drop the whitelist from > > gen_autoksyms.sh. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/1369 > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > Reviewed-by: Alexander Lobakin <alobakin@pm.me> > > Tested-by: Alexander Lobakin <alobakin@pm.me> > > --- > > Changes in v4: > > - Added .lto.o to targets to actually fix the use of if_changed. > > - Replaced an unnecessary mod-prelink-ext with .lto. > > > > Changes in v3: > > - Added missing FORCE. > > > > Changes in v2: > > - Fixed a couple of typos. > > - Fixed objtool arguments for .lto.o to always include --module. > > > > --- > > scripts/Makefile.build | 28 +++++++++++++++++++++++++++- > > scripts/Makefile.lib | 7 +++++++ > > scripts/Makefile.modfinal | 21 ++------------------- > > scripts/Makefile.modpost | 22 +++------------------- > > scripts/gen_autoksyms.sh | 12 ------------ > > 5 files changed, 39 insertions(+), 51 deletions(-) > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > > index 02197cb8e3a7..a6f43afd09bb 100644 > > --- a/scripts/Makefile.build > > +++ b/scripts/Makefile.build > > @@ -88,6 +88,10 @@ 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 > > + > > ifdef need-modorder > > targets-for-modules += $(obj)/modules.order > > endif > > @@ -271,12 +275,34 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE > > $(call if_changed_rule,cc_o_c) > > $(call cmd,force_checksrc) > > > > +ifdef CONFIG_LTO_CLANG > > +# Module .o files may contain LLVM bitcode, compile them into native code > > +# before ELF processing > > +quiet_cmd_cc_lto_link_modules = LTO [M] $@ > > +cmd_cc_lto_link_modules = \ > > + $(LD) $(ld_flags) -r -o $@ \ > > + $(shell [ -s $(@:.lto.o=.o.symversions) ] && \ > > + echo -T $(@:.lto.o=.o.symversions)) \ > > + --whole-archive $(filter-out FORCE,$^) > > + > > +ifdef CONFIG_STACK_VALIDATION > > +# objtool was skipped for LLVM bitcode, run it now that we have compiled > > +# modules into native code > > +cmd_cc_lto_link_modules += ; \ > > + $(objtree)/tools/objtool/objtool $(objtool_args) --module \ > > + $(@:.ko=.lto.o) > > > What is this "$(@:.ko=.lto.o)" doing ? > > The target is already suffixed with .lto.o > so $(@:.ko=.lto.o) should be the same as $@ Good catch, probably a leftover from an earlier iteration. > Shall I fix it locally unless > I find more questionable code? Please do. Sami
On Thu, Aug 19, 2021 at 12:06 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > On Tue, Aug 17, 2021 at 7:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Tue, Aug 17, 2021 at 3:05 AM Sami Tolvanen <samitolvanen@google.com> wrote: > > > > > > With CONFIG_LTO_CLANG, we currently link modules into native > > > code just before modpost, which means with TRIM_UNUSED_KSYMS > > > enabled, we still look at the LLVM bitcode in the .o files when > > > generating the list of used symbols. As the bitcode doesn't > > > yet have calls to compiler intrinsics and llvm-nm doesn't see > > > function references that only exist in function-level inline > > > assembly, we currently need a whitelist for TRIM_UNUSED_KSYMS to > > > work with LTO. > > > > > > This change moves module LTO linking to happen earlier, and > > > thus avoids the issue with LLVM bitcode and TRIM_UNUSED_KSYMS > > > entirely, allowing us to also drop the whitelist from > > > gen_autoksyms.sh. > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/1369 > > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > > > Reviewed-by: Alexander Lobakin <alobakin@pm.me> > > > Tested-by: Alexander Lobakin <alobakin@pm.me> > > > --- > > > Changes in v4: > > > - Added .lto.o to targets to actually fix the use of if_changed. > > > - Replaced an unnecessary mod-prelink-ext with .lto. > > > > > > Changes in v3: > > > - Added missing FORCE. > > > > > > Changes in v2: > > > - Fixed a couple of typos. > > > - Fixed objtool arguments for .lto.o to always include --module. > > > > > > --- > > > scripts/Makefile.build | 28 +++++++++++++++++++++++++++- > > > scripts/Makefile.lib | 7 +++++++ > > > scripts/Makefile.modfinal | 21 ++------------------- > > > scripts/Makefile.modpost | 22 +++------------------- > > > scripts/gen_autoksyms.sh | 12 ------------ > > > 5 files changed, 39 insertions(+), 51 deletions(-) > > > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > > > index 02197cb8e3a7..a6f43afd09bb 100644 > > > --- a/scripts/Makefile.build > > > +++ b/scripts/Makefile.build > > > @@ -88,6 +88,10 @@ 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 > > > + > > > ifdef need-modorder > > > targets-for-modules += $(obj)/modules.order > > > endif > > > @@ -271,12 +275,34 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE > > > $(call if_changed_rule,cc_o_c) > > > $(call cmd,force_checksrc) > > > > > > +ifdef CONFIG_LTO_CLANG > > > +# Module .o files may contain LLVM bitcode, compile them into native code > > > +# before ELF processing > > > +quiet_cmd_cc_lto_link_modules = LTO [M] $@ > > > +cmd_cc_lto_link_modules = \ > > > + $(LD) $(ld_flags) -r -o $@ \ > > > + $(shell [ -s $(@:.lto.o=.o.symversions) ] && \ > > > + echo -T $(@:.lto.o=.o.symversions)) \ > > > + --whole-archive $(filter-out FORCE,$^) > > > + > > > +ifdef CONFIG_STACK_VALIDATION > > > +# objtool was skipped for LLVM bitcode, run it now that we have compiled > > > +# modules into native code > > > +cmd_cc_lto_link_modules += ; \ > > > + $(objtree)/tools/objtool/objtool $(objtool_args) --module \ > > > + $(@:.ko=.lto.o) > > > > > > What is this "$(@:.ko=.lto.o)" doing ? > > > > The target is already suffixed with .lto.o > > so $(@:.ko=.lto.o) should be the same as $@ > > Good catch, probably a leftover from an earlier iteration. > > > Shall I fix it locally unless > > I find more questionable code? > > Please do. > > Sami Applied with $(@:.ko=.lto.o) -> $@ Thanks.
diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 02197cb8e3a7..a6f43afd09bb 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -88,6 +88,10 @@ 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 + ifdef need-modorder targets-for-modules += $(obj)/modules.order endif @@ -271,12 +275,34 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE $(call if_changed_rule,cc_o_c) $(call cmd,force_checksrc) +ifdef CONFIG_LTO_CLANG +# Module .o files may contain LLVM bitcode, compile them into native code +# before ELF processing +quiet_cmd_cc_lto_link_modules = LTO [M] $@ +cmd_cc_lto_link_modules = \ + $(LD) $(ld_flags) -r -o $@ \ + $(shell [ -s $(@:.lto.o=.o.symversions) ] && \ + echo -T $(@:.lto.o=.o.symversions)) \ + --whole-archive $(filter-out FORCE,$^) + +ifdef CONFIG_STACK_VALIDATION +# objtool was skipped for LLVM bitcode, run it now that we have compiled +# modules into native code +cmd_cc_lto_link_modules += ; \ + $(objtree)/tools/objtool/objtool $(objtool_args) --module \ + $(@:.ko=.lto.o) +endif + +$(obj)/%.lto.o: $(obj)/%.o FORCE + $(call if_changed,cc_lto_link_modules) +endif + cmd_mod = { \ echo $(if $($*-objs)$($*-y)$($*-m), $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)), $(@:.mod=.o)); \ $(undefined_syms) echo; \ } > $@ -$(obj)/%.mod: $(obj)/%.o FORCE +$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE $(call if_changed,mod) quiet_cmd_cc_lst_c = MKLST $@ diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 10950559b223..af1c920a585c 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -225,6 +225,13 @@ 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 + # Objtool arguments are also needed for modfinal with LTO, so we define # then here to avoid duplication. objtool_args = \ diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 5e9b8057fb24..ff805777431c 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 objtool_args +# for c_flags and mod-prelink-ext include $(srctree)/scripts/Makefile.lib # find all modules listed in modules.order @@ -30,23 +30,6 @@ quiet_cmd_cc_o_c = CC [M] $@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) -ifdef CONFIG_LTO_CLANG -# With CONFIG_LTO_CLANG, reuse the object file we compiled for modpost to -# avoid a second slow LTO link -prelink-ext := .lto - -# ELF processing was skipped earlier because we didn't have native code, -# so let's now process the prelinked binary before we link the module. - -ifdef CONFIG_STACK_VALIDATION -cmd_ld_ko_o += \ - $(objtree)/tools/objtool/objtool $(objtool_args) \ - $(@:.ko=$(prelink-ext).o); - -endif # CONFIG_STACK_VALIDATION - -endif # CONFIG_LTO_CLANG - quiet_cmd_ld_ko_o = LD [M] $@ cmd_ld_ko_o += \ $(LD) -r $(KBUILD_LDFLAGS) \ @@ -72,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: %$(prelink-ext).o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE +$(modules): %.ko: %$(mod-prelink-ext).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 c383ba33d837..eef56d629799 100644 --- a/scripts/Makefile.modpost +++ b/scripts/Makefile.modpost @@ -41,7 +41,7 @@ __modpost: include include/config/auto.conf include $(srctree)/scripts/Kbuild.include -# for ld_flags +# for mod-prelink-ext include $(srctree)/scripts/Makefile.lib MODPOST = scripts/mod/modpost \ @@ -118,22 +118,6 @@ $(input-symdump): @echo >&2 ' Modules may not have dependencies or modversions.' @echo >&2 ' You may get many unresolved symbol warnings.' -ifdef CONFIG_LTO_CLANG -# With CONFIG_LTO_CLANG, .o files might be LLVM bitcode, so we need to run -# LTO to compile them into native code before running modpost -prelink-ext := .lto - -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 $^ - -%.lto.o: %.o - $(call if_changed,cc_lto_link_modules) -endif - modules := $(sort $(shell cat $(MODORDER))) # KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined symbols @@ -144,9 +128,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$$/$(prelink-ext)\.o/' $< | $(MODPOST) -T - + cmd_modpost = sed 's/\.ko$$/$(mod-prelink-ext)\.o/' $< | $(MODPOST) -T - -$(output-symdump): $(MODORDER) $(input-symdump) $(modules:.ko=$(prelink-ext).o) FORCE +$(output-symdump): $(MODORDER) $(input-symdump) $(modules:.ko=$(mod-prelink-ext).o) FORCE $(call if_changed,modpost) targets += $(output-symdump) diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh index da320151e7c3..6ed0d225c8b1 100755 --- a/scripts/gen_autoksyms.sh +++ b/scripts/gen_autoksyms.sh @@ -26,18 +26,6 @@ if [ -n "$CONFIG_MODVERSIONS" ]; then needed_symbols="$needed_symbols module_layout" fi -# With CONFIG_LTO_CLANG, LLVM bitcode has not yet been compiled into a binary -# when the .mod files are generated, which means they don't yet contain -# references to certain symbols that will be present in the final binaries. -if [ -n "$CONFIG_LTO_CLANG" ]; then - # intrinsic functions - needed_symbols="$needed_symbols memcpy memmove memset" - # ftrace - needed_symbols="$needed_symbols _mcount" - # stack protector symbols - needed_symbols="$needed_symbols __stack_chk_fail __stack_chk_guard" -fi - ksym_wl= if [ -n "$CONFIG_UNUSED_KSYMS_WHITELIST" ]; then # Use 'eval' to expand the whitelist path and check if it is relative