Restrict support for `#[derive(IntoBytes)]` on unions, work to guarantee forwards-compatible soundness

[joshlf]

Do you need IntoBytes support on unions? Let us know at #1802.

Progress

• Put #[derive(IntoBytes)] on unions behind a --cfg
  • [derive] IntoBytes on unions requires --cfg #1804
• Update 0.8 release notes to mention Do you need `IntoBytes` support on unions? Let us know! #1802
• (Optionally) Update IntoBytes docs to mention Do you need `IntoBytes` support on unions? Let us know! #1802
• Get Rust to guarantee union bit validity or safety; either:
  • Get Rust to guarantee union bit validity, or
  • Get Rust to guarantee that union values can't be uninitialized by safe code
    • Get Rust to guarantee that constructing a union value that is not a combination of bit-valid sub-regions of its sub-fields will always require unsafe code
      • Minimum guarantees for union construction and typed copies? rust-lang/unsafe-code-guidelines#533
    • Get Rust to guarantee that, in a typed copy of a union value, any bytes previously written are preserved so long as those bytes are not written to a byte offset at which there is padding in all fields of the union
      • Minimum guarantees for union construction and typed copies? rust-lang/unsafe-code-guidelines#533
    • Update documentation on both FromBytes and IntoBytes to be clear about what safety invariants are added

Details

Support for #[derive(IntoBytes)] on unions was added in 9c19cbe (reviewed at fxrev.dev/639087). This support is sound on the assumption that if every field of a union is IntoBytes and there is no extra padding before or after any field, then no bit-valid instance of that union can have uninitialized bytes.

However, this assumes too much. It is currently up in the air whether this actually holds. It's not clear from reading the history why we were okay adding this implementation in the first place, but it now seems like it may have been premature.

It may eventually become the case that this assumption is guaranteed by Rust, so this may eventually become a non-issue. However, for the time being, it's a violation of our soundness policy, which promises to be sound on all future Rust compilers.

Short-term mitigation

Unfortunately, existing users rely on #[derive(IntoBytes)] support on unions, so removing this support will break users. This is likely not a problem on today's Rust, so forcing users to migrate to something else might be too drastic of a solution. Instead, I propose that we discourage further use by gating #[derive(IntoBytes)] on unions behind a --cfg. The reason for a --cfg instead of a Cargo feature is that Cargo features are footguns in cases like this - it's easy for crate A to enable the feature and then for crate B, which depends on A, to accidentally rely on that feature despite not enabling that feature itself. For something with soundness and stability implications, that's risky. By contrast, a --cfg requires the top-level crate to enable it for all downstream crates.

Long-term mitigation

There are two options for long-term mitigations.

Union validity

We can try to get Rust to promise what we need in order for #[derive(IntoBytes)] on unions to be guaranteed sound on all future compilers. In particular, this entails restricting union bit validity so that, if the byte at a given offset is initialized in every valid value of every field, then it must be initialized in the union as well.

Union safety

Alternatively, we can take a weaker approach that only requires a safety rather than bit validity constraint: #1792 (comment)

[joshlf]

Summarizing a discussion with @jswrenn

See also: #260

The situation may be more permissive than I originally thought. In particular, while it might be the case at some point in the future that any union with all uninitialized bytes is considered bit-valid (ie, it is sound to produce such a union value), it is currently the case that creating such a value requires unsafe code. Safe code is only permitted to instantiate or modify a union value using its actual fields.

It is already the case that zerocopy's traits imply both bit validity and safety requirements. For example, FromBytes attests that a type has a particular bit validity, but it also permits users to construct arbitrary instances of a type, which implies that that type must not have certain safety invariants which are inconsistent with constructing a value from arbitrary bytes. We can say that IntoBytes is similar - it adds a new safety invariant to types that they must only be constructed from their fields.

In order for this to continue to be sound, we need the following forwards-compatibility guarantees from Rust:

• It will never be safe to construct a union value other than from its fields or by overwriting one of its sub-fields
• In a typed copy of a union value, any bytes previously written are preserved so long as those bytes are not written to a byte offset at which there is padding in all fields of the union