Re: [PATCH v2 09/10] kbuild: refactor cmd_modversions_c

From: Nick Desaulniers <ndesaulniers@google.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Michal Marek <michal.lkml@markovi.net>,
	Nathan Chancellor <nathan@kernel.org>,
	llvm@lists.linux.dev,  Sami Tolvanen <samitolvanen@google.com>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [PATCH v2 09/10] kbuild: refactor cmd_modversions_c
Date: Tue, 5 Apr 2022 09:42:36 -0700	[thread overview]
Message-ID: <CAKwvOd=oCivWGHLFLCuPeH1R-gKGRmsvg=NV5ZbSVhd13cqgSw@mail.gmail.com> (raw)
In-Reply-To: <20220405113359.2880241-10-masahiroy@kernel.org>

On Tue, Apr 5, 2022 at 4:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> cmd_modversions_c implements two parts; run genksyms to calculate CRCs
> of exported symbols, run $(LD) to update the object with the CRCs. The
> latter is not executed for CONFIG_LTO_CLANG=y since the object is not
> ELF but LLVM bit code at this point.
>
> The first part can be unified because we can always use $(NM) instead
> of "$(OBJDUMP) -h" to dump the symbols.
>
> Split the code into the two macros, cmd_gen_symversions_c and
> cmd_modversions.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> Changes in v2:
>  - new
>
>  scripts/Makefile.build | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index d934bdf84de4..ba2be555f942 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -169,29 +169,25 @@ ifdef CONFIG_MODVERSIONS
>  #   the actual value of the checksum generated by genksyms
>  # o remove .tmp_<file>.o to <file>.o
>
> -ifdef CONFIG_LTO_CLANG
>  # Generate .o.symversions files for each .o with exported symbols, and link these
>  # to the kernel and/or modules at the end.
> -cmd_modversions_c =                                                            \
> +gen_symversions =                                                              \
>         if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then                       \
> -               $(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))  \
> +               $(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
>                     > $@.symversions;                                           \
>         else                                                                    \
>                 rm -f $@.symversions;                                           \
> -       fi;
> -else
> -cmd_modversions_c =                                                            \
> -       if $(OBJDUMP) -h $@ | grep -q __ksymtab; then                           \
> -               $(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))  \
> -                   > $(@D)/.tmp_$(@F:.o=.ver);                                 \
> -                                                                               \
> +       fi
> +
> +cmd_gen_symversions_c =        $(call gen_symversions,c)
> +
> +cmd_modversions =                                                              \
> +       if [ -r $@.symversions ]; then                                          \
>                 $(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@               \
> -                       -T $(@D)/.tmp_$(@F:.o=.ver);                            \
> +                       -T $@.symversions;                                      \
>                 mv -f $(@D)/.tmp_$(@F) $@;                                      \
> -               rm -f $(@D)/.tmp_$(@F:.o=.ver);                                 \
>         fi
>  endif
> -endif
>
>  ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>  # compiler will not generate __mcount_loc use recordmcount or recordmcount.pl
> @@ -269,7 +265,8 @@ define rule_cc_o_c
>         $(call cmd,checksrc)
>         $(call cmd,checkdoc)
>         $(call cmd,gen_objtooldep)
> -       $(call cmd,modversions_c)
> +       $(call cmd,gen_symversions_c)
> +       $(if $(CONFIG_LTO_CLANG),,$(call cmd,modversions))
>         $(call cmd,record_mcount)
>  endef
>
> --
> 2.32.0
>

-- 
Thanks,
~Nick Desaulniers