rust-bindgen icon indicating copy to clipboard operation
rust-bindgen copied to clipboard

BindgenUnion can get incorrect alignment on i686

Open bertschingert opened this issue 1 year ago • 1 comments

When targeting an architecture where the natural alignment of 64-bit integer types is 4 instead of 8, bindgen can create BindgenUnion types that get an alignment of 4 when the source type has an alignment of 8. For example:

// test.h
union outer {
    struct {
        long long a;
        long long b[];
    } __attribute__((aligned(8)));
}
// bindgen test.h -- --target=i686-unknown-linux-gnu

// Actual results:
#[repr(C)]
pub struct outer {
    pub __bindgen_anon_1: __BindgenUnionField<outer__bindgen_ty_1>,
    pub bindgen_union_field: u64,
}
#[repr(C)]
#[repr(align(8))]
#[derive(Debug)]
pub struct outer__bindgen_ty_1 {
    pub a: ::std::os::raw::c_longlong,
    pub b: __IncompleteArrayField<::std::os::raw::c_longlong>,
}

// Expected results:
#[repr(C)]
#[repr(align(8))]
pub struct outer {
    pub __bindgen_anon_1: __BindgenUnionField<outer__bindgen_ty_1>,
    pub bindgen_union_field: u64,
}
#[repr(C)]
#[repr(align(8))]
#[derive(Debug)]
pub struct outer__bindgen_ty_1 {
    pub a: ::std::os::raw::c_longlong,
    pub b: __IncompleteArrayField<::std::os::raw::c_longlong>,
}

This comes up for two types in bcachefs: struct btree_node and struct btree_node_entry--specifically for the anonymous unions in these types.

It looks like two factors combine to cause this issue:

  • StructLayoutTracker assumes that fields in unions contribute alignment to the parent type and so StructLayoutTracker::requires_explicit_align() returns false for outer -- however, for BindgenUnions, the inner types do not actually contribute any alignment to the parent type because they are just PhantomData within a __BindgenUnionField
  • Opaque::known_rust_type_for_array() creates an integer type that does influence the alignment of outer, but it assumes that sizeof == alignof for standard int types, which isn't true for u64 on i686

I am not sure of the best way to resolve this. I think the simplest way would be to add a check to StructLayoutTracker::requires_explicit_align(): if we are a BindgenUnion that needs an alignment of 8, and the natural alignment of u64 is not 8 on the target arch, then return true. However, that requires a method that returns the alignment of u64 at runtime and I wasn't sure how to do this. Rust's std::mem::alignof doesn't use the target arch at runtime, and clang_sys::clang_Type_getAlignOf() requires a node in Clang's AST rather than a primitive type.

What do you think? Am I overlooking a public function in clang_sys (or somewhere in Rust) that will return the alignment of a primitive type for the target arch, at runtime? Any other ideas on the best way to handle this?

bertschingert avatar Feb 27 '24 21:02 bertschingert