* [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS @ 2021-07-02 3:29 Lecopzer Chen 2021-07-02 3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen 2021-07-02 3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen 0 siblings, 2 replies; 11+ messages in thread From: Lecopzer Chen @ 2021-07-02 3:29 UTC (permalink / raw) To: ndesaulniers, keescook, samitolvanen, linux-kbuild, nathan Cc: clang-built-linux, linux-kernel, yj.chiang, masahiroy, michal.lkml, Lecopzer Chen LTO with MODVERSIONS will fail in generating correct CRC because the makefile rule doesn't work for make with version 3.8X. Refer to [1]: > When building modules(CONFIG_...=m), I found some of module versions > are incorrect and set to 0. > This can be found in build log for first clean build which shows > WARNING: EXPORT symbol "XXXX" [drivers/XXX/XXX.ko] version generation failed, > symbol will not be versioned. > But in second build(incremental build), the WARNING disappeared and the > module version becomes valid CRC and make someone who want to change > modules without updating kernel image can't insert their modules. > The problematic code is > + $(foreach n, $(filter-out FORCE,$^), \ > + $(if $(wildcard $(n).symversions), \ > + ; cat $(n).symversions >> $@.symversions)) The issue is fixed when make version upgrading to 4.2. Thus we need to check make version during selecting on LTO Kconfig. Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits for arithmetic comparisons. [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> ---- v3: - fix comment in make-version.sh v2: https://lore.kernel.org/lkml/20210701092841.1419-1-lecopzer.chen@mediatek.com/ - change MAKE_VERSION_INT to MAKE_VERSION - remove $(make-version) - tweak commit message v1: https://lore.kernel.org/lkml/20210630121436.19581-1-lecopzer.chen@mediatek.com/ ---- Lecopzer Chen (2): Kbuild: lto: add CONFIG_MAKE_VERSION Kbuild: lto: add make version checking Makefile | 2 +- arch/Kconfig | 1 + init/Kconfig | 4 ++++ scripts/make-version.sh | 13 +++++++++++++ 4 files changed, 19 insertions(+), 1 deletion(-) create mode 100755 scripts/make-version.sh -- 2.18.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION 2021-07-02 3:29 [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS Lecopzer Chen @ 2021-07-02 3:29 ` Lecopzer Chen 2021-07-04 0:15 ` Nathan Chancellor 2021-07-05 2:02 ` Masahiro Yamada 2021-07-02 3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen 1 sibling, 2 replies; 11+ messages in thread From: Lecopzer Chen @ 2021-07-02 3:29 UTC (permalink / raw) To: ndesaulniers, keescook, samitolvanen, linux-kbuild, nathan Cc: clang-built-linux, linux-kernel, yj.chiang, masahiroy, michal.lkml, Lecopzer Chen To check the GNU make version. Used by the LTO Kconfig. LTO with MODVERSIONS will fail in generating correct CRC because the makefile rule doesn't work for make with version 3.8X.[1] Thus we need to check make version during selecting on LTO Kconfig. Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits for arithmetic comparisons. [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> --- Makefile | 2 +- init/Kconfig | 4 ++++ scripts/make-version.sh | 13 +++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100755 scripts/make-version.sh diff --git a/Makefile b/Makefile index 88888fff4c62..2402745b2ba9 100644 --- a/Makefile +++ b/Makefile @@ -516,7 +516,7 @@ CLANG_FLAGS := export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL -export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX +export PERL PYTHON3 CHECK CHECKFLAGS MAKE MAKE_VERSION UTS_MACHINE HOSTCXX export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE diff --git a/init/Kconfig b/init/Kconfig index 55f9f7738ebb..ecc110504f87 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -86,6 +86,10 @@ config CC_HAS_ASM_INLINE config CC_HAS_NO_PROFILE_FN_ATTR def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror) +config MAKE_VERSION + int + default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION)) + config CONSTRUCTORS bool diff --git a/scripts/make-version.sh b/scripts/make-version.sh new file mode 100755 index 000000000000..3a451db3c067 --- /dev/null +++ b/scripts/make-version.sh @@ -0,0 +1,13 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Print the GNU make version in a 5 or 6-digit form. + +set -e + +# Convert the version string x.y.z to a canonical 5 or 6-digit form. +IFS=. +set -- $1 + +# If the 2nd or 3rd field is missing, fill it with a zero. +echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) -- 2.18.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION 2021-07-02 3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen @ 2021-07-04 0:15 ` Nathan Chancellor 2021-07-05 2:02 ` Masahiro Yamada 1 sibling, 0 replies; 11+ messages in thread From: Nathan Chancellor @ 2021-07-04 0:15 UTC (permalink / raw) To: Lecopzer Chen Cc: ndesaulniers, keescook, samitolvanen, linux-kbuild, clang-built-linux, linux-kernel, yj.chiang, masahiroy, michal.lkml On Fri, Jul 02, 2021 at 11:29:42AM +0800, Lecopzer Chen wrote: > To check the GNU make version. Used by the LTO Kconfig. > > LTO with MODVERSIONS will fail in generating correct CRC because > the makefile rule doesn't work for make with version 3.8X.[1] > > Thus we need to check make version during selecting on LTO Kconfig. > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits > for arithmetic comparisons. > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > Makefile | 2 +- > init/Kconfig | 4 ++++ > scripts/make-version.sh | 13 +++++++++++++ > 3 files changed, 18 insertions(+), 1 deletion(-) > create mode 100755 scripts/make-version.sh > > diff --git a/Makefile b/Makefile > index 88888fff4c62..2402745b2ba9 100644 > --- a/Makefile > +++ b/Makefile > @@ -516,7 +516,7 @@ CLANG_FLAGS := > > export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC > export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL > -export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX > +export PERL PYTHON3 CHECK CHECKFLAGS MAKE MAKE_VERSION UTS_MACHINE HOSTCXX > export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD > export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE > > diff --git a/init/Kconfig b/init/Kconfig > index 55f9f7738ebb..ecc110504f87 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -86,6 +86,10 @@ config CC_HAS_ASM_INLINE > config CC_HAS_NO_PROFILE_FN_ATTR > def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror) > > +config MAKE_VERSION > + int > + default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION)) > + > config CONSTRUCTORS > bool > > diff --git a/scripts/make-version.sh b/scripts/make-version.sh > new file mode 100755 > index 000000000000..3a451db3c067 > --- /dev/null > +++ b/scripts/make-version.sh > @@ -0,0 +1,13 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Print the GNU make version in a 5 or 6-digit form. > + > +set -e > + > +# Convert the version string x.y.z to a canonical 5 or 6-digit form. > +IFS=. > +set -- $1 > + > +# If the 2nd or 3rd field is missing, fill it with a zero. > +echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) > -- > 2.18.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION 2021-07-02 3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen 2021-07-04 0:15 ` Nathan Chancellor @ 2021-07-05 2:02 ` Masahiro Yamada 2021-07-05 17:59 ` Sami Tolvanen 1 sibling, 1 reply; 11+ messages in thread From: Masahiro Yamada @ 2021-07-05 2:02 UTC (permalink / raw) To: Lecopzer Chen Cc: Nick Desaulniers, Kees Cook, Sami Tolvanen, Linux Kbuild mailing list, Nathan Chancellor, clang-built-linux, Linux Kernel Mailing List, yj.chiang, Michal Marek On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen <lecopzer.chen@mediatek.com> wrote: > > To check the GNU make version. Used by the LTO Kconfig. > > LTO with MODVERSIONS will fail in generating correct CRC because > the makefile rule doesn't work for make with version 3.8X.[1] > > Thus we need to check make version during selecting on LTO Kconfig. > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits > for arithmetic comparisons. > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> > --- NACK. "Let's add MAKE_VERSION >= 40200 restriction just because I cannot write correct code that works for older Make" is a horrible idea. Also, Kconfig is supposed to check the compiler (or toolchains) capability, not host tool versions. > Makefile | 2 +- > init/Kconfig | 4 ++++ > scripts/make-version.sh | 13 +++++++++++++ > 3 files changed, 18 insertions(+), 1 deletion(-) > create mode 100755 scripts/make-version.sh > > diff --git a/Makefile b/Makefile > index 88888fff4c62..2402745b2ba9 100644 > --- a/Makefile > +++ b/Makefile > @@ -516,7 +516,7 @@ CLANG_FLAGS := > > export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC > export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL > -export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX > +export PERL PYTHON3 CHECK CHECKFLAGS MAKE MAKE_VERSION UTS_MACHINE HOSTCXX > export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ ZSTD > export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE > > diff --git a/init/Kconfig b/init/Kconfig > index 55f9f7738ebb..ecc110504f87 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -86,6 +86,10 @@ config CC_HAS_ASM_INLINE > config CC_HAS_NO_PROFILE_FN_ATTR > def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror) > > +config MAKE_VERSION > + int > + default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION)) > + > config CONSTRUCTORS > bool > > diff --git a/scripts/make-version.sh b/scripts/make-version.sh > new file mode 100755 > index 000000000000..3a451db3c067 > --- /dev/null > +++ b/scripts/make-version.sh > @@ -0,0 +1,13 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Print the GNU make version in a 5 or 6-digit form. > + > +set -e > + > +# Convert the version string x.y.z to a canonical 5 or 6-digit form. > +IFS=. > +set -- $1 > + > +# If the 2nd or 3rd field is missing, fill it with a zero. > +echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) > -- > 2.18.0 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION 2021-07-05 2:02 ` Masahiro Yamada @ 2021-07-05 17:59 ` Sami Tolvanen 2021-07-06 9:06 ` Lecopzer Chen 0 siblings, 1 reply; 11+ messages in thread From: Sami Tolvanen @ 2021-07-05 17:59 UTC (permalink / raw) To: Masahiro Yamada Cc: Lecopzer Chen, Nick Desaulniers, Kees Cook, Linux Kbuild mailing list, Nathan Chancellor, clang-built-linux, Linux Kernel Mailing List, yj.chiang, Michal Marek On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen > <lecopzer.chen@mediatek.com> wrote: > > > > To check the GNU make version. Used by the LTO Kconfig. > > > > LTO with MODVERSIONS will fail in generating correct CRC because > > the makefile rule doesn't work for make with version 3.8X.[1] > > > > Thus we need to check make version during selecting on LTO Kconfig. > > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits > > for arithmetic comparisons. > > > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> > > --- > > > NACK. > > "Let's add MAKE_VERSION >= 40200 restriction > just because I cannot write correct code that > works for older Make" is a horrible idea. > > Also, Kconfig is supposed to check the compiler > (or toolchains) capability, not host tool versions. I feel like requiring a Make that's half a decade old for a feature that also requires a toolchain released last October ago isn't entirely unreasonable. That being said, if Masahiro prefers not to rely on the wildcard function's behavior here, which is a reasonable request, we could simply use the shell to test for the file's existence: diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 34d257653fb4..c6bd62f518ff 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y) cmd_update_lto_symversions = \ rm -f $@.symversions \ $(foreach n, $(filter-out FORCE,$^), \ - $(if $(wildcard $(n).symversions), \ + $(if $(shell test -s $(n).symversions && echo y), \ ; cat $(n).symversions >> $@.symversions)) else cmd_update_lto_symversions = echo >/dev/null This is not quite as efficient as using wildcard, but should work with older Make versions too. Thoughts? Sami ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION 2021-07-05 17:59 ` Sami Tolvanen @ 2021-07-06 9:06 ` Lecopzer Chen 2021-07-07 17:02 ` Sami Tolvanen 0 siblings, 1 reply; 11+ messages in thread From: Lecopzer Chen @ 2021-07-06 9:06 UTC (permalink / raw) To: samitolvanen Cc: clang-built-linux, keescook, lecopzer.chen, linux-kbuild, linux-kernel, masahiroy, michal.lkml, nathan, ndesaulniers, yj.chiang > On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen > > <lecopzer.chen@mediatek.com> wrote: > > > > > > To check the GNU make version. Used by the LTO Kconfig. > > > > > > LTO with MODVERSIONS will fail in generating correct CRC because > > > the makefile rule doesn't work for make with version 3.8X.[1] > > > > > > Thus we need to check make version during selecting on LTO Kconfig. > > > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits > > > for arithmetic comparisons. > > > > > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > > > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> > > > --- > > > > > > NACK. > > > > "Let's add MAKE_VERSION >= 40200 restriction > > just because I cannot write correct code that > > works for older Make" is a horrible idea. > > > > Also, Kconfig is supposed to check the compiler > > (or toolchains) capability, not host tool versions. > > I feel like requiring a Make that's half a decade old for a feature > that also requires a toolchain released last October ago isn't > entirely unreasonable. > > That being said, if Masahiro prefers not to rely on the wildcard > function's behavior here, which is a reasonable request, we could > simply use the shell to test for the file's existence: > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index 34d257653fb4..c6bd62f518ff 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y) > cmd_update_lto_symversions = \ > rm -f $@.symversions \ > $(foreach n, $(filter-out FORCE,$^), \ > - $(if $(wildcard $(n).symversions), \ > + $(if $(shell test -s $(n).symversions && echo y), \ > ; cat $(n).symversions >> $@.symversions)) > else > cmd_update_lto_symversions = echo >/dev/null > > This is not quite as efficient as using wildcard, but should work with > older Make versions too. Thoughts? > I've tested this in both make-4.3 and 3.81, and the CRC is correct. But I'm not sure if anyone would have the "arg list too long" issue. Tested-by: Lecopzer Chen <lecopzer.chen@mediatek.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION 2021-07-06 9:06 ` Lecopzer Chen @ 2021-07-07 17:02 ` Sami Tolvanen 2021-07-09 8:25 ` Lecopzer Chen 0 siblings, 1 reply; 11+ messages in thread From: Sami Tolvanen @ 2021-07-07 17:02 UTC (permalink / raw) To: Lecopzer Chen Cc: clang-built-linux, keescook, linux-kbuild, linux-kernel, masahiroy, michal.lkml, nathan, ndesaulniers, yj.chiang On Tue, Jul 6, 2021 at 2:06 AM Lecopzer Chen <lecopzer.chen@mediatek.com> wrote: > > > On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen > > > <lecopzer.chen@mediatek.com> wrote: > > > > > > > > To check the GNU make version. Used by the LTO Kconfig. > > > > > > > > LTO with MODVERSIONS will fail in generating correct CRC because > > > > the makefile rule doesn't work for make with version 3.8X.[1] > > > > > > > > Thus we need to check make version during selecting on LTO Kconfig. > > > > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits > > > > for arithmetic comparisons. > > > > > > > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > > > > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> > > > > --- > > > > > > > > > NACK. > > > > > > "Let's add MAKE_VERSION >= 40200 restriction > > > just because I cannot write correct code that > > > works for older Make" is a horrible idea. > > > > > > Also, Kconfig is supposed to check the compiler > > > (or toolchains) capability, not host tool versions. > > > > I feel like requiring a Make that's half a decade old for a feature > > that also requires a toolchain released last October ago isn't > > entirely unreasonable. > > > > That being said, if Masahiro prefers not to rely on the wildcard > > function's behavior here, which is a reasonable request, we could > > simply use the shell to test for the file's existence: > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > > index 34d257653fb4..c6bd62f518ff 100644 > > --- a/scripts/Makefile.build > > +++ b/scripts/Makefile.build > > @@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y) > > cmd_update_lto_symversions = \ > > rm -f $@.symversions \ > > $(foreach n, $(filter-out FORCE,$^), \ > > - $(if $(wildcard $(n).symversions), \ > > + $(if $(shell test -s $(n).symversions && echo y), \ > > ; cat $(n).symversions >> $@.symversions)) > > else > > cmd_update_lto_symversions = echo >/dev/null > > > > This is not quite as efficient as using wildcard, but should work with > > older Make versions too. Thoughts? > > > > > I've tested this in both make-4.3 and 3.81, and the CRC is correct. > But I'm not sure if anyone would have the "arg list too long" issue. > > Tested-by: Lecopzer Chen <lecopzer.chen@mediatek.com> Thank you for testing. This should produce a command identical to the wildcard version (with newer Make versions), so that shouldn't be an issue. If nobody objects to this approach, would you mind putting this into a proper patch and sending it as v4? Sami ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION 2021-07-07 17:02 ` Sami Tolvanen @ 2021-07-09 8:25 ` Lecopzer Chen 0 siblings, 0 replies; 11+ messages in thread From: Lecopzer Chen @ 2021-07-09 8:25 UTC (permalink / raw) To: samitolvanen Cc: clang-built-linux, keescook, lecopzer.chen, linux-kbuild, linux-kernel, masahiroy, michal.lkml, nathan, ndesaulniers, yj.chiang > On Tue, Jul 6, 2021 at 2:06 AM Lecopzer Chen <lecopzer.chen@mediatek.com> wrote: > > > > > On Sun, Jul 4, 2021 at 7:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen > > > > <lecopzer.chen@mediatek.com> wrote: > > > > > > > > > > To check the GNU make version. Used by the LTO Kconfig. > > > > > > > > > > LTO with MODVERSIONS will fail in generating correct CRC because > > > > > the makefile rule doesn't work for make with version 3.8X.[1] > > > > > > > > > > Thus we need to check make version during selecting on LTO Kconfig. > > > > > Add CONFIG_MAKE_VERSION which means MAKE_VERSION in canonical digits > > > > > for arithmetic comparisons. > > > > > > > > > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > > > > > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> > > > > > --- > > > > > > > > > > > > NACK. > > > > > > > > "Let's add MAKE_VERSION >= 40200 restriction > > > > just because I cannot write correct code that > > > > works for older Make" is a horrible idea. > > > > > > > > Also, Kconfig is supposed to check the compiler > > > > (or toolchains) capability, not host tool versions. > > > > > > I feel like requiring a Make that's half a decade old for a feature > > > that also requires a toolchain released last October ago isn't > > > entirely unreasonable. > > > > > > That being said, if Masahiro prefers not to rely on the wildcard > > > function's behavior here, which is a reasonable request, we could > > > simply use the shell to test for the file's existence: > > > > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > > > index 34d257653fb4..c6bd62f518ff 100644 > > > --- a/scripts/Makefile.build > > > +++ b/scripts/Makefile.build > > > @@ -388,7 +388,7 @@ ifeq ($(CONFIG_LTO_CLANG) $(CONFIG_MODVERSIONS),y y) > > > cmd_update_lto_symversions = \ > > > rm -f $@.symversions \ > > > $(foreach n, $(filter-out FORCE,$^), \ > > > - $(if $(wildcard $(n).symversions), \ > > > + $(if $(shell test -s $(n).symversions && echo y), \ > > > ; cat $(n).symversions >> $@.symversions)) > > > else > > > cmd_update_lto_symversions = echo >/dev/null > > > > > > This is not quite as efficient as using wildcard, but should work with > > > older Make versions too. Thoughts? > > > > > > > > > I've tested this in both make-4.3 and 3.81, and the CRC is correct. > > But I'm not sure if anyone would have the "arg list too long" issue. > > > > Tested-by: Lecopzer Chen <lecopzer.chen@mediatek.com> > > Thank you for testing. This should produce a command identical to the > wildcard version (with newer Make versions), so that shouldn't be an > issue. If nobody objects to this approach, would you mind putting this > into a proper patch and sending it as v4? Sure, I'll rebase the whole commit and send as v4 soon. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] Kbuild: lto: add make version checking 2021-07-02 3:29 [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS Lecopzer Chen 2021-07-02 3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen @ 2021-07-02 3:29 ` Lecopzer Chen 2021-07-04 0:16 ` Nathan Chancellor 2021-07-05 2:04 ` Masahiro Yamada 1 sibling, 2 replies; 11+ messages in thread From: Lecopzer Chen @ 2021-07-02 3:29 UTC (permalink / raw) To: ndesaulniers, keescook, samitolvanen, linux-kbuild, nathan Cc: clang-built-linux, linux-kernel, yj.chiang, masahiroy, michal.lkml, Lecopzer Chen LTO with MODVERSIONS will fail in generating correct CRC because the makefile rule doesn't work for make with version 3.8X.[1] Thus we need to check make version during selecting on LTO Kconfig. and the suitable version should be 4.2(40200) which released in 2016[2]. [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ [2] https://ftp.gnu.org/gnu/make/ Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> --- arch/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/Kconfig b/arch/Kconfig index 129df498a8e1..beee68149da8 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -639,6 +639,7 @@ config HAS_LTO_CLANG def_bool y # Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510 depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM + depends on !MODVERSIONS || MAKE_VERSION >= 40200 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 -- 2.18.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] Kbuild: lto: add make version checking 2021-07-02 3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen @ 2021-07-04 0:16 ` Nathan Chancellor 2021-07-05 2:04 ` Masahiro Yamada 1 sibling, 0 replies; 11+ messages in thread From: Nathan Chancellor @ 2021-07-04 0:16 UTC (permalink / raw) To: Lecopzer Chen Cc: ndesaulniers, keescook, samitolvanen, linux-kbuild, clang-built-linux, linux-kernel, yj.chiang, masahiroy, michal.lkml On Fri, Jul 02, 2021 at 11:29:43AM +0800, Lecopzer Chen wrote: > LTO with MODVERSIONS will fail in generating correct CRC because > the makefile rule doesn't work for make with version 3.8X.[1] > > Thus we need to check make version during selecting on LTO Kconfig. > and the suitable version should be 4.2(40200) which released in 2016[2]. > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > [2] https://ftp.gnu.org/gnu/make/ > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > arch/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 129df498a8e1..beee68149da8 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -639,6 +639,7 @@ config HAS_LTO_CLANG > def_bool y > # Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510 > depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM > + depends on !MODVERSIONS || MAKE_VERSION >= 40200 > 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 > -- > 2.18.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] Kbuild: lto: add make version checking 2021-07-02 3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen 2021-07-04 0:16 ` Nathan Chancellor @ 2021-07-05 2:04 ` Masahiro Yamada 1 sibling, 0 replies; 11+ messages in thread From: Masahiro Yamada @ 2021-07-05 2:04 UTC (permalink / raw) To: Lecopzer Chen Cc: Nick Desaulniers, Kees Cook, Sami Tolvanen, Linux Kbuild mailing list, Nathan Chancellor, clang-built-linux, Linux Kernel Mailing List, yj.chiang, Michal Marek On Fri, Jul 2, 2021 at 12:29 PM Lecopzer Chen <lecopzer.chen@mediatek.com> wrote: > > LTO with MODVERSIONS will fail in generating correct CRC because > the makefile rule doesn't work for make with version 3.8X.[1] > > Thus we need to check make version during selecting on LTO Kconfig. > and the suitable version should be 4.2(40200) which released in 2016[2]. > > [1] https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@mediatek.com/ > [2] https://ftp.gnu.org/gnu/make/ > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com> > --- NACK for the same reason as 1/2. We need to write correct code. If we cannot, we need to add 'depends on BROKEN'. > arch/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 129df498a8e1..beee68149da8 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -639,6 +639,7 @@ config HAS_LTO_CLANG > def_bool y > # Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510 > depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM > + depends on !MODVERSIONS || MAKE_VERSION >= 40200 > 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 > -- > 2.18.0 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-07-09 8:25 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-02 3:29 [PATCH v3 0/2] Kbuild: lto: add make version checking for MODVERSIONS Lecopzer Chen 2021-07-02 3:29 ` [PATCH v3 1/2] Kbuild: lto: add CONFIG_MAKE_VERSION Lecopzer Chen 2021-07-04 0:15 ` Nathan Chancellor 2021-07-05 2:02 ` Masahiro Yamada 2021-07-05 17:59 ` Sami Tolvanen 2021-07-06 9:06 ` Lecopzer Chen 2021-07-07 17:02 ` Sami Tolvanen 2021-07-09 8:25 ` Lecopzer Chen 2021-07-02 3:29 ` [PATCH v3 2/2] Kbuild: lto: add make version checking Lecopzer Chen 2021-07-04 0:16 ` Nathan Chancellor 2021-07-05 2:04 ` Masahiro Yamada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).