[PATCH 0/2] Normalized CFI integer tags Kconfig fixes

[PATCH 0/2] Normalized CFI integer tags Kconfig fixes

[Alice Ryhl]

This series adds a config option for the LLVM version in rustc and uses
it to fix the detection for the CFI integer tags constructors bugfix.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (1):
      cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS

Gary Guo (1):
      kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

 arch/Kconfig             | 26 ++++++++++++--------------
 init/Kconfig             |  8 ++++++--
 scripts/Kconfig.include  |  4 ++++
 scripts/rustc-version.sh | 31 +++++++++++++++++++++++++------
 4 files changed, 47 insertions(+), 22 deletions(-)
---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241010-icall-detect-vers-79c2dc3cd41d

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Alice Ryhl]

From: Gary Guo <gary@garyguo.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@garyguo.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

Re: [PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Miguel Ojeda]

On Thu, Oct 10, 2024 at 11:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> From: Gary Guo <gary@garyguo.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@garyguo.net>

Alice: when I apply this, I will need to add your Signed-off-by here
(i.e. when handling patches from others, you need to add your SoB
too).

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

I guess if we don't find "LLVM" in the output, something weird is
going on, so I guess it is reasonable not printing either here.
Although, in principle, we could preserve information and print at
least the `$rustc` one.

Anyway, we may need to rethink this when we start supporting e.g. the
GCC backend, so I think it is fine as it is.

Thanks!

Cheers,
Miguel

Re: [PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Alice Ryhl]

On Thu, Oct 10, 2024 at 12:55 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Thu, Oct 10, 2024 at 11:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > From: Gary Guo <gary@garyguo.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@garyguo.net>
>
> Alice: when I apply this, I will need to add your Signed-off-by here
> (i.e. when handling patches from others, you need to add your SoB
> too).

Oh, sorry I forgot about this.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Alice Ryhl]

On Thu, Oct 10, 2024 at 12:55 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Thu, Oct 10, 2024 at 11:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > From: Gary Guo <gary@garyguo.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@garyguo.net>
>
> Alice: when I apply this, I will need to add your Signed-off-by here
> (i.e. when handling patches from others, you need to add your SoB
> too).
>
> > +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
>
> I guess if we don't find "LLVM" in the output, something weird is
> going on, so I guess it is reasonable not printing either here.
> Although, in principle, we could preserve information and print at
> least the `$rustc` one.
>
> Anyway, we may need to rethink this when we start supporting e.g. the
> GCC backend, so I think it is fine as it is.

I guess we can just do

rustc_llvm_version=0

in that case?

Re: [PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Sami Tolvanen]

On Thu, Oct 10, 2024 at 01:02:17PM +0200, Alice Ryhl wrote:
> On Thu, Oct 10, 2024 at 12:55 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > On Thu, Oct 10, 2024 at 11:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
> > >
> > > From: Gary Guo <gary@garyguo.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@garyguo.net>
> >
> > Alice: when I apply this, I will need to add your Signed-off-by here
> > (i.e. when handling patches from others, you need to add your SoB
> > too).
> >
> > > +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
> >
> > I guess if we don't find "LLVM" in the output, something weird is
> > going on, so I guess it is reasonable not printing either here.
> > Although, in principle, we could preserve information and print at
> > least the `$rustc` one.
> >
> > Anyway, we may need to rethink this when we start supporting e.g. the
> > GCC backend, so I think it is fine as it is.
> 
> I guess we can just do
> 
> rustc_llvm_version=0
> 
> in that case?

Agreed, that should be sufficient. I'm not sure what the GCC back-end
status is, but this patch looks good to me whether you plan to fold in
this change or not. FWIW:

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami

Re: [PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Masahiro Yamada]

On Thu, Oct 10, 2024 at 6:38 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> From: Gary Guo <gary@garyguo.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@garyguo.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

Re: [PATCH 1/2] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Miguel Ojeda]

On Fri, Oct 11, 2024 at 4:04 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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.

Yeah, I think we can do that: one is `--verbose`, the other isn't, but
the verbose one should (hopefully) always contain the non-verbose as
the first line. Since it is in the first line, we can directly do the
`set` without having to "extract" that line -- something like [1]
below?

