* Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
2023-03-02 14:16 [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type Arnaldo Carvalho de Melo
@ 2023-03-02 14:58 ` Miguel Ojeda
2023-03-02 14:59 ` Martin Rodriguez Reboredo
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Miguel Ojeda @ 2023-03-02 14:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Derek Barbosa, rust-for-linux,
Linux Kernel Mailing List, Vincenzo Palazzo
On Thu, Mar 2, 2023 at 3:16 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> To address this build error:
This is due to commit 5d1dd961e743 ("x86/alternatives: Add
alt_instr.flags") that will land in v6.3-rc1.
Vincenzo reported it in Zulip too, so I will add a `Reported-by` for
him. There was a LKP report too in tip.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
2023-03-02 14:16 [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type Arnaldo Carvalho de Melo
2023-03-02 14:58 ` Miguel Ojeda
@ 2023-03-02 14:59 ` Martin Rodriguez Reboredo
2023-03-02 15:35 ` Arnaldo Carvalho de Melo
2023-03-02 21:02 ` Miguel Ojeda
2023-03-02 15:15 ` Vincenzo Palazzo
2023-03-02 22:07 ` Miguel Ojeda
3 siblings, 2 replies; 8+ messages in thread
From: Martin Rodriguez Reboredo @ 2023-03-02 14:59 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Miguel Ojeda
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Derek Barbosa, rust-for-linux,
Linux Kernel Mailing List
On 3/2/23 11:16, Arnaldo Carvalho de Melo wrote:
> To address this build error:
>
> BINDGEN rust/bindings/bindings_generated.rs
> BINDGEN rust/bindings/bindings_helpers_generated.rs
> EXPORTS rust/exports_core_generated.h
> RUSTC P rust/libmacros.so
> RUSTC L rust/compiler_builtins.o
> RUSTC L rust/alloc.o
> RUSTC L rust/bindings.o
> RUSTC L rust/build_error.o
> EXPORTS rust/exports_alloc_generated.h
> error[E0588]: packed type cannot transitively contain a `#[repr(align)]` type
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10094:1
> |
> 10094 | / pub struct alt_instr {
> 10095 | | pub instr_offset: s32,
> 10096 | | pub repl_offset: s32,
> 10097 | | pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
> 10098 | | pub instrlen: u8_,
> 10099 | | pub replacementlen: u8_,
> 10100 | | }
> | |_^
> |
> note: `alt_instr__bindgen_ty_1__bindgen_ty_1` has a `#[repr(align)]` attribute
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10111:1
> |
> 10111 | / pub struct alt_instr__bindgen_ty_1__bindgen_ty_1 {
> 10112 | | pub _bitfield_1: __BindgenBitfieldUnit<[u8; 4usize], u16>,
> 10113 | | }
> | |_^
> note: `alt_instr` contains a field of type `alt_instr__bindgen_ty_1`
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10097:9
> |
> 10097 | pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
> | ^^^^^^^^^^^^^^^^
> note: ...which contains a field of type `alt_instr__bindgen_ty_1__bindgen_ty_1`
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10104:9
> |
> 10104 | pub __bindgen_anon_1: alt_instr__bindgen_ty_1__bindgen_ty_1,
> | ^^^^^^^^^^^^^^^^
>
Reading the kernel sources this field corresponds to an u16 which indeed
represents a bit set and it says so in a comment on the field. I
couldn't replicate this issue, though, because this struct is used only
inside arch pretty much internally, then there's no problem to make it
opaque. Still, we have to be careful if these kind of things appear in
the future.
And I notice that You haven't mentioned the version of Bindgen that
You've used, including its linked libclang too. Otherwise I think this
could be accepted.
Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
> error: aborting due to previous error
>
> For more information about this error, try `rustc --explain E0588`.
> make[1]: *** [rust/Makefile:389: rust/bindings.o] Error 1
> make: *** [Makefile:1293: prepare] Error 2
>
> Cc: Derek Barbosa <debarbos@redhat.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> rust/bindgen_parameters | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
> index be4963bf720304da..552d9a85925b9945 100644
> --- a/rust/bindgen_parameters
> +++ b/rust/bindgen_parameters
> @@ -6,6 +6,7 @@
> --opaque-type local_apic
>
> # Packed type cannot transitively contain a `#[repr(align)]` type.
> +--opaque-type alt_instr
> --opaque-type x86_msi_data
> --opaque-type x86_msi_addr_lo
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
2023-03-02 14:59 ` Martin Rodriguez Reboredo
@ 2023-03-02 15:35 ` Arnaldo Carvalho de Melo
2023-03-02 21:02 ` Miguel Ojeda
1 sibling, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-03-02 15:35 UTC (permalink / raw)
To: Martin Rodriguez Reboredo
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Derek Barbosa, rust-for-linux,
Linux Kernel Mailing List
Em Thu, Mar 02, 2023 at 11:59:00AM -0300, Martin Rodriguez Reboredo escreveu:
> On 3/2/23 11:16, Arnaldo Carvalho de Melo wrote:
> > To address this build error:
> >
> > BINDGEN rust/bindings/bindings_generated.rs
> > BINDGEN rust/bindings/bindings_helpers_generated.rs
> > EXPORTS rust/exports_core_generated.h
> > RUSTC P rust/libmacros.so
> > RUSTC L rust/compiler_builtins.o
> > RUSTC L rust/alloc.o
> > RUSTC L rust/bindings.o
> > RUSTC L rust/build_error.o
> > EXPORTS rust/exports_alloc_generated.h
> > error[E0588]: packed type cannot transitively contain a `#[repr(align)]` type
> > --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10094:1
> > |
> > 10094 | / pub struct alt_instr {
> > 10095 | | pub instr_offset: s32,
> > 10096 | | pub repl_offset: s32,
> > 10097 | | pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
> > 10098 | | pub instrlen: u8_,
> > 10099 | | pub replacementlen: u8_,
> > 10100 | | }
> > | |_^
> > |
> > note: `alt_instr__bindgen_ty_1__bindgen_ty_1` has a `#[repr(align)]` attribute
> > --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10111:1
> > |
> > 10111 | / pub struct alt_instr__bindgen_ty_1__bindgen_ty_1 {
> > 10112 | | pub _bitfield_1: __BindgenBitfieldUnit<[u8; 4usize], u16>,
> > 10113 | | }
> > | |_^
> > note: `alt_instr` contains a field of type `alt_instr__bindgen_ty_1`
> > --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10097:9
> > |
> > 10097 | pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
> > | ^^^^^^^^^^^^^^^^
> > note: ...which contains a field of type `alt_instr__bindgen_ty_1__bindgen_ty_1`
> > --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10104:9
> > |
> > 10104 | pub __bindgen_anon_1: alt_instr__bindgen_ty_1__bindgen_ty_1,
> > | ^^^^^^^^^^^^^^^^
> >
>
> Reading the kernel sources this field corresponds to an u16 which indeed
> represents a bit set and it says so in a comment on the field. I
> couldn't replicate this issue, though, because this struct is used only
> inside arch pretty much internally, then there's no problem to make it
> opaque. Still, we have to be careful if these kind of things appear in
> the future.
ok
> And I notice that You haven't mentioned the version of Bindgen that
> You've used, including its linked libclang too. Otherwise I think this
> could be accepted.
⬢[acme@toolbox linux]$ bindgen --version
bindgen 0.56.0
⬢[acme@toolbox linux]$ which bindgen
/var/home/acme/.cargo/bin/bindgen
⬢[acme@toolbox linux]$ ldd /var/home/acme/.cargo/bin/bindgen
linux-vdso.so.1 (0x00007ffe543be000)
libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f7b69e94000)
libm.so.6 => /lib64/libm.so.6 (0x00007f7b69db4000)
libc.so.6 => /lib64/libc.so.6 (0x00007f7b69bd7000)
/lib64/ld-linux-x86-64.so.2 (0x00007f7b6a235000)
⬢[acme@toolbox linux]$ clang --version &| head -2
bash: syntax error near unexpected token `|'
⬢[acme@toolbox linux]$ clang --version |& head -2
clang version 15.0.7 (Fedora 15.0.7-1.fc37)
Target: x86_64-redhat-linux-gnu
⬢[acme@toolbox linux]$
> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
>
> > error: aborting due to previous error
> >
> > For more information about this error, try `rustc --explain E0588`.
> > make[1]: *** [rust/Makefile:389: rust/bindings.o] Error 1
> > make: *** [Makefile:1293: prepare] Error 2
> >
> > Cc: Derek Barbosa <debarbos@redhat.com>
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> > rust/bindgen_parameters | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
> > index be4963bf720304da..552d9a85925b9945 100644
> > --- a/rust/bindgen_parameters
> > +++ b/rust/bindgen_parameters
> > @@ -6,6 +6,7 @@
> > --opaque-type local_apic
> >
> > # Packed type cannot transitively contain a `#[repr(align)]` type.
> > +--opaque-type alt_instr
> > --opaque-type x86_msi_data
> > --opaque-type x86_msi_addr_lo
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
2023-03-02 14:59 ` Martin Rodriguez Reboredo
2023-03-02 15:35 ` Arnaldo Carvalho de Melo
@ 2023-03-02 21:02 ` Miguel Ojeda
2023-03-03 0:07 ` Martin Rodriguez Reboredo
1 sibling, 1 reply; 8+ messages in thread
From: Miguel Ojeda @ 2023-03-02 21:02 UTC (permalink / raw)
To: Martin Rodriguez Reboredo
Cc: Arnaldo Carvalho de Melo, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron,
Derek Barbosa, rust-for-linux, Linux Kernel Mailing List
On Thu, Mar 2, 2023 at 3:59 PM Martin Rodriguez Reboredo
<yakoyoku@gmail.com> wrote:
>
> Still, we have to be careful if these kind of things appear in
> the future.
Not entirely sure what you mean -- do you mean if some Rust
abstractions used its fields? But if we were in that case, it would
not compile, so we would notice. Or what do you mean?
> And I notice that You haven't mentioned the version of Bindgen that
> You've used, including its linked libclang too. Otherwise I think this
> could be accepted.
I could reproduce this with the expected versions. Since, for now,
those are the only ones supported and the build system emits a warning
otherwise, I think it is fair to assume those versions were used
unless otherwise stated.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
2023-03-02 21:02 ` Miguel Ojeda
@ 2023-03-03 0:07 ` Martin Rodriguez Reboredo
0 siblings, 0 replies; 8+ messages in thread
From: Martin Rodriguez Reboredo @ 2023-03-03 0:07 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Arnaldo Carvalho de Melo, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Boqun Feng, Gary Guo, Björn Roy Baron,
Derek Barbosa, rust-for-linux, Linux Kernel Mailing List
On 3/2/23 18:02, Miguel Ojeda wrote:
> On Thu, Mar 2, 2023 at 3:59 PM Martin Rodriguez Reboredo
> <yakoyoku@gmail.com> wrote:
>>
>> Still, we have to be careful if these kind of things appear in
>> the future.
>
> Not entirely sure what you mean -- do you mean if some Rust
> abstractions used its fields? But if we were in that case, it would
> not compile, so we would notice. Or what do you mean?
>
I've meant a general case with any abstraction, but that would be
noticed right away.
>> And I notice that You haven't mentioned the version of Bindgen that
>> You've used, including its linked libclang too. Otherwise I think this
>> could be accepted.
>
> I could reproduce this with the expected versions. Since, for now,
> those are the only ones supported and the build system emits a warning
> otherwise, I think it is fair to assume those versions were used
> unless otherwise stated.
>
> Cheers,
> Miguel
Sounds fair.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
2023-03-02 14:16 [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type Arnaldo Carvalho de Melo
2023-03-02 14:58 ` Miguel Ojeda
2023-03-02 14:59 ` Martin Rodriguez Reboredo
@ 2023-03-02 15:15 ` Vincenzo Palazzo
2023-03-02 22:07 ` Miguel Ojeda
3 siblings, 0 replies; 8+ messages in thread
From: Vincenzo Palazzo @ 2023-03-02 15:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Miguel Ojeda
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Derek Barbosa, rust-for-linux,
Linux Kernel Mailing List
> To address this build error:
>
> BINDGEN rust/bindings/bindings_generated.rs
> BINDGEN rust/bindings/bindings_helpers_generated.rs
> EXPORTS rust/exports_core_generated.h
> RUSTC P rust/libmacros.so
> RUSTC L rust/compiler_builtins.o
> RUSTC L rust/alloc.o
> RUSTC L rust/bindings.o
> RUSTC L rust/build_error.o
> EXPORTS rust/exports_alloc_generated.h
> error[E0588]: packed type cannot transitively contain a `#[repr(align)]` type
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10094:1
> |
> 10094 | / pub struct alt_instr {
> 10095 | | pub instr_offset: s32,
> 10096 | | pub repl_offset: s32,
> 10097 | | pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
> 10098 | | pub instrlen: u8_,
> 10099 | | pub replacementlen: u8_,
> 10100 | | }
> | |_^
> |
> note: `alt_instr__bindgen_ty_1__bindgen_ty_1` has a `#[repr(align)]` attribute
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10111:1
> |
> 10111 | / pub struct alt_instr__bindgen_ty_1__bindgen_ty_1 {
> 10112 | | pub _bitfield_1: __BindgenBitfieldUnit<[u8; 4usize], u16>,
> 10113 | | }
> | |_^
> note: `alt_instr` contains a field of type `alt_instr__bindgen_ty_1`
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10097:9
> |
> 10097 | pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
> | ^^^^^^^^^^^^^^^^
> note: ...which contains a field of type `alt_instr__bindgen_ty_1__bindgen_ty_1`
> --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10104:9
> |
> 10104 | pub __bindgen_anon_1: alt_instr__bindgen_ty_1__bindgen_ty_1,
> | ^^^^^^^^^^^^^^^^
>
> error: aborting due to previous error
>
> For more information about this error, try `rustc --explain E0588`.
> make[1]: *** [rust/Makefile:389: rust/bindings.o] Error 1
> make: *** [Makefile:1293: prepare] Error 2
>
> Cc: Derek Barbosa <debarbos@redhat.com>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Ah good catch!
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
2023-03-02 14:16 [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2023-03-02 15:15 ` Vincenzo Palazzo
@ 2023-03-02 22:07 ` Miguel Ojeda
3 siblings, 0 replies; 8+ messages in thread
From: Miguel Ojeda @ 2023-03-02 22:07 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Derek Barbosa, rust-for-linux,
Linux Kernel Mailing List
On Thu, Mar 2, 2023 at 3:16 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> To address this build error:
Applied to `rust-fixes` for tomorrow's -next run.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 8+ messages in thread