Message-ID: <37578649-c696-f3b5-a216-196e210929e5@gmail.com>
Date:   Thu, 2 Mar 2023 11:59:00 -0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type
Content-Language: en-US
To:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Miguel Ojeda <ojeda@kernel.org>
Cc:     Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>,
        =?UTF-8?Q?Bj=c3=b6rn_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>
References: <ZACvxNOuuyifQ9Nx@kernel.org>
From:   Martin Rodriguez Reboredo <yakoyoku@gmail.com>
In-Reply-To: <ZACvxNOuuyifQ9Nx@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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
>