Independent scripts would be simpler, so that sounds good too.

By the way, this is a prerequisite for an actual fix we need, so I
would like to send it as part of a `rust-fixes` PR -- your Ack would
be great!

Cheers,
Miguel

[1]

if ! output=$("$@" --version --verbose 2>/dev/null); then
    echo 0 0
    exit 1
fi

set -- $output
rustc_version=$(get_rustc_canonical_version $2)

if ! output=$(echo "$output" | grep ^LLVM); then
    echo $rustc_version 0
    exit 1
fi

set -- $output
echo $rustc_version $(get_llvm_canonical_version $3)

[PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Gary Guo]

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].

The invocation of rustc-version is being moved from init/Kconfig to
scripts/Kconfig.include 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@garyguo.net>
---
 init/Kconfig                  |  6 +++++-
 scripts/Kconfig.include       |  3 +++
 scripts/rustc-llvm-version.sh | 22 ++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100755 scripts/rustc-llvm-version.sh

diff --git a/init/Kconfig b/init/Kconfig
index fbd0cb06a50a..304e2bd32bfd 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..33193ca6e803 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -65,6 +65,9 @@ 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-version := $(shell,$(srctree)/scripts/rustc-version.sh $(RUSTC))
+rustc-llvm-version := $(shell,$(srctree)/scripts/rustc-llvm-version.sh $(RUSTC))
+
 # $(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-llvm-version.sh b/scripts/rustc-llvm-version.sh
new file mode 100755
index 000000000000..b8ffa24afea8
--- /dev/null
+++ b/scripts/rustc-llvm-version.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Usage: $ ./rustc-version.sh rustc
+#
+# Print the LLVM version that the Rust compiler uses in a 6 digit form.
+
+# Convert the version string x.y.z to a canonical up-to-6-digits form.
+get_canonical_version()
+{
+	IFS=.
+	set -- $1
+	echo $((10000 * $1 + 100 * $2 + $3))
+}
+
+if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then
+	set -- $output
+	get_canonical_version $3
+else
+	echo 0
+	exit 1
+fi

base-commit: f5e50614e39e74401b492a2fa125f2e2b6458bab
-- 
2.44.1

Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Miguel Ojeda]

On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
>
> The invocation of rustc-version is being moved from init/Kconfig to
> scripts/Kconfig.include for consistency with cc-version.

Yeah, I am ambivalent. Dropping them would minimize changes and avoid
introducing something only used once, which would be nice too. Happy
either way.

> +if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then

Similarly, I wonder if we should use '^LLVM version: ' here or similar
to minimize the chances the "LLVM" string appears elsewhere in the
future (perhaps in a custom string a vendor adds, though I would
expect them to add it lowercase). We are relying on having a $3 when
splitting anyway.

Depending on what Masahiro prefers, I will take this one or the
one-invocation-only one.

Thanks Gary!

Cheers,
Miguel

Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Gary Guo]

On Fri, 11 Oct 2024 13:53:47 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
> >
> > The invocation of rustc-version is being moved from init/Kconfig to
> > scripts/Kconfig.include for consistency with cc-version.  
> 
> Yeah, I am ambivalent. Dropping them would minimize changes and avoid
> introducing something only used once, which would be nice too. Happy
> either way.
> 

Another motivation is that in my helper inline series, I need to do
arithmetic on LLVM version (divide it by 10000 to get major verison),
which isn't possible for config options, but will work for variables
defiend in Kconfig.include.

I didn't mention it in the commit message for this patch because this
is not my patch series :)

Best,
Gary

> > +if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then  
> 
> Similarly, I wonder if we should use '^LLVM version: ' here or similar
> to minimize the chances the "LLVM" string appears elsewhere in the
> future (perhaps in a custom string a vendor adds, though I would
> expect them to add it lowercase). We are relying on having a $3 when
> splitting anyway.
> 
> Depending on what Masahiro prefers, I will take this one or the
> one-invocation-only one.
> 
> Thanks Gary!
> 
> Cheers,
> Miguel

Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Masahiro Yamada]

