From f427697f3133016f307916a3f982041641699366 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 24 Jun 2024 10:34:46 -0700 Subject: [PATCH] Uplift repr(packed) fix to zerovec 0.9 (#5091) Uplifts #5049 to zerovec 0.9.7 --- utils/zerovec/Cargo.toml | 2 +- utils/zerovec/derive/Cargo.toml | 2 +- utils/zerovec/derive/examples/derives.rs | 4 +-- utils/zerovec/derive/src/make_ule.rs | 2 +- utils/zerovec/derive/src/ule.rs | 4 +-- utils/zerovec/derive/src/utils.rs | 42 +++++++++++++++++++----- utils/zerovec/derive/src/varule.rs | 4 +-- utils/zerovec/src/flexzerovec/slice.rs | 2 +- utils/zerovec/src/ule/mod.rs | 4 +-- utils/zerovec/src/ule/option.rs | 6 ++-- utils/zerovec/src/ule/tuple.rs | 6 ++-- 11 files changed, 51 insertions(+), 27 deletions(-) diff --git a/utils/zerovec/Cargo.toml b/utils/zerovec/Cargo.toml index b3ed87e8581..d57a30f21af 100644 --- a/utils/zerovec/Cargo.toml +++ b/utils/zerovec/Cargo.toml @@ -5,7 +5,7 @@ [package] name = "zerovec" description = "Zero-copy vector backed by a byte array" -version = "0.9.6" +version = "0.9.7" categories = ["rust-patterns", "memory-management", "caching", "no-std", "data-structures"] keywords = ["zerocopy", "serialization", "zero-copy", "serde"] diff --git a/utils/zerovec/derive/Cargo.toml b/utils/zerovec/derive/Cargo.toml index a59cc2aab5a..788d42e692b 100644 --- a/utils/zerovec/derive/Cargo.toml +++ b/utils/zerovec/derive/Cargo.toml @@ -5,7 +5,7 @@ [package] name = "zerovec-derive" description = "Custom derive for the zerovec crate" -version = "0.9.6" +version = "0.9.7" authors = ["Manish Goregaokar "] categories = ["rust-patterns", "memory-management", "caching", "no-std", "data-structures"] keywords = ["zerocopy", "serialization", "zero-copy", "serde"] diff --git a/utils/zerovec/derive/examples/derives.rs b/utils/zerovec/derive/examples/derives.rs index 40f821023d9..e53c444aaa2 100644 --- a/utils/zerovec/derive/examples/derives.rs +++ b/utils/zerovec/derive/examples/derives.rs @@ -6,7 +6,7 @@ use zerovec::ule::AsULE; use zerovec::ule::EncodeAsVarULE; use zerovec::*; -#[repr(packed)] +#[repr(C, packed)] #[derive(ule::ULE, Copy, Clone)] pub struct FooULE { a: u8, @@ -40,7 +40,7 @@ impl AsULE for Foo { } } -#[repr(packed)] +#[repr(C, packed)] #[derive(ule::VarULE)] pub struct RelationULE { /// This maps to (AndOr, Polarity, Operand), diff --git a/utils/zerovec/derive/src/make_ule.rs b/utils/zerovec/derive/src/make_ule.rs index 92a200e0bec..20b89171e4f 100644 --- a/utils/zerovec/derive/src/make_ule.rs +++ b/utils/zerovec/derive/src/make_ule.rs @@ -83,7 +83,7 @@ fn make_ule_enum_impl( attrs: ZeroVecAttrs, ) -> TokenStream2 { // We could support more int reprs in the future if needed - if !utils::has_valid_repr(&input.attrs, |r| r == "u8") { + if !utils::ReprInfo::compute(&input.attrs).u8 { return Error::new( input.span(), "#[make_ule] can only be applied to #[repr(u8)] enums", diff --git a/utils/zerovec/derive/src/ule.rs b/utils/zerovec/derive/src/ule.rs index 6a03c008f4f..755e66a229b 100644 --- a/utils/zerovec/derive/src/ule.rs +++ b/utils/zerovec/derive/src/ule.rs @@ -10,10 +10,10 @@ use syn::spanned::Spanned; use syn::{Data, DeriveInput, Error}; pub fn derive_impl(input: &DeriveInput) -> TokenStream2 { - if !utils::has_valid_repr(&input.attrs, |r| r == "packed" || r == "transparent") { + if !utils::ReprInfo::compute(&input.attrs).cpacked_or_transparent() { return Error::new( input.span(), - "derive(ULE) must be applied to a #[repr(packed)] or #[repr(transparent)] type", + "derive(ULE) must be applied to a #[repr(C, packed)] or #[repr(transparent)] type", ) .to_compile_error(); } diff --git a/utils/zerovec/derive/src/utils.rs b/utils/zerovec/derive/src/utils.rs index 6d37444ca31..b65b848b069 100644 --- a/utils/zerovec/derive/src/utils.rs +++ b/utils/zerovec/derive/src/utils.rs @@ -11,14 +11,38 @@ use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::{Attribute, Error, Field, Fields, Ident, Index, Result, Token}; -// Check that there are repr attributes satisfying the given predicate -pub fn has_valid_repr(attrs: &[Attribute], predicate: impl Fn(&Ident) -> bool + Copy) -> bool { - attrs.iter().filter(|a| a.path().is_ident("repr")).any(|a| { - a.parse_args::() - .ok() - .and_then(|s| s.idents.iter().find(|s| predicate(s)).map(|_| ())) - .is_some() - }) +#[derive(Default)] +pub struct ReprInfo { + pub c: bool, + pub transparent: bool, + pub u8: bool, + pub packed: bool, +} + +impl ReprInfo { + pub fn compute(attrs: &[Attribute]) -> Self { + let mut info = ReprInfo::default(); + for attr in attrs.iter().filter(|a| a.path().is_ident("repr")) { + if let Ok(pieces) = attr.parse_args::() { + for piece in pieces.idents.iter() { + if piece == "C" || piece == "c" { + info.c = true; + } else if piece == "transparent" { + info.transparent = true; + } else if piece == "packed" { + info.packed = true; + } else if piece == "u8" { + info.u8 = true; + } + } + } + } + info + } + + pub fn cpacked_or_transparent(self) -> bool { + (self.c && self.packed) || self.transparent + } } // An attribute that is a list of idents @@ -60,7 +84,7 @@ pub fn repr_for(f: &Fields) -> TokenStream2 { if f.len() == 1 { quote!(transparent) } else { - quote!(packed) + quote!(C, packed) } } diff --git a/utils/zerovec/derive/src/varule.rs b/utils/zerovec/derive/src/varule.rs index 4a586f9547b..82fd702578e 100644 --- a/utils/zerovec/derive/src/varule.rs +++ b/utils/zerovec/derive/src/varule.rs @@ -15,10 +15,10 @@ pub fn derive_impl( input: &DeriveInput, custom_varule_validator: Option, ) -> TokenStream2 { - if !utils::has_valid_repr(&input.attrs, |r| r == "packed" || r == "transparent") { + if !utils::ReprInfo::compute(&input.attrs).cpacked_or_transparent() { return Error::new( input.span(), - "derive(VarULE) must be applied to a #[repr(packed)] or #[repr(transparent)] type", + "derive(VarULE) must be applied to a #[repr(C, packed)] or #[repr(transparent)] type", ) .to_compile_error(); } diff --git a/utils/zerovec/src/flexzerovec/slice.rs b/utils/zerovec/src/flexzerovec/slice.rs index 41cb7116f90..8e8f7570646 100644 --- a/utils/zerovec/src/flexzerovec/slice.rs +++ b/utils/zerovec/src/flexzerovec/slice.rs @@ -13,7 +13,7 @@ use core::ops::Range; const USIZE_WIDTH: usize = mem::size_of::(); /// A zero-copy "slice" that efficiently represents `[usize]`. -#[repr(packed)] +#[repr(C, packed)] pub struct FlexZeroSlice { // Hard Invariant: 1 <= width <= USIZE_WIDTH (which is target_pointer_width) // Soft Invariant: width == the width of the largest element diff --git a/utils/zerovec/src/ule/mod.rs b/utils/zerovec/src/ule/mod.rs index 5a6d9cd4713..757307ada94 100644 --- a/utils/zerovec/src/ule/mod.rs +++ b/utils/zerovec/src/ule/mod.rs @@ -61,7 +61,7 @@ use core::{mem, slice}; /// 6. Acknowledge the following note about the equality invariant. /// /// If the ULE type is a struct only containing other ULE types (or other types which satisfy invariants 1 and 2, -/// like `[u8; N]`), invariants 1 and 2 can be achieved via `#[repr(packed)]` or `#[repr(transparent)]`. +/// like `[u8; N]`), invariants 1 and 2 can be achieved via `#[repr(C, packed)]` or `#[repr(transparent)]`. /// /// # Equality invariant /// @@ -271,7 +271,7 @@ where /// 7. Acknowledge the following note about the equality invariant. /// /// If the ULE type is a struct only containing other ULE/VarULE types (or other types which satisfy invariants 1 and 2, -/// like `[u8; N]`), invariants 1 and 2 can be achieved via `#[repr(packed)]` or `#[repr(transparent)]`. +/// like `[u8; N]`), invariants 1 and 2 can be achieved via `#[repr(C, packed)]` or `#[repr(transparent)]`. /// /// # Equality invariant /// diff --git a/utils/zerovec/src/ule/option.rs b/utils/zerovec/src/ule/option.rs index 9b0dc5b28a1..e8bd0fae498 100644 --- a/utils/zerovec/src/ule/option.rs +++ b/utils/zerovec/src/ule/option.rs @@ -28,7 +28,7 @@ use core::mem::{self, MaybeUninit}; // Invariants: // The MaybeUninit is zeroed when None (bool = false), // and is valid when Some (bool = true) -#[repr(packed)] +#[repr(C, packed)] pub struct OptionULE(bool, MaybeUninit); impl OptionULE { @@ -62,11 +62,11 @@ impl core::fmt::Debug for OptionULE { // Safety (based on the safety checklist on the ULE trait): // 1. OptionULE does not include any uninitialized or padding bytes. -// (achieved by `#[repr(packed)]` on a struct containing only ULE fields, +// (achieved by `#[repr(C, packed)]` on a struct containing only ULE fields, // in the context of this impl. The MaybeUninit is valid for all byte sequences, and we only generate /// zeroed or valid-T byte sequences to fill it) // 2. OptionULE is aligned to 1 byte. -// (achieved by `#[repr(packed)]` on a struct containing only ULE fields, in the context of this impl) +// (achieved by `#[repr(C, packed)]` on a struct containing only ULE fields, in the context of this impl) // 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/src/ule/tuple.rs b/utils/zerovec/src/ule/tuple.rs index 3e0f291b3fa..457a10f41b0 100644 --- a/utils/zerovec/src/ule/tuple.rs +++ b/utils/zerovec/src/ule/tuple.rs @@ -30,15 +30,15 @@ use core::mem; macro_rules! tuple_ule { ($name:ident, $len:literal, [ $($t:ident $i:tt),+ ]) => { #[doc = concat!("ULE type for tuples with ", $len, " elements.")] - #[repr(packed)] + #[repr(C, packed)] #[allow(clippy::exhaustive_structs)] // stable pub struct $name<$($t),+>($(pub $t),+); // Safety (based on the safety checklist on the ULE trait): // 1. TupleULE does not include any uninitialized or padding bytes. - // (achieved by `#[repr(packed)]` on a struct containing only ULE fields) + // (achieved by `#[repr(C, packed)]` on a struct containing only ULE fields) // 2. TupleULE is aligned to 1 byte. - // (achieved by `#[repr(packed)]` on a struct containing only ULE fields) + // (achieved by `#[repr(C, packed)]` on a struct containing 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 there are extra bytes. // 5. The other ULE methods use the default impl.