Re: [PATCH BCACHEFS-TOOLS] use upstream bindgen; fix packed and aligned types

Kent Overstreet
2024-01-22T19:22:52Z

On Mon, Jan 22, 2024 at 07:36:43AM -0700, Thomas Bertschinger wrote:
> bcachefs-tools has been using a patched bindgen to work around a
> limitation of rustc that prevents compiling structs with
> both #[repr(packed(N)] and #[repr(align(N)] attributes. The patch:
> e8168ceda507 "codegen: Don't generate conflicting packed() and align()
> representation hints." discards the "align" attribute in cases where
> bindgen produces a type with both.
>
> This may be correct for some types, but it turns out that for each
> bcachefs type with this problem, keeping the "align" attribute and
> discarding the "packed" attribute generates a type with the same ABI as
> the original C type.
>
> This can be tested automatically by running:
> $ cargo test --manifest-path bch_bindgen/Cargo.toml
> in the bcachefs-tools tree.
>
> There has been pressure recently to start using upstream bindgen; both
> externally, from distribution maintainers who want to build
> bcachefs-tools with standard dependencies, and internally, in order to
> enable using Rust for bcachefs in-kernel.
>
> This patch updates bcachefs-tools to use upstream bindgen. It works
> around the rustc limitation with a post-processing step in the bindgen
> build that adjusts the attributes to include "#[repr(C, align(N))]" and
> exclude #[repr(packed(N)] only for the 4 types that need it.
>
> Some types that had been manually implemented in
> bch_bindgen/src/bcachefs.rs are now automatically generated by bindgen,
> so that they will be covered by the ABI compatibility testing mentioned
> above.
>
> I intentionally targeted the post-processing to the exact 4 types with
> the issue currently, so that any changes to bcachefs that result in this
> issue appearing for a new type will require manual intervention. I
> figured any such changes should require careful consideration.
>
> Ideally, bindgen can be updated to handle situations where "align(N)"
> is needed and "packed(N)" can be safely discarded. If a patch for this
> is accepted into bindgen, the post-processing hack can be removed.
>
> Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>

Doesn't build here...?

Compiling bch_bindgen v0.1.0 (/home/kent/bcachefs-tools/bch_bindgen)
error[E0587]: type has conflicting packed and align representation hints
--> /home/kent/bcachefs-tools/target/release/build/bch_bindgen-6ee8b9c8755f1deb/out/bcachefs.rs:3:147196
|
3 | ...r (align (8))] # [derive (Debug , Default , Copy , Clone)] pub struct bkey { pub u64s : __u8 , pub _bitfield_align_1 : [u8 ; 0] , pub ...
| ^^^^^^^^^^^^^^^

error[E0587]: type has conflicting packed and align representation hints
--> /home/kent/bcachefs-tools/target/release/build/bch_bindgen-6ee8b9c8755f1deb/out/bcachefs.rs:3:162454
|
3 | ...gn (8))] # [derive (Debug , Default , Copy , Clone)] pub struct bch_extent_crc32 { pub _bitfield_align_1 : [u8 ; 0] , pub _bitfield_1 ...
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0587]: type has conflicting packed and align representation hints
--> /home/kent/bcachefs-tools/target/release/build/bch_bindgen-6ee8b9c8755f1deb/out/bcachefs.rs:3:176663
|
3 | ...ign (8))] # [derive (Debug , Default , Copy , Clone)] pub struct bch_extent_ptr { pub _bitfield_align_1 : [u8 ; 0] , pub _bitfield_1 :...
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0588]: packed type cannot transitively contain a `#[repr(align)]` type
--> /home/kent/bcachefs-tools/target/release/build/bch_bindgen-6ee8b9c8755f1deb/out/bcachefs.rs:3:326505
|
3 | ...1) ; s . assume_init () } } } # [repr (C , packed (8))] pub struct btree_node { pub csum : bch_csum , pub magic : __le64 , pub flags :...
| ^^^^^^^^^^^^^^^^^^^^^
|
note: `bch_extent_ptr` has a `#[repr(align)]` attribute
--> /home/kent/bcachefs-tools/target/release/build/bch_bindgen-6ee8b9c8755f1deb/out/bcachefs.rs:3:176663
|
3 | ...ign (8))] # [derive (Debug , Default , Copy , Clone)] pub struct bch_extent_ptr { pub _bitfield_align_1 : [u8 ; 0] , pub _bitfield_1 :...
| ^^^^^^^^^^^^^^^^^^^^^^^^^

Some errors have detailed explanations: E0587, E0588.
For more information about an error, try `rustc --explain E0587`.
error: could not compile `bch_bindgen` (lib) due to 4 previous errors
make: *** [Makefile:178: bcachefs] Error 101