[PATCH] kbuild: lto: fix module versionings mismatch in incremental build

[Lecopzer Chen]

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

For example:
rm -f fs/notify/built-in.a.symversions ; rm -f fs/notify/built-in.a; \
llvm-ar cDPrST fs/notify/built-in.a fs/notify/fsnotify.o \
fs/notify/notification.o fs/notify/group.o ...

`foreach n` shows nothing to `cat` into $(n).symversions because
`if $(wildcard $(n).symversions)` return nothing, but actually
they do exist during this line was executed.

-rw-r--r-- 1 root root 168580 Jun 13 19:10 fs/notify/fsnotify.o
-rw-r--r-- 1 root root 111 Jun 13 19:10 fs/notify/fsnotify.o.symversions

The reason is the $(n).symversions are generated at runtime, but
Makefile wildcard function expends and checks the file exist or not
during parsing the Makefile.

Thus fix this by use `test` shell command to check the file
existence in runtime.

Fixes: 38e89184900385 ("kbuild: lto: fix module versioning")
Signed-off-by: Lecopzer Chen <lecopz...@mediatek.com>
---
scripts/Makefile.build | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 949f723efe53..a91012b06ebb 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -387,8 +387,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), \
- ; cat $(n).symversions >> $@.symversions))
+ ; test -s $(n).symversions && cat $(n).symversions >> $@.symversions)
else
cmd_update_lto_symversions = echo >/dev/null
endif
--
2.18.0

[Lecopzer Chen]

Andy Lavr <andy...@gmail.com> point out there is a build failed for

M] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gpfifog84.o

make[6]: /bin/sh: Argument list too long

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

make[6]: *** [/scripts/Makefile.build:463:
drivers/gpu/drm/amd/amdgpu/amdgpu.o] Error 127
make[5]: *** [/scripts/Makefile.build:529: drivers/gpu/drm/amd/amdgpu]
Error 2
make[5]: *** Waiting for unfinished jobs....
CC [M] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gpfifogf100.o


I'll fix it in patch v2.

thanks Andy

Lecopzer

[Kees Cook]

On Mon, Jun 14, 2021 at 01:51:09PM +0800, Lecopzer Chen wrote:
> 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.

I'm doing this, and I don't see the problem:

$ make LLVM=1 LLVM_IAS=1 distclean
$ make LLVM=1 LLVM_IAS=1 menuconfig
*enable LTO*
*enable a module*
$ make LLVM=1 LLVM_IAS=1 -j...

What series of commands (and .config) shows this for you?

--
Kees Cook

[Lecopzer Chen]

Hi Kees,

Thanks for you checking.

After double checking in clean android kernel build, this causes by
make version.
(I have build failed in Linux LTO,
seems it's not well support in contract to android?)

I knew Google has LTO first in Android and upstream later, and most code
are same as upstream, so the env here I use Android common kernel for
easily testing.


Test env is android common kernel: android12-5.4 [1] with its latest code
and it builds from build.sh[2]

$ BUILD_CONFIG=common/build.config.gki.aarch64 build/build.sh
+ make O=.... LLVM=1 LLVM_IAS=1 DEPMOD=depmod -j12 Image modules Image.lz4

With make set to v3.81, this can be reproduced with CONFIG_TEE=m.
With version >= 4.2 this is not reproducible.


Our build env default set make to v3.81, although Android uses hermetic build
and v4.3 now, but Linux doesn't have such things.

Maybe we can add build time checking or comment before CFI module versioning
build rules to avoid anyone struggling with this again:).

[1] https://android.googlesource.com/kernel/common/+/refs/heads/android12-5.4
[2] https://android.googlesource.com/kernel/build/+/refs/heads/master

thanks,
Lecopzer

[Kees Cook]

Ah, very interesting. While there are tests in Makefile for
MAKE_VERSION, if we want to do this, it should likely be extended to
Kconfig, as that's where the initial version tests for things happen. We
could require MAKE_VERSION >= 4.2 for LTO?

-Kees

>
>
> Our build env default set make to v3.81, although Android uses hermetic build
> and v4.3 now, but Linux doesn't have such things.
>
> Maybe we can add build time checking or comment before CFI module versioning
> build rules to avoid anyone struggling with this again:).
>
> [1] https://android.googlesource.com/kernel/common/+/refs/heads/android12-5.4
> [2] https://android.googlesource.com/kernel/build/+/refs/heads/master
>
> thanks,
> Lecopzer
>
>

--
Kees Cook

[Lecopzer Chen]

Yes, We can imitate how CLANG_VERSION was implemented in Kconfig.

Accroding to GNU make release page[1], I've only tested for 3.81,
4.2 and 4.3.
4.2 was released in 2016, I think it's fine for LTO lowest version.


[1] https://ftp.gnu.org/gnu/make/


thanks,
Lecopzer

[Kees Cook]

On Wed, Jun 16, 2021 at 04:02:52PM +0800, Lecopzer Chen wrote:
> Yes, We can imitate how CLANG_VERSION was implemented in Kconfig.
>
> Accroding to GNU make release page[1], I've only tested for 3.81,
> 4.2 and 4.3.
> 4.2 was released in 2016, I think it's fine for LTO lowest version.

Okay, sounds good. Are you able to build a patch for this?

Thanks for figuring it out!

--
Kees Cook

[Lecopzer Chen]

Okay, I'll send a patch in a couple of weeks.

Thanks,
Lecopzer