[1/1] rust: bindgen: Add `alt_instr` as opaque type

Message ID ZACvxNOuuyifQ9Nx@kernel.org
State New
Headers
Series [1/1] rust: bindgen: Add `alt_instr` as opaque type |

Commit Message

Arnaldo Carvalho de Melo March 2, 2023, 2:16 p.m. UTC
  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>
---
 rust/bindgen_parameters | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Miguel Ojeda March 2, 2023, 2:58 p.m. UTC | #1
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
  
Martin Rodriguez Reboredo March 2, 2023, 2:59 p.m. UTC | #2
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
>
  
Vincenzo Palazzo March 2, 2023, 3:15 p.m. UTC | #3
> 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>
  
Arnaldo Carvalho de Melo March 2, 2023, 3:35 p.m. UTC | #4
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
> >
  
Miguel Ojeda March 2, 2023, 9:02 p.m. UTC | #5
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
  
Miguel Ojeda March 2, 2023, 10:07 p.m. UTC | #6
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
  
Martin Rodriguez Reboredo March 3, 2023, 12:07 a.m. UTC | #7
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.
  

Patch

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