2021-06-30 12:15:45

by Lecopzer Chen

[permalink] [raw]
Subject: [PATCH 0/2] Kbuild: lto: add make version checking

LTO with MODVERSION 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 >> [email protected]))

The issue fixed when make version upgrading to 4.2.

Thus we need to check make version during selecting on LTO Kconfig.
The MAKE_VERSION_INT means MAKE_VERSION in canonical digits integer and
implemnted by imitating CLANG_VERSION.

[1] https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Lecopzer Chen <[email protected]>

Lecopzer Chen (2):
Kbuild: lto: add make-version macros
Kbuild: lto: add make version checking

Makefile | 2 +-
arch/Kconfig | 1 +
init/Kconfig | 4 ++++
scripts/Kconfig.include | 3 +++
scripts/make-version.sh | 13 +++++++++++++
5 files changed, 22 insertions(+), 1 deletion(-)
create mode 100755 scripts/make-version.sh

--
2.18.0


2021-06-30 12:15:46

by Lecopzer Chen

[permalink] [raw]
Subject: [PATCH 2/2] Kbuild: lto: add make version checking

LTO with MODVERSION 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 release in 2016[2].

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://ftp.gnu.org/gnu/make/
Signed-off-by: Lecopzer Chen <[email protected]>
---
arch/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index c45b770d3579..1571957bade5 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -632,6 +632,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 MAKE_VERSION_INT >= 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

2021-06-30 12:15:57

by Lecopzer Chen

[permalink] [raw]
Subject: [PATCH 1/2] Kbuild: lto: add make-version macros

To check the GNU make version. Used by the LTO Kconfig.

LTO with MODVERSION 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.
The MAKE_VERSION_INT means MAKE_VERSION in canonical digits integer and
implemnted by imitating CLANG_VERSION.

[1] https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Lecopzer Chen <[email protected]>
---
Makefile | 2 +-
init/Kconfig | 4 ++++
scripts/Kconfig.include | 3 +++
scripts/make-version.sh | 13 +++++++++++++
4 files changed, 21 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 a61c92066c2e..9f2b71fdf23e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -83,6 +83,10 @@ config TOOLS_SUPPORT_RELR
config CC_HAS_ASM_INLINE
def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)

+config MAKE_VERSION_INT
+ int
+ default $(make-version)
+
config CONSTRUCTORS
bool

diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 0496efd6e117..f956953d0236 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -63,3 +63,6 @@ ld-version := $(shell,set -- $(ld-info) && echo $2)
cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1))
m32-flag := $(cc-option-bit,-m32)
m64-flag := $(cc-option-bit,-m64)
+
+# Get the GNU make version with a canonical digit.
+make-version := $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))
diff --git a/scripts/make-version.sh b/scripts/make-version.sh
new file mode 100755
index 000000000000..ce5af96696cc
--- /dev/null
+++ b/scripts/make-version.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Print the linker name and its 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

2021-06-30 17:03:42

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 1/2] Kbuild: lto: add make-version macros

Hi Lecopzer,

On 6/30/2021 5:14 AM, Lecopzer Chen wrote:
> To check the GNU make version. Used by the LTO Kconfig.
>
> LTO with MODVERSION 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.
> The MAKE_VERSION_INT means MAKE_VERSION in canonical digits integer and
> implemnted by imitating CLANG_VERSION.

implemented

>
> [1] https://lore.kernel.org/lkml/[email protected]/
> Signed-off-by: Lecopzer Chen <[email protected]>
> ---
> Makefile | 2 +-
> init/Kconfig | 4 ++++
> scripts/Kconfig.include | 3 +++
> scripts/make-version.sh | 13 +++++++++++++
> 4 files changed, 21 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 a61c92066c2e..9f2b71fdf23e 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -83,6 +83,10 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>
> +config MAKE_VERSION_INT

It might be cleaner to make this "config MAKE_VERSION". It will not
conflict with the builtin MAKE_VERSION because this is really
CONFIG_MAKE_VERSION, which is how MAKE_VERSION will be handled in Kconfig.

> + int
> + default $(make-version)
> +
> config CONSTRUCTORS
> bool
>
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 0496efd6e117..f956953d0236 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -63,3 +63,6 @@ ld-version := $(shell,set -- $(ld-info) && echo $2)
> cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1))
> m32-flag := $(cc-option-bit,-m32)
> m64-flag := $(cc-option-bit,-m64)
> +
> +# Get the GNU make version with a canonical digit.
> +make-version := $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))

It might be better for this to just be used directly by "config
MAKE_VERSION":

