From aa40dc7ee4738827e0a015c2a336352ccdd4832a Mon Sep 17 00:00:00 2001 From: Josh Liebow-Feeser's Agent Date: Sat, 5 Sep 2026 00:10:30 +0000 Subject: [PATCH] Preserve nested DST padding in layout formulas DstLayout flattened a nested slice DST's trailing-padding operation into the alignment of its packed outer container. That could understate the Rust object size, causing safe parsers and allocators to construct references and boxes whose referents extended beyond their backing storage. Represent slice-DST sizes as a normalized prefix plus an independently rounded inner size. Carry that formula through composition, padding, metadata inference, casts, projections, allocation, and split overlap checks. Add regressions for the reported 8-byte-versus-10-byte packed layout, parsing, allocation, zeroing, projection, metadata plateaus, and dynamic padding. Closes #3617 *Authored by an AI agent acting on Josh Liebow-Feeser's behalf.* gherrit-pr-id: Gxauzb34ce7e537xqailxsfsyzknfjcnl --- zerocopy/AGENTS.md | 7 + ...t_via_runtime_check_dynamic_padding.x86-64 | 8 +- ...a_runtime_check_dynamic_padding.x86-64.mca | 34 +- zerocopy/src/layout.rs | 954 ++++++++++++++---- zerocopy/src/lib.rs | 179 +++- zerocopy/src/macros.rs | 32 + zerocopy/src/pointer/mod.rs | 28 +- zerocopy/src/split_at.rs | 310 +++++- zerocopy/src/util/mod.rs | 36 +- .../tests/ui/struct.nightly.stderr | 2 +- .../tests/ui/struct.stable.stderr | 4 +- 11 files changed, 1282 insertions(+), 312 deletions(-) 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)