On Fri, Oct 11, 2024 at 9:06 PM Gary Guo <gary@garyguo.net> wrote:
>
> On Fri, 11 Oct 2024 13:53:47 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>
> > On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
> > >
> > > The invocation of rustc-version is being moved from init/Kconfig to
> > > scripts/Kconfig.include for consistency with cc-version.
> >
> > Yeah, I am ambivalent. Dropping them would minimize changes and avoid
> > introducing something only used once, which would be nice too. Happy
> > either way.
>
> Another motivation is that in my helper inline series, I need to do
> arithmetic on LLVM version (divide it by 10000 to get major verison),
> which isn't possible for config options, but will work for variables
> defiend in Kconfig.include.
>
> I didn't mention it in the commit message for this patch because this
> is not my patch series :)


I tend to agree with Muguel.
The motivation of having cc-version in scripts/Kconfig.include
is to check the presence of the supported C compiler, as C compiler
is mandatory (in contrast, Rust compiler is optional).

If you have a reason to have it in scripts/Kconfig.include
for your future works, it is OK with me, but I cannot judge it.






>
> Best,
> Gary
>
> > > +if output=$("$@" --version --verbose 2>/dev/null | grep LLVM); then
> >
> > Similarly, I wonder if we should use '^LLVM version: ' here or similar
> > to minimize the chances the "LLVM" string appears elsewhere in the
> > future (perhaps in a custom string a vendor adds, though I would
> > expect them to add it lowercase). We are relying on having a $3 when
> > splitting anyway.
> >
> > Depending on what Masahiro prefers, I will take this one or the
> > one-invocation-only one.
> >
> > Thanks Gary!
> >
> > Cheers,
> > Miguel
>
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Miguel Ojeda]

On Mon, Oct 14, 2024 at 6:27 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I tend to agree with Muguel.
> The motivation of having cc-version in scripts/Kconfig.include
> is to check the presence of the supported C compiler, as C compiler
> is mandatory (in contrast, Rust compiler is optional).
>
> If you have a reason to have it in scripts/Kconfig.include
> for your future works, it is OK with me, but I cannot judge it.

Thanks Masahiro -- OK, since it seems Gary needs it, and we already
have the patch here, I will take that one.

Cheers,
Miguel

Re: [PATCH] kbuild: rust: add `CONFIG_RUSTC_LLVM_VERSION`

[Miguel Ojeda]

On Fri, Oct 11, 2024 at 1:41 PM Gary Guo <gary@garyguo.net> wrote:
>
> 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].
>
> The invocation of rustc-version is being moved from init/Kconfig to
> scripts/Kconfig.include 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@garyguo.net>

Applied to `rust-fixes` -- thanks everyone!

    [ Added missing `-llvm` to the Usage documentation. - Miguel ]

Cheers,
Miguel

[PATCH 2/2] cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS

[Alice Ryhl]

The HAVE_CFI_ICALL_NORMALIZE_INTEGERS option has some tricky conditions
when KASAN or GCOV are turned on, as in that case we need some clang and
rustc fixes [1][2] to avoid boot failures. The intent with the current
setup is that you should be able to override the check and turn on the
option if your clang/rustc has the fix. However, this override does not
work in practice. Thus, use the new RUSTC_LLVM_VERSION to correctly
implement the check for whether the fix is available.

Additionally, remove KASAN_HW_TAGS from the list of incompatible
options. The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with
KASAN because LLVM will emit some constructors when using KASAN that are
assigned incorrect CFI tags. These constructors are emitted due to use
of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are
respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled.
However, the KASAN_HW_TAGS option relies on hardware support for MTE
instead and does not pass either flag. (Note also that KASAN_HW_TAGS
does not `select CONSTRUCTORS`.)

Link: https://github.com/llvm/llvm-project/pull/104826 [1]
Link: https://github.com/rust-lang/rust/pull/129373 [2]
Fixes: 4c66f8307ac0 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig")
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 arch/Kconfig | 26 ++++++++++++--------------
 init/Kconfig |  2 +-
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8af374ea1adc..00163e4a237c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -838,7 +838,7 @@ config CFI_CLANG
 config CFI_ICALL_NORMALIZE_INTEGERS
 	bool "Normalize CFI tags for integers"
 	depends on CFI_CLANG
