Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210715073716.17477-1-lecopzer.chen@mediatek.com>
In-Reply-To: <20210715073716.17477-1-lecopzer.chen@mediatek.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Sun, 18 Jul 2021 23:43:21 +0900
Message-ID: <CAK7LNAToKcvCLSrAbjb9RJn3mmwt_fzGTq+3ejNb034PVNeG2g@mail.gmail.com>
Subject: Re: [PATCH v4] Kbuild: lto: fix module versionings mismatch in GNU
 make 3.X
To:     Lecopzer Chen <lecopzer.chen@mediatek.com>
Cc:     clang-built-linux <clang-built-linux@googlegroups.com>,
        Kees Cook <keescook@chromium.org>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Michal Marek <michal.lkml@markovi.net>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Sami Tolvanen <samitolvanen@google.com>, yj.chiang@mediatek.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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


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@mediatek.com>
> ---
>  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