Do you need IntoBytes support on unions? Let us know!

[joshlf]

Tracking issue: #1792

TL;DR: If you want to use IntoBytes with union types, upvote this discussion!

In 0.8, we've restricted #[derive (IntoBytes)] so that it only supports unions when —-cfg zerocopy_derive_union_into_bytes is
passed. Union bit validity is up in the air, and it's not currently guaranteed that implementing IntoBytes on unions will remain
sound on all future Rust versions.

The exact bit validity of unions is actively being discussed, and we're advocating for bit validity requirements that will permit IntoBytes on unions to be sound.

If you would like to use IntoBytes with unions, please let us know by upvoting this discussion or by commenting. We will use
this as evidence that there is demand for this feature, and try to persuade Rust to adopt the necessary union bit validity
requirements. Please do not spam other threads besides this one, especially threads in rust-lang repositories. Let's be friendly
participants in this discussion 🙂

[anp]

It looks like Fuchsia currently uses IntoBytes on some #[repr(C)] unions. We generate the wrappers with bindgen and the referenced types include unions.

EDIT: My initial search was too narrow, we also do this when we generate bindings to the Linux uapi.

[joshlf]

Fuchsia is the reason for the --cfg escape hatch 🙂

But yes, good to have this use case recorded. Thanks!

[phy1729]

The ppv-lite86 crate uses AsBytes/IntoByes on a union at https://github.com/cryptocorrosion/cryptocorrosion/blob/16e2fe894fc39944c2625d735fff3d697d052e42/utils-simd/ppv-lite86/src/x86_64/mod.rs#L110-L112.

[a1akris]

The ppv-lite86 crate uses AsBytes/IntoByes on a union at https://github.com/cryptocorrosion/cryptocorrosion/blob/16e2fe894fc39944c2625d735fff3d697d052e42/utils-simd/ppv-lite86/src/x86_64/mod.rs#L110-L112.

I have a similar usage to this one in a private project for versioned binary blobs. The classic approach

#[derive(TryFromBytes, Immutable, IntoBytes)]
#[repr(C)]
pub enum Blob {
    V1(BlobV1),
    V2(BlobV2),
}

doesn't work because enums cannot be packed so I have to rewrite them as

#[derive(TryFromBytes, Immutable, IntoBytes)]
#[repr(C, packed)]
pub struct Blob {
    version: BlobVersion,
    blob: BlobUnion
}

#[derive(TryFromBytes, IntoBytes, Immutable)]
#[repr(u8)]
pub enum BlobVersion {
    V1,
    V2,
}

#[derive(TryFromBytes, IntoBytes, Immutable)]
union BlobUnion {
    v1: BlobV1,
    v2: BlobV2,
}

with some extra code for conversions.

Also, --cfg doesn't work for me on 0.8.9 for some reason so currently I put all this boilerplate in a macro where I implement

unsafe impl IntoBytes for BlobUnion

myself and there every field type is checked by fn is_into_bytes<T: Immutable + IntoBytes>().

[pmj]

I'm using this in a device driver; the vendor supplies C header files with nested struct/union definitions for the various messages the device understands.

It looks something like this:

struct DeviceMessage
{
    uint32_t signature; // Can be DEVICE_MESSAGE_DISPLAY, DEVICE_MESSAGE_AUDIO, etc.
    // […more shared fields…]

    union
    {
        struct
        {
            uint8_t parameter;
            uint8_t reserved[7];
        } display;
        struct
        {
            uint8_t flags;
            uint8_t index;
            uint8_t reserved[6];
        } audio;
        // […]
    } message_specific;
};

The ability to just import these definitions into the rust code with bindgen and emit the byte stream with IntoBytes saves me from error-prone manual translation.

In my particular case, there is no padding. Every byte is accounted for in every branch, because we don't want to send a buffer containing uninitialised bytes to the device. So I could happily live with this only working with the subset of well-defined unions.

For what it's worth, I'd also be entirely fine with implementing IntoBytes myself for types containing unions, if it was documented how I should go about this. However, the stern warning of

