bch_bindgen: fix packed and aligned structs on i686, ppc64

This patch addresses build issues with bch_bindgen on architectures
where the natural alignment of u64 is 4 instead of 8, such as i686 and
ppc64. The issue arises from rustc's refusing to compile structs with a
"packed" attribute that have members with an explicit "align(N)"
attribute.

rust-bindgen generally does not place "align(N)" attributes on types
when it is redundant with the type's natural alignment. There are a few
types in bcachefs with "__aligned(8)" in C where the type's natural
alignment is 8 except on architectures like those mentioned above where
it is 4. rust-bindgen places the "align(8)" attribute on these types,
for those architectures. The affected types include:

- bch_csum
- bch_sb_layout
- bch_ioctl_data_progress

bch_ioctl_data_progress, and all types that depend on it, are not used
on the Rust side currently and bindings are only generated for them
because they are covered by `.allowlist_type("bch_.*")` in
bch_bindgen/build.rs. This patch resolves the build failure for this
type by excluding it from bch_bindgen. If/when accessing this type in
Rust is needed, a decision can be made then about how to represent its
layout in Rust.

bch_csum and bch_sb_layout, and types that depend on them, are currently
used in Rust so they can't be excluded. This patch addresses these types
by stripping off the "packed" attribute in Rust on types that embed
these types, since that attribute happens to not be necessary for proper
layout.

Signed-off-by: Thomas Bertschinger <tahbertschinger@gmail.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/bch_bindgen/build.rs b/bch_bindgen/build.rs
index 1baa2666b8f878b23ba06ee493d222db27894cc8..0329f1aed039a0ba2f20b7c962f312697e815bd4 100644 (file)
--- a/bch_bindgen/build.rs
+++ b/bch_bindgen/build.rs
@@ -64,6 +64,7 @@ fn main() {
         .allowlist_function("printbuf.*")
         .blocklist_type("rhash_lock_head")
         .blocklist_type("srcu_struct")
+        .blocklist_type("bch_ioctl_data.*")
         .allowlist_var("BCH_.*")
         .allowlist_var("KEY_SPEC_.*")
         .allowlist_var("Fix753_.*")
@@ -121,14 +122,42 @@ fn main() {
 }
 
 // rustc has a limitation where it does not allow structs with a "packed" attribute to contain a
-// member with an "align(N)" attribute. There is one type in bcachefs with this issue:
-// struct btree_node.
+// member with an "align(N)" attribute. There are a few types in bcachefs with this problem. We can
+// "fix" these types by stripping off "packed" from the outer type, or "align(N)" from the inner
+// type. For all of the affected types, stripping "packed" from the outer type happens to preserve
+// the same layout in Rust as in C.
 //
-// Luckily, it happens that this type does not need the "packed(N)" attribute in Rust to have the
-// same ABI as C, so we can strip it off the bindgen output.
+// Some types are only affected on attributes on architectures where the natural alignment of u64
+// is 4 instead of 8, for example i686 or ppc64: struct bch_csum and struct bch_sb_layout have
+// "align(8)" added on such architecutres. These types are included by several "packed" types:
+//   - bch_extent_crc128
+//   - jset
+//   - btree_node_entry
+//   - bch_sb
+//
+// TODO: find a way to conditionally include arch-specific modifications when compiling for that
+// target arch. Regular conditional compilation won't work here since build scripts are always
+// compiled for the host arch, not the target arch, so that won't work when cross-compiling.
 fn packed_and_align_fix(bindings: std::string::String) -> std::string::String {
-    bindings.replace(
-        "#[repr(C, packed(8))]\npub struct btree_node {",
-        "#[repr(C, align(8))]\npub struct btree_node {",
-    )
+    bindings
+        .replace(
+            "#[repr(C, packed(8))]\npub struct btree_node {",
+            "#[repr(C, align(8))]\npub struct btree_node {",
+        )
+        .replace(
+            "#[repr(C, packed(8))]\n#[derive(Debug, Default, Copy, Clone)]\npub struct bch_extent_crc128 {",
+            "#[repr(C, align(8))]\n#[derive(Debug, Default, Copy, Clone)]\npub struct bch_extent_crc128 {",
+        )
+        .replace(
+            "#[repr(C, packed(8))]\npub struct jset {",
+            "#[repr(C, align(8))]\npub struct jset {",
+        )
+        .replace(
+            "#[repr(C, packed(8))]\npub struct btree_node_entry {",
+            "#[repr(C, align(8))]\npub struct btree_node_entry {",
+        )
+        .replace(
+            "#[repr(C, packed(8))]\npub struct bch_sb {",
+            "#[repr(C, align(8))]\npub struct bch_sb {",
+        )
 }