[PATCH] rust: kbuild: set `bindgen`'s Rust target version

[PATCH] rust: kbuild: set `bindgen`'s Rust target version

[Miguel Ojeda]

Each `bindgen` release may upgrade the list of Rust targets. For instance,
currently, in their master branch [1], the latest ones are:

    Nightly => {
        vectorcall_abi: #124485,
        ptr_metadata: #81513,
        layout_for_ptr: #69835,
    },
    Stable_1_77(77) => { offset_of: #106655 },
    Stable_1_73(73) => { thiscall_abi: #42202 },
    Stable_1_71(71) => { c_unwind_abi: #106075 },
    Stable_1_68(68) => { abi_efiapi: #105795 },

By default, the highest stable release in their list is used, and users
are expected to set one if they need to support older Rust versions
(e.g. see [2]).

Thus, over time, new Rust features are used by default, and at some
point, it is likely that `bindgen` will emit Rust code that requires a
Rust version higher than our minimum (or perhaps enabling an unstable
feature). Currently, there is no problem because the maximum they have,
as seen above, is Rust 1.77.0, and our current minimum is Rust 1.78.0.

Therefore, set a Rust target explicitly now to prevent going forward in
time too much and thus getting potential build failures at some point.

Since we also support a minimum `bindgen` version, and since `bindgen`
does not support passing unknown Rust target versions, we need to use
the list of our minimum `bindgen` version, rather than the latest. So,
since `bindgen` 0.65.1 had this list [3], we need to use Rust 1.68.0:

    /// Rust stable 1.64
    ///  * `core_ffi_c` ([Tracking issue](https://github.com/rust-lang/rust/issues/94501))
    => Stable_1_64 => 1.64;
    /// Rust stable 1.68
    ///  * `abi_efiapi` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/65815))
    => Stable_1_68 => 1.68;
    /// Nightly rust
    ///  * `thiscall` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/42202))
    ///  * `vectorcall` calling convention (no tracking issue)
    ///  * `c_unwind` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/74990))
    => Nightly => nightly;

    ...

    /// Latest stable release of Rust
    pub const LATEST_STABLE_RUST: RustTarget = RustTarget::Stable_1_68;

Thus add the `--rust-target 1.68` parameter. Add a comment as well
explaining this.

An alternative would be to use the currently running (i.e. actual) `rustc`
and `bindgen` versions to pick a "better" Rust target version. However,
that would introduce more moving parts depending on the user setup and
is also more complex to implement.

Cc: Christian Poveda <git@pvdrz.com>
Cc: Emilio Cobos Álvarez <emilio@crisal.io>
Link: https://github.com/rust-lang/rust-bindgen/blob/21c60f473f4e824d4aa9b2b508056320d474b110/bindgen/features.rs#L97-L105 [1]
Link: https://github.com/rust-lang/rust-bindgen/issues/2960 [2]
Link: https://github.com/rust-lang/rust-bindgen/blob/7d243056d335fdc4537f7bca73c06d01aae24ddc/bindgen/features.rs#L131-L150 [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/Makefile | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/rust/Makefile b/rust/Makefile
index f349e7b067ea..1ca63ffee6cd 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -278,9 +278,19 @@ endif
 # architecture instead of generating `usize`.
 bindgen_c_flags_final = $(bindgen_c_flags_lto) -fno-builtin -D__BINDGEN__
 
+# Each `bindgen` release may upgrade the list of Rust target versions. By
+# default, the highest stable release in their list is used. Thus we need to set
+# a `--rust-target` to avoid future `bindgen` releases emitting code that
+# `rustc` may not understand. On top of that, `bindgen` does not support passing
+# an unknown Rust target version.
+#
+# Therefore, the Rust target for `bindgen` can be only as high as the minimum
+# Rust version the kernel supports and only as high as the greatest stable Rust
+# target supported by the minimum `bindgen` version the kernel supports (that
+# is, if we do not test the actual `rustc`/`bindgen` versions running).
 quiet_cmd_bindgen = BINDGEN $@
       cmd_bindgen = \
-	$(BINDGEN) $< $(bindgen_target_flags) \
+	$(BINDGEN) $< $(bindgen_target_flags) --rust-target 1.68 \
 		--use-core --with-derive-default --ctypes-prefix ffi --no-layout-tests \
 		--no-debug '.*' --enable-function-attribute-detection \
 		-o $@ -- $(bindgen_c_flags_final) -DMODULE \

base-commit: b2603f8ac8217bc59f5c7f248ac248423b9b99cb
-- 
2.47.0

Re: [PATCH] rust: kbuild: set `bindgen`'s Rust target version

[Alice Ryhl]

On Sat, Nov 23, 2024 at 7:03 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Each `bindgen` release may upgrade the list of Rust targets. For instance,
> currently, in their master branch [1], the latest ones are:
>
>     Nightly => {
>         vectorcall_abi: #124485,
>         ptr_metadata: #81513,
>         layout_for_ptr: #69835,
>     },
>     Stable_1_77(77) => { offset_of: #106655 },
>     Stable_1_73(73) => { thiscall_abi: #42202 },
>     Stable_1_71(71) => { c_unwind_abi: #106075 },
>     Stable_1_68(68) => { abi_efiapi: #105795 },
>
> By default, the highest stable release in their list is used, and users
> are expected to set one if they need to support older Rust versions
> (e.g. see [2]).
>
> Thus, over time, new Rust features are used by default, and at some
> point, it is likely that `bindgen` will emit Rust code that requires a
> Rust version higher than our minimum (or perhaps enabling an unstable
> feature). Currently, there is no problem because the maximum they have,
> as seen above, is Rust 1.77.0, and our current minimum is Rust 1.78.0.
>
> Therefore, set a Rust target explicitly now to prevent going forward in
> time too much and thus getting potential build failures at some point.
>
> Since we also support a minimum `bindgen` version, and since `bindgen`
> does not support passing unknown Rust target versions, we need to use
> the list of our minimum `bindgen` version, rather than the latest. So,
> since `bindgen` 0.65.1 had this list [3], we need to use Rust 1.68.0:
>
>     /// Rust stable 1.64
>     ///  * `core_ffi_c` ([Tracking issue](https://github.com/rust-lang/rust/issues/94501))
>     => Stable_1_64 => 1.64;
>     /// Rust stable 1.68
>     ///  * `abi_efiapi` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/65815))
>     => Stable_1_68 => 1.68;
>     /// Nightly rust
>     ///  * `thiscall` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/42202))
>     ///  * `vectorcall` calling convention (no tracking issue)
>     ///  * `c_unwind` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/74990))
>     => Nightly => nightly;
>
>     ...
>
>     /// Latest stable release of Rust
>     pub const LATEST_STABLE_RUST: RustTarget = RustTarget::Stable_1_68;
>
> Thus add the `--rust-target 1.68` parameter. Add a comment as well
> explaining this.
>
> An alternative would be to use the currently running (i.e. actual) `rustc`
> and `bindgen` versions to pick a "better" Rust target version. However,
> that would introduce more moving parts depending on the user setup and
> is also more complex to implement.
>
> Cc: Christian Poveda <git@pvdrz.com>
> Cc: Emilio Cobos Álvarez <emilio@crisal.io>
> Link: https://github.com/rust-lang/rust-bindgen/blob/21c60f473f4e824d4aa9b2b508056320d474b110/bindgen/features.rs#L97-L105 [1]
> Link: https://github.com/rust-lang/rust-bindgen/issues/2960 [2]
> Link: https://github.com/rust-lang/rust-bindgen/blob/7d243056d335fdc4537f7bca73c06d01aae24ddc/bindgen/features.rs#L131-L150 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Just to double-check, the problem is that bindgen currently doesn't
get any information about the rustc we're using, so it may generate
code invalid on the rustc we are actually using?

Alice

Re: [PATCH] rust: kbuild: set `bindgen`'s Rust target version

[Miguel Ojeda]

On Mon, Nov 25, 2024 at 10:08 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Just to double-check, the problem is that bindgen currently doesn't
> get any information about the rustc we're using, so it may generate
> code invalid on the rustc we are actually using?

Exactly: it doesn't at the moment, but eventually a future release could.

Of course, people using the latest stable `rustc` would likely never
see an issue. But there may be people e.g. building `bindgen` on their
own, and thus likely picking the latest version, while using an older
Rust toolchain from their distribution instead of, say, `rustup`.

Cheers,
Miguel

Re: [PATCH] rust: kbuild: set `bindgen`'s Rust target version

[Alice Ryhl]

On Mon, Nov 25, 2024 at 10:42 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, Nov 25, 2024 at 10:08 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Just to double-check, the problem is that bindgen currently doesn't
> > get any information about the rustc we're using, so it may generate
> > code invalid on the rustc we are actually using?
>
> Exactly: it doesn't at the moment, but eventually a future release could.
>
> Of course, people using the latest stable `rustc` would likely never
> see an issue. But there may be people e.g. building `bindgen` on their
> own, and thus likely picking the latest version, while using an older
> Rust toolchain from their distribution instead of, say, `rustup`.

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

Sounds like this might need a backport to 6.12, to prevent issues
appearing on the LTS in the future?

Alice

Re: [PATCH] rust: kbuild: set `bindgen`'s Rust target version

[Miguel Ojeda]

On Mon, Nov 25, 2024 at 10:45 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
> Sounds like this might need a backport to 6.12, to prevent issues
> appearing on the LTS in the future?

Yeah (I am not sure how likely it will be that someone hits the issue
in an LTS use case, since it requires using a `bindgen` that does not
exist yet, even in `HEAD`, at the same time as an old enough `rustc`,
but it is definitely possible, and life will keep surprising us).

Thanks!

Cheers,
Miguel

Re: [PATCH] rust: kbuild: set `bindgen`'s Rust target version

[Miguel Ojeda]

On Sat, Nov 23, 2024 at 7:03 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Each `bindgen` release may upgrade the list of Rust targets. For instance,
> currently, in their master branch [1], the latest ones are:
>
>     Nightly => {
>         vectorcall_abi: #124485,
>         ptr_metadata: #81513,
>         layout_for_ptr: #69835,
>     },
>     Stable_1_77(77) => { offset_of: #106655 },
>     Stable_1_73(73) => { thiscall_abi: #42202 },
>     Stable_1_71(71) => { c_unwind_abi: #106075 },
>     Stable_1_68(68) => { abi_efiapi: #105795 },
>
> By default, the highest stable release in their list is used, and users
> are expected to set one if they need to support older Rust versions
> (e.g. see [2]).
>
> Thus, over time, new Rust features are used by default, and at some
> point, it is likely that `bindgen` will emit Rust code that requires a
> Rust version higher than our minimum (or perhaps enabling an unstable
> feature). Currently, there is no problem because the maximum they have,
> as seen above, is Rust 1.77.0, and our current minimum is Rust 1.78.0.
>
> Therefore, set a Rust target explicitly now to prevent going forward in
> time too much and thus getting potential build failures at some point.
>
> Since we also support a minimum `bindgen` version, and since `bindgen`
> does not support passing unknown Rust target versions, we need to use
> the list of our minimum `bindgen` version, rather than the latest. So,
> since `bindgen` 0.65.1 had this list [3], we need to use Rust 1.68.0:
>
>     /// Rust stable 1.64
>     ///  * `core_ffi_c` ([Tracking issue](https://github.com/rust-lang/rust/issues/94501))
>     => Stable_1_64 => 1.64;
>     /// Rust stable 1.68
>     ///  * `abi_efiapi` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/65815))
>     => Stable_1_68 => 1.68;
>     /// Nightly rust
>     ///  * `thiscall` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/42202))
>     ///  * `vectorcall` calling convention (no tracking issue)
>     ///  * `c_unwind` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/74990))
>     => Nightly => nightly;
>
>     ...
>
>     /// Latest stable release of Rust
>     pub const LATEST_STABLE_RUST: RustTarget = RustTarget::Stable_1_68;
>
> Thus add the `--rust-target 1.68` parameter. Add a comment as well
> explaining this.
>
> An alternative would be to use the currently running (i.e. actual) `rustc`
> and `bindgen` versions to pick a "better" Rust target version. However,
> that would introduce more moving parts depending on the user setup and
> is also more complex to implement.
>
> Cc: Christian Poveda <git@pvdrz.com>
> Cc: Emilio Cobos Álvarez <emilio@crisal.io>
> Link: https://github.com/rust-lang/rust-bindgen/blob/21c60f473f4e824d4aa9b2b508056320d474b110/bindgen/features.rs#L97-L105 [1]
> Link: https://github.com/rust-lang/rust-bindgen/issues/2960 [2]
> Link: https://github.com/rust-lang/rust-bindgen/blob/7d243056d335fdc4537f7bca73c06d01aae24ddc/bindgen/features.rs#L131-L150 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Applied to `rust-fixes` -- thanks!

I added the following note to the commit message and added a small
sentence to the comment too:

    Starting with `bindgen` 0.71.0 [4], we will be able to set any future
    Rust version instead, i.e. we will be able to set here our minimum
    supported Rust version. Christian implemented it [5] after seeing this
    patch. Thanks!

I also queued it for stable:

    Cc: stable@vger.kernel.org # needed for 6.12.y; unneeded for
6.6.y; do not apply to 6.1.y
    Fixes: c844fa64a2d4 ("rust: start supporting several `bindgen` versions")

Cheers,
Miguel

Re: [PATCH] rust: kbuild: set `bindgen`'s Rust target version

[Miguel Ojeda]

On Tue, Dec 10, 2024 at 1:09 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Applied to `rust-fixes` -- thanks!
>
> I added the following note to the commit message and added a small
> sentence to the comment too:
>
>     Starting with `bindgen` 0.71.0 [4], we will be able to set any future
>     Rust version instead, i.e. we will be able to set here our minimum
>     supported Rust version. Christian implemented it [5] after seeing this
>     patch. Thanks!
>
> I also queued it for stable:
>
>     Cc: stable@vger.kernel.org # needed for 6.12.y; unneeded for
> 6.6.y; do not apply to 6.1.y
>     Fixes: c844fa64a2d4 ("rust: start supporting several `bindgen` versions")

This is now also needed to support `bindgen` >= 0.71.0 and higher
together with `rustc` < 1.82.0, since a week ago `bindgen`'s release
started using `unsafe extern` blocks.

Cheers,
Miguel