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

Implemented derives fail when packed

Open chrysn opened this issue 3 years ago • 1 comments

This is the "it also applies to impl_derive" version of #2200. Long story short: The compiler now catches what used to be UB-but-worked when accessing packed fields through possibly unaligned pointers, and is now strict about that, breaking Debug on packed structs all around.

Input C Header

struct packed_wo_copy {
        short f1;
        char f2[0];
} __attribute__((packed));

This is exactly copied from #2200.

That's not exactly the case that triggered errors for me, but that one involved nested types with attribute aligned and whatsonot -- effect should be the same.

Bindgen Invocation

Diverging from #2200, we now use impl_debug:

$ bindgen test.h --impl-debug

Actual Results

[...]
impl ::std::fmt::Debug for packed_wo_copy {
    fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
        write!(
            f,
            "packed_wo_copy {{ f1: {:?}, f2: {:?} }}",
            self.f1, self.f2
        )
    }
}
[...]

and when compiling:

error: reference to packed field is unaligned
  --> /dev/stdin:90:13
   |
90 |             self.f1, self.f2
   |             ^^^^^^^
   |
   = note: `#[deny(unaligned_references)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
   = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
   = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
   = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

Expected Results

For the f1 part, bindgen should see that it is Copy and thus apply the common misalignment workaround of putting it in curly braces when used:

        write!(
            f,
            "packed_wo_copy {{ f1: {:?}, f2: {:?} }}",
            { self.f1 }, self.f2
        )

For the f2 argument, a good fallback might be to not emit anything, or to emit a text value (eg. resulting in packed_wo_copy { f1: 42, f2: not printable }). There might be sharper ways to salvage even that case (eg. check whether it is pinned, if not "temporarily move it out"), but restoring the regressed behavior to give some implementation of Debug is probably more pressing.


Some notes on where to fix this were already left in https://github.com/rust-lang/rust-bindgen/issues/1491#issuecomment-1143427177 (from where this issue is split out).

chrysn avatar Jun 07 '22 08:06 chrysn

This bit me today, as it seems to be a hard error as of rust 1.69 (latest stable is 1.70 now). There is probably more old code out there in the wild that is breaking as we speak, without an easy migration path

TheButlah avatar Jun 26 '23 22:06 TheButlah