* [PATCH 1/3] kbuild: provide a new place to check necessary host tools
2018-10-16 9:10 [PATCH 0/3] kbuild: add scripts/Makefile.toolcheck to check necessary host tools Masahiro Yamada
@ 2018-10-16 9:10 ` Masahiro Yamada
2018-10-16 15:47 ` Sam Ravnborg
2018-10-16 9:10 ` [PATCH 2/3] objtool: move libelf check out of top Makefile Masahiro Yamada
2018-10-16 9:10 ` [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary Masahiro Yamada
2 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2018-10-16 9:10 UTC (permalink / raw)
To: linux-kbuild
Cc: Ingo Molnar, Josh Poimboeuf, Bernd Edlinger, Borislav Petkov,
Sam Ravnborg, Masahiro Yamada, Michal Marek, linux-kernel
Enabling CONFIG options often requires additional features, like
target compiler capabilities, host tools, etc.
As for the target compiler capabilities, Kconfig is able to add
proper dependencies. For example,
config STACKPROTECTOR
...
depends on $(cc-option,-fstack-protector)
The CONFIG option is simply disabled if unsupported by the compiler.
This seems sensible since otherwise, there is no way to avoid the
build error.
In the discussion [1], doing similar for host tools was unacceptable.
Build errors caused by missing host tools are easy to fix; just
install them to your build machine. So, let the build fail instead
of disabling CONFIG options silently.
We already have such checks in random places in makefiles. What I
do not like is that people tend to do that in the top Makefile.
The top Makefile is too cluttered, and what is worse, $(error ...)
functions in the top Makefile could be false positive. (Please note
the top Makefile includes the stale include/config/auto.conf before
syncconfig updates it.)
This adds a new file scripts/Makefile.toolcheck to check additional
tools depending on the kernel configuration. This check is run
immediately after syncconfig, i.e., when a user attempts to build
something with a new set of CONFIG options.
[1] https://patchwork.kernel.org/patch/10516049/
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 1 +
scripts/Makefile.toolcheck | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
create mode 100644 scripts/Makefile.toolcheck
diff --git a/Makefile b/Makefile
index bf3786e..23a204a 100644
--- a/Makefile
+++ b/Makefile
@@ -633,6 +633,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
# include/config/auto.conf (which mirrors .config).
include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
$(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.toolcheck
else
# External modules and some install targets need include/generated/autoconf.h
# and include/config/auto.conf but do not care if they are up-to-date.
diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
new file mode 100644
index 0000000..f3c165d
--- /dev/null
+++ b/scripts/Makefile.toolcheck
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0
+# ===========================================================================
+# Host tools check
+# ===========================================================================
+#
+# Some additional tools might be required depending on the kernel configuration.
+# Check if they are installed on the host machine, and if missing, error out
+# with a user-friendly message.
+
+include include/config/auto.conf
+
+__toolcheck:
+ @:
+
+PHONY += $(toolcheck-y)
+__toolcheck: $(toolcheck-y)
+
+define populate_toolcheck
+__toolcheck: check_$(1)
+PHONY += check_$(1)
+check_$(1):
+ $(Q){ $(chk_$(1)); } >/dev/null 2>&1 || { \
+ echo "*" >&2; \
+ for msg in $(msg_$(1)); \
+ do \
+ echo "* $$$${msg}" >&2; \
+ done; \
+ echo "*" >&2; \
+ /bin/false; \
+ }
+endef
+
+$(foreach c, $(toolcheck-y), $(eval $(call populate_toolcheck,$(c))))
+
+.PHONY: $(PHONY)
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] kbuild: provide a new place to check necessary host tools
2018-10-16 9:10 ` [PATCH 1/3] kbuild: provide a new place " Masahiro Yamada
@ 2018-10-16 15:47 ` Sam Ravnborg
0 siblings, 0 replies; 14+ messages in thread
From: Sam Ravnborg @ 2018-10-16 15:47 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, Ingo Molnar, Josh Poimboeuf, Bernd Edlinger,
Borislav Petkov, Michal Marek, linux-kernel
Hi Masahiro.
Looks good, a few nits below.
Sam
> The top Makefile is too cluttered
So true...
> This adds a new file scripts/Makefile.toolcheck to check additional
> tools depending on the kernel configuration. This check is run
> immediately after syncconfig, i.e., when a user attempts to build
> something with a new set of CONFIG options.
>
> [1] https://patchwork.kernel.org/patch/10516049/
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
The use of toolcheck-y should be documented in Documentation/kbuild/...
>
> Makefile | 1 +
> scripts/Makefile.toolcheck | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
> create mode 100644 scripts/Makefile.toolcheck
>
> diff --git a/Makefile b/Makefile
> index bf3786e..23a204a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -633,6 +633,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
> # include/config/auto.conf (which mirrors .config).
> include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
> $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
> + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.toolcheck
> else
> # External modules and some install targets need include/generated/autoconf.h
> # and include/config/auto.conf but do not care if they are up-to-date.
> diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
> new file mode 100644
> index 0000000..f3c165d
> --- /dev/null
> +++ b/scripts/Makefile.toolcheck
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ===========================================================================
> +# Host tools check
> +# ===========================================================================
> +#
> +# Some additional tools might be required depending on the kernel configuration.
> +# Check if they are installed on the host machine, and if missing, error out
> +# with a user-friendly message.
> +
> +include include/config/auto.conf
> +
> +__toolcheck:
> + @:
> +
> +PHONY += $(toolcheck-y)
> +__toolcheck: $(toolcheck-y)
> +
> +define populate_toolcheck
> +__toolcheck: check_$(1)
> +PHONY += check_$(1)
> +check_$(1):
> + $(Q){ $(chk_$(1)); } >/dev/null 2>&1 || { \
> + echo "*" >&2; \
> + for msg in $(msg_$(1)); \
> + do \
> + echo "* $$$${msg}" >&2; \
> + done; \
> + echo "*" >&2; \
> + /bin/false; \
> + }
Very dense, but usign if [ ... ] may result is somethign that
is a little more readable.
And maybe a litle hint on the usage...
> +$(foreach c, $(toolcheck-y), $(eval $(call populate_toolcheck,$(c))))
Good use of $(eval ...)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] objtool: move libelf check out of top Makefile
2018-10-16 9:10 [PATCH 0/3] kbuild: add scripts/Makefile.toolcheck to check necessary host tools Masahiro Yamada
2018-10-16 9:10 ` [PATCH 1/3] kbuild: provide a new place " Masahiro Yamada
@ 2018-10-16 9:10 ` Masahiro Yamada
2018-10-16 14:25 ` Josh Poimboeuf
2018-10-16 9:10 ` [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary Masahiro Yamada
2 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2018-10-16 9:10 UTC (permalink / raw)
To: linux-kbuild
Cc: Ingo Molnar, Josh Poimboeuf, Bernd Edlinger, Borislav Petkov,
Sam Ravnborg, Masahiro Yamada, Michal Marek, linux-kernel
Bernd Edlinger reports:
The next make after an oldconfig reads in the outdated
include/config/auto.conf which can kill the make before
it is able to call the syncconfig target.
$ make defconfig
*** Default configuration is based on 'x86_64_defconfig'
$ make
scripts/kconfig/conf --syncconfig Kconfig
Makefile:936: *** "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y,
please install libelf-dev, libelf-devel or elfutils-libelf-devel". Stop.
$ sed -i s/CONFIG_UNWINDER_ORC=y// .configs
$ make oldconfig
Choose kernel unwinder
> 1. ORC unwinder (UNWINDER_ORC) (NEW)
2. Frame pointer unwinder (UNWINDER_FRAME_POINTER)
choice[1-2?]: 2
$ make
Makefile:936: *** "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y,
please install libelf-dev, libelf-devel or elfutils-libelf-devel". Stop.
I tried to fix it by moving the libelf evaluation to the Kconfig
stage [1], but it was rejected.
Kconfig is now able to run shell commands to evaluate any prerequisites
needed to enable CONFIG options.
For example, as shown in commit 2a61f4747eea ("stack-protector: test
compiler capability in Kconfig and drop AUTO mode"), the new Kconfig
syntax is useful to evaluate target compiler capabilities.
However, disabling the CONFIG option silently just because of missing
libelf would not be a preferred behavior; in this case, installing an
appropriate package will solve the problem. Hence, this check will be
kept in a makefile, but somewhere else than the top Makefile.
Move the check to scripts/Makefile.toolcheck so that it is run after
syncconfig.
Another behavioral change is, missing libelf for CONFIG_STACK_VALIDATION
was previously a warning, but now a error.
[1] https://patchwork.kernel.org/patch/10516049/
Reported-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
Makefile | 21 +++------------------
scripts/Makefile.build | 2 --
scripts/Makefile.toolcheck | 5 +++++
3 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/Makefile b/Makefile
index 23a204a..71940b7 100644
--- a/Makefile
+++ b/Makefile
@@ -949,23 +949,6 @@ mod_sign_cmd = true
endif
export mod_sign_cmd
-ifdef CONFIG_STACK_VALIDATION
- has_libelf := $(call try-run,\
- echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,1,0)
- ifeq ($(has_libelf),1)
- objtool_target := tools/objtool FORCE
- else
- ifdef CONFIG_UNWINDER_ORC
- $(error "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
- else
- $(warning "Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
- endif
- SKIP_STACK_VALIDATION := 1
- export SKIP_STACK_VALIDATION
- endif
-endif
-
-
ifeq ($(KBUILD_EXTMOD),)
core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
@@ -1115,7 +1098,9 @@ uapi-asm-generic:
src=uapi/asm obj=arch/$(SRCARCH)/include/generated/uapi/asm
PHONY += prepare-objtool
-prepare-objtool: $(objtool_target)
+ifdef CONFIG_STACK_VALIDATION
+prepare-objtool: tools/objtool
+endif
# Generate some files
# ---------------------------------------------------------------------------
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 54da4b0..e9dabe4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -233,7 +233,6 @@ endif # CC_USING_RECORD_MCOUNT
endif # CONFIG_FTRACE_MCOUNT_RECORD
ifdef CONFIG_STACK_VALIDATION
-ifneq ($(SKIP_STACK_VALIDATION),1)
__objtool_obj := $(objtree)/tools/objtool/objtool
@@ -270,7 +269,6 @@ objtool_obj = $(if $(patsubst y%,, \
$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
$(__objtool_obj))
-endif # SKIP_STACK_VALIDATION
endif # CONFIG_STACK_VALIDATION
# Rebuild all objects when objtool changes, or is enabled/disabled.
diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
index f3c165d..bc26fc0 100644
--- a/scripts/Makefile.toolcheck
+++ b/scripts/Makefile.toolcheck
@@ -12,6 +12,11 @@ include include/config/auto.conf
__toolcheck:
@:
+chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
+msg_stack_validation = "libelf is necessary for building the objtool." \
+ "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
+toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
+
PHONY += $(toolcheck-y)
__toolcheck: $(toolcheck-y)
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] objtool: move libelf check out of top Makefile
2018-10-16 9:10 ` [PATCH 2/3] objtool: move libelf check out of top Makefile Masahiro Yamada
@ 2018-10-16 14:25 ` Josh Poimboeuf
2018-10-16 15:51 ` Masahiro Yamada
0 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2018-10-16 14:25 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, Ingo Molnar, Bernd Edlinger, Borislav Petkov,
Sam Ravnborg, Michal Marek, linux-kernel
On Tue, Oct 16, 2018 at 06:10:52PM +0900, Masahiro Yamada wrote:
> Another behavioral change is, missing libelf for CONFIG_STACK_VALIDATION
> was previously a warning, but now a error.
This behavioral change should be fine. It was already an error for
UNWINDER_ORC, so this would only upgrade a warning to an error for
people using STACK_VALIDATION without ORC, which should be a small
number of people by this point.
> diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
> index f3c165d..bc26fc0 100644
> --- a/scripts/Makefile.toolcheck
> +++ b/scripts/Makefile.toolcheck
> @@ -12,6 +12,11 @@ include include/config/auto.conf
> __toolcheck:
> @:
>
> +chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> +msg_stack_validation = "libelf is necessary for building the objtool." \
> + "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> +toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> +
> PHONY += $(toolcheck-y)
> __toolcheck: $(toolcheck-y)
This is a nice improvement.
It would probably be a good idea to clarify to the user which config
option(s) are the cause for the error, by putting
"CONFIG_STACK_VALIDATION" in the error string, for example.
Though, for this particular case, it would be clearer to have a
different error, based on which option is enabled, like we had before.
Like:
ifdef CONFIG_UNWINDER_ORC
chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y. " \
"Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
else
chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y. " \
"Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
endif
What do you think?
--
Josh
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] objtool: move libelf check out of top Makefile
2018-10-16 14:25 ` Josh Poimboeuf
@ 2018-10-16 15:51 ` Masahiro Yamada
2018-10-16 16:15 ` Josh Poimboeuf
0 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2018-10-16 15:51 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Linux Kbuild mailing list, Ingo Molnar, Bernd Edlinger,
Borislav Petkov, Sam Ravnborg, Michal Marek,
Linux Kernel Mailing List
On Tue, Oct 16, 2018 at 11:25 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Tue, Oct 16, 2018 at 06:10:52PM +0900, Masahiro Yamada wrote:
> > Another behavioral change is, missing libelf for CONFIG_STACK_VALIDATION
> > was previously a warning, but now a error.
>
> This behavioral change should be fine. It was already an error for
> UNWINDER_ORC, so this would only upgrade a warning to an error for
> people using STACK_VALIDATION without ORC, which should be a small
> number of people by this point.
>
> > diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
> > index f3c165d..bc26fc0 100644
> > --- a/scripts/Makefile.toolcheck
> > +++ b/scripts/Makefile.toolcheck
> > @@ -12,6 +12,11 @@ include include/config/auto.conf
> > __toolcheck:
> > @:
> >
> > +chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > +msg_stack_validation = "libelf is necessary for building the objtool." \
> > + "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > +toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> > +
> > PHONY += $(toolcheck-y)
> > __toolcheck: $(toolcheck-y)
>
> This is a nice improvement.
>
> It would probably be a good idea to clarify to the user which config
> option(s) are the cause for the error, by putting
> "CONFIG_STACK_VALIDATION" in the error string, for example.
>
> Though, for this particular case, it would be clearer to have a
> different error, based on which option is enabled, like we had before.
>
> Like:
>
>
> ifdef CONFIG_UNWINDER_ORC
>
> chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y. " \
> "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
>
> else
>
> chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y. " \
> "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
>
> endif
>
>
> What do you think?
It is ugly.
Do you need such detailed information like ORC metadata stuff here?
This Makefile aims to error out, showing why the build failed.
That's it.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] objtool: move libelf check out of top Makefile
2018-10-16 15:51 ` Masahiro Yamada
@ 2018-10-16 16:15 ` Josh Poimboeuf
2018-10-19 6:04 ` Masahiro Yamada
0 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2018-10-16 16:15 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, Ingo Molnar, Bernd Edlinger,
Borislav Petkov, Sam Ravnborg, Michal Marek,
Linux Kernel Mailing List
On Wed, Oct 17, 2018 at 12:51:40AM +0900, Masahiro Yamada wrote:
> > ifdef CONFIG_UNWINDER_ORC
> >
> > chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y. " \
> > "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
> >
> > else
> >
> > chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y. " \
> > "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> >
> > endif
> >
> >
> > What do you think?
>
>
> It is ugly.
>
> Do you need such detailed information like ORC metadata stuff here?
>
> This Makefile aims to error out, showing why the build failed.
> That's it.
Yeah, it is kind of ugly. But the "showing why the build failed" part
is important. I was trying to give the user a clear error message,
similar to what we have today.
Without context, the user won't know what objtool is, or why it needs to
be built.
If we have just a single error message for all cases, it should at least
mention the config option. Like
"Cannot build objtool for CONFIG_STACK_VALIDATION."
But then, most users will only have that enabled because of ORC. So an
ORC-specific message would be more appropriate in most cases.
So maybe it can just be something more vague:
msg_stack_validation = "Cannot build objtool for CONFIG_UNWINDER_ORC and/or CONFIG_STACK_VALIDATION. " \
"Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
That would probably be good enough. Then we could drop the ugly ifdef.
--
Josh
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] objtool: move libelf check out of top Makefile
2018-10-16 16:15 ` Josh Poimboeuf
@ 2018-10-19 6:04 ` Masahiro Yamada
2018-10-19 19:28 ` Josh Poimboeuf
0 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2018-10-19 6:04 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Linux Kbuild mailing list, Ingo Molnar, Bernd Edlinger,
Borislav Petkov, Sam Ravnborg, Michal Marek,
Linux Kernel Mailing List
Hi Josh,
On Wed, Oct 17, 2018 at 1:16 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Wed, Oct 17, 2018 at 12:51:40AM +0900, Masahiro Yamada wrote:
> > > ifdef CONFIG_UNWINDER_ORC
> > >
> > > chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y. " \
> > > "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
> > >
> > > else
> > >
> > > chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y. " \
> > > "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> > >
> > > endif
> > >
> > >
> > > What do you think?
> >
> >
> > It is ugly.
> >
> > Do you need such detailed information like ORC metadata stuff here?
> >
> > This Makefile aims to error out, showing why the build failed.
> > That's it.
>
> Yeah, it is kind of ugly. But the "showing why the build failed" part
> is important. I was trying to give the user a clear error message,
> similar to what we have today.
>
> Without context, the user won't know what objtool is, or why it needs to
> be built.
>
> If we have just a single error message for all cases, it should at least
> mention the config option. Like
>
> "Cannot build objtool for CONFIG_STACK_VALIDATION."
>
> But then, most users will only have that enabled because of ORC. So an
> ORC-specific message would be more appropriate in most cases.
>
> So maybe it can just be something more vague:
>
> msg_stack_validation = "Cannot build objtool for CONFIG_UNWINDER_ORC and/or CONFIG_STACK_VALIDATION. " \
> "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
>
> That would probably be good enough. Then we could drop the ugly ifdef.
Fair point, but I am confused by the current
STACK_VALIDATION / UNWINDER_ORC logic.
In my understanding, objtool is
an all-in-one object check/manipulation tool.
STACK_VALIDATION and UNWINDER_ORC
is a selection of a sub-command, 'check' or 'orc generate'.
(Correct me if am wrong.)
However, STACK_VALIDATION is still used to
decide whether or not to compile the objtool.
Adding a new symbol OBJTOOL would clarify the logic.
config OBJTOOL
bool
config STACK_VALIDATION
bool "Compile-time stack metadata validation"
depends on HAVE_STACK_VALIDATION
select OBJTOOL
...
config UNWINDER_ORC
bool "ORC unwinder"
depends on X86_64
select OBJTOOL
...
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] objtool: move libelf check out of top Makefile
2018-10-19 6:04 ` Masahiro Yamada
@ 2018-10-19 19:28 ` Josh Poimboeuf
0 siblings, 0 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2018-10-19 19:28 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kbuild mailing list, Ingo Molnar, Bernd Edlinger,
Borislav Petkov, Sam Ravnborg, Michal Marek,
Linux Kernel Mailing List
On Fri, Oct 19, 2018 at 03:04:22PM +0900, Masahiro Yamada wrote:
> Hi Josh,
>
>
> On Wed, Oct 17, 2018 at 1:16 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > On Wed, Oct 17, 2018 at 12:51:40AM +0900, Masahiro Yamada wrote:
> > > > ifdef CONFIG_UNWINDER_ORC
> > > >
> > > > chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > > msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y. " \
> > > > "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > > toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
> > > >
> > > > else
> > > >
> > > > chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > > msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y. " \
> > > > "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > > toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> > > >
> > > > endif
> > > >
> > > >
> > > > What do you think?
> > >
> > >
> > > It is ugly.
> > >
> > > Do you need such detailed information like ORC metadata stuff here?
> > >
> > > This Makefile aims to error out, showing why the build failed.
> > > That's it.
> >
> > Yeah, it is kind of ugly. But the "showing why the build failed" part
> > is important. I was trying to give the user a clear error message,
> > similar to what we have today.
> >
> > Without context, the user won't know what objtool is, or why it needs to
> > be built.
> >
> > If we have just a single error message for all cases, it should at least
> > mention the config option. Like
> >
> > "Cannot build objtool for CONFIG_STACK_VALIDATION."
> >
> > But then, most users will only have that enabled because of ORC. So an
> > ORC-specific message would be more appropriate in most cases.
> >
> > So maybe it can just be something more vague:
> >
> > msg_stack_validation = "Cannot build objtool for CONFIG_UNWINDER_ORC and/or CONFIG_STACK_VALIDATION. " \
> > "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> >
> > That would probably be good enough. Then we could drop the ugly ifdef.
>
>
> Fair point, but I am confused by the current
> STACK_VALIDATION / UNWINDER_ORC logic.
>
> In my understanding, objtool is
> an all-in-one object check/manipulation tool.
>
> STACK_VALIDATION and UNWINDER_ORC
> is a selection of a sub-command, 'check' or 'orc generate'.
>
> (Correct me if am wrong.)
>
>
> However, STACK_VALIDATION is still used to
> decide whether or not to compile the objtool.
>
>
> Adding a new symbol OBJTOOL would clarify the logic.
>
>
>
> config OBJTOOL
> bool
>
> config STACK_VALIDATION
> bool "Compile-time stack metadata validation"
> depends on HAVE_STACK_VALIDATION
> select OBJTOOL
> ...
>
>
> config UNWINDER_ORC
> bool "ORC unwinder"
> depends on X86_64
> select OBJTOOL
> ...
While 'orc generate' and 'check' are indeed separate subcommands of
objtool, the functionality of 'orc generate' is actually a superset of
the functionality of 'check'. In other words, ORC generation relies on
the stack validation feature, which is consistent with the current
config logic.
--
Josh
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary
2018-10-16 9:10 [PATCH 0/3] kbuild: add scripts/Makefile.toolcheck to check necessary host tools Masahiro Yamada
2018-10-16 9:10 ` [PATCH 1/3] kbuild: provide a new place " Masahiro Yamada
2018-10-16 9:10 ` [PATCH 2/3] objtool: move libelf check out of top Makefile Masahiro Yamada
@ 2018-10-16 9:10 ` Masahiro Yamada
[not found] ` <20181016195420.GE27990@zn.tnic>
2 siblings, 1 reply; 14+ messages in thread
From: Masahiro Yamada @ 2018-10-16 9:10 UTC (permalink / raw)
To: linux-kbuild
Cc: Ingo Molnar, Josh Poimboeuf, Bernd Edlinger, Borislav Petkov,
Sam Ravnborg, Masahiro Yamada, Michal Marek, linux-kernel
If CONFIG_KERNEL_LZ4 is enabled without lz4 tool installed on the
system, the build fails at the very last stage (reported by
Borislav Petkov [1]).
LZO arch/x86/boot/compressed/vmlinux.bin.lzo
/bin/sh: 1: lzop: not found
arch/x86/boot/compressed/Makefile:141: recipe for target 'arch/x86/boot/compressed/vmlinux.bin.lzo' failed
make[2]: *** [arch/x86/boot/compressed/vmlinux.bin.lzo] Error 1
arch/x86/boot/Makefile:112: recipe for target 'arch/x86/boot/compressed/vmlinux' failed
make[1]: *** [arch/x86/boot/compressed/vmlinux] Error 2
arch/x86/Makefile:284: recipe for target 'bzImage' failed
make: *** [bzImage] Error 2
Check the tools in scripts/Makefile.toolcheck to fail the build
earlier with a more readable message.
[1] https://patchwork.kernel.org/patch/10635381/
Suggested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/Makefile.toolcheck | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
index bc26fc0..5e336e4 100644
--- a/scripts/Makefile.toolcheck
+++ b/scripts/Makefile.toolcheck
@@ -17,6 +17,14 @@ msg_stack_validation = "libelf is necessary for building the objtool." \
"Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
+chk_lzo = command -v lzop
+msg_lzo = "lzo tool not found. Please install it."
+toolcheck-$(CONFIG_KERNEL_LZO) += lzo
+
+chk_lz4 = command -v lz4c
+msg_lz4 = "lz4 tool not found. Please install it."
+toolcheck-$(CONFIG_KERNEL_LZ4) += lz4
+
PHONY += $(toolcheck-y)
__toolcheck: $(toolcheck-y)
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread