diff --git a/CHANGELOG.md b/CHANGELOG.md index 91f0402da62..0cbaa9f6bc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - `zerovec` - (0.10.3) Fix size regression by making `twox-hash` dep `no_std` (https://github.com/unicode-org/icu4x/pull/5007) - (0.10.3) Enforce C,packed, not just packed, on ULE types, fixing for incoming changes to `repr(Rust)` (https://github.com/unicode-org/icu4x/pull/5049) + - (0.10.4) Enforce C,packed on OptionVarULE (https://github.com/unicode-org/icu4x/pull/5143) - `zerovec_derive` - (0.10.3) Enforce C,packed, not just packed, on ULE types, fixing for incoming changes to `repr(Rust)` (https://github.com/unicode-org/icu4x/pull/5049) diff --git a/utils/zerovec/derive/src/ule.rs b/utils/zerovec/derive/src/ule.rs index 755e66a229b..2f3e6f9e324 100644 --- a/utils/zerovec/derive/src/ule.rs +++ b/utils/zerovec/derive/src/ule.rs @@ -48,9 +48,9 @@ pub fn derive_impl(input: &DeriveInput) -> TokenStream2 { // Safety (based on the safety checklist on the ULE trait): // 1. #name does not include any uninitialized or padding bytes. - // (achieved by enforcing #[repr(transparent)] or #[repr(packed)] on a struct of only ULE types) + // (achieved by enforcing #[repr(transparent)] or #[repr(C, packed)] on a struct of only ULE types) // 2. #name is aligned to 1 byte. - // (achieved by enforcing #[repr(transparent)] or #[repr(packed)] on a struct of only ULE types) + // (achieved by enforcing #[repr(transparent)] or #[repr(C, packed)] on a struct of only ULE types) // 3. The impl of validate_byte_slice() returns an error if any byte is not valid. // 4. The impl of validate_byte_slice() returns an error if there are extra bytes. // 5. The other ULE methods use the default impl. diff --git a/utils/zerovec/derive/src/varule.rs b/utils/zerovec/derive/src/varule.rs index 82fd702578e..01f8d42c2e0 100644 --- a/utils/zerovec/derive/src/varule.rs +++ b/utils/zerovec/derive/src/varule.rs @@ -88,9 +88,9 @@ pub fn derive_impl( // Safety (based on the safety checklist on the ULE trait): // 1. #name does not include any uninitialized or padding bytes - // (achieved by enforcing #[repr(transparent)] or #[repr(packed)] on a struct of only ULE types) + // (achieved by enforcing #[repr(transparent)] or #[repr(C, packed)] on a struct of only ULE types) // 2. #name is aligned to 1 byte. - // (achieved by enforcing #[repr(transparent)] or #[repr(packed)] on a struct of only ULE types) + // (achieved by enforcing #[repr(transparent)] or #[repr(C, packed)] on a struct of only ULE types) // 3. The impl of `validate_byte_slice()` returns an error if any byte is not valid. // 4. The impl of `validate_byte_slice()` returns an error if the slice cannot be used in its entirety // 5. The impl of `from_byte_slice_unchecked()` returns a reference to the same data. diff --git a/utils/zerovec/design_doc.md b/utils/zerovec/design_doc.md index 11621a3d6b1..e4c3643ac3d 100644 --- a/utils/zerovec/design_doc.md +++ b/utils/zerovec/design_doc.md @@ -182,7 +182,7 @@ where The trait is `unsafe` to implement since `ZeroVec` will rely on invariants promised by the trait. The main feature here is that this trait lets `ZeroVec` take a bytestream it is decoding and certify that it contains valid `Self` types. It allows `ZeroVec` to turn `&[u8]` into `&[T::ULE]` during parsing or deserialization. -As `ULE` requires types to not have any alignment restrictions, most `ULE` types will be `#[repr(transparent)]` or `#[repr(packed)]` wrappers around other ULE types (or in general, types known to have no alignment requirements). Note that `#[repr(Rust)]` isn't defined or stable, so ULE types _must_ have _some_ `#[repr(..)]` tag for them to be able to stably uphold the invariants. +As `ULE` requires types to not have any alignment restrictions, most `ULE` types will be `#[repr(transparent)]` or `#[repr(C, packed)]` wrappers around other ULE types (or in general, types known to have no alignment requirements). Note that `#[repr(Rust)]` isn't defined or stable, so ULE types _must_ have _some_ `#[repr(..)]` tag for them to be able to stably uphold the invariants. If you wish to make a custom ULE type, it will likely wrap [`RawBytesULE`] with added invariants (and `#[repr(transparent)]`, or do something like the following: @@ -195,7 +195,7 @@ struct Foo { } // Implements ULE -#[repr(packed)] +#[repr(C, packed)] struct FooULE { field1: u32::ULE, field2: char::ULE, @@ -223,7 +223,7 @@ pub unsafe trait VarULE: 'static { Similarly to [`ULE`], `VarULE` is an `unsafe` trait which mainly requires the user to specify whether a `&[u8]` slice contains a valid bit pattern for a _single_ `Self` instance. Since pointer metadata can vary between unsized types, `from_byte_slice_unchecked()` must also be specified by the implementor so that `VarZeroVec` can materialize `&Self` instances out of known-valid bit patterns after validation. -`VarULE` types must also accept any alignment, so most custom `VarULE` types will be `#[repr(packed)]` wrappers around structs containing `ULE` and `VarULE` types (like `str`, `[u8]`, [`VarZeroSlice`], [`ZeroSlice`]). +`VarULE` types must also accept any alignment, so most custom `VarULE` types will be `#[repr(C, packed)]` wrappers around structs containing `ULE` and `VarULE` types (like `str`, `[u8]`, [`VarZeroSlice`], [`ZeroSlice`]). ### `EncodeAsVarULE` @@ -315,7 +315,7 @@ These are basic derives that can be applied to types to _just_ generate ULE and These can only be applied to structs where all fields are ULE types (for `#[derive(VarULE)]`, the last field must be an unsized `VarULE` type). These derives will do the following things: - - Apply `#[repr(packed)]` to the type (or perhaps `#[repr(C)]` if we can determine that that will always work) + - Apply `#[repr(C, packed)]` to the type (or perhaps `#[repr(C)]` if we can determine that that will always work) - Generate the appropriate `ZeroMapKV` impl (an opt-out can be provided) - Generate a `ULE` or `VarULE` implementation that applies offsetted `validate_byte_slice()` for each field to implement the final `validate_byte_slice()` - Generate `Copy`/`Clone` impls as necessary (`#[derive()]` does not work with packed types) @@ -382,7 +382,7 @@ For an initial pass, we may only support single-heap-field structs for `#[make_v Ideally, enums can have ULE impls autogenerated for them, but handling the discriminants gets tricky. -One way to handle this is to generate a private internal struct for each variant and do the usual ULE or VarULE generation for each. Then, manaully construct a `#[repr(packed)]` tagged union with a `u8` tag and a `union` of all of the internal structs. This is somewhat complicated but actually relatively simple to implement since it can use `#[make_ule]` and `#[make_varule]`. +One way to handle this is to generate a private internal struct for each variant and do the usual ULE or VarULE generation for each. Then, manaully construct a `#[repr(C, packed)]` tagged union with a `u8` tag and a `union` of all of the internal structs. This is somewhat complicated but actually relatively simple to implement since it can use `#[make_ule]` and `#[make_varule]`. We may also do this completely manually, which gives us the opportunity for bitpacking the discriminant further, if combined with the bitpacking scheme discussed below. diff --git a/utils/zerovec/src/ule/custom.rs b/utils/zerovec/src/ule/custom.rs index 8cc6e9de4e9..5a31c66e4b6 100644 --- a/utils/zerovec/src/ule/custom.rs +++ b/utils/zerovec/src/ule/custom.rs @@ -47,9 +47,9 @@ //! # field3: ZeroVec<'a, u32> //! # } //! -//! // Must be repr(packed) for safety of VarULE! +//! // Must be repr(C, packed) for safety of VarULE! //! // Must also only contain ULE types -//! #[repr(packed)] +//! #[repr(C, packed)] //! struct FooULE { //! field1: ::ULE, //! field2: ::ULE, @@ -57,9 +57,9 @@ //! } //! //! // Safety (based on the safety checklist on the VarULE trait): -//! // 1. FooULE does not include any uninitialized or padding bytes. (achieved by `#[repr(packed)]` on +//! // 1. FooULE does not include any uninitialized or padding bytes. (achieved by `#[repr(C, packed)]` on //! // a struct with only ULE fields) -//! // 2. FooULE is aligned to 1 byte. (achieved by `#[repr(packed)]` on +//! // 2. FooULE is aligned to 1 byte. (achieved by `#[repr(C, packed)]` on //! // a struct with only ULE fields) //! // 3. The impl of `validate_byte_slice()` returns an error if any byte is not valid. //! // 4. The impl of `validate_byte_slice()` returns an error if the slice cannot be used in its entirety diff --git a/utils/zerovec/src/ule/niche.rs b/utils/zerovec/src/ule/niche.rs index ae61faca0b0..b7574f65df0 100644 --- a/utils/zerovec/src/ule/niche.rs +++ b/utils/zerovec/src/ule/niche.rs @@ -108,7 +108,7 @@ impl + ULE + Eq, const N: usize> Eq for NichedOptionULE { /// containing only ULE fields. /// NichedOptionULE either contains NICHE_BIT_PATTERN or valid U byte sequences. /// In both cases the data is initialized. -/// 2. NichedOptionULE is aligned to 1 byte due to `#[repr(packed)]` on a struct containing only +/// 2. NichedOptionULE is aligned to 1 byte due to `#[repr(C, packed)]` on a struct containing only /// ULE fields. /// 3. validate_byte_slice impl returns an error if invalid bytes are encountered. /// 4. validate_byte_slice impl returns an error there are extra bytes. diff --git a/utils/zerovec/src/ule/option.rs b/utils/zerovec/src/ule/option.rs index e8bd0fae498..303bb908a64 100644 --- a/utils/zerovec/src/ule/option.rs +++ b/utils/zerovec/src/ule/option.rs @@ -141,7 +141,7 @@ impl Eq for OptionULE {} /// ``` // The slice field is empty when None (bool = false), // and is a valid T when Some (bool = true) -#[repr(packed)] +#[repr(C, packed)] pub struct OptionVarULE(PhantomData, bool, [u8]); impl OptionVarULE { @@ -166,8 +166,8 @@ impl core::fmt::Debug for OptionVarULE // Safety (based on the safety checklist on the VarULE trait): // 1. OptionVarULE does not include any uninitialized or padding bytes -// (achieved by being repr(packed) on ULE types) -// 2. OptionVarULE is aligned to 1 byte (achieved by being repr(packed) on ULE types) +// (achieved by being repr(C, packed) on ULE types) +// 2. OptionVarULE is aligned to 1 byte (achieved by being repr(C, packed) on ULE types) // 3. The impl of `validate_byte_slice()` returns an error if any byte is not valid. // 4. The impl of `validate_byte_slice()` returns an error if the slice cannot be used in its entirety // 5. The impl of `from_byte_slice_unchecked()` returns a reference to the same data.