From 8a3ed749db5882240b5a8b9c7cb90423427199b9 Mon Sep 17 00:00:00 2001 From: Cameron Bytheway Date: Wed, 14 Feb 2024 11:02:26 -0700 Subject: [PATCH] feat(s2n-codec): add bytes methods for unaligned types --- .github/workflows/ci.yml | 2 +- common/s2n-codec/Cargo.toml | 8 ++ common/s2n-codec/src/decoder/checked_range.rs | 17 +++ common/s2n-codec/src/decoder/mod.rs | 2 +- common/s2n-codec/src/decoder/value.rs | 2 +- common/s2n-codec/src/encoder/value.rs | 18 +++ common/s2n-codec/src/testing/mod.rs | 58 ++++++++- common/s2n-codec/src/unaligned.rs | 111 ++++++++++++++++-- 8 files changed, 204 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5ac33755cf..66a08df3f9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -698,7 +698,7 @@ jobs: strategy: fail-fast: false matrix: - crate: [quic/s2n-quic-core, quic/s2n-quic-platform] + crate: [common/s2n-codec, quic/s2n-quic-core, quic/s2n-quic-platform] steps: - uses: actions/checkout@v4 with: diff --git a/common/s2n-codec/Cargo.toml b/common/s2n-codec/Cargo.toml index ba63f10be3..b32f6e6e70 100644 --- a/common/s2n-codec/Cargo.toml +++ b/common/s2n-codec/Cargo.toml @@ -23,3 +23,11 @@ bolero-generator = { version = "0.10", default-features = false, optional = true byteorder = { version = "1.1", default-features = false } bytes = { version = "1", default-features = false, optional = true } zerocopy = { version = "0.7", features = ["derive"] } + +[dev-dependencies] +bolero = "0.10" +bolero-generator = "0.10" + +[package.metadata.kani] +flags = { tests = true } +unstable = { stubbing = true } diff --git a/common/s2n-codec/src/decoder/checked_range.rs b/common/s2n-codec/src/decoder/checked_range.rs index aa46d052e3..d016b4dba0 100644 --- a/common/s2n-codec/src/decoder/checked_range.rs +++ b/common/s2n-codec/src/decoder/checked_range.rs @@ -43,6 +43,23 @@ impl CheckedRange { &slice[self.start..self.end] } + #[cfg(feature = "checked_range_unsafe")] + #[inline] + pub fn get_mut<'a>(&self, slice: &'a mut [u8]) -> &'a mut [u8] { + unsafe { + #[cfg(debug_assertions)] + debug_assert_eq!(slice.as_ptr().add(self.start), self.original_ptr); + + slice.get_unchecked_mut(self.start..self.end) + } + } + + #[cfg(not(feature = "checked_range_unsafe"))] + #[inline] + pub fn get_mut<'a>(&self, slice: &'a mut [u8]) -> &'a mut [u8] { + &mut slice[self.start..self.end] + } + #[inline] pub fn len(&self) -> usize { self.end - self.start diff --git a/common/s2n-codec/src/decoder/mod.rs b/common/s2n-codec/src/decoder/mod.rs index 443a224c63..424116ed7b 100644 --- a/common/s2n-codec/src/decoder/mod.rs +++ b/common/s2n-codec/src/decoder/mod.rs @@ -413,7 +413,7 @@ pub use buffer_mut::*; pub use checked_range::*; pub use value::*; -#[derive(Debug)] +#[derive(Clone, Copy, Debug)] pub enum DecoderError { UnexpectedEof(usize), UnexpectedBytes(usize), diff --git a/common/s2n-codec/src/decoder/value.rs b/common/s2n-codec/src/decoder/value.rs index 9d5ca2655f..dd6e06fc34 100644 --- a/common/s2n-codec/src/decoder/value.rs +++ b/common/s2n-codec/src/decoder/value.rs @@ -87,7 +87,7 @@ macro_rules! decoder_value_unaligned_integer { let (value, buffer) = buffer.decode_slice($bitsize / 8)?; let value = value.as_less_safe_slice(); let value = NetworkEndian::$call(value); - Ok(($ty(value), buffer)) + Ok(($ty::new_truncated(value), buffer)) } } ); diff --git a/common/s2n-codec/src/encoder/value.rs b/common/s2n-codec/src/encoder/value.rs index fb29c4fe64..6371c12f13 100644 --- a/common/s2n-codec/src/encoder/value.rs +++ b/common/s2n-codec/src/encoder/value.rs @@ -180,6 +180,24 @@ impl EncoderValue for () { } } +impl EncoderValue for (A, B) { + #[inline] + fn encode(&self, encoder: &mut E) { + self.0.encode(encoder); + self.1.encode(encoder); + } + + #[inline] + fn encoding_size(&self) -> usize { + self.0.encoding_size() + self.1.encoding_size() + } + + #[inline] + fn encoding_size_for_encoder(&self, encoder: &E) -> usize { + self.0.encoding_size_for_encoder(encoder) + self.1.encoding_size_for_encoder(encoder) + } +} + impl EncoderValue for Option { #[inline] fn encode(&self, buffer: &mut E) { diff --git a/common/s2n-codec/src/testing/mod.rs b/common/s2n-codec/src/testing/mod.rs index 0a7a006b81..ac645bfac7 100644 --- a/common/s2n-codec/src/testing/mod.rs +++ b/common/s2n-codec/src/testing/mod.rs @@ -212,16 +212,66 @@ pub fn ensure_decoding_mut_matches<'a, T: DecoderValueMut<'a> + PartialEq + core #[cfg(test)] mod tests { + use crate::{i24, i48, u24, u48}; + use bolero::check; + #[test] - fn test_u8_round_trip_value() { - for i in 0..core::u8::MAX { + fn u8_round_trip_value_test() { + for i in 0..=core::u8::MAX { ensure_codec_round_trip_value!(u8, i).unwrap(); } } #[test] - fn test_u8_round_trip_bytes() { - let bytes = (0..core::u8::MAX).collect::>(); + fn u8_round_trip_bytes_test() { + let bytes = (0..=core::u8::MAX).collect::>(); ensure_codec_round_trip_bytes!(u8, &bytes).unwrap(); } + + #[test] + fn u16_round_trip_value_test() { + for i in 0..=u16::MAX { + ensure_codec_round_trip_value!(u16, i).unwrap(); + } + } + + #[test] + fn u16_round_trip_bytes_test() { + let mut bytes = vec![]; + for v in 0..=u16::MAX { + bytes.extend_from_slice(&v.to_be_bytes()); + } + ensure_codec_round_trip_bytes!(u16, &bytes).unwrap(); + } + + macro_rules! fuzz_tests { + ($ty:ident, $value_test:ident, $bytes_test:ident) => { + #[test] + #[cfg_attr(kani, kani::proof, kani::solver(cadical))] + fn $value_test() { + check!().with_type().cloned().for_each(|v| { + ensure_codec_round_trip_value!($ty, v).unwrap(); + let bytes = v.to_be_bytes(); + let actual = $ty::from_be_bytes(bytes); + assert_eq!(v, actual); + }); + } + + #[test] + fn $bytes_test() { + check!().for_each(|bytes| { + let _ = ensure_codec_round_trip_bytes!($ty, &bytes); + }); + } + }; + } + + fuzz_tests!(u24, u24_round_trip_value_test, u24_round_trip_bytes_test); + fuzz_tests!(i24, i24_round_trip_value_test, i24_round_trip_bytes_test); + fuzz_tests!(u32, u32_round_trip_value_test, u32_round_trip_bytes_test); + fuzz_tests!(i32, i32_round_trip_value_test, i32_round_trip_bytes_test); + fuzz_tests!(u48, u48_round_trip_value_test, u48_round_trip_bytes_test); + fuzz_tests!(i48, i48_round_trip_value_test, i48_round_trip_bytes_test); + fuzz_tests!(u64, u64_round_trip_value_test, u64_round_trip_bytes_test); + fuzz_tests!(i64, i64_round_trip_value_test, i64_round_trip_bytes_test); } diff --git a/common/s2n-codec/src/unaligned.rs b/common/s2n-codec/src/unaligned.rs index 25c150ff5b..33b2bb3779 100644 --- a/common/s2n-codec/src/unaligned.rs +++ b/common/s2n-codec/src/unaligned.rs @@ -16,19 +16,34 @@ use core::{ // // 48-bit integers are also implemented for completeness. macro_rules! unaligned_integer_type { - ($name:ident, $bitsize:expr, $storage_type:ty, [$($additional_conversions:ty),*]) => { + ($name:ident, $bitsize:expr, $storage_type:ty, $min:expr, $max:expr, [$($additional_conversions:ty),*]) => { #[allow(non_camel_case_types)] #[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Eq, Ord, Hash, Default)] - pub struct $name(pub(crate) $storage_type); + pub struct $name($storage_type); impl $name { + pub const ZERO: Self = Self(0); + pub const MIN: Self = Self($min); + pub const MAX: Self = Self($max); + /// Truncate the storage value into the allowed range + #[inline] pub fn new_truncated(value: $storage_type) -> Self { Self(value & ((1 << $bitsize) - 1)) } + + #[inline] + pub fn from_be_bytes(bytes: [u8; ($bitsize / 8)]) -> Self { + Self(UnalignedBytes::be_bytes_to_storage(bytes) as _) + } + + #[inline] + pub fn to_be_bytes(self) -> [u8; ($bitsize / 8)] { + UnalignedBytes::storage_to_be_bytes(self.0 as _) + } } - #[cfg(feature = "generator")] + #[cfg(any(test, feature = "generator"))] impl bolero_generator::TypeGenerator for $name { fn generate(driver: &mut D) -> Option { Some(Self::new_truncated(driver.gen()?)) @@ -38,6 +53,7 @@ macro_rules! unaligned_integer_type { impl TryFrom<$storage_type> for $name { type Error = TryFromIntError; + #[inline] fn try_from(value: $storage_type) -> Result { if value < (1 << $bitsize) { Ok(Self(value)) @@ -48,6 +64,7 @@ macro_rules! unaligned_integer_type { } impl From<$name> for $storage_type { + #[inline] fn from(value: $name) -> $storage_type { value.0 } @@ -55,12 +72,14 @@ macro_rules! unaligned_integer_type { $( impl From<$additional_conversions> for $name { + #[inline] fn from(value: $additional_conversions) -> Self { $name(value.into()) } } impl From<$name> for $additional_conversions { + #[inline] fn from(value: $name) -> Self { value.0 as $additional_conversions } @@ -70,6 +89,7 @@ macro_rules! unaligned_integer_type { impl Deref for $name { type Target = $storage_type; + #[inline] fn deref(&self) -> &Self::Target { &self.0 } @@ -77,12 +97,80 @@ macro_rules! unaligned_integer_type { }; } -unaligned_integer_type!(u24, 24, u32, [u8, u16]); -unaligned_integer_type!(i24, 24, i32, [u8, i8, u16, i16]); +/// A trait defining how to convert between storage types and unaligned bytes +trait UnalignedBytes: Sized { + type Storage; + + fn storage_to_be_bytes(storage: Self::Storage) -> Self; + fn be_bytes_to_storage(self) -> Self::Storage; +} + +impl UnalignedBytes for [u8; 3] { + type Storage = u32; + + #[inline] + fn storage_to_be_bytes(storage: Self::Storage) -> Self { + let [_, a, b, c] = storage.to_be_bytes(); + [a, b, c] + } + + #[inline] + fn be_bytes_to_storage(self) -> Self::Storage { + let [a, b, c] = self; + let bytes = [0, a, b, c]; + Self::Storage::from_be_bytes(bytes) + } +} + +impl UnalignedBytes for [u8; 6] { + type Storage = u64; + + #[inline] + fn storage_to_be_bytes(storage: Self::Storage) -> Self { + let [_, _, a, b, c, d, e, f] = storage.to_be_bytes(); + [a, b, c, d, e, f] + } + + #[inline] + fn be_bytes_to_storage(self) -> Self::Storage { + let [a, b, c, d, e, f] = self; + let bytes = [0, 0, a, b, c, d, e, f]; + Self::Storage::from_be_bytes(bytes) + } +} + +macro_rules! signed_min { + ($bitsize:expr) => { + -(1 << ($bitsize - 1)) + }; +} + +macro_rules! signed_max { + ($bitsize:expr) => { + ((1 << ($bitsize - 1)) - 1) + }; +} + +#[test] +fn signed_min_max_test() { + assert_eq!(i8::MIN as i16, signed_min!(8)); + assert_eq!(i8::MAX as i16, signed_max!(8)); +} + +unaligned_integer_type!(u24, 24, u32, 0, (1 << 24) - 1, [u8, u16]); +unaligned_integer_type!( + i24, + 24, + i32, + signed_min!(24), + signed_max!(24), + [u8, i8, u16, i16] +); impl TryFrom for u24 { type Error = TryFromIntError; + #[inline] fn try_from(value: u64) -> Result { let storage_value: u32 = value.try_into()?; storage_value.try_into() @@ -90,18 +178,27 @@ impl TryFrom for u24 { } impl From for u64 { + #[inline] fn from(value: u24) -> u64 { value.0.into() } } -unaligned_integer_type!(u48, 24, u64, [u8, u16, u32]); -unaligned_integer_type!(i48, 24, i64, [u8, i8, u16, i16, u32, i32]); +unaligned_integer_type!(u48, 48, u64, 0, (1 << 48) - 1, [u8, u16, u32]); +unaligned_integer_type!( + i48, + 48, + i64, + signed_min!(48), + signed_max!(48), + [u8, i8, u16, i16, u32, i32] +); #[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Eq, Ord, Hash)] pub struct TryFromIntError(()); impl From for TryFromIntError { + #[inline] fn from(_: core::num::TryFromIntError) -> Self { Self(()) }