Do not implement this trait yourself! Instead, use `#[derive(IntoBytes)]

has led me to not pursue this approach for now. (Although I suspect I will have to do something about this soon because the --cfg flag approach is incredibly annoying; cargo doesn't seem to be able to reliably set it from Cargo.toml files, so it always needs to be passed in via environment variables.)

[Allen-Webb]

In my particular case, there is no padding. Every byte is accounted for in every branch, because we don't want to send a buffer containing uninitialised bytes to the device. So I could happily live with this only working with the subset of well-defined unions.

I agree that it probably makes sense to enforce that all branches of the union are the same size.

[m-hilgendorf]

I would like this feature for using zerocopy with mmap'd files, whose formats are defined in C headers and can be easily generated with bindgen. My specific use case is reading and writing ELF files.

It would be nice if this was a feature flag instead of a custom --cfg. Not only are they hard to set (for example, in .cargo/config.toml, which you may not want to commit to a repo), rustc gives a loud warning about unexpected configs. Perhaps a scary feature flag name unsafe-experimental-derive-unions-into and restrictions like have been suggested (failing if the union variants have different sizes).

Another problem with --cfg is that you can't use it in libraries. All dependents have to be aware of it.

[ianthetechie]

Hmm... that will definitely bite me eventually (also developing a libarry) 😅

[ianthetechie]

Writing in that I also use this feature. My use case is interoperability with a binary file format whose reference implementation is in C++, and relies heavily on essentially transmuting chunks of memory. One of the fields is a union to conserve a few bytes (the original authors cared a lot about that sort of thing).

In my case, I think it's guaranteed to be a safe operation since both variants of the union are the exact same size. Of course it may not be valid in the sense that the interpretation of the data could be nonsense (logic error) if you don't uphold the invariants. But it is still "safe" in memory terms.

(Have I understood the issue correctly? The part that's underspecified seems to be what happens to "extra" bits that are only used by a single variant.)

Here is a code sample illustrating the usage.

#[bitfield(u32,
    repr = U32<LE>,
    from = bit_twiddling_helpers::conv_u32le::from_inner,
    into = bit_twiddling_helpers::conv_u32le::into_inner
)]
#[derive(FromBytes, IntoBytes, Immutable, Unaligned)]
struct StopImpact {
    #[bits(24, from = bit_twiddling_helpers::conv_u32le::from_inner, into = bit_twiddling_helpers::conv_u32le::into_inner)]
    impact_between_edges: U32<LE>,
    #[bits(8)]
    edge_to_right: u8,
}

/// Stores either the stop impact or the transit line identifier.
/// Since transit lines are schedule-based, they have no need for edge transition logic,
/// so we can freely share this field.
#[repr(C)]
#[derive(FromBytes, IntoBytes, Immutable, Unaligned, Copy, Clone)]
union StopOrLine {
    stop_impact: StopImpact,
    line_id: U32<LE>,
}

I'll also second the concern of @m-hilgendorf above with the current approach of a config switch. I can confirm this does not work for libraries, which is also the way I'm using it. config.toml in a library will not apply to any projects importing it. I would also prefer a feature flag.

[ianthetechie]

Aside: I get the following warning when compiling now. I don't see the same behavior when running my regular build on stable rust, but do see it on nightly, which use for miri tests.

warning: unexpected `cfg` condition name: `zerocopy_derive_union_into_bytes`
   --> valhalla-graphtile/src/graph_tile/directed_edge.rs:202:21
    |
202 | #[derive(FromBytes, IntoBytes, Immutable, Unaligned, Copy, Clone)]
    |                     ^^^^^^^^^
    |
    = help: expected names are: `docsrs`, `feature`, and `test` and 31 more
    = note: using a cfg inside a derive macro will use the cfgs from the destination crate and not the ones from the defining crate
    = help: try referring to `IntoBytes` crate for guidance on how handle this unexpected cfg
    = help: the derive macro `IntoBytes` may come from an old version of the `zerocopy_derive` crate, try updating your dependency with `cargo update -p zerocopy_derive`
    = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
    = note: `#[warn(unexpected_cfgs)]` on by default
    = note: this warning originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)