diff mbox series

[v2,11/28] kbuild: lto: postpone objtool

Message ID 20200903203053.3411268-12-samitolvanen@google.com (mailing list archive)
State New, archived
Headers show
Series [v2,01/28] x86/boot/compressed: Disable relocation relaxation | expand

Commit Message

Sami Tolvanen Sept. 3, 2020, 8:30 p.m. UTC 
With LTO, LLVM bitcode won't be compiled into native code until
modpost_link, or modfinal for modules. This change postpones calls
to objtool until after these steps.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 arch/Kconfig              |  2 +-
 scripts/Makefile.build    |  2 ++
 scripts/Makefile.modfinal | 24 ++++++++++++++++++++++--
 scripts/link-vmlinux.sh   | 23 ++++++++++++++++++++++-
 4 files changed, 47 insertions(+), 4 deletions(-)

Comments

Kees Cook Sept. 3, 2020, 10:19 p.m. UTC | #1 
On Thu, Sep 03, 2020 at 01:30:36PM -0700, Sami Tolvanen wrote:
> With LTO, LLVM bitcode won't be compiled into native code until
> modpost_link, or modfinal for modules. This change postpones calls
> to objtool until after these steps.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

For a "fail fast" style of building, it makes sense to have objtool run
as early as possible, so it makes sense to keep the current behavior in
non-LTO mode. I do wonder, though, if there is a real benefit to having
"fail fast" case. I imagine a lot of automated builds are using
--keep-going with make, and actually waiting until the end to do the
validation means more code will get build-tested before objtool rejects
the results. *shrug*

