rust-lang / rust-lang/rust-bindgen

Implemented derives fail when packed

Open
#2,221 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

A-derive bug
Dominant language
Rust
Stars
5.3k
Forks
829
Avg merge
1d 1h
Merged PRs (30d)
15

Description

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).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the issue with the packed C header and bindgen test.h --impl-debug, then inspect the impl_derive Debug generation described in the issue and the linked comment. Done means generated Debug implementations compile for packed structs, copying Copy fields safely and providing a defined fallback for non-Copy fields such as the array.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.