diff --git a/zerocopy/AGENTS.md b/zerocopy/AGENTS.md index dcc4b7d4ad..56029d14ff 100644 --- a/zerocopy/AGENTS.md +++ b/zerocopy/AGENTS.md @@ -37,6 +37,13 @@ you **MUST** also read [agent_docs/reviewing.md](./agent_docs/reviewing.md). - **Documentation:** **DO** ensure that changes do not cause documentation to become out of date (e.g., renaming files referenced here). +- **Derived traits in tests:** For traits whose documentation requires + derive-only implementation, test fixture types **MUST** use Zerocopy's + derives. Do not bypass a derive rejection with a manual implementation; + instead, test the lower-level internal primitive or extend the derive. + Generated derive-output snapshots and crate-owned built-in implementations + are exempt. + ## Project Context ### Overview diff --git a/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64 b/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64 index 03684cb09f..1638faa762 100644 --- a/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64 +++ b/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64 @@ -1,13 +1,13 @@ bench_split_via_runtime_check_dynamic_padding: mov rax, rdi mov rdx, qword ptr [rsi + 16] - mov ecx, edx - and ecx, 3 - cmp ecx, 1 + lea rdi, [rdx + 2*rdx] + mov ecx, edi + not ecx + test cl, 3 jne .LBB5_1 mov rcx, qword ptr [rsi] mov rsi, qword ptr [rsi + 8] - lea rdi, [rdx + 2*rdx] add rdi, rcx add rdi, 9 sub rsi, rdx diff --git a/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64.mca b/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64.mca index 5034ab0583..849cc45014 100644 --- a/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64.mca +++ b/zerocopy/benches/split_via_runtime_check_dynamic_padding.x86-64.mca @@ -1,11 +1,11 @@ Iterations: 100 Instructions: 2000 -Total Cycles: 708 +Total Cycles: 807 Total uOps: 2000 Dispatch Width: 4 -uOps Per Cycle: 2.82 -IPC: 2.82 +uOps Per Cycle: 2.48 +IPC: 2.48 Block RThroughput: 5.0 @@ -20,13 +20,13 @@ Instruction Info: [1] [2] [3] [4] [5] [6] Instructions: 1 1 0.33 mov rax, rdi 1 5 0.50 * mov rdx, qword ptr [rsi + 16] - 1 1 0.33 mov ecx, edx - 1 1 0.33 and ecx, 3 - 1 1 0.33 cmp ecx, 1 + 1 1 0.50 lea rdi, [rdx + 2*rdx] + 1 1 0.33 mov ecx, edi + 1 1 0.33 not ecx + 1 1 0.33 test cl, 3 1 1 1.00 jne .LBB5_1 1 5 0.50 * mov rcx, qword ptr [rsi] 1 5 0.50 * mov rsi, qword ptr [rsi + 8] - 1 1 0.50 lea rdi, [rdx + 2*rdx] 1 1 0.33 add rdi, rcx 1 1 0.33 add rdi, 9 1 1 0.33 sub rsi, rdx @@ -53,26 +53,26 @@ Resources: Resource pressure per iteration: [0] [1] [2] [3] [4] [5] [6.0] [6.1] - - - 3.00 3.02 5.00 4.98 4.00 4.00 + - - 3.01 3.01 5.00 4.98 4.00 4.00 Resource pressure by instruction: [0] [1] [2] [3] [4] [5] [6.0] [6.1] Instructions: - - - - 0.99 - 0.01 - - mov rax, rdi - - - - - - - - 1.00 mov rdx, qword ptr [rsi + 16] - - - 0.99 0.01 - - - - mov ecx, edx - - - 0.99 0.01 - - - - and ecx, 3 - - - 0.97 0.03 - - - - cmp ecx, 1 + - - 0.02 0.01 - 0.97 - - mov rax, rdi + - - - - - - 0.99 0.01 mov rdx, qword ptr [rsi + 16] + - - 0.02 0.98 - - - - lea rdi, [rdx + 2*rdx] + - - 0.98 0.02 - - - - mov ecx, edi + - - 0.98 0.01 - 0.01 - - not ecx + - - 0.97 0.02 - 0.01 - - test cl, 3 - - - - - 1.00 - - jne .LBB5_1 - - - - - - 1.00 - mov rcx, qword ptr [rsi] - - - - - - 0.99 0.01 mov rsi, qword ptr [rsi + 8] - - - 0.01 0.99 - - - - lea rdi, [rdx + 2*rdx] - - - - 0.96 - 0.04 - - add rdi, rcx - - - 0.03 - - 0.97 - - add rdi, 9 + - - 0.02 0.98 - - - - add rdi, rcx + - - 0.01 0.96 - 0.03 - - add rdi, 9 - - 0.01 0.03 - 0.96 - - sub rsi, rdx - - - - 1.00 - 1.00 - mov qword ptr [rax + 8], rdx - - - - 1.00 - - 1.00 mov qword ptr [rax + 16], rdi - - - - 1.00 - 0.01 0.99 mov qword ptr [rax + 24], rsi - - - - - 1.00 - 0.99 0.01 mov qword ptr [rax], rcx + - - - - 1.00 - - 1.00 mov qword ptr [rax], rcx - - - - - 1.00 - - ret - - - - - - - - xor ecx, ecx - - - - 1.00 - 0.01 0.99 mov qword ptr [rax], rcx diff --git a/zerocopy/src/layout.rs b/zerocopy/src/layout.rs index d58786d637..dc59c08bb4 100644 --- a/zerocopy/src/layout.rs +++ b/zerocopy/src/layout.rs @@ -65,6 +65,99 @@ pub(crate) struct TrailingSliceLayout { pub(crate) offset: usize, // The size of the element type of the trailing slice field. pub(crate) elem_size: E, + // `size_align` is the alignment used by the sole `round_up` operation in + // the normalized object-size formula below. It is a rounding modulus, not + // necessarily the alignment of the enclosing type (`DstLayout::align`) or + // the effective alignment at which its trailing slice field is placed. + // In particular, `repr(packed)` can lower both without removing trailing + // padding required by a nested DST's own layout. For example, a + // `repr(packed(2))` wrapper around an alignment-4 DST can have + // `DstLayout::align == 2` but `size_align == 4`. + // + // When this describes a valid Rust type, `size_align` is a power of two, + // `size_prefix < size_align`, and, for a trailing slice length `elems`, the + // size of the enclosing type is: + // + // size_prefix + // + round_up(size_offset + elems * elem_size, size_align) + // + // `size_prefix` is always less than `size_align`. These fields are + // separate from `offset` because packing a field changes the field's + // placement and the enclosing type's alignment, but does not change + // trailing padding internal to the field's type. In particular, a nested + // slice DST can require an inner rounding operation whose origin differs + // from the physical offset of the trailing slice. + pub(crate) size_prefix: usize, + pub(crate) size_offset: usize, + pub(crate) size_align: NonZeroUsize, +} + +impl TrailingSliceLayout { + /// Computes the size described by this layout for `elems` trailing slice + /// elements. + #[inline(always)] + pub(crate) const fn size_for_elems(self, elems: usize) -> Option { + let trailing_size = match self.elem_size.checked_mul(elems) { + Some(size) => size, + None => return None, + }; + let without_padding = match self.size_offset.checked_add(trailing_size) { + Some(size) => size, + None => return None, + }; + let with_padding = match without_padding + .checked_add(util::padding_needed_for(without_padding, self.size_align)) + { + Some(size) => size, + None => return None, + }; + self.size_prefix.checked_add(with_padding) + } + + /// Returns whether `self` and `other` describe the same size for every + /// trailing slice length. + /// + /// This recognizes two sufficient forms. If both formulas use the same + /// rounding alignment and begin at the same residue, each future rounding + /// decision is identical. If the element size is a multiple of both + /// rounding alignments, neither formula's padding changes as the length + /// increases. In both cases, equality at length zero proves equality for + /// every length. + #[inline(always)] + pub(crate) const fn has_same_size_sequence(self, other: Self) -> bool { + if self.elem_size != other.elem_size { + return false; + } + + match (self.size_for_elems(0), other.size_for_elems(0)) { + (Some(self_size), Some(other_size)) if self_size == other_size => {} + _ => return false, + } + + let self_align = self.size_align.get(); + let other_align = other.size_align.get(); + let max_align = if self_align > other_align { self_align } else { other_align }; + + // Since valid layout alignments are powers of two, `max_align` is a + // multiple of both alignments. Every metadata increment therefore + // adds exactly `elem_size` to both rounded sizes. + #[allow(clippy::arithmetic_side_effects)] + if self.elem_size % max_align == 0 { + return true; + } + + if self_align != other_align { + return false; + } + + // Both formulas visit the same residue after every metadata increment, + // so they add and remove padding in lockstep. + #[allow(clippy::arithmetic_side_effects)] + let self_residue = self.size_offset % self_align; + #[allow(clippy::arithmetic_side_effects)] + let other_residue = other.size_offset % other_align; + self_residue == other_residue + } } impl SizeInfo { @@ -76,9 +169,21 @@ impl SizeInfo { const fn try_to_nonzero_elem_size(&self) -> Option> { Some(match *self { SizeInfo::Sized { size } => SizeInfo::Sized { size }, - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) => { + SizeInfo::SliceDst(TrailingSliceLayout { + offset, + elem_size, + size_prefix, + size_offset, + size_align, + }) => { if let Some(elem_size) = NonZeroUsize::new(elem_size) { - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) + SizeInfo::SliceDst(TrailingSliceLayout { + offset, + elem_size, + size_prefix, + size_offset, + size_align, + }) } else { return None; } @@ -256,6 +361,12 @@ impl DstLayout { size_info: SizeInfo::SliceDst(TrailingSliceLayout { offset: 0, elem_size: mem::size_of::(), + size_prefix: 0, + size_offset: 0, + size_align: match NonZeroUsize::new(mem::align_of::()) { + Some(align) => align, + None => const_unreachable!(), + }, }), statically_shallow_unpadded: true, } @@ -427,6 +538,9 @@ impl DstLayout { SizeInfo::SliceDst(TrailingSliceLayout { offset: trailing_offset, elem_size, + size_prefix, + size_offset, + size_align, }) => { // If the trailing field is dynamically sized, so too // will the resulting layout. The offset of the trailing @@ -440,11 +554,42 @@ impl DstLayout { // panic otherwise (e.g., combining or aligning the // components would create a size exceeding // `usize::MAX`). - let offset = match offset.checked_add(trailing_offset) { + let trailing_offset = match offset.checked_add(trailing_offset) { + Some(offset) => offset, + None => const_panic!("`field` cannot be appended without the total size overflowing `usize`"), + }; + + // Before normalization, the composite's size is: + // + // offset + size_prefix + // + round_up(size_offset + elems * elem_size, size_align) + // + // Split `offset + size_prefix` into a remainder + // less than `size_align` and a multiple of + // `size_align`. The multiple can move inside the + // rounding operation without changing the result. + let unnormalized_prefix = match offset.checked_add(size_prefix) { + Some(prefix) => prefix, + None => const_panic!("`field` cannot be appended without the total size overflowing `usize`"), + }; + #[allow(clippy::arithmetic_side_effects)] + let size_prefix = unnormalized_prefix % size_align.get(); + // `size_prefix` is a remainder of + // `unnormalized_prefix`. + #[allow(clippy::arithmetic_side_effects)] + let prefix_multiple = unnormalized_prefix - size_prefix; + let size_offset = match prefix_multiple.checked_add(size_offset) { Some(offset) => offset, None => const_panic!("`field` cannot be appended without the total size overflowing `usize`"), }; - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) + + SizeInfo::SliceDst(TrailingSliceLayout { + offset: trailing_offset, + elem_size, + size_prefix, + size_offset, + size_align, + }) } }, ) @@ -459,18 +604,11 @@ impl DstLayout { } /// Like `Layout::pad_to_align`, this routine rounds the size of this layout - /// up to the nearest multiple of this type's alignment or `repr_packed` - /// (whichever is less). This method leaves DST layouts unchanged, since the - /// trailing padding of DSTs is computed at runtime. - /// - /// The accompanying boolean is `true` if the resulting composition of - /// fields necessitated static (as opposed to dynamic) padding; otherwise - /// `false`. + /// up to the nearest multiple of this type's alignment. For DST layouts, + /// this updates the runtime size formula to account for trailing padding. /// /// In order to match the layout of a `#[repr(C)]` struct, this method - /// should be invoked after the invocations of [`DstLayout::extend`]. If - /// `self` corresponds to a type marked with `repr(packed(N))`, then - /// `repr_packed` should be set to `Some(N)`, otherwise `None`. + /// should be invoked after the invocations of [`DstLayout::extend`]. /// /// This method cannot be used to match the layout of a record with the /// default representation, as that representation is mostly unspecified. @@ -480,7 +618,7 @@ impl DstLayout { /// If a (potentially hypothetical) valid `repr(C)` type begins with fields /// whose layout are `self` followed only by zero or more bytes of trailing /// padding (not included in `self`), then unsafe code may rely on - /// `self.pad_to_align(repr_packed)` producing a layout that correctly + /// `self.pad_to_align()` producing a layout that correctly /// encapsulates the layout of that type. /// /// We make no guarantees to the behavior of this method if `self` cannot @@ -504,10 +642,56 @@ impl DstLayout { (padding, SizeInfo::Sized { size }) } // For DST layouts, trailing padding depends on the length of the - // trailing DST and is computed at runtime. This does not alter the - // offset or element size of the layout, so we leave `size_info` - // unchanged. - size_info @ SizeInfo::SliceDst(_) => (0, size_info), + // trailing DST and is computed at runtime. Normalize the composed + // rounding operation back into the representation documented on + // `TrailingSliceLayout`. + SizeInfo::SliceDst(mut trailing) => { + if trailing.size_align.get() < self.align.get() { + // `self.align` and `trailing.size_align` are powers of two, + // so `self.align` is a multiple of `trailing.size_align`. + // Rounding + // + // size_prefix + round_up(x, size_align) + // + // to `self.align` is equivalent to rounding + // + // round_up(size_prefix, size_align) + x + // + // directly to `self.align`. Since `size_prefix` is less + // than `size_align`, its rounded value is either zero or + // `size_align`. + let prefix = + if trailing.size_prefix == 0 { 0 } else { trailing.size_align.get() }; + trailing.size_offset = match trailing.size_offset.checked_add(prefix) { + Some(offset) => offset, + None => const_panic!("Adding padding caused size to overflow `usize`."), + }; + trailing.size_prefix = 0; + trailing.size_align = self.align; + } else { + // `trailing.size_align` is a multiple of `self.align`, so + // the inner rounded size is already aligned to + // `self.align`. Only `size_prefix` needs to be rounded. + let padding = padding_needed_for(trailing.size_prefix, self.align); + let rounded_prefix = match trailing.size_prefix.checked_add(padding) { + Some(prefix) => prefix, + None => const_panic!("Adding padding caused size to overflow `usize`."), + }; + #[allow(clippy::arithmetic_side_effects)] + let size_prefix = rounded_prefix % trailing.size_align.get(); + #[allow(clippy::arithmetic_side_effects)] + let prefix_multiple = rounded_prefix - size_prefix; + trailing.size_offset = match trailing.size_offset.checked_add(prefix_multiple) { + Some(offset) => offset, + None => { + const_panic!("Adding padding caused size to overflow `usize`.") + } + }; + trailing.size_prefix = size_prefix; + } + + (0, SizeInfo::SliceDst(trailing)) + } }; let statically_shallow_unpadded = self.statically_shallow_unpadded && static_padding == 0; @@ -527,15 +711,57 @@ impl DstLayout { #[must_use] #[inline(always)] pub const fn requires_dynamic_padding(self) -> bool { - // A `% self.align.get()` cannot panic, since `align` is non-zero. + // A `% size_align.get()` cannot panic, since `size_align` is non-zero. #[allow(clippy::arithmetic_side_effects)] match self.size_info { SizeInfo::Sized { .. } => false, SizeInfo::SliceDst(trailing_slice_layout) => { // SAFETY: This predicate is formally proved sound by // `proofs::prove_requires_dynamic_padding`. - trailing_slice_layout.offset % self.align.get() != 0 - || trailing_slice_layout.elem_size % self.align.get() != 0 + let initial_padding = util::padding_needed_for( + trailing_slice_layout.size_offset, + trailing_slice_layout.size_align, + ); + let initial_rounded_size = + trailing_slice_layout.size_offset.checked_add(initial_padding); + let initial_size = match initial_rounded_size { + Some(size) => trailing_slice_layout.size_prefix.checked_add(size), + None => None, + }; + + match initial_size { + Some(initial_size) => { + initial_size != trailing_slice_layout.offset + || trailing_slice_layout.elem_size + % trailing_slice_layout.size_align.get() + != 0 + } + None => true, + } + } + } + } + + /// Returns the largest trailing slice length whose object size is exactly + /// `size`. + /// + /// Returns `None` if there is no such length, if this describes a sized + /// type, or if the trailing slice element is zero-sized. + #[inline(always)] + const fn metadata_for_exact_size(&self, size: usize) -> Option { + match self.size_info { + SizeInfo::Sized { .. } + | SizeInfo::SliceDst(TrailingSliceLayout { elem_size: 0, .. }) => None, + SizeInfo::SliceDst(_) => { + // Address zero is aligned to every supported alignment, and + // `0 + size` cannot overflow. The validator returns the + // largest metadata whose object fits in `size`; accepting it + // only when the resulting object consumes all `size` bytes + // turns that bounded query into an exact-size query. + match self.validate_cast_and_convert_metadata(0, size, CastType::Prefix) { + Ok((elems, object_size)) if object_size == size => Some(elems), + _ => None, + } } } } @@ -680,19 +906,28 @@ impl DstLayout { } (0, size) } - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) => { - // Calculate the maximum number of bytes that could be consumed - // - any number of bytes larger than this will either not be a - // multiple of the alignment, or will be larger than - // `bytes_len`. - let max_total_bytes = - util::round_down_to_next_multiple_of_alignment(bytes_len, self.align); + SizeInfo::SliceDst(TrailingSliceLayout { + elem_size, + size_prefix, + size_offset, + size_align, + .. + }) => { + let bytes_after_prefix = match bytes_len.checked_sub(size_prefix) { + Some(bytes) => bytes, + None => return Err(MetadataCastError::Size), + }; + // The inner portion of every valid size is a multiple of + // `size_align`. Round the available bytes down to the largest + // such multiple. + let max_rounded_bytes = + util::round_down_to_next_multiple_of_alignment(bytes_after_prefix, size_align); // Calculate the maximum number of bytes that could be consumed // by the trailing slice. // // FIXME(#67): Once our MSRV is 1.65, use let-else: // https://blog.rust-lang.org/2022/11/03/Rust-1.65.0.html#let-else-statements - let max_slice_and_padding_bytes = match max_total_bytes.checked_sub(offset) { + let max_slice_and_padding_bytes = match max_rounded_bytes.checked_sub(size_offset) { Some(max) => max, // `bytes_len` too small even for 0 trailing slice elements. None => return Err(MetadataCastError::Size), @@ -710,11 +945,11 @@ impl DstLayout { // elem_size) * elem_size`. // // Guaranteed not to overflow on addition: - // - max_slice_and_padding_bytes == max_total_bytes - offset - // - elems * elem_size <= max_slice_and_padding_bytes == max_total_bytes - offset - // - elems * elem_size + offset <= max_total_bytes <= usize::MAX + // - max_slice_and_padding_bytes == max_rounded_bytes - size_offset + // - elems * elem_size <= max_slice_and_padding_bytes + // - elems * elem_size + size_offset <= max_rounded_bytes #[allow(clippy::arithmetic_side_effects)] - let without_padding = offset + elems * elem_size.get(); + let without_padding = size_offset + elems * elem_size.get(); // `self_bytes` is equal to the offset bytes plus the bytes // consumed by the trailing slice plus any padding bytes // required to satisfy the alignment. Note that we have computed @@ -723,16 +958,20 @@ impl DstLayout { // the size of an extra element. // // Guaranteed not to overflow: - // - By previous comment: without_padding == elems * elem_size + - // offset <= max_total_bytes - // - By construction, `max_total_bytes` is a multiple of - // `self.align`. + // - By previous comment: without_padding <= max_rounded_bytes + // - By construction, `max_rounded_bytes` is a multiple of + // `size_align`. // - At most, adding padding needed to round `without_padding` - // up to the next multiple of the alignment will bring - // `self_bytes` up to `max_total_bytes`. + // up to the next multiple of `size_align` will bring the + // rounded size up to `max_rounded_bytes`. + #[allow(clippy::arithmetic_side_effects)] + let rounded = + without_padding + util::padding_needed_for(without_padding, size_align); + // `rounded <= max_rounded_bytes <= bytes_after_prefix`, so + // adding `size_prefix` cannot overflow and cannot exceed + // `bytes_len`. #[allow(clippy::arithmetic_side_effects)] - let self_bytes = - without_padding + util::padding_needed_for(without_padding, self.align); + let self_bytes = size_prefix + rounded; (elems, self_bytes) } }; @@ -745,7 +984,7 @@ impl DstLayout { // - In the `Sized` branch, only returns `size` if `size <= // bytes_len`. // - In the `SliceDst` branch, calculates `self_bytes <= - // max_toatl_bytes`, which is upper-bounded by `bytes_len`. + // bytes_len`. #[allow(clippy::arithmetic_side_effects)] CastType::Suffix => bytes_len - self_bytes, }; @@ -823,52 +1062,19 @@ mod cast_from { // - If this is possible, any information necessary to perform // the `Src`->`Dst` metadata conversion at runtime. // - // Assume that `Src` and `Dst` are slice DSTs, and define: - // - `S_OFF = Src::LAYOUT.size_info.offset` - // - `S_ELEM = Src::LAYOUT.size_info.elem_size` - // - `D_OFF = Dst::LAYOUT.size_info.offset` - // - `D_ELEM = Dst::LAYOUT.size_info.elem_size` - // - // We are trying to solve the following equation: - // - // D_OFF + d_meta * D_ELEM = S_OFF + s_meta * S_ELEM - // - // At runtime, we will be attempting to compute `d_meta`, given - // `s_meta` (a runtime value) and all other parameters (which - // are compile-time values). We can solve like so: - // - // D_OFF + d_meta * D_ELEM = S_OFF + s_meta * S_ELEM - // - // d_meta * D_ELEM = S_OFF - D_OFF + s_meta * S_ELEM - // - // d_meta = (S_OFF - D_OFF + s_meta * S_ELEM)/D_ELEM - // - // Since `d_meta` will be a `usize`, we need the right-hand side - // to be an integer, and this needs to hold for *any* value of - // `s_meta` (in order for our conversion to be infallible - ie, - // to not have to reject certain values of `s_meta` at runtime). - // This means that: - // - // - `s_meta * S_ELEM` must be a multiple of `D_ELEM` - // - Since this must hold for any value of `s_meta`, `S_ELEM` - // must be a multiple of `D_ELEM` - // - `S_OFF - D_OFF` must be a multiple of `D_ELEM` - // - // Thus, let `OFFSET_DELTA_ELEMS = (S_OFF - D_OFF)/D_ELEM` and - // `ELEM_MULTIPLE = S_ELEM/D_ELEM`. We can rewrite the above - // expression as: - // - // d_meta = (S_OFF - D_OFF + s_meta * S_ELEM)/D_ELEM + // For slice DSTs, destination metadata is an affine function + // of source metadata: // // d_meta = OFFSET_DELTA_ELEMS + s_meta * ELEM_MULTIPLE // - // Thus, we just need to compute the following and confirm that - // they have integer solutions in order to both a) determine - // whether infallible `Src` -> `Dst` casts are possible and, b) - // pre-compute the parameters necessary to perform those casts - // at runtime. These parameters are encapsulated in - // `CastParams`, which acts as a witness that such infallible - // casts are possible. + // `ELEM_MULTIPLE` scales the destination's trailing element + // size to the source's. `OFFSET_DELTA_ELEMS` selects a + // destination size equal to the source's zero-element size. + // We then prove that the two complete rounded size formulas + // produce the same sequence for all metadata. This is + // necessary because a packed outer DST can retain a nested + // field's rounding operation; comparing only physical + // trailing-slice offsets is not sufficient. /// The parameters required in order to perform an /// unsized-to-unsized pointer cast from `Src` to `Dst` as /// described above. @@ -910,15 +1116,41 @@ mod cast_from { } impl CastParams { + /// Returns whether mapping source metadata `n` to destination + /// metadata `dst_base + n * (src.elem_size / dst.elem_size)` + /// preserves object size for every `n`. + const fn size_sequences_match( + src: TrailingSliceLayout, + dst: TrailingSliceLayout, + dst_base: usize, + ) -> bool { + let base_bytes = match dst_base.checked_mul(dst.elem_size) { + Some(bytes) => bytes, + None => return false, + }; + let size_offset = match dst.size_offset.checked_add(base_bytes) { + Some(offset) => offset, + None => return false, + }; + let shifted_dst = TrailingSliceLayout { + offset: dst.offset, + elem_size: src.elem_size, + size_prefix: dst.size_prefix, + size_offset, + size_align: dst.size_align, + }; + src.has_same_size_sequence(shifted_dst) + } + const fn try_compute( - src: &DstLayout, - dst: &DstLayout, + src_layout: &DstLayout, + dst_layout: &DstLayout, ) -> Option> { - if src.align.get() < dst.align.get() { + if src_layout.align.get() < dst_layout.align.get() { return None; } - let inner = match (src.size_info, dst.size_info) { + let inner = match (src_layout.size_info, dst_layout.size_info) { ( SizeInfo::Sized { size: src_size }, SizeInfo::Sized { size: dst_size }, @@ -931,82 +1163,83 @@ mod cast_from { // dst_size`. CastParamsInner::SizedToSized } - (SizeInfo::Sized { size: src_size }, SizeInfo::SliceDst(dst)) => { - let offset_delta = if let Some(od) = src_size.checked_sub(dst.offset) { - od - } else { - return None; - }; - - let dst_elem_size = if let Some(e) = NonZeroUsize::new(dst.elem_size) { - e - } else { - return None; + (SizeInfo::Sized { size: src_size }, SizeInfo::SliceDst(_)) => { + let dst_meta = match dst_layout.metadata_for_exact_size(src_size) { + Some(meta) => meta, + None => return None, }; - // PANICS: `dst_elem_size: NonZeroUsize`, so this won't - // divide by zero. - #[allow(clippy::arithmetic_side_effects)] - let delta_mod_other_elem = offset_delta % dst_elem_size.get(); - - if delta_mod_other_elem != 0 { - return None; - } - - // PANICS: `dst_elem_size: NonZeroUsize`, so this won't - // divide by zero. - #[allow(clippy::arithmetic_side_effects)] - let dst_meta = offset_delta / dst_elem_size.get(); - // SAFETY: The preceding math ensures that a `Dst` // with `dst_meta` addresses `src_size` bytes. CastParamsInner::SizedToUnsized { dst_meta } } (SizeInfo::SliceDst(src), SizeInfo::SliceDst(dst)) => { - let offset_delta = if let Some(od) = src.offset.checked_sub(dst.offset) - { - od - } else { - return None; - }; - let dst_elem_size = if let Some(e) = NonZeroUsize::new(dst.elem_size) { e } else { return None; }; - // PANICS: `dst_elem_size: NonZeroUsize`, so this won't - // divide by zero. - #[allow(clippy::arithmetic_side_effects)] - let delta_mod_other_elem = offset_delta % dst_elem_size.get(); - // PANICS: `dst_elem_size: NonZeroUsize`, so this won't // divide by zero. #[allow(clippy::arithmetic_side_effects)] let elem_remainder = src.elem_size % dst_elem_size.get(); - if delta_mod_other_elem != 0 - || src.elem_size < dst.elem_size - || elem_remainder != 0 - { + if src.elem_size < dst.elem_size || elem_remainder != 0 { return None; } - // PANICS: `dst_elem_size: NonZeroUsize`, so this won't - // divide by zero. - #[allow(clippy::arithmetic_side_effects)] - let offset_delta_elems = offset_delta / dst_elem_size.get(); - // PANICS: `dst_elem_size: NonZeroUsize`, so this won't // divide by zero. #[allow(clippy::arithmetic_side_effects)] let elem_multiple = src.elem_size / dst_elem_size.get(); + // Prefer the old exact-inner-offset solution. In + // addition to preserving existing accepted casts, + // it avoids selecting another metadata value from a + // rounding plateau when that value would have a + // different future padding sequence. + let exact_offset_delta_elems = + match src.size_offset.checked_sub(dst.size_offset) { + Some(delta) => { + #[allow(clippy::arithmetic_side_effects)] + let remainder = delta % dst_elem_size.get(); + if remainder == 0 { + #[allow(clippy::arithmetic_side_effects)] + let elems = delta / dst_elem_size.get(); + Some(elems) + } else { + None + } + } + None => None, + }; + + let offset_delta_elems = match exact_offset_delta_elems { + Some(elems) if Self::size_sequences_match(src, dst, elems) => elems, + _ => { + let src_zero_size = match src.size_for_elems(0) { + Some(size) => size, + None => return None, + }; + let elems = + match dst_layout.metadata_for_exact_size(src_zero_size) { + Some(elems) => elems, + None => return None, + }; + if !Self::size_sequences_match(src, dst, elems) { + return None; + } + elems + } + }; + CastParamsInner::UnsizedToUnsized { - // SAFETY: We checked above that this is an exact ratio. + // SAFETY: `size_sequences_match` proves that + // this affine metadata map preserves size. offset_delta_elems, - // SAFETY: We checked above that this is an exact ratio. + // SAFETY: We checked above that this is an exact + // ratio of source to destination element size. elem_multiple, } } @@ -1041,15 +1274,58 @@ mod cast_from { unstable_name_collisions, clippy::multiple_unsafe_ops_per_block )] - // SAFETY: `self` is a witness that the following - // equation holds: + // SAFETY: `self` witnesses that this affine map + // makes `Src` and `Dst`'s complete rounded size + // formulas equal for every source metadata value. + // Interpret the arithmetic in this proof over the + // mathematical nonnegative integers. Since the + // caller promises that `src_meta` is valid `Src` + // metadata, // - // D_OFF + d_meta * D_ELEM = S_OFF + s_meta * S_ELEM + // src_meta * S_ELEM + // <= size_of_val(src) + // <= isize::MAX. // - // Since the caller promises that `src_meta` is - // valid `Src` metadata, this math will not - // overflow, and the returned value will describe a + // Since `elem_multiple = S_ELEM / D_ELEM` and + // `D_ELEM >= 1`, + // + // src_meta * elem_multiple + // <= src_meta * S_ELEM + // <= isize::MAX + // <= usize::MAX. + // + // Thus the inner multiplication is neither above + // `usize::MAX` nor below `usize::MIN == 0`, exactly + // ruling out the UB cases documented for + // `usize::unchecked_mul` [1]. Sequence equality + // also gives + // + // (offset_delta_elems + // + src_meta * elem_multiple) * D_ELEM + // <= size_of_val(src). + // + // Since `D_ELEM >= 1`, the sum is at most + // `size_of_val(src)`, and so is at most + // `isize::MAX <= usize::MAX`. Its addends are + // nonnegative, so the sum is not below + // `usize::MIN` either. This exactly rules out the + // UB cases documented for `usize::unchecked_add` + // [2]. The returned metadata therefore describes a // `Dst` of the same size. + // + // [1] Per https://doc.rust-lang.org/1.93.1/std/primitive.usize.html#method.unchecked_mul: + // + // This results in undefined behavior when + // `self * rhs > usize::MAX` or + // `self * rhs < usize::MIN`, i.e. when + // `checked_mul` would return `None`. + // + // [2] Per https://doc.rust-lang.org/1.93.1/std/primitive.usize.html#method.unchecked_add: + // + // This results in undefined behavior when + // `self + rhs > usize::MAX` or + // `self + rhs < usize::MIN`, i.e. when + // `checked_add` would return `None`. unsafe { offset_delta_elems .unchecked_add(src_meta.unchecked_mul(elem_multiple)) @@ -1108,7 +1384,7 @@ mod tests { fn test_dst_layout_for_slice() { let layout = DstLayout::for_slice::(); match layout.size_info { - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) => { + SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size, .. }) => { assert_eq!(offset, 0); assert_eq!(elem_size, 4); } @@ -1192,15 +1468,21 @@ mod tests { let aligns = (0..29).map(|p| NonZeroUsize::new(2usize.pow(p)).unwrap()); let packs = core::iter::once(None).chain(aligns.clone().map(Some)); - for align in aligns { + for field_type_align in aligns { for pack in packs.clone() { let base = DstLayout::for_type::(); let elem_size = 42; let trailing_field_offset = 11; let trailing_field = DstLayout { - align, - size_info: SizeInfo::SliceDst(TrailingSliceLayout { elem_size, offset: 11 }), + align: field_type_align, + size_info: SizeInfo::SliceDst(TrailingSliceLayout { + elem_size, + offset: trailing_field_offset, + size_prefix: 0, + size_offset: trailing_field_offset, + size_align: field_type_align, + }), statically_shallow_unpadded: false, }; @@ -1208,7 +1490,10 @@ mod tests { let max_align = pack.unwrap_or(DstLayout::CURRENT_MAX_ALIGN).get(); - let align = align.get().min(max_align); + let align = field_type_align.get().min(max_align); + let field_offset = align; + let size_prefix = field_offset % field_type_align.get(); + let size_offset = field_offset - size_prefix + trailing_field_offset; assert_eq!( composite, @@ -1216,7 +1501,10 @@ mod tests { align: NonZeroUsize::new(align).unwrap(), size_info: SizeInfo::SliceDst(TrailingSliceLayout { elem_size, - offset: align + trailing_field_offset, + offset: field_offset + trailing_field_offset, + size_prefix, + size_offset, + size_align: field_type_align, }), statically_shallow_unpadded: false, } @@ -1292,7 +1580,8 @@ mod tests { => padded { size: current_max_align * 2, align: current_max_align }); } - /// Tests that calling `pad_to_align` on a DST `DstLayout` is a no-op. + /// Tests that calling `pad_to_align` on a DST `DstLayout` incorporates the + /// type's trailing-padding operation into its runtime size formula. #[test] fn test_dst_layout_pad_to_align_with_dst() { for align in (0..29).map(|p| NonZeroUsize::new(2usize.pow(p)).unwrap()) { @@ -1300,15 +1589,64 @@ mod tests { for elem_size in 0..10 { let layout = DstLayout { align, - size_info: SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }), + size_info: SizeInfo::SliceDst(TrailingSliceLayout { + offset, + elem_size, + size_prefix: 0, + size_offset: offset, + size_align: DstLayout::MIN_ALIGN, + }), statically_shallow_unpadded: false, }; - assert_eq!(layout.pad_to_align(), layout); + assert_eq!( + layout.pad_to_align(), + DstLayout { + align, + size_info: SizeInfo::SliceDst(TrailingSliceLayout { + offset, + elem_size, + size_prefix: 0, + size_offset: offset, + size_align: align, + }), + statically_shallow_unpadded: false, + } + ); } } } } + #[test] + fn test_trailing_slice_layout_size_sequence_equivalence() { + // These are the two normalized descriptions produced by a + // `repr(C, packed(2))` wrapper around slice DSTs whose element types + // both have size four but alignments four and two respectively. Their + // formulas differ structurally, but both simplify to `2 + 4 * elems`. + let align4 = TrailingSliceLayout { + offset: 2, + elem_size: 4, + size_prefix: 2, + size_offset: 0, + size_align: NonZeroUsize::new(4).unwrap(), + }; + let align2 = TrailingSliceLayout { + offset: 2, + elem_size: 4, + size_prefix: 0, + size_offset: 2, + size_align: NonZeroUsize::new(2).unwrap(), + }; + assert!(align4.has_same_size_sequence(align2)); + for elems in 0..8 { + assert_eq!(align4.size_for_elems(elems), Some(2 + 4 * elems)); + assert_eq!(align4.size_for_elems(elems), align2.size_for_elems(elems)); + } + + let dynamically_padded = TrailingSliceLayout { elem_size: 1, ..align4 }; + assert!(!dynamically_padded.has_same_size_sequence(align2)); + } + // This test takes a long time when running under Miri, so we skip it in // that case. This is acceptable because this is a logic test that doesn't // attempt to expose UB. @@ -1325,16 +1663,26 @@ mod tests { #[allow(non_local_definitions)] impl From<(usize, usize)> for SizeInfo { fn from((offset, elem_size): (usize, usize)) -> SizeInfo { - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) + SizeInfo::SliceDst(TrailingSliceLayout { + offset, + elem_size, + size_prefix: 0, + size_offset: offset, + size_align: DstLayout::MIN_ALIGN, + }) } } fn layout>(s: S, align: usize) -> DstLayout { - DstLayout { - size_info: s.into(), - align: NonZeroUsize::new(align).unwrap(), - statically_shallow_unpadded: false, - } + let align = NonZeroUsize::new(align).unwrap(); + let size_info = match s.into() { + SizeInfo::SliceDst(mut trailing) => { + trailing.size_align = align; + SizeInfo::SliceDst(trailing) + } + size_info => size_info, + }; + DstLayout { size_info, align, statically_shallow_unpadded: false } } /// This macro accepts arguments in the form of: @@ -1531,10 +1879,18 @@ mod tests { let resulting_size = match layout.size_info { SizeInfo::Sized { size } => size, - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) => { + SizeInfo::SliceDst(TrailingSliceLayout { + elem_size, + size_prefix, + size_offset, + size_align, + .. + }) => { let padded_size = |elems| { - let without_padding = offset + elems * elem_size; - without_padding + util::padding_needed_for(without_padding, align) + let without_padding = size_offset + elems * elem_size; + size_prefix + + without_padding + + util::padding_needed_for(without_padding, size_align) }; let resulting_size = padded_size(elems); @@ -1562,8 +1918,15 @@ mod tests { } else { let min_size = match layout.size_info { SizeInfo::Sized { size } => size, - SizeInfo::SliceDst(TrailingSliceLayout { offset, .. }) => { - offset + util::padding_needed_for(offset, layout.align) + SizeInfo::SliceDst(TrailingSliceLayout { + size_prefix, + size_offset, + size_align, + .. + }) => { + size_prefix + + size_offset + + util::padding_needed_for(size_offset, size_align) } }; @@ -1592,6 +1955,33 @@ mod tests { .map(|(size_info, align)| layout(size_info, align)); itertools::iproduct!(layouts, 0..8, 0..8, [CastType::Prefix, CastType::Suffix]) .for_each(validate_behavior); + + // Exercise a normalized formula which cannot be represented as + // `round_up(offset + elems * elem_size, align)`: the physical trailing + // slice begins at offset 7, while an inner aligned DST contributes the + // size formula `2 + round_up(5 + elems, 4)` to a packed outer type. + let nested_packed = DstLayout { + align: NonZeroUsize::new(2).unwrap(), + size_info: SizeInfo::SliceDst(TrailingSliceLayout { + offset: 7, + elem_size: 1, + size_prefix: 2, + size_offset: 5, + size_align: NonZeroUsize::new(4).unwrap(), + }), + statically_shallow_unpadded: false, + }; + itertools::iproduct!([nested_packed], 0..8, 0..24, [CastType::Prefix, CastType::Suffix]) + .for_each(validate_behavior); + + assert!(matches!( + nested_packed.validate_cast_and_convert_metadata(0, 8, CastType::Prefix), + Err(MetadataCastError::Size) + )); + assert!(matches!( + nested_packed.validate_cast_and_convert_metadata(0, 10, CastType::Prefix), + Ok((3, 10)) + )); } #[test] @@ -1631,9 +2021,13 @@ mod tests { let dst = args.elem_size.is_some(); let layout = { let size_info = match args.elem_size { - Some(elem_size) => { - SizeInfo::SliceDst(TrailingSliceLayout { offset: args.offset, elem_size }) - } + Some(elem_size) => SizeInfo::SliceDst(TrailingSliceLayout { + offset: args.offset, + elem_size, + size_prefix: 0, + size_offset: args.offset, + size_align: args.align, + }), None => SizeInfo::Sized { // Rust only supports types whose sizes are a multiple // of their alignment. If the macro created a type like @@ -1955,14 +2349,31 @@ mod proofs { use super::*; + fn slice_dst_size_for_trailing_bytes( + layout: TrailingSliceLayout, + trailing_size: usize, + ) -> Option { + let without_padding = layout.size_offset.checked_add(trailing_size)?; + let padding = util::padding_needed_for(without_padding, layout.size_align); + let rounded = without_padding.checked_add(padding)?; + layout.size_prefix.checked_add(rounded) + } + + fn any_layout_align() -> NonZeroUsize { + let exponent: u8 = kani::any(); + kani::assume(usize::from(exponent) < POINTER_WIDTH_BITS - 1); + + // `exponent < POINTER_WIDTH_BITS`, so the shift is in range and its + // result is non-zero. This construction is surjective over precisely + // the power-of-two alignments below `THEORETICAL_MAX_ALIGN`. + NonZeroUsize::new(1usize << exponent).unwrap() + } + impl kani::Arbitrary for DstLayout { fn any() -> Self { - let align: NonZeroUsize = kani::any(); + let align = any_layout_align(); let size_info: SizeInfo = kani::any(); - kani::assume(align.is_power_of_two()); - kani::assume(align < DstLayout::THEORETICAL_MAX_ALIGN); - // For testing purposes, we most care about instantiations of // `DstLayout` that can correspond to actual Rust types. We use // `Layout` to verify that our `DstLayout` satisfies the validity @@ -1970,10 +2381,27 @@ mod proofs { kani::assume( match size_info { SizeInfo::Sized { size } => Layout::from_size_align(size, align.get()), - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size: _ }) => { - // `SliceDst` cannot encode an exact size, but we know - // it is at least `offset` bytes. - Layout::from_size_align(offset, align.get()) + SizeInfo::SliceDst(trailing) => { + // `SliceDst` cannot encode one exact size. Validate its + // minimum size and the invariants required of the size + // formula for layouts of real Rust types. + let padding = + util::padding_needed_for(trailing.size_offset, trailing.size_align); + let rounded = trailing.size_offset.checked_add(padding); + let min_size = + rounded.and_then(|size| trailing.size_prefix.checked_add(size)); + let unrounded_min_size = + trailing.size_prefix.checked_add(trailing.size_offset); + + kani::assume(matches!( + unrounded_min_size, + Some(size) if size >= trailing.offset + )); + + match min_size { + Some(min_size) => Layout::from_size_align(min_size, align.get()), + None => Layout::from_size_align(usize::MAX, align.get()), + } } } .is_ok(), @@ -2004,11 +2432,19 @@ mod proofs { fn any() -> Self { let elem_size: usize = kani::any(); let offset: usize = kani::any(); + let size_offset: usize = kani::any(); + let size_align = any_layout_align(); + let raw_size_prefix: usize = kani::any(); + // Since `size_align` is a power of two, masking is surjective over + // precisely the values in `0..size_align`. + #[allow(clippy::arithmetic_side_effects)] + let size_prefix = raw_size_prefix & (size_align.get() - 1); kani::assume(elem_size < DstLayout::MAX_SIZE); kani::assume(offset < DstLayout::MAX_SIZE); + kani::assume(size_offset < DstLayout::MAX_SIZE); - TrailingSliceLayout { elem_size, offset } + TrailingSliceLayout { elem_size, offset, size_prefix, size_offset, size_align } } } @@ -2029,22 +2465,34 @@ mod proofs { loop {} }; - let Some(unpadded_size) = size_info.offset.checked_add(trailing_slice_size) else { - // The `unpadded_size` exceeds `usize::MAX`; `meta`` is invalid. + let Some(trailing_slice_end) = size_info.offset.checked_add(trailing_slice_size) else { + // The trailing slice end exceeds `usize::MAX`; `meta` is invalid. kani::assume(false); loop {} }; - if unpadded_size >= DstLayout::MAX_SIZE { - // The `unpadded_size` exceeds `isize::MAX`; `meta` is invalid. + let Some(without_padding) = size_info.size_offset.checked_add(trailing_slice_size) else { kani::assume(false); loop {} - } + }; + let inner_padding = util::padding_needed_for(without_padding, size_info.size_align); + let Some(rounded_size) = without_padding.checked_add(inner_padding) else { + kani::assume(false); + loop {} + }; + let Some(size) = size_info.size_prefix.checked_add(rounded_size) else { + kani::assume(false); + loop {} + }; - let trailing_padding = util::padding_needed_for(unpadded_size, layout.align); + if size > DstLayout::MAX_SIZE || trailing_slice_end > size { + // This metadata cannot describe a valid instance. + kani::assume(false); + loop {} + } if !layout.requires_dynamic_padding() { - assert!(trailing_padding == 0); + assert_eq!(trailing_slice_end, size); } } @@ -2069,9 +2517,6 @@ mod proofs { unreachable!(); }; - // Under the above conditions, `DstLayout::extend` will not panic. - let composite = base.extend(field, packed); - // The field's alignment is clamped by `max_align` (i.e., the // `packed` attribute, if any) [1]. // @@ -2082,10 +2527,6 @@ mod proofs { // alignment of the field's type. let field_align = min(field.align, packed.unwrap_or(DstLayout::THEORETICAL_MAX_ALIGN)); - // The struct's alignment is the maximum of its previous alignment and - // `field_align`. - assert_eq!(composite.align, max(base.align, field_align)); - // Compute the minimum amount of inter-field padding needed to // satisfy the field's alignment, and offset of the trailing field. // [1] @@ -2095,7 +2536,36 @@ mod proofs { // Inter-field padding is guaranteed to be the minimum required in // order to satisfy each field's (possibly altered) alignment. let padding = padding_needed_for(base_size, field_align); - let offset = base_size + padding; + let offset = match base_size.checked_add(padding) { + Some(offset) => offset, + None => { + kani::assume(false); + loop {} + } + }; + + // Restrict the proof to fragments whose extension can correspond to a + // valid Rust layout. Outside this domain, `extend` is permitted to + // panic. + match field.size_info { + SizeInfo::Sized { size } => kani::assume(offset.checked_add(size).is_some()), + SizeInfo::SliceDst(trailing) => { + kani::assume(offset.checked_add(trailing.offset).is_some()); + let unnormalized_prefix = offset.checked_add(trailing.size_prefix); + kani::assume(unnormalized_prefix.is_some()); + let unnormalized_prefix = unnormalized_prefix.unwrap(); + let size_prefix = unnormalized_prefix % trailing.size_align.get(); + let prefix_multiple = unnormalized_prefix - size_prefix; + kani::assume(prefix_multiple.checked_add(trailing.size_offset).is_some()); + } + } + + // Under the above conditions, `DstLayout::extend` will not panic. + let composite = base.extend(field, packed); + + // The struct's alignment is the maximum of its previous alignment and + // `field_align`. + assert_eq!(composite.align, max(base.align, field_align)); // For testing purposes, we'll also construct `alloc::Layout` // stand-ins for `DstLayout`, and show that `extend` behaves @@ -2132,29 +2602,52 @@ mod proofs { panic!("The composite of two sized layouts must be sized.") } } - SizeInfo::SliceDst(TrailingSliceLayout { - offset: field_offset, - elem_size: field_elem_size, - }) => { - if let SizeInfo::SliceDst(TrailingSliceLayout { - offset: composite_offset, - elem_size: composite_elem_size, - }) = composite.size_info - { + SizeInfo::SliceDst(field_trailing) => { + if let SizeInfo::SliceDst(composite_trailing) = composite.size_info { // The offset of the trailing slice component is the sum // of the offset of the trailing field and the trailing // slice offset within that field. - assert_eq!(composite_offset, offset + field_offset); + assert_eq!(composite_trailing.offset, offset + field_trailing.offset); // The elem size is unchanged. - assert_eq!(composite_elem_size, field_elem_size); + assert_eq!(composite_trailing.elem_size, field_trailing.elem_size); + + // The normalized formula must continue to describe the + // size of the field at every trailing byte contribution + // for which the checked size computation succeeds. This + // is stronger than considering only products of a slice + // length and `elem_size`, and avoids a nonlinear + // multiplication in the verifier. + let trailing_size: usize = kani::any(); + let field_size = + match slice_dst_size_for_trailing_bytes(field_trailing, trailing_size) { + Some(size) => size, + None => { + kani::assume(false); + loop {} + } + }; + let expected_size = match offset.checked_add(field_size) { + Some(size) => size, + None => { + kani::assume(false); + loop {} + } + }; + assert_eq!( + slice_dst_size_for_trailing_bytes(composite_trailing, trailing_size), + Some(expected_size) + ); + // `alloc::Layout` permits sizes which are not a multiple + // of alignment, so this represents the unpadded prefix of + // the DST through the start of its trailing slice. let field_analog = - Layout::from_size_align(field_offset, field_align.get()).unwrap(); + Layout::from_size_align(field_trailing.offset, field_align.get()).unwrap(); if let Ok((actual_composite, actual_offset)) = base_analog.extend(field_analog) { assert_eq!(actual_offset, offset); - assert_eq!(actual_composite.size(), composite_offset); + assert_eq!(actual_composite.size(), composite_trailing.offset); assert_eq!(actual_composite.align(), composite.align.get()); } else { // An error here reflects that composite of `base` @@ -2195,6 +2688,24 @@ mod proofs { let layout: DstLayout = kani::any(); + if let SizeInfo::SliceDst(trailing) = layout.size_info { + // Restrict the proof to fragments whose padded layout can + // correspond to a valid Rust type. Outside this domain, + // `pad_to_align` is permitted to panic. + if trailing.size_align.get() < layout.align.get() { + let prefix = if trailing.size_prefix == 0 { 0 } else { trailing.size_align.get() }; + kani::assume(trailing.size_offset.checked_add(prefix).is_some()); + } else { + let padding = padding_needed_for(trailing.size_prefix, layout.align); + let rounded_prefix = trailing.size_prefix.checked_add(padding); + kani::assume(rounded_prefix.is_some()); + let rounded_prefix = rounded_prefix.unwrap(); + let size_prefix = rounded_prefix % trailing.size_align.get(); + let prefix_multiple = rounded_prefix - size_prefix; + kani::assume(trailing.size_offset.checked_add(prefix_multiple).is_some()); + } + } + let padded = layout.pad_to_align(); // Calling `pad_to_align` does not alter the `DstLayout`'s alignment. @@ -2219,9 +2730,40 @@ mod proofs { } else { panic!("The padding of a sized layout must result in a sized layout.") } + } else if let (SizeInfo::SliceDst(unpadded_trailing), SizeInfo::SliceDst(padded_trailing)) = + (layout.size_info, padded.size_info) + { + assert_eq!(padded_trailing.offset, unpadded_trailing.offset); + assert_eq!(padded_trailing.elem_size, unpadded_trailing.elem_size); + + // Prove the normalization for every trailing byte contribution + // for which the checked size computation succeeds. This is + // stronger than considering only products of a slice length and + // `elem_size`, and avoids a nonlinear multiplication in the + // verifier. + let trailing_size: usize = kani::any(); + let unpadded_size = + match slice_dst_size_for_trailing_bytes(unpadded_trailing, trailing_size) { + Some(size) => size, + None => { + kani::assume(false); + loop {} + } + }; + let trailing_padding = padding_needed_for(unpadded_size, layout.align); + let expected_size = match unpadded_size.checked_add(trailing_padding) { + Some(size) => size, + None => { + kani::assume(false); + loop {} + } + }; + assert_eq!( + slice_dst_size_for_trailing_bytes(padded_trailing, trailing_size), + Some(expected_size) + ); } else { - // If the layout is a DST, padding cannot be statically added. - assert_eq!(padded.size_info, layout.size_info); + panic!("The padding of a DST layout must result in a DST layout.") } } } diff --git a/zerocopy/src/lib.rs b/zerocopy/src/lib.rs index b5ecbb58a8..ffc07dfd27 100644 --- a/zerocopy/src/lib.rs +++ b/zerocopy/src/lib.rs @@ -786,10 +786,15 @@ pub unsafe trait KnownLayout { /// where `size == size_of::()` /// - If `Self` is a slice DST, then `LAYOUT.size_info == /// SizeInfo::SliceDst(slice_layout)` where: + /// - `slice_layout.size_align` is a power of two + /// - `slice_layout.size_prefix < slice_layout.size_align` /// - The size, `size`, of an instance of `Self` with `elems` trailing - /// slice elements is equal to `slice_layout.offset + - /// slice_layout.elem_size * elems` rounded up to the nearest multiple - /// of `LAYOUT.align` + /// slice elements is equal to `slice_layout.size_prefix + + /// round_up(slice_layout.size_offset + slice_layout.elem_size * elems, + /// slice_layout.size_align)` + /// - The trailing slice begins at byte offset `slice_layout.offset` + /// - `slice_layout.offset + slice_layout.elem_size * elems` does not + /// overflow and is no greater than `size` /// - For such an instance, any bytes in the range `[slice_layout.offset + /// slice_layout.elem_size * elems, size)` are padding and must not be /// assumed to be initialized @@ -1004,11 +1009,7 @@ impl PointerMetadata for usize { #[inline] fn size_for_metadata(self, layout: DstLayout) -> Option { match layout.size_info { - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) => { - let slice_len = elem_size.checked_mul(self)?; - let without_padding = offset.checked_add(slice_len)?; - without_padding.checked_add(util::padding_needed_for(without_padding, layout.align)) - } + SizeInfo::SliceDst(trailing) => trailing.size_for_elems(self), // NOTE: This branch is unreachable, but we return `None` rather // than `unreachable!()` to avoid generating panic paths. SizeInfo::Sized { .. } => None, @@ -6497,6 +6498,33 @@ mod tests { } } + // These types exercise a nested slice DST whose inner trailing-padding + // operation cannot be flattened into the alignment of the packed outer + // type. For `NestedPackedDstOuter<[u8]>`, the trailing slice begins at + // offset 7, but the size for `elems` elements is + // `2 + round_up(5 + elems, 4)`. + #[derive(FromBytes, Immutable, KnownLayout)] + #[repr(C, align(4))] + struct NestedPackedDstWord([u8; 4]); + + #[derive(FromBytes, Immutable, KnownLayout)] + #[repr(C)] + struct NestedPackedDstInner { + a: NestedPackedDstWord, + b: u8, + tail: T, + } + + #[derive(FromBytes, Immutable, KnownLayout)] + #[repr(C, packed(2))] + struct NestedPackedDstOuter { + z: u8, + inner: ManuallyDrop>, + } + + #[repr(C, align(2))] + struct Align2(T); + #[test] fn test_known_layout() { // Test that `$ty` and `ManuallyDrop<$ty>` have the expected layout. @@ -6516,9 +6544,13 @@ mod tests { align: NonZeroUsize::new(align).unwrap(), size_info: match trailing_slice_elem_size { None => SizeInfo::Sized { size: offset }, - Some(elem_size) => { - SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }) - } + Some(elem_size) => SizeInfo::SliceDst(TrailingSliceLayout { + offset, + elem_size, + size_prefix: 0, + size_offset: offset, + size_align: NonZeroUsize::new(align).unwrap(), + }), }, statically_shallow_unpadded, }; @@ -6537,6 +6569,113 @@ mod tests { test!(str, layout(0, 1, Some(1), true)); } + #[test] + fn test_nested_packed_dst_layout() { + fn assert_native_size(expected: usize) { + let value = NestedPackedDstOuter { + z: 0, + inner: ManuallyDrop::new(NestedPackedDstInner { + a: NestedPackedDstWord([0; 4]), + b: 0, + tail: [0; N], + }), + }; + let value: &NestedPackedDstOuter<[u8]> = &value; + assert_eq!(mem::size_of_val(value), expected); + assert_eq!(NestedPackedDstOuter::<[u8]>::size_for_metadata(N), Some(expected)); + } + + assert_native_size::<0>(10); + assert_native_size::<1>(10); + assert_native_size::<2>(10); + assert_native_size::<3>(10); + assert_native_size::<4>(14); + assert_native_size::<7>(14); + assert_native_size::<8>(18); + + let layout = NestedPackedDstOuter::<[u8]>::LAYOUT; + assert_eq!(layout.align.get(), 2); + assert!(layout.requires_dynamic_padding()); + assert_eq!( + layout.size_info, + SizeInfo::SliceDst(TrailingSliceLayout { + offset: 7, + elem_size: 1, + size_prefix: 2, + size_offset: 5, + size_align: NonZeroUsize::new(4).unwrap(), + }) + ); + + // Eight bytes used to be incorrectly accepted for a zero-element + // value. The actual Rust layout requires ten bytes. + let short = Align2([0u8; 8]); + assert!(NestedPackedDstOuter::<[u8]>::ref_from_bytes_with_elems(&short.0, 0).is_err()); + assert!(pointer::Ptr::from_ref(&short.0[..]) + .try_cast_into::, pointer::BecauseImmutable>( + CastType::Prefix, + Some(0), + ) + .is_err()); + + let full = Align2([0, 0, 0, 0, 0, 0, 0, 0, b'S', b'!']); + let value = NestedPackedDstOuter::<[u8]>::ref_from_bytes_with_elems(&full.0, 0).unwrap(); + assert_eq!(mem::size_of_val(value), 10); + + // When metadata is inferred, lengths 0 through 3 all produce a + // ten-byte object. Parsing chooses the largest trailing-slice length + // which fits exactly. + let inferred = NestedPackedDstOuter::<[u8]>::ref_from_bytes(&full.0).unwrap(); + assert_eq!(pointer::Ptr::from_ref(inferred).len(), 3); + + // `MetadataOf::padding_needed_for` is consumed by `SplitAt` to decide + // whether two mutable projections would overlap. It must include the + // inner type's trailing padding, not just padding to the outer packed + // alignment. + for (elems, expected) in [(0, 3), (1, 2), (2, 1), (3, 0), (4, 3)] { + // SAFETY: The `assert_native_size` calls above construct an + // instance for every metadata value used by this loop and + // establish that its size is at most 14 bytes. The Reference + // states that the upper bound on object size is `isize::MAX` [1]. + // Thus the 14-byte maximum does not exceed `isize::MAX`, exactly + // satisfying `MetadataOf::new_unchecked`'s precondition. + // + // [1] Per https://doc.rust-lang.org/1.92.0/reference/types/numeric.html#machine-dependent-integer-types: + // + // The theoretical upper bound on object and array size is the + // maximum `isize` value. + let meta = + unsafe { util::MetadataOf::>::new_unchecked(elems) }; + assert_eq!(meta.padding_needed_for(), expected); + } + } + + // Before Rust 1.76, rust-lang/rust#118537 caused `addr_of_mut!` to + // miscompute the offset of an unsized field in a packed struct. The fix is + // rust-lang/rust#118540. Keep the layout and parsing regression above on + // all supported compilers, and gate only the projection regression which + // necessarily exercises that independent compiler bug. + #[rustversion::since(1.76.0)] + #[test] + fn test_nested_packed_dst_projection() { + let full = Align2([0, 0, 0, 0, 0, 0, 0, 0, b'S', b'!']); + + // The projected inner value occupies the final eight bytes of the full + // ten-byte input, including the two sentinel bytes. + let (value, rest) = pointer::Ptr::from_ref(&full.0[..]) + .try_cast_into::, pointer::BecauseImmutable>( + CastType::Prefix, + Some(0), + ) + .unwrap(); + assert!(rest.as_ref().is_empty()); + let inner = value.project::<_, { STRUCT_VARIANT_ID }, { ident_id!(inner) }>().unwrap(); + assert_eq!( + inner.as_bytes::().as_ref(), + &[0, 0, 0, 0, 0, 0, b'S', b'!'] + ); + } + #[cfg(feature = "derive")] #[test] fn test_known_layout_derive() { @@ -6594,7 +6733,13 @@ mod tests { let unsized_layout = |align, elem_size, offset, statically_shallow_unpadded| DstLayout { align: NonZeroUsize::new(align).unwrap(), - size_info: SizeInfo::SliceDst(TrailingSliceLayout { offset, elem_size }), + size_info: SizeInfo::SliceDst(TrailingSliceLayout { + offset, + elem_size, + size_prefix: 0, + size_offset: offset, + size_align: NonZeroUsize::new(align).unwrap(), + }), statically_shallow_unpadded, }; @@ -7506,6 +7651,16 @@ mod tests { assert_eq!(&*s, &[0, 3, 0]); } + #[test] + fn test_new_box_zeroed_with_elems_nested_packed_dst() { + let mut value = NestedPackedDstOuter::<[u8]>::new_box_zeroed_with_elems(0).unwrap(); + assert_eq!(mem::size_of_val(&*value), 10); + + // Exercise the whole-object write that exposed the former + // under-allocation under Miri and AddressSanitizer. + value.zero(); + } + #[test] fn test_new_box_zeroed_with_elems_empty() { let s: Box<[u64]> = <[u64]>::new_box_zeroed_with_elems(0).unwrap(); diff --git a/zerocopy/src/macros.rs b/zerocopy/src/macros.rs index bae08d19cc..e194823f21 100644 --- a/zerocopy/src/macros.rs +++ b/zerocopy/src/macros.rs @@ -1411,6 +1411,38 @@ mod tests { let x: &SliceDst = transmute_ref!(slice_dst_big); assert_eq!(x, slice_dst_small); + // A packed outer DST can preserve the trailing field's stronger + // rounding alignment. These two types consequently have different + // normalized layout formulas even though both have size `2 + 4 * n` + // for every trailing slice length. Ensure the semantic size-sequence + // comparison accepts this representation-preserving transmutation. + #[derive(FromBytes, Immutable, IntoBytes, KnownLayout)] + #[repr(C, align(2))] + struct TransmuteAlign2([u8; 4]); + + #[derive(FromBytes, Immutable, IntoBytes, KnownLayout)] + #[repr(C, packed(2))] + struct TransmutePacked4 { + head: u16, + tail: [u32], + } + + #[derive(FromBytes, Immutable, IntoBytes, KnownLayout)] + #[repr(C, packed(2))] + struct TransmutePacked2 { + head: u16, + tail: [TransmuteAlign2], + } + + #[repr(C, align(2))] + struct TransmuteBytes([u8; 10]); + + let bytes = TransmuteBytes([0; 10]); + let src = TransmutePacked4::ref_from_bytes(&bytes.0).unwrap(); + let dst: &TransmutePacked2 = transmute_ref!(src); + assert_eq!(mem::size_of_val(src), 10); + assert_eq!(mem::size_of_val(dst), 10); + // Test that it's legal to transmute a reference while shrinking the // lifetime (note that `X` has the lifetime `'static`). let x: &[u8; 8] = transmute_ref!(X); diff --git a/zerocopy/src/pointer/mod.rs b/zerocopy/src/pointer/mod.rs index bdb283fdac..cf96bf58fb 100644 --- a/zerocopy/src/pointer/mod.rs +++ b/zerocopy/src/pointer/mod.rs @@ -47,10 +47,7 @@ where pub mod cast { use core::{marker::PhantomData, mem}; - use crate::{ - layout::{SizeInfo, TrailingSliceLayout}, - HasField, KnownLayout, PtrInner, - }; + use crate::{layout::SizeInfo, HasField, KnownLayout, PtrInner}; /// A pointer cast or projection. /// @@ -182,11 +179,9 @@ pub mod cast { // SAFETY: By the `static_assert!`, `Src` and `Dst` are either: // - Both sized and equal in size - // - Both slice DSTs with the same trailing slice offset and element size - // and with align_of::() == align_of::(). These ensure that any - // given pointer metadata encodes the same size for both `Src` and `Dst` - // (note that the alignment is required as it affects the amount of - // trailing padding). Thus, `project` preserves the set of referent bytes. + // - Both slice DSTs with the same trailing-slice offset, the same object + // size for every metadata value, and with align_of::() == + // align_of::(). Thus, `project` preserves the set of referent bytes. unsafe impl Project for CastUnsized where Src: ?Sized + KnownLayout, @@ -201,10 +196,11 @@ pub mod cast { let dst = ::LAYOUT; match (src.size_info, dst.size_info) { (SizeInfo::Sized { size: src_size }, SizeInfo::Sized { size: dst_size }) => src_size == dst_size, - ( - SizeInfo::SliceDst(TrailingSliceLayout { offset: src_offset, elem_size: src_elem_size }), - SizeInfo::SliceDst(TrailingSliceLayout { offset: dst_offset, elem_size: dst_elem_size }) - ) => src.align.get() == dst.align.get() && src_offset == dst_offset && src_elem_size == dst_elem_size, + (SizeInfo::SliceDst(src_trailing), SizeInfo::SliceDst(dst_trailing)) => { + src.align.get() == dst.align.get() + && src_trailing.offset == dst_trailing.offset + && src_trailing.has_same_size_sequence(dst_trailing) + }, _ => false, } }); @@ -225,10 +221,8 @@ pub mod cast { // SAFETY: By the `static_assert!` in `Project::project`, `Src` and `Dst` // are either: // - Both sized and equal in size - // - Both slice DSTs with the same alignment, trailing slice offset, and - // element size. These ensure that any given pointer metadata encodes the - // same size for both `Src` and `Dst` (note that the alignment is required - // as it affects the amount of trailing padding). + // - Both slice DSTs with the same alignment and trailing-slice offset, and + // with equal sizes for every metadata value. unsafe impl CastExact for CastUnsized where Src: ?Sized + KnownLayout, diff --git a/zerocopy/src/split_at.rs b/zerocopy/src/split_at.rs index 1695a0d740..21fa70765a 100644 --- a/zerocopy/src/split_at.rs +++ b/zerocopy/src/split_at.rs @@ -188,10 +188,7 @@ pub unsafe trait SplitAt: KnownLayout { /// Attempts to split `self` in two. /// /// Returns `None` if `l_len` is greater than the length of `self`'s - /// trailing slice, or if the given `l_len` would result in [the trailing - /// padding](KnownLayout#slice-dst-layout) of the left portion overlapping - /// the right portion. - /// + /// trailing slice. /// /// # Examples /// @@ -270,10 +267,12 @@ unsafe impl SplitAt for [T] { /// requires trailing padding, the trailing padding of the left part of the /// split `T` will overlap the right part. If `T` is a mutable reference or /// permits interior mutation, you must ensure that the left and right parts do -/// not overlap. You can do this at zero-cost using using -/// [`Self::via_immutable`], [`Self::via_into_bytes`], or -/// [`Self::via_unaligned`], or with a dynamic check by using -/// [`Self::via_runtime_check`]. +/// not overlap. You can do this without a runtime check using +/// [`Self::via_immutable`] or [`Self::via_into_bytes`]. When the layout never +/// requires dynamic trailing padding, you can also use +/// [`Self::via_unaligned`] if the trailing element type is [`Unaligned`], or +/// [`Self::via_no_dynamic_padding`] without that bound. Otherwise, use the +/// dynamic check in [`Self::via_runtime_check`]. #[derive(Debug)] pub struct Split { /// A pointer to the source slice DST. @@ -423,8 +422,16 @@ where (l.as_ref(), r.as_ref()) } - /// Produces the split parts of `self`, using [`Unaligned`] to ensure that - /// it is sound to have concurrent references to both parts. + /// Produces the split parts of `self`, requiring the trailing slice's + /// element type to be [`Unaligned`]. + /// + /// This bound makes the method available to generic code that can name the + /// trailing element's alignment. At monomorphization, this method also + /// verifies that `T`'s layout never requires dynamic trailing padding. + /// This verification performs no runtime check. Use + /// [`Self::via_no_dynamic_padding`] when no [`Unaligned`] bound is + /// available, or [`Self::via_runtime_check`] for dynamically padded + /// layouts. /// /// # Examples /// @@ -432,7 +439,7 @@ where /// use zerocopy::{SplitAt, FromBytes}; /// # use zerocopy_derive::*; /// - /// #[derive(SplitAt, FromBytes, KnownLayout, Immutable, Unaligned)] + /// #[derive(SplitAt, FromBytes, KnownLayout, Immutable)] /// #[repr(C)] /// struct Packet { /// length: u8, @@ -450,8 +457,8 @@ where /// // Attempt to split `packet` at `length`. /// let split = packet.split_at(packet.length as usize).unwrap(); /// - /// // Use the `Unaligned` bound on `Packet` to prove that it's okay to - /// // return concurrent references to `packet` and `rest`. + /// // Use the `Unaligned` bound on `Packet`'s trailing element type to make + /// // this no-runtime-check method available. /// let (packet, rest) = split.via_unaligned(); /// /// assert_eq!(packet.length, 4); @@ -459,6 +466,26 @@ where /// assert_eq!(rest, [5, 6, 7, 8, 9]); /// ``` /// + /// The [`Unaligned`] bound on the trailing element does not by itself + /// prove that the full layout has no dynamic trailing padding. Such a + /// layout is rejected at monomorphization: + /// + /// ```compile_fail,E0080 + /// use zerocopy::{FromBytes, Immutable, KnownLayout, SplitAt}; + /// # use zerocopy_derive::*; + /// + /// #[derive(FromBytes, Immutable, KnownLayout, SplitAt)] + /// #[repr(C, align(2))] + /// struct DynamicallyPadded { + /// prefix: u8, + /// trailing: [u8], + /// } + /// + /// let bytes = [0u8; 4]; + /// let value = DynamicallyPadded::ref_from_bytes(&bytes).unwrap(); + /// let _ = value.split_at(1).unwrap().via_unaligned(); + /// ``` + /// #[doc = codegen_header!("h5", "split_via_unaligned")] /// /// See [`Split::via_immutable`](#method.split_via_immutable.codegen). @@ -466,16 +493,71 @@ where #[inline(always)] pub fn via_unaligned(self) -> (&'a T, &'a [T::Elem]) where - T: Unaligned, + T::Elem: Unaligned, { let (l, r) = self.into_ptr().via_unaligned(); (l.as_ref(), r.as_ref()) } + /// Produces the split parts of `self` when `T`'s layout never requires + /// dynamic trailing padding. + /// + /// This method performs no runtime check and requires no trait bound that + /// expresses this layout property. Instead, it verifies the property at + /// monomorphization and produces a post-monomorphization error when the + /// concrete `T` may require dynamic trailing padding. Generic code that + /// can name an alignment bound may prefer [`Self::via_unaligned`]; + /// dynamically padded layouts must use [`Self::via_runtime_check`]. This + /// method's check applies to every valid metadata value for `T`; in + /// contrast, [`Self::via_runtime_check`] can accept a particular split of + /// a type whose other metadata values require trailing padding. + /// + /// # Examples + /// + /// ``` + /// use zerocopy::SplitAt; + /// + /// let words = [1u16, 2, 3, 4]; + /// let split = SplitAt::split_at(&words[..], 2).unwrap(); + /// let (left, right) = split.via_no_dynamic_padding(); + /// assert_eq!(left, [1, 2]); + /// assert_eq!(right, [3, 4]); + /// ``` + /// + /// A layout that can require dynamic trailing padding is rejected at + /// monomorphization: + /// + /// ```compile_fail,E0080 + /// use zerocopy::{FromBytes, Immutable, KnownLayout, SplitAt}; + /// # use zerocopy_derive::*; + /// + /// #[derive(FromBytes, Immutable, KnownLayout, SplitAt)] + /// #[repr(C, align(2))] + /// struct DynamicallyPadded { + /// prefix: u8, + /// trailing: [u8], + /// } + /// + /// let bytes = [0u8; 4]; + /// let value = DynamicallyPadded::ref_from_bytes(&bytes).unwrap(); + /// let _ = value.split_at(1).unwrap().via_no_dynamic_padding(); + /// ``` + /// + #[doc = codegen_header!("h5", "split_via_no_dynamic_padding")] + /// + /// See [`Split::via_immutable`](#method.split_via_immutable.codegen). + #[must_use = "has no side effects"] + #[inline(always)] + pub fn via_no_dynamic_padding(self) -> (&'a T, &'a [T::Elem]) { + let (l, r) = self.into_ptr().via_no_dynamic_padding(); + (l.as_ref(), r.as_ref()) + } + /// Produces the split parts of `self`, using a dynamic check to ensure that /// it is sound to have concurrent references to both parts. You should - /// prefer using [`Self::via_immutable`], [`Self::via_into_bytes`], or - /// [`Self::via_unaligned`], which have no runtime cost. + /// prefer using [`Self::via_immutable`], [`Self::via_into_bytes`], + /// [`Self::via_unaligned`], or [`Self::via_no_dynamic_padding`], which + /// perform no runtime check. /// /// Note that this check is overly conservative if `T` is [`Immutable`]; for /// some types, this check will reject some splits which @@ -665,8 +747,16 @@ where (l.as_mut(), r.as_mut()) } - /// Produces the split parts of `self`, using [`Unaligned`] to ensure that - /// it is sound to have concurrent references to both parts. + /// Produces the split parts of `self`, requiring the trailing slice's + /// element type to be [`Unaligned`]. + /// + /// This bound makes the method available to generic code that can name the + /// trailing element's alignment. At monomorphization, this method also + /// verifies that `T`'s layout never requires dynamic trailing padding. + /// This verification performs no runtime check. Use + /// [`Self::via_no_dynamic_padding`] when no [`Unaligned`] bound is + /// available, or [`Self::via_runtime_check`] for dynamically padded + /// layouts. /// /// # Examples /// @@ -674,7 +764,7 @@ where /// use zerocopy::{SplitAt, FromBytes}; /// # use zerocopy_derive::*; /// - /// #[derive(SplitAt, FromBytes, KnownLayout, IntoBytes, Unaligned)] + /// #[derive(SplitAt, FromBytes, KnownLayout, IntoBytes)] /// #[repr(C)] /// struct Packet { /// length: u8, @@ -693,8 +783,8 @@ where /// // Attempt to split `packet` at `length`. /// let split = packet.split_at_mut(packet.length as usize).unwrap(); /// - /// // Use the `Unaligned` bound on `Packet` to prove that it's okay to - /// // return concurrent references to `packet` and `rest`. + /// // Use the `Unaligned` bound on `Packet`'s trailing element type to + /// // make this no-runtime-check method available. /// let (packet, rest) = split.via_unaligned(); /// /// assert_eq!(packet.length, 4); @@ -715,16 +805,52 @@ where #[inline(always)] pub fn via_unaligned(self) -> (&'a mut T, &'a mut [T::Elem]) where - T: Unaligned, + T::Elem: Unaligned, { let (l, r) = self.into_ptr().via_unaligned(); (l.as_mut(), r.as_mut()) } + /// Produces the split parts of `self` when `T`'s layout never requires + /// dynamic trailing padding. + /// + /// This is the mutable counterpart to + /// [`Split::via_no_dynamic_padding`] for shared references. It performs no + /// runtime check and verifies the layout property at monomorphization. + /// Generic code that can name an alignment bound may prefer + /// [`Self::via_unaligned`]; dynamically padded layouts must use + /// [`Self::via_runtime_check`]. This method's check applies to every valid + /// metadata value for `T`; in contrast, [`Self::via_runtime_check`] can + /// accept a particular split of a type whose other metadata values require + /// trailing padding. + /// + /// # Examples + /// + /// ``` + /// use zerocopy::SplitAt; + /// + /// let mut words = [1u16, 2, 3, 4]; + /// let split = SplitAt::split_at_mut(&mut words[..], 2).unwrap(); + /// let (left, right) = split.via_no_dynamic_padding(); + /// left[0] = 5; + /// right[0] = 6; + /// assert_eq!(words, [5, 2, 6, 4]); + /// ``` + /// + /// # Code Generation + /// + /// See [`Split::via_immutable`](#method.split_via_immutable.codegen). + #[must_use = "has no side effects"] + #[inline(always)] + pub fn via_no_dynamic_padding(self) -> (&'a mut T, &'a mut [T::Elem]) { + let (l, r) = self.into_ptr().via_no_dynamic_padding(); + (l.as_mut(), r.as_mut()) + } + /// Produces the split parts of `self`, using a dynamic check to ensure that /// it is sound to have concurrent references to both parts. You should - /// prefer using [`Self::via_into_bytes`] or [`Self::via_unaligned`], which - /// have no runtime cost. + /// prefer using [`Self::via_into_bytes`], [`Self::via_unaligned`], or + /// [`Self::via_no_dynamic_padding`], which perform no runtime check. /// /// # Examples /// @@ -860,26 +986,55 @@ where unsafe { self.via_unchecked() } } - /// Produces the split parts of `self`, using [`Unaligned`] to ensure that - /// it is sound to have concurrent references to both parts. + /// Produces the split parts of `self`, requiring the trailing slice's + /// element type to be [`Unaligned`]. #[inline(always)] fn via_unaligned(self) -> (Ptr<'a, T, I>, Ptr<'a, [T::Elem], I>) where - T: Unaligned, + T::Elem: Unaligned, { - // SAFETY: By `T: SplitAt + Unaligned`, `T` is either a slice or a - // `repr(C)` or `repr(transparent)` slice DST that is well-aligned at - // any address and length. If `T` is a slice DST with alignment 1, - // `repr(C)` or `repr(transparent)` ensures that no padding is placed - // after the final element of the trailing slice. Consequently, `T` can - // be split into strictly non-overlapping parts any any index. + self.via_no_dynamic_padding() + } + + /// Produces the split parts of `self` when `T`'s layout never requires + /// dynamic trailing padding. + #[inline(always)] + fn via_no_dynamic_padding(self) -> (Ptr<'a, T, I>, Ptr<'a, [T::Elem], I>) { + static_assert!( + T: ?Sized + KnownLayout => !T::LAYOUT.requires_dynamic_padding(), + "`Split::via_unaligned` and `Split::via_no_dynamic_padding` cannot be used with a type whose layout may require dynamic trailing padding; use `Split::via_runtime_check` instead" + ); + + // SAFETY: `requires_dynamic_padding` returns `true` iff some valid + // metadata requires trailing padding. The assertion therefore proves + // that `self.l_len()`, which is valid metadata by `Split`'s invariant, + // has `padding_needed_for() == 0`. By the postcondition of + // `PtrInner::split_at_unchecked`, the two returned referents are then + // contiguous and non-overlapping. Since the source already conforms to + // `I::Aliasing`, splitting it into disjoint byte ranges preserves that + // invariant: under `Exclusive`, neither result aliases or accesses the + // other's bytes; under `Shared`, interior mutation through one result + // cannot mutate bytes pointed to by the other. This satisfies Rust's + // reference aliasing rules [1]. + // + // [1] Per https://doc.rust-lang.org/1.93.1/std/ptr/index.html#pointer-to-reference-conversion: + // + // When creating a mutable reference, then while this reference + // exists, the memory it points to must not get accessed (read or + // written) through any other pointer or reference not derived from + // this reference. + // + // When creating a shared reference, then while this reference + // exists, the memory it points to must not get mutated (except inside + // `UnsafeCell`). unsafe { self.via_unchecked() } } /// Produces the split parts of `self`, using a dynamic check to ensure that /// it is sound to have concurrent references to both parts. You should - /// prefer using [`Self::via_immutable`], [`Self::via_into_bytes`], or - /// [`Self::via_unaligned`], which have no runtime cost. + /// prefer using [`Self::via_immutable`], [`Self::via_into_bytes`], + /// [`Self::via_unaligned`], or [`Self::via_no_dynamic_padding`], which + /// perform no runtime check. #[inline(always)] fn via_runtime_check(self) -> Result<(Ptr<'a, T, I>, Ptr<'a, [T::Elem], I>), Self> { let l_len = self.l_len(); @@ -1011,6 +1166,8 @@ mod tests { trailing: [u8], } + assert!(SliceDst::LAYOUT.requires_dynamic_padding()); + const N: usize = 16; let arr = [1u16; N]; @@ -1071,20 +1228,81 @@ mod tests { } #[test] fn test_split_at_via_unaligned() { - use crate::{FromBytes, Immutable, IntoBytes, KnownLayout, SplitAt, Unaligned}; - #[derive(FromBytes, KnownLayout, SplitAt, IntoBytes, Immutable, Unaligned)] - #[repr(C)] - struct Packet { - length: u8, - body: [u8], + use crate::{Immutable, KnownLayout, Split, SplitAt, Unaligned}; + + fn via_unaligned<'a, T>(split: Split<&'a T>) -> (&'a T, &'a [T::Elem]) + where + T: ?Sized + SplitAt, + T::Elem: Unaligned, + { + split.via_unaligned() } - let arr = [1, 2, 3, 4]; - let packet = Packet::ref_from_bytes(&arr[..]).unwrap(); + fn via_unaligned_mut<'a, T>(split: Split<&'a mut T>) -> (&'a mut T, &'a mut [T::Elem]) + where + T: ?Sized + SplitAt, + T::Elem: Unaligned, + { + split.via_unaligned() + } + + #[derive(KnownLayout, SplitAt, Immutable)] + #[repr(C, align(2))] + struct Packet { + prefix: [u8; 2], + body: B, + } + + // `Packet` has alignment 2 and does not implement `Unaligned`, but its + // two-byte trailing elements do. Since its size changes in two-byte + // increments, its concrete layout never requires dynamic padding. + assert!(!Packet::<[[u8; 2]]>::LAYOUT.requires_dynamic_padding()); + let packet = Packet { prefix: [0, 1], body: [[2, 3], [4, 5], [6, 7]] }; + let packet: &Packet<[[u8; 2]]> = &packet; let split = packet.split_at(2).unwrap(); - let (l, r) = split.via_unaligned(); - assert_eq!(l.length, 1); - assert_eq!(r, &[4]); + let (l, r) = via_unaligned(split); + assert_eq!(l.body, [[2, 3], [4, 5]]); + assert_eq!(r, &[[6, 7]]); + + let mut packet = Packet { prefix: [0, 1], body: [[2, 3], [4, 5], [6, 7]] }; + { + let packet: &mut Packet<[[u8; 2]]> = &mut packet; + let split = packet.split_at_mut(2).unwrap(); + let (l, r) = via_unaligned_mut(split); + l.body[0] = [8, 9]; + r[0] = [10, 11]; + } + assert_eq!(packet.body, [[8, 9], [4, 5], [10, 11]]); + } + + #[test] + fn test_split_at_via_no_dynamic_padding() { + use core::cell::Cell; + + use crate::SplitAt; + + // `u16` does not implement `Unaligned`, but `[u16]` never requires + // dynamic trailing padding. + let words = [1u16, 2, 3, 4]; + let split = SplitAt::split_at(&words[..], 2).unwrap(); + let (left, right) = split.via_no_dynamic_padding(); + assert_eq!(left, [1, 2]); + assert_eq!(right, [3, 4]); + + let mut words = [1u16, 2, 3, 4]; + let split = SplitAt::split_at_mut(&mut words[..], 2).unwrap(); + let (left, right) = split.via_no_dynamic_padding(); + left[0] = 5; + right[0] = 6; + assert_eq!(words, [5, 2, 6, 4]); + + // Exercise the safety-sensitive shared route with interior mutation. + let cells = [Cell::new(1u16), Cell::new(2), Cell::new(3)]; + let split = SplitAt::split_at(&cells[..], 2).unwrap(); + let (left, right) = split.via_no_dynamic_padding(); + left[0].set(4); + right[0].set(5); + assert_eq!([cells[0].get(), cells[1].get(), cells[2].get()], [4, 2, 5]); } } diff --git a/zerocopy/src/util/mod.rs b/zerocopy/src/util/mod.rs index bd0d83e78c..7d968f1b3a 100644 --- a/zerocopy/src/util/mod.rs +++ b/zerocopy/src/util/mod.rs @@ -144,9 +144,10 @@ pub(crate) fn validate_aligned_to(t: T) -> Result<(), Alignment /// on the answer it gives if this is not the case. #[cfg_attr( kani, - kani::requires(len <= DstLayout::MAX_SIZE), kani::requires(align.is_power_of_two()), - kani::ensures(|&p| (len + p) % align.get() == 0), + // A power-of-two alignment divides the `usize` modulus, so wrapping + // preserves congruence even when the next aligned value exceeds `usize`. + kani::ensures(|&p| len.wrapping_add(p) % align.get() == 0), // Ensures that we add the minimum required padding. kani::ensures(|&p| p < align.get()), )] @@ -576,15 +577,36 @@ mod len_of { clippy::multiple_unsafe_ops_per_block )] // SAFETY: By invariant on `self`, a `&T` with metadata `self.meta` - // describes an object of size `<= isize::MAX`. This computes the - // size of such a `&T` without any trailing padding, and so neither - // the multiplication nor the addition will overflow. - let unpadded_size = unsafe { + // describes an object of size `<= isize::MAX`. The end of the + // trailing slice is no larger than the size of the object, and so + // neither the multiplication nor the addition will overflow. + let trailing_slice_end = unsafe { let trailing_size = self.meta.unchecked_mul(trailing_slice_layout.elem_size); trailing_size.unchecked_add(trailing_slice_layout.offset) }; - util::padding_needed_for(unpadded_size, T::LAYOUT.align) + let size = match T::size_for_metadata(self.meta) { + Some(size) => size, + // SAFETY: The `MetadataOf` invariant states that the size + // of `T` with metadata `self.meta` is no larger than + // `isize::MAX`, which fits in `usize`. + // `KnownLayout::size_for_metadata` promises to return `None` + // if and only if the resulting size would not fit in `usize`, + // so this arm cannot be reached. Reaching + // `unreachable_unchecked` would be undefined behavior [1]. + // + // [1] Per https://doc.rust-lang.org/1.92.0/std/hint/fn.unreachable_unchecked.html#safety: + // + // Reaching this function is Undefined Behavior. + None => unsafe { core::hint::unreachable_unchecked() }, + }; + + // Guaranteed not to underflow: By the `KnownLayout::LAYOUT` + // contract, all bytes after the trailing slice are padding in the + // object described by `size`. + #[allow(clippy::arithmetic_side_effects)] + let padding = size - trailing_slice_end; + padding } #[inline(always)] diff --git a/zerocopy/zerocopy-derive/tests/ui/struct.nightly.stderr b/zerocopy/zerocopy-derive/tests/ui/struct.nightly.stderr index d217751683..3b8034e53f 100644 --- a/zerocopy/zerocopy-derive/tests/ui/struct.nightly.stderr +++ b/zerocopy/zerocopy-derive/tests/ui/struct.nightly.stderr @@ -558,7 +558,7 @@ help: the following other types implement trait `SplitAt` ... 363 | #[derive(SplitAt, KnownLayout)] | ^^^^^^^ `SplitAtSized` - --> src/split_at.rs:253:0 + --> src/split_at.rs:250:0 | = note: `[T]` = help: see issue #48214 diff --git a/zerocopy/zerocopy-derive/tests/ui/struct.stable.stderr b/zerocopy/zerocopy-derive/tests/ui/struct.stable.stderr index f7c5fc87d2..4797ee7b19 100644 --- a/zerocopy/zerocopy-derive/tests/ui/struct.stable.stderr +++ b/zerocopy/zerocopy-derive/tests/ui/struct.stable.stderr @@ -529,9 +529,9 @@ help: the following other types implement trait `SplitAt` 363 | #[derive(SplitAt, KnownLayout)] | ^^^^^^^ `SplitAtSized` | - ::: $WORKSPACE/src/split_at.rs:253:1 + ::: $WORKSPACE/src/split_at.rs:250:1 | -253 | unsafe impl SplitAt for [T] { +250 | unsafe impl SplitAt for [T] { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `[T]` = help: see issue #48214 = note: this error originates in the derive macro `SplitAt` (in Nightly builds, run with -Z macro-backtrace for more info)