Re: [PATCH v4] Kbuild: lto: fix module versionings mismatch in GNU make 3.X

[Masahiro Yamada]

On Thu, Jul 15, 2021 at 4:37 PM Lecopzer Chen
<lecopzer.chen@xxxxxxxxxxxx> 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.
>
> 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.
>
> Rebase from both:
> 1. [https://lore.kernel.org/lkml/20210616080252.32046-1-lecopzer.chen@xxxxxxxxxxxx/]
> 2. [https://lore.kernel.org/lkml/20210702032943.7865-1-lecopzer.chen@xxxxxxxxxxxx/]
>
> Fixes: 38e89184900385 ("kbuild: lto: fix module versioning")
> Sign-off-byed: Sami Tolvanen <samitolvanen@xxxxxxxxxx>


This Signed-off-by is not correct usage, I think.

I replaced it with Co-developed-by.

I usually use 'for ... do ... done' instead of $(foreach ...),
and 'if ... then ... fi' instead of $(if ...).

But, this is the minimal change without causing
too long command line.

Applied to linux-kbuild. Thanks.





> Signed-off-by: Lecopzer Chen <lecopzer.chen@xxxxxxxxxxxx>
> ---
>  scripts/Makefile.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 10b2f2380d6f..02197cb8e3a7 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -386,7 +386,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
> --
> 2.18.0
>


-- 
Best Regards
Masahiro Yamada