> ---
>  arch/Kconfig              |  2 +-
>  scripts/Makefile.build    |  2 ++
>  scripts/Makefile.modfinal | 24 ++++++++++++++++++++++--
>  scripts/link-vmlinux.sh   | 23 ++++++++++++++++++++++-
>  4 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 71392e4a8900..7a418907e686 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -599,7 +599,7 @@ config LTO_CLANG
>  	depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
>  	depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
>  	depends on ARCH_SUPPORTS_LTO_CLANG
> -	depends on !FTRACE_MCOUNT_RECORD
> +	depends on HAVE_OBJTOOL_MCOUNT || !(X86_64 && DYNAMIC_FTRACE)
>  	depends on !KASAN
>  	depends on !GCOV_KERNEL
>  	select LTO
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index c348e6d6b436..b8f1f0d65a73 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -218,6 +218,7 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
>  endif # USE_RECORDMCOUNT
>  
>  ifdef CONFIG_STACK_VALIDATION
> +ifndef CONFIG_LTO_CLANG
>  ifneq ($(SKIP_STACK_VALIDATION),1)
>  
>  __objtool_obj := $(objtree)/tools/objtool/objtool
> @@ -253,6 +254,7 @@ objtool_obj = $(if $(patsubst y%,, \
>  	$(__objtool_obj))
>  
>  endif # SKIP_STACK_VALIDATION
> +endif # CONFIG_LTO_CLANG
>  endif # CONFIG_STACK_VALIDATION
>  
>  # Rebuild all objects when objtool changes, or is enabled/disabled.
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 1005b147abd0..909bd509edb4 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -34,10 +34,30 @@ 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
> -endif
> +
> +# 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
> +ifneq ($(SKIP_STACK_VALIDATION),1)
> +cmd_ld_ko_o +=								\
> +	$(objtree)/tools/objtool/objtool				\
> +		$(if $(CONFIG_UNWINDER_ORC),orc generate,check)		\
> +		--module						\
> +		$(if $(CONFIG_FRAME_POINTER),,--no-fp)			\
> +		$(if $(CONFIG_GCOV_KERNEL),--no-unreachable,)		\
> +		$(if $(CONFIG_RETPOLINE),--retpoline,)			\
> +		$(if $(CONFIG_X86_SMAP),--uaccess,)			\
> +		$(if $(USE_OBJTOOL_MCOUNT),--mcount,)			\
> +		$(@:.ko=$(prelink-ext).o);
> +
> +endif # SKIP_STACK_VALIDATION
> +endif # CONFIG_STACK_VALIDATION

I wonder if objtool_args could be reused here instead of having two
places to keep in sync? It looks like that might mean moving things
around a bit before this patch, since I can't quite see if
Makefile.build's variables are visible to Makefile.modfinal?

> +
> +endif # CONFIG_LTO_CLANG
>  
>  quiet_cmd_ld_ko_o = LD [M]  $@
> -      cmd_ld_ko_o =                                                     \
> +      cmd_ld_ko_o +=                                                     \
>  	$(LD) -r $(KBUILD_LDFLAGS)					\
>  		$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)		\
>  		$(addprefix -T , $(KBUILD_LDS_MODULE))			\
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 3e99a19b9195..a352a5ad9ef7 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -93,8 +93,29 @@ objtool_link()
>  {
>  	local objtoolopt;
>  
> +	if [ "${CONFIG_LTO_CLANG} ${CONFIG_STACK_VALIDATION}" = "y y" ]; then
> +		# Don't perform vmlinux validation unless explicitly requested,
> +		# but run objtool on vmlinux.o now that we have an object file.
> +		if [ -n "${CONFIG_UNWINDER_ORC}" ]; then
> +			objtoolopt="orc generate"
> +		else
> +			objtoolopt="check"
> +		fi
> +
> +		if [ -n ${USE_OBJTOOL_MCOUNT} ]; then
> +			objtoolopt="${objtoolopt} --mcount"
> +		fi
> +	fi
> +
>  	if [ -n "${CONFIG_VMLINUX_VALIDATION}" ]; then
> -		objtoolopt="check --vmlinux"
> +		if [ -z "${objtoolopt}" ]; then
> +			objtoolopt="check --vmlinux"
> +		else
> +			objtoolopt="${objtoolopt} --vmlinux"
> +		fi
> +	fi
> +
> +	if [ -n "${objtoolopt}" ]; then
>  		if [ -z "${CONFIG_FRAME_POINTER}" ]; then
>  			objtoolopt="${objtoolopt} --no-fp"
>  		fi
> -- 
> 2.28.0.402.g5ffc5be6b7-goog
>
Sami Tolvanen Sept. 8, 2020, 8:56 p.m. UTC | #2 
On Thu, Sep 03, 2020 at 03:19:43PM -0700, Kees Cook wrote:
> On Thu, Sep 03, 2020 at 01:30:36PM -0700, Sami Tolvanen wrote:
> > With LTO, LLVM bitcode won't be compiled into native code until
> > modpost_link, or modfinal for modules. This change postpones calls
> > to objtool until after these steps.
> > 
> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> 
> For a "fail fast" style of building, it makes sense to have objtool run
> as early as possible, so it makes sense to keep the current behavior in
> non-LTO mode. I do wonder, though, if there is a real benefit to having
> "fail fast" case. I imagine a lot of automated builds are using
> --keep-going with make, and actually waiting until the end to do the
> validation means more code will get build-tested before objtool rejects
> the results. *shrug*
> 
> > ---
> >  arch/Kconfig              |  2 +-
> >  scripts/Makefile.build    |  2 ++
> >  scripts/Makefile.modfinal | 24 ++++++++++++++++++++++--
> >  scripts/link-vmlinux.sh   | 23 ++++++++++++++++++++++-
> >  4 files changed, 47 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 71392e4a8900..7a418907e686 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -599,7 +599,7 @@ config LTO_CLANG
> >  	depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
> >  	depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
> >  	depends on ARCH_SUPPORTS_LTO_CLANG
> > -	depends on !FTRACE_MCOUNT_RECORD
> > +	depends on HAVE_OBJTOOL_MCOUNT || !(X86_64 && DYNAMIC_FTRACE)
> >  	depends on !KASAN
> >  	depends on !GCOV_KERNEL
> >  	select LTO
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index c348e6d6b436..b8f1f0d65a73 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -218,6 +218,7 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
> >  endif # USE_RECORDMCOUNT
> >  
> >  ifdef CONFIG_STACK_VALIDATION
> > +ifndef CONFIG_LTO_CLANG
> >  ifneq ($(SKIP_STACK_VALIDATION),1)
> >  
> >  __objtool_obj := $(objtree)/tools/objtool/objtool
> > @@ -253,6 +254,7 @@ objtool_obj = $(if $(patsubst y%,, \
> >  	$(__objtool_obj))
> >  
> >  endif # SKIP_STACK_VALIDATION
> > +endif # CONFIG_LTO_CLANG
> >  endif # CONFIG_STACK_VALIDATION
> >  
> >  # Rebuild all objects when objtool changes, or is enabled/disabled.
> > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> > index 1005b147abd0..909bd509edb4 100644
> > --- a/scripts/Makefile.modfinal
> > +++ b/scripts/Makefile.modfinal
> > @@ -34,10 +34,30 @@ 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
> > -endif
> > +
> > +# 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
> > +ifneq ($(SKIP_STACK_VALIDATION),1)
> > +cmd_ld_ko_o +=								\
> > +	$(objtree)/tools/objtool/objtool				\
> > +		$(if $(CONFIG_UNWINDER_ORC),orc generate,check)		\
> > +		--module						\
> > +		$(if $(CONFIG_FRAME_POINTER),,--no-fp)			\
> > +		$(if $(CONFIG_GCOV_KERNEL),--no-unreachable,)		\
> > +		$(if $(CONFIG_RETPOLINE),--retpoline,)			\
> > +		$(if $(CONFIG_X86_SMAP),--uaccess,)			\
> > +		$(if $(USE_OBJTOOL_MCOUNT),--mcount,)			\
> > +		$(@:.ko=$(prelink-ext).o);
> > +
> > +endif # SKIP_STACK_VALIDATION
> > +endif # CONFIG_STACK_VALIDATION
> 
> I wonder if objtool_args could be reused here instead of having two
> places to keep in sync? It looks like that might mean moving things
> around a bit before this patch, since I can't quite see if
> Makefile.build's variables are visible to Makefile.modfinal?

It doesn't look like they are. I suppose we could move objtool_args to
Makefile.lib. Masahiro, any thoughts?

Sami
diff mbox series

Patch

diff --git a/arch/Kconfig b/arch/Kconfig
index 71392e4a8900..7a418907e686 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -599,7 +599,7 @@  config LTO_CLANG
 	depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
 	depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
 	depends on ARCH_SUPPORTS_LTO_CLANG
-	depends on !FTRACE_MCOUNT_RECORD
+	depends on HAVE_OBJTOOL_MCOUNT || !(X86_64 && DYNAMIC_FTRACE)
 	depends on !KASAN
 	depends on !GCOV_KERNEL
 	select LTO
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index c348e6d6b436..b8f1f0d65a73 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -218,6 +218,7 @@  cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
 endif # USE_RECORDMCOUNT
 
 ifdef CONFIG_STACK_VALIDATION
+ifndef CONFIG_LTO_CLANG
 ifneq ($(SKIP_STACK_VALIDATION),1)
 
 __objtool_obj := $(objtree)/tools/objtool/objtool
@@ -253,6 +254,7 @@  objtool_obj = $(if $(patsubst y%,, \
 	$(__objtool_obj))
 
 endif # SKIP_STACK_VALIDATION
+endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
 
 # Rebuild all objects when objtool changes, or is enabled/disabled.
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 1005b147abd0..909bd509edb4 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -34,10 +34,30 @@  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
-endif
+
+# 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
+ifneq ($(SKIP_STACK_VALIDATION),1)
+cmd_ld_ko_o +=								\
+	$(objtree)/tools/objtool/objtool				\
+		$(if $(CONFIG_UNWINDER_ORC),orc generate,check)		\
+		--module						\
+		$(if $(CONFIG_FRAME_POINTER),,--no-fp)			\
+		$(if $(CONFIG_GCOV_KERNEL),--no-unreachable,)		\
+		$(if $(CONFIG_RETPOLINE),--retpoline,)			\
+		$(if $(CONFIG_X86_SMAP),--uaccess,)			\
+		$(if $(USE_OBJTOOL_MCOUNT),--mcount,)			\
+		$(@:.ko=$(prelink-ext).o);
+
+endif # SKIP_STACK_VALIDATION
+endif # CONFIG_STACK_VALIDATION
+
+endif # CONFIG_LTO_CLANG
 
 quiet_cmd_ld_ko_o = LD [M]  $@
-      cmd_ld_ko_o =                                                     \
+      cmd_ld_ko_o +=                                                     \
 	$(LD) -r $(KBUILD_LDFLAGS)					\
 		$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)		\
 		$(addprefix -T , $(KBUILD_LDS_MODULE))			\
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 3e99a19b9195..a352a5ad9ef7 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -93,8 +93,29 @@  objtool_link()
 {
 	local objtoolopt;
 
+	if [ "${CONFIG_LTO_CLANG} ${CONFIG_STACK_VALIDATION}" = "y y" ]; then
+		# Don't perform vmlinux validation unless explicitly requested,
+		# but run objtool on vmlinux.o now that we have an object file.
+		if [ -n "${CONFIG_UNWINDER_ORC}" ]; then
+			objtoolopt="orc generate"
+		else
+			objtoolopt="check"
+		fi
+
+		if [ -n ${USE_OBJTOOL_MCOUNT} ]; then
+			objtoolopt="${objtoolopt} --mcount"
+		fi
+	fi
+
 	if [ -n "${CONFIG_VMLINUX_VALIDATION}" ]; then
-		objtoolopt="check --vmlinux"
+		if [ -z "${objtoolopt}" ]; then
+			objtoolopt="check --vmlinux"
+		else
+			objtoolopt="${objtoolopt} --vmlinux"
+		fi
+	fi
+
+	if [ -n "${objtoolopt}" ]; then
 		if [ -z "${CONFIG_FRAME_POINTER}" ]; then
 			objtoolopt="${objtoolopt} --no-fp"
 		fi