-	depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS
+	depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG
 	help
 	  This option normalizes the CFI tags for integer types so that all
 	  integer types of the same size and signedness receive the same CFI
@@ -851,21 +851,19 @@ config CFI_ICALL_NORMALIZE_INTEGERS
 
 	  This option is necessary for using CFI with Rust. If unsure, say N.
 
-config HAVE_CFI_ICALL_NORMALIZE_INTEGERS
-	def_bool !GCOV_KERNEL && !KASAN
-	depends on CFI_CLANG
+config HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG
+	def_bool y
 	depends on $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers)
-	help
-	  Is CFI_ICALL_NORMALIZE_INTEGERS supported with the set of compilers
-	  currently in use?
+	# With GCOV/KASAN we need this fix: https://github.com/llvm/llvm-project/pull/104826
+	depends on CLANG_VERSION >= 190000 || (!GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS)
 
-	  This option defaults to false if GCOV or KASAN is enabled, as there is
-	  an LLVM bug that makes normalized integers tags incompatible with
-	  KASAN and GCOV. Kconfig currently does not have the infrastructure to
-	  detect whether your rustc compiler contains the fix for this bug, so
-	  it is assumed that it doesn't. If your compiler has the fix, you can
-	  explicitly enable this option in your config file. The Kconfig logic
-	  needed to detect this will be added in a future kernel release.
+config HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC
+	def_bool y
+	depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG
+	depends on RUSTC_VERSION >= 107900
+	# With GCOV/KASAN we need this fix: https://github.com/rust-lang/rust/pull/129373
+	depends on (RUSTC_LLVM_VERSION >= 190000 && RUSTC_VERSION >= 108200) || \
+		(!GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS)
 
 config CFI_PERMISSIVE
 	bool "Use CFI in permissive mode"
diff --git a/init/Kconfig b/init/Kconfig
index 98cf859d58c2..c521e1421ad4 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1950,7 +1950,7 @@ config RUST
 	depends on !GCC_PLUGIN_RANDSTRUCT
 	depends on !RANDSTRUCT
 	depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
-	depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
+	depends on !CFI_CLANG || HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC
 	select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG
 	depends on !CALL_PADDING || RUSTC_VERSION >= 108100
 	depends on !KASAN_SW_TAGS

-- 
2.47.0.rc0.187.ge670bccf7e-goog

Re: [PATCH 2/2] cfi: fix conditions for HAVE_CFI_ICALL_NORMALIZE_INTEGERS

[Sami Tolvanen]

On Thu, Oct 10, 2024 at 09:38:27AM +0000, Alice Ryhl wrote:
> The HAVE_CFI_ICALL_NORMALIZE_INTEGERS option has some tricky conditions
> when KASAN or GCOV are turned on, as in that case we need some clang and
> rustc fixes [1][2] to avoid boot failures. The intent with the current
> setup is that you should be able to override the check and turn on the
> option if your clang/rustc has the fix. However, this override does not
> work in practice. Thus, use the new RUSTC_LLVM_VERSION to correctly
> implement the check for whether the fix is available.
> 
> Additionally, remove KASAN_HW_TAGS from the list of incompatible
> options. The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with
> KASAN because LLVM will emit some constructors when using KASAN that are
> assigned incorrect CFI tags. These constructors are emitted due to use
> of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are
> respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled.
> However, the KASAN_HW_TAGS option relies on hardware support for MTE
> instead and does not pass either flag. (Note also that KASAN_HW_TAGS
> does not `select CONSTRUCTORS`.)
> 
> Link: https://github.com/llvm/llvm-project/pull/104826 [1]
> Link: https://github.com/rust-lang/rust/pull/129373 [2]
> Fixes: 4c66f8307ac0 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

This looks correct to me. Checking the LLVM version is definitely a
better option. Thanks!

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami

Re: [PATCH 0/2] Normalized CFI integer tags Kconfig fixes

[Miguel Ojeda]

On Thu, Oct 10, 2024 at 11:38 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> This series adds a config option for the LLVM version in rustc and uses
> it to fix the detection for the CFI integer tags constructors bugfix.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Applied to `rust-fixes` (using Gary's alternative patch) -- thanks everyone!

Cheers,
Miguel