`_bindgen_union_align` field in generated Rust union results in garbage data on AArch64 (e.g. Apple Silicon)

[frewsxcv]

See https://users.rust-lang.org/t/on-apple-silicon-bindgen-union-is-constructed-with-random-data/54469/ for a write-up and context. This may or may not be a security issue, since it's reading arbitrary memory.

[frewsxcv]

@saleemrashid just discovered the root cause and is going to respond here later with a minimal reproducing example and a description of the issue. It does seem to be a bug in bindgen, as the _bindgen_union_align field causes the function return semantics to change.

[saleemrashid]

TL;DR: bindgen uses a u64 as the opaque layout for a union containing an f64, which causes a different calling convention to be used on AArch64. I'm not sure if this affects ARM, but Clang doesn't seem to generate different code like it does for AArch64.

If you have the following C code:

#include <stdint.h>

typedef union {
  double v[4];
} coord;

coord make_coord(double x, double y, double z, double t) {
  return (coord){.v = {x, y, z, t}};
}

On AArch64, make_coord takes 4 parameters in the floating point registers (d0-d3) and returns a union, also using the floating point registers. Therefore, it's a no-op.

https://godbolt.org/z/7r4x3d

make_coord:                             // @make_coord
        ret

When bindgen generates bindings for that union, it'll add an opaque member to ensure the union has the correct alignment and size. In this case, it will generate:

#[repr(C)]
pub union Coord {
  pub v: [f64; 4],
  _bindgen_union_align: [u64; 4],
}

The [u64; 4] member will actually result in a different calling convention being used. If we add a uint64_t[4] member to the C code, we'll see that make_coord will now write the parameters to the return location (i.e. the stack).

typedef union {
  double v[4];
  uint64_t _bindgen_union_align[4];
} coord;

https://godbolt.org/z/KacYTx

make_coord:                             // @make_coord
        stp     d0, d1, [x8]
        stp     d2, d3, [x8, #16]
        ret

Because of this, in the example given by @frewsxcv, the _bindgen_union_align member causes Rust to read garbage - proj_coord returned the union in the floating-point registers, but Rust expected it on the stack. Removing _bindgen_union_align: [u64; 4] fixes the issue.

[cuviper]

In the compiler, I think this comes from classify_ret checking is_homogeneous_aggregate, where bindgen's alignment field will change the answer.

[frewsxcv]

Does anyone know how alignment should be done in this case? A special check for this case (floats in a union on AArch64) doesn't feel right to me.

[cuviper]

A ZST [u64; 0] would also influence the alignment, and should be ignored for the homogeneous aggregate check. It wouldn't have any effect on size though, if that's important for this union definition.

[frewsxcv]

Also, is it possible to create a regression test for this in bindgen? My understanding is we'd need to link Rust code with C code to exhibit the behavior, but I don't see that happening anywhere already in the test suite.

[saleemrashid]

A ZST [u64; 0] would also influence the alignment, and should be ignored for the homogeneous aggregate check. It wouldn't have any effect on size though, if that's important for this union definition.

The bug is in Bindgen's opaque blob feature, which is implemented in src/ir/layout.rs and used to implement _bindgen_union_align. The opaque blob has to have the correct alignment and size.

[saleemrashid]

Does anyone know how alignment should be done in this case? A special check for this case (floats in a union on AArch64) doesn't feel right to me.

My initial thoughts are that if a type is a homogenous aggregate (i.e. it meets the AArch64 ABI criteria to use the return in registers behaviour), the opaque layout should use the element type rather than a uN of the appropriate size/alignment. (i.e. an opaque layout for this example would use [f64; 4]). It's not pleasant to have architecture-specific edge cases, but probably necessary since Bindgen works with low-level ABI details.

LLVM definitely has the functions to identify homogenous aggregates and retrieve the element types, but I couldn't find them exposed in the LibClang API.

[frewsxcv]

Based off what I'm hearing, it sounds like the fix for this is to add a check in the opaque blob feature if the current platform is AArch64, and all the union variants consist of floating points, then the _bindgen_union_align field should be an array of f32s or f64s. Does that sound right? If so, what happens if the unionhas two variants, one with af32and the other with au32`?

[cuviper]

I wouldn't limit this to AArch64 -- multiple targets have similar ABI differences for homogeneous aggregates.

If the union has its own mix of f32 and u32, then it's already not homogeneous. Maybe the blob can just pick any one type that's already present in the union?

But it's also not clear to me what this added field is solving in the first place, so I'm probably missing something. Don't the union's own fields already set its size and alignment appropriately?