| Message ID | 20241010-icall-detect-vers-v1-1-8f114956aa88@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Normalized CFI integer tags Kconfig fixes | expand |
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
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>
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?
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
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
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)
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