Message-ID: <CAK7LNARBXt=CWy5CgtHqdePw5L6EtD15emS2Fvre4QWfm_LjUg@mail.gmail.com>
Date: Fri, 11 Oct 2024 11:03:24 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Sami Tolvanen <samitolvanen@...gle.com>, 
	Kees Cook <kees@...nel.org>, Nathan Chancellor <nathan@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, linux-kernel@...r.kernel.org, 
	linux-kbuild@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	llvm@...ts.linux.dev
Subject: Re: [PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

On Thu, Oct 10, 2024 at 6:38 PM Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> From: Gary Guo <gary@...yguo.net>
>
> Each version of Rust supports a range of LLVM versions. There are cases where
> we want to gate a config on the LLVM version instead of the Rust version.
> Normalized cfi integer tags are one example [1].
>
> For consistency with cc-version and ld-version, the new version number is added
> to the existing rustc-version script, rather than being added to a new script.
>
> The invocation of rustc-version is being moved from init/Kconfig to
> scripts/Kconfig.include to avoid invoking rustc-version.sh twice and for
> consistency with cc-version.
>
> Link: https://lore.kernel.org/all/20240925-cfi-norm-kasan-fix-v1-1-0328985cdf33@google.com/ [1]
> Signed-off-by: Gary Guo <gary@...yguo.net>
> ---
>  init/Kconfig             |  6 +++++-
>  scripts/Kconfig.include  |  4 ++++
>  scripts/rustc-version.sh | 31 +++++++++++++++++++++++++------
>  3 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 530a382ee0fe..98cf859d58c2 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -62,7 +62,7 @@ config LLD_VERSION
>
>  config RUSTC_VERSION
>         int
> -       default $(shell,$(srctree)/scripts/rustc-version.sh $(RUSTC))
> +       default $(rustc-version)
>         help
>           It does not depend on `RUST` since that one may need to use the version
>           in a `depends on`.
> @@ -78,6 +78,10 @@ config RUST_IS_AVAILABLE
>           In particular, the Makefile target 'rustavailable' is useful to check
>           why the Rust toolchain is not being detected.
>
> +config RUSTC_LLVM_VERSION
> +       int
> +       default $(rustc-llvm-version)
> +
>  config CC_CAN_LINK
>         bool
>         default $(success,$(srctree)/scripts/cc-can-link.sh $(CC) $(CLANG_FLAGS) $(USERCFLAGS) $(USERLDFLAGS) $(m64-flag)) if 64BIT
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 785a491e5996..788097a55731 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -65,6 +65,10 @@ cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$
>  m32-flag := $(cc-option-bit,-m32)
>  m64-flag := $(cc-option-bit,-m64)
>
> +rustc-info := $(shell,$(srctree)/scripts/rustc-version.sh $(RUSTC))
> +rustc-version := $(shell,set -- $(rustc-info) && echo $1)
> +rustc-llvm-version := $(shell,set -- $(rustc-info) && echo $2)
> +
>  # $(rustc-option,<flag>)
>  # Return y if the Rust compiler supports <flag>, n otherwise
>  # Calls to this should be guarded so that they are not evaluated if
> diff --git a/scripts/rustc-version.sh b/scripts/rustc-version.sh
> index 4e22593e2eab..24e19ed8f234 100755
> --- a/scripts/rustc-version.sh
> +++ b/scripts/rustc-version.sh
> @@ -3,14 +3,23 @@
>  #
>  # Usage: $ ./rustc-version.sh rustc
>  #
> -# Print the Rust compiler version in a 6 or 7-digit form.
> +# Print the Rust compiler version and the LLVM version it uses in a 6 or
> +# 7-digit form.
> +
> +# Convert the version string x.y.z to a canonical up-to-6-digits form.
> +get_llvm_canonical_version()
> +{
> +       IFS=.
> +       set -- $1
> +       echo $((10000 * $1 + 100 * $2 + $3))
> +}
>
>  # Convert the version string x.y.z to a canonical up-to-7-digits form.
>  #
> -# Note that this function uses one more digit (compared to other
> -# instances in other version scripts) to give a bit more space to
> +# Note that this function uses one more digit (compared to other instances in
> +# other version scripts and the instance above) to give a bit more space to
>  # `rustc` since it will reach 1.100.0 in late 2026.
> -get_canonical_version()
> +get_rustc_canonical_version()
>  {
>         IFS=.
>         set -- $1
> @@ -19,8 +28,18 @@ get_canonical_version()
>
>  if output=$("$@" --version 2>/dev/null); then
>         set -- $output
> -       get_canonical_version $2
> +       rustc_version=$(get_rustc_canonical_version $2)
>  else
> -       echo 0
> +       echo 0 0
>         exit 1
>  fi
> +
> +if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then
> +       set -- $output
> +       rustc_llvm_version=$(get_llvm_canonical_version $3)
> +else
> +       echo 0 0
> +       exit 1
> +fi
> +
> +echo $rustc_version $rustc_llvm_version
>
> --
> 2.47.0.rc0.187.ge670bccf7e-goog
>
>



Please notice there is no reason to process
rustc_version and rustc_llvm_version in the same script
because they are completely independent.
It even invokes "rustc --version" twice.

If you implement it this way, you do not need to
touch scripts/rustc-version.sh at all.



If both rustc_version and rustc_llvm_version were
retrieved from the single invocation of "rustc --version",
I would agree "OK, it makes sense to implement it in the
same script". But, it isn't.





--
Best Regards


Masahiro Yamada

Powered by blists - more mailing lists