config MAKE_VERSION
int
default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))

> diff --git a/scripts/make-version.sh b/scripts/make-version.sh
> new file mode 100755
> index 000000000000..ce5af96696cc
> --- /dev/null
> +++ b/scripts/make-version.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Print the linker name and its 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}))
>

2021-06-30 17:10:22

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH 2/2] Kbuild: lto: add make version checking

On 6/30/2021 5:14 AM, Lecopzer Chen wrote:
> LTO with MODVERSION 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 release in 2016[2].
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://ftp.gnu.org/gnu/make/
> Signed-off-by: Lecopzer Chen <[email protected]>
> ---
> arch/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index c45b770d3579..1571957bade5 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -632,6 +632,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 MAKE_VERSION_INT >= 40200

If the bug depends on CONFIG_MODVERSIONS, should this be

depends on !MODVERSIONS || MAKE_VERSION >= 40200

? Especially since the problematic block in your original report is
gated on CONFIG_LTO_CLANG + CONFIG_MODVERSIONS.

> 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
>

2021-07-01 08:55:14

by Lecopzer Chen

[permalink] [raw]
Subject: Re: [PATCH 1/2] Kbuild: lto: add make-version macros

> Hi Lecopzer,
>
> On 6/30/2021 5:14 AM, Lecopzer Chen wrote:
> > To check the GNU make version. Used by the LTO Kconfig.
> >
> > LTO with MODVERSION 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.
> > The MAKE_VERSION_INT means MAKE_VERSION in canonical digits integer and
> > implemnted by imitating CLANG_VERSION.
>
> implemented

Thanks!
>
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> > Signed-off-by: Lecopzer Chen <[email protected]>
> > ---
> > Makefile | 2 +-
> > init/Kconfig | 4 ++++
> > scripts/Kconfig.include | 3 +++
> > scripts/make-version.sh | 13 +++++++++++++
> > 4 files changed, 21 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 a61c92066c2e..9f2b71fdf23e 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -83,6 +83,10 @@ config TOOLS_SUPPORT_RELR
> > config CC_HAS_ASM_INLINE
> > def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
> >
> > +config MAKE_VERSION_INT
>
> It might be cleaner to make this "config MAKE_VERSION". It will not
> conflict with the builtin MAKE_VERSION because this is really
> CONFIG_MAKE_VERSION, which is how MAKE_VERSION will be handled in Kconfig.

Okat, thanks, I'll try and fix it in patch v2.
>
> > + int
> > + default $(make-version)
> > +
> > config CONSTRUCTORS
> > bool
> >
> > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> > index 0496efd6e117..f956953d0236 100644
> > --- a/scripts/Kconfig.include
> > +++ b/scripts/Kconfig.include
> > @@ -63,3 +63,6 @@ ld-version := $(shell,set -- $(ld-info) && echo $2)
> > cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1))
> > m32-flag := $(cc-option-bit,-m32)
> > m64-flag := $(cc-option-bit,-m64)
> > +
> > +# Get the GNU make version with a canonical digit.
> > +make-version := $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))
>
> It might be better for this to just be used directly by "config
> MAKE_VERSION":
>
> config MAKE_VERSION
> int
> default $(shell,$(srctree)/scripts/make-version.sh $(MAKE_VERSION))

Sure, I'll fix in patch v2, thank you.

>
> > diff --git a/scripts/make-version.sh b/scripts/make-version.sh
> > new file mode 100755
> > index 000000000000..ce5af96696cc
> > --- /dev/null
> > +++ b/scripts/make-version.sh
> > @@ -0,0 +1,13 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Print the linker name and its 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}))
> >

2021-07-01 08:56:16

by Lecopzer Chen

[permalink] [raw]
Subject: Re: [PATCH 2/2] Kbuild: lto: add make version checking


> On 6/30/2021 5:14 AM, Lecopzer Chen wrote:
> > LTO with MODVERSION 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 release in 2016[2].
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> > [2] https://ftp.gnu.org/gnu/make/
> > Signed-off-by: Lecopzer Chen <[email protected]>
> > ---
> > arch/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index c45b770d3579..1571957bade5 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -632,6 +632,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 MAKE_VERSION_INT >= 40200
>
> If the bug depends on CONFIG_MODVERSIONS, should this be
>
> depends on !MODVERSIONS || MAKE_VERSION >= 40200
>
> ? Especially since the problematic block in your original report is
> gated on CONFIG_LTO_CLANG + CONFIG_MODVERSIONS.
>

You're right, I'll fix it in v2, thanks for reviewing.


> > 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
> >