Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce C,packed, not just packed, on ULE types #5049

Merged
merged 1 commit into from
Jun 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/calendar/src/provider/chinese_based.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl<'data> ChineseBasedCacheV1<'data> {
derive(databake::Bake),
databake(path = icu_calendar::provider),
)]
#[repr(packed)]
#[repr(C, packed)]
pub struct PackedChineseBasedYearInfo(pub u8, pub u8, pub u8);

impl PackedChineseBasedYearInfo {
Expand Down
2 changes: 1 addition & 1 deletion components/calendar/src/provider/islamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl<'data> IslamicCacheV1<'data> {
databake(path = icu_calendar::provider),
)]
#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
#[repr(packed)]
#[repr(C, packed)]
pub struct PackedIslamicYearInfo(pub u8, pub u8);

impl fmt::Debug for PackedIslamicYearInfo {
Expand Down
2 changes: 1 addition & 1 deletion components/casemap/src/provider/exceptions_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl ExceptionHeader {
/// In this struct the RESERVED bit is still allowed to be set, and it will produce a different
/// exception header, but it will not have any other effects.
#[derive(Copy, Clone, PartialEq, Eq, ULE)]
#[repr(packed)]
#[repr(C, packed)]
pub struct ExceptionHeaderULE {
slot_presence: SlotPresence,
bits: ExceptionBitsULE,
Expand Down
2 changes: 1 addition & 1 deletion components/properties/src/provider/bidi_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub enum CheckedBidiPairedBracketType {
/// numbers and a byte has 8 bits
/// needed for datagen but not intended for users
#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
#[repr(packed)]
#[repr(C, packed)]
pub struct MirroredPairedBracketDataULE([u8; 3]);

// Safety (based on the safety checklist on the ULE trait):
Expand Down
4 changes: 2 additions & 2 deletions utils/zerovec/derive/examples/derives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion utils/zerovec/derive/src/make_ule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions utils/zerovec/derive/src/ule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
42 changes: 33 additions & 9 deletions utils/zerovec/derive/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<IdentListAttribute>()
.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::<IdentListAttribute>() {
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
Expand Down Expand Up @@ -60,7 +84,7 @@ pub fn repr_for(f: &Fields) -> TokenStream2 {
if f.len() == 1 {
quote!(transparent)
} else {
quote!(packed)
quote!(C, packed)
}
}

Expand Down
4 changes: 2 additions & 2 deletions utils/zerovec/derive/src/varule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ pub fn derive_impl(
input: &DeriveInput,
custom_varule_validator: Option<TokenStream2>,
) -> 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();
}
Expand Down
2 changes: 1 addition & 1 deletion utils/zerovec/src/flexzerovec/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use core::ops::Range;
const USIZE_WIDTH: usize = mem::size_of::<usize>();

/// 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
Expand Down
4 changes: 2 additions & 2 deletions utils/zerovec/src/ule/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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
///
Expand Down
6 changes: 3 additions & 3 deletions utils/zerovec/src/ule/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<U>(bool, MaybeUninit<U>);

impl<U: Copy> OptionULE<U> {
Expand Down Expand Up @@ -62,11 +62,11 @@ impl<U: Copy + core::fmt::Debug> core::fmt::Debug for OptionULE<U> {

// 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.
Expand Down
6 changes: 3 additions & 3 deletions utils/zerovec/src/ule/tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading