Date:   Thu, 2 Mar 2023 12:35:46 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Martin Rodriguez Reboredo <yakoyoku@gmail.com>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>,
        =?iso-8859-1?Q?Bj=F6rn?= Roy Baron <bjorn3_gh@protonmail.com>,
        Derek Barbosa <debarbos@redhat.com>,
        rust-for-linux@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
Message-ID: <ZADCUvbeZrbjz8QR@kernel.org>
References: <ZACvxNOuuyifQ9Nx@kernel.org>
 <37578649-c696-f3b5-a216-196e210929e5@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <37578649-c696-f3b5-a216-196e210929e5@gmail.com>
Precedence: bulk

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