google / google/zerocopy

DST fails with SizeError if dynamic part size results in overall type size not multiple of alignment

Open
#3,061 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2.6k
Forks
179
Avg merge
1d 19h
Merged PRs (30d)
29

Description

For a DST with a trailing `[u8]` contained on a type with an implicit alignment > 1, `ref_from_bytes` fails with SizeError if the [u8] len would result in a size not a multiple of the struct alignment.

The below code example showcases the issue. It's adapted from the "Support for Dynamically Sized Types" example in https://github.com/google/zerocopy/discussions/1680 to model the fields in PacketHeader as u16, which upgrades PacketHeader to have an alignment of 2. As a result, the `body` slice can only be a multiple of 2.

```rust
#[test]
fn flexible_size_aligned_dst() {
use zerocopy::{FromBytes, Immutable, KnownLayout};

#[derive(FromBytes, KnownLayout, Immutable)]
#[repr(C)]
struct PacketHeader {
src_port: u16,
dst_port: u16,
length: u16,
checksum: u16,
}

#[derive(FromBytes, KnownLayout, Immutable)]
#[repr(C)]
struct Packet {
header: PacketHeader,
body: [u8],
}

assert_eq!(2, align_of::());

#[repr(align(8))]
struct Bytes([u8; 14]);
let bytes = &Bytes([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]).0[..];

// body len is multiple of 2, works
let msg = Packet::ref_from_bytes(&bytes[..10]).expect("should work");
assert_eq!(msg.body.len(), 2);

// body len is not multiple of 2, fails with SizeErr
let msg = Packet::ref_from_bytes(&bytes[..11]).expect("should work but doesn't");
assert_eq!(msg.body.len(), 3);
}
```

There are at least 2 possible workarounds but not sure either is good:
1. In Packet, set `header: Unalign`, which downgrades the alignment requirements of Packet, but I believe potentially slows down the runtime access to the PacketHeader fields.
2. Ensure the provided &[u8] to `ref_from_bytes` have any required padding and skip it when accessing the field. The code to skip the padding is risky, and a copy may be required to add padding to the original &[u8].

Is there some clean way of making this work with no perf penalty?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.