From 74e76288f4da4b2d7ce89b0112ef06ec206cc45d Mon Sep 17 00:00:00 2001 From: Walnut <39544927+Walnut356@users.noreply.github.com> Date: Wed, 16 Aug 2023 07:44:07 -0500 Subject: [PATCH 1/5] added bytes-specific get_X functions to reduce dead code --- src/bytes.rs | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/src/bytes.rs b/src/bytes.rs index 0404a72db..225dbcd68 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -533,6 +533,23 @@ impl Clone for Bytes { } } +macro_rules! buf_get_impl { + ($this:ident, $typ:tt::$conv:tt) => {{ + const SIZE: usize = mem::size_of::<$typ>(); + // slice.get() returns None on failing bounds check, resulting in a panic, but skips the unnecessary code of the + // default buf impl that needs to account for non-contiguous memory + let ret = $this + .chunk() + .get(..SIZE) + .map(|src| unsafe { $typ::$conv(*(src as *const _ as *const [_; SIZE])) }) + .unwrap(); + + // if the direct conversion was possible, advance and return + $this.advance(SIZE); + return ret; + }}; +} + impl Buf for Bytes { #[inline] fn remaining(&self) -> usize { @@ -567,6 +584,102 @@ impl Buf for Bytes { ret } } + + fn get_u16(&mut self) -> u16 { + buf_get_impl!(self, u16::from_be_bytes); + } + + fn get_u16_le(&mut self) -> u16 { + buf_get_impl!(self, u16::from_le_bytes); + } + + fn get_u16_ne(&mut self) -> u16 { + buf_get_impl!(self, u16::from_ne_bytes); + } + + fn get_i16(&mut self) -> i16 { + buf_get_impl!(self, i16::from_be_bytes); + } + + fn get_i16_le(&mut self) -> i16 { + buf_get_impl!(self, i16::from_le_bytes); + } + + fn get_i16_ne(&mut self) -> i16 { + buf_get_impl!(self, i16::from_ne_bytes); + } + + fn get_u32(&mut self) -> u32 { + buf_get_impl!(self, u32::from_be_bytes); + } + + fn get_u32_le(&mut self) -> u32 { + buf_get_impl!(self, u32::from_le_bytes); + } + + fn get_u32_ne(&mut self) -> u32 { + buf_get_impl!(self, u32::from_ne_bytes); + } + + fn get_i32(&mut self) -> i32 { + buf_get_impl!(self, i32::from_be_bytes); + } + + fn get_i32_le(&mut self) -> i32 { + buf_get_impl!(self, i32::from_le_bytes); + } + + fn get_i32_ne(&mut self) -> i32 { + buf_get_impl!(self, i32::from_ne_bytes); + } + + fn get_u64(&mut self) -> u64 { + buf_get_impl!(self, u64::from_be_bytes); + } + + fn get_u64_le(&mut self) -> u64 { + buf_get_impl!(self, u64::from_le_bytes); + } + + fn get_u64_ne(&mut self) -> u64 { + buf_get_impl!(self, u64::from_ne_bytes); + } + + fn get_i64(&mut self) -> i64 { + buf_get_impl!(self, i64::from_be_bytes); + } + + fn get_i64_le(&mut self) -> i64 { + buf_get_impl!(self, i64::from_le_bytes); + } + + fn get_i64_ne(&mut self) -> i64 { + buf_get_impl!(self, i64::from_ne_bytes); + } + + fn get_u128(&mut self) -> u128 { + buf_get_impl!(self, u128::from_be_bytes); + } + + fn get_u128_le(&mut self) -> u128 { + buf_get_impl!(self, u128::from_le_bytes); + } + + fn get_u128_ne(&mut self) -> u128 { + buf_get_impl!(self, u128::from_ne_bytes); + } + + fn get_i128(&mut self) -> i128 { + buf_get_impl!(self, i128::from_be_bytes); + } + + fn get_i128_le(&mut self) -> i128 { + buf_get_impl!(self, i128::from_le_bytes); + } + + fn get_i128_ne(&mut self) -> i128 { + buf_get_impl!(self, i128::from_ne_bytes); + } } impl Deref for Bytes { From b49f3d1bc449a75c7e7a6e3976ce90934afcb78e Mon Sep 17 00:00:00 2001 From: Walnut <39544927+Walnut356@users.noreply.github.com> Date: Wed, 16 Aug 2023 08:21:39 -0500 Subject: [PATCH 2/5] added bytes-specific get_X tests --- tests/test_bytes.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index 5ec60a5b0..a12816ab5 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -1208,3 +1208,18 @@ fn test_bytes_capacity_len() { } } } + +#[test] +fn test_get_u16() { + let mut buf = Bytes::from(&b"\x21\x54zomg"[..]); + assert_eq!(0x2154, buf.get_u16()); + let mut buf = Bytes::from(&b"\x21\x54zomg"[..]); + assert_eq!(0x5421, buf.get_u16_le()); +} + +#[test] +#[should_panic] +fn test_get_u16_buffer_underflow() { + let mut buf = Bytes::from(&b"\x21"[..]); + buf.get_u16(); +} \ No newline at end of file From 329acdb2fcc7e787b93969e976c48167a79634a9 Mon Sep 17 00:00:00 2001 From: Walnut <39544927+Walnut356@users.noreply.github.com> Date: Wed, 16 Aug 2023 22:34:27 -0500 Subject: [PATCH 3/5] added inlining for Bytes get_X functions --- src/bytes.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/bytes.rs b/src/bytes.rs index 225dbcd68..fc4dccf1d 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -585,98 +585,122 @@ impl Buf for Bytes { } } + #[inline] fn get_u16(&mut self) -> u16 { buf_get_impl!(self, u16::from_be_bytes); } + #[inline] fn get_u16_le(&mut self) -> u16 { buf_get_impl!(self, u16::from_le_bytes); } + #[inline] fn get_u16_ne(&mut self) -> u16 { buf_get_impl!(self, u16::from_ne_bytes); } + #[inline] fn get_i16(&mut self) -> i16 { buf_get_impl!(self, i16::from_be_bytes); } + #[inline] fn get_i16_le(&mut self) -> i16 { buf_get_impl!(self, i16::from_le_bytes); } + #[inline] fn get_i16_ne(&mut self) -> i16 { buf_get_impl!(self, i16::from_ne_bytes); } + #[inline] fn get_u32(&mut self) -> u32 { buf_get_impl!(self, u32::from_be_bytes); } + #[inline] fn get_u32_le(&mut self) -> u32 { buf_get_impl!(self, u32::from_le_bytes); } + #[inline] fn get_u32_ne(&mut self) -> u32 { buf_get_impl!(self, u32::from_ne_bytes); } + #[inline] fn get_i32(&mut self) -> i32 { buf_get_impl!(self, i32::from_be_bytes); } + #[inline] fn get_i32_le(&mut self) -> i32 { buf_get_impl!(self, i32::from_le_bytes); } + #[inline] fn get_i32_ne(&mut self) -> i32 { buf_get_impl!(self, i32::from_ne_bytes); } + #[inline] fn get_u64(&mut self) -> u64 { buf_get_impl!(self, u64::from_be_bytes); } + #[inline] fn get_u64_le(&mut self) -> u64 { buf_get_impl!(self, u64::from_le_bytes); } + #[inline] fn get_u64_ne(&mut self) -> u64 { buf_get_impl!(self, u64::from_ne_bytes); } + #[inline] fn get_i64(&mut self) -> i64 { buf_get_impl!(self, i64::from_be_bytes); } + #[inline] fn get_i64_le(&mut self) -> i64 { buf_get_impl!(self, i64::from_le_bytes); } + #[inline] fn get_i64_ne(&mut self) -> i64 { buf_get_impl!(self, i64::from_ne_bytes); } + #[inline] fn get_u128(&mut self) -> u128 { buf_get_impl!(self, u128::from_be_bytes); } + #[inline] fn get_u128_le(&mut self) -> u128 { buf_get_impl!(self, u128::from_le_bytes); } + #[inline] fn get_u128_ne(&mut self) -> u128 { buf_get_impl!(self, u128::from_ne_bytes); } + #[inline] fn get_i128(&mut self) -> i128 { buf_get_impl!(self, i128::from_be_bytes); } + #[inline] fn get_i128_le(&mut self) -> i128 { buf_get_impl!(self, i128::from_le_bytes); } + #[inline] fn get_i128_ne(&mut self) -> i128 { buf_get_impl!(self, i128::from_ne_bytes); } From f6e110bb02fe807101213b211be1c1b8a303ba25 Mon Sep 17 00:00:00 2001 From: Walnut <39544927+Walnut356@users.noreply.github.com> Date: Sun, 24 Sep 2023 18:19:29 -0500 Subject: [PATCH 4/5] fixed formatting --- tests/test_bytes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index a12816ab5..779c2e924 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -1222,4 +1222,4 @@ fn test_get_u16() { fn test_get_u16_buffer_underflow() { let mut buf = Bytes::from(&b"\x21"[..]); buf.get_u16(); -} \ No newline at end of file +} From 61945724ffb673d35acb7f373833ceacd3cb27ba Mon Sep 17 00:00:00 2001 From: Walnut <39544927+Walnut356@users.noreply.github.com> Date: Mon, 11 Dec 2023 19:11:20 -0600 Subject: [PATCH 5/5] more direct get strategy and additional tests --- src/bytes.rs | 66 +++++++++++++++++++++++++-------------------- tests/test_bytes.rs | 17 ++++++++++++ 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/bytes.rs b/src/bytes.rs index f01617a2d..a4c4563c1 100644 --- a/src/bytes.rs +++ b/src/bytes.rs @@ -534,13 +534,11 @@ impl Clone for Bytes { macro_rules! buf_get_impl { ($this:ident, $typ:tt::$conv:tt) => {{ const SIZE: usize = mem::size_of::<$typ>(); + + assert!($this.len >= SIZE); // slice.get() returns None on failing bounds check, resulting in a panic, but skips the unnecessary code of the // default buf impl that needs to account for non-contiguous memory - let ret = $this - .chunk() - .get(..SIZE) - .map(|src| unsafe { $typ::$conv(*(src as *const _ as *const [_; SIZE])) }) - .unwrap(); + let ret = unsafe { $typ::$conv(($this.ptr as *const $typ).read_unaligned()) }; // if the direct conversion was possible, advance and return $this.advance(SIZE); @@ -583,124 +581,134 @@ impl Buf for Bytes { } } + #[inline] + fn get_u8(&mut self) -> u8 { + buf_get_impl!(self, u8::from) + } + + #[inline] + fn get_i8(&mut self) -> i8 { + buf_get_impl!(self, i8::from) + } + #[inline] fn get_u16(&mut self) -> u16 { - buf_get_impl!(self, u16::from_be_bytes); + buf_get_impl!(self, u16::from_be); } #[inline] fn get_u16_le(&mut self) -> u16 { - buf_get_impl!(self, u16::from_le_bytes); + buf_get_impl!(self, u16::from_le); } #[inline] fn get_u16_ne(&mut self) -> u16 { - buf_get_impl!(self, u16::from_ne_bytes); + buf_get_impl!(self, u16::from); } #[inline] fn get_i16(&mut self) -> i16 { - buf_get_impl!(self, i16::from_be_bytes); + buf_get_impl!(self, i16::from_be); } #[inline] fn get_i16_le(&mut self) -> i16 { - buf_get_impl!(self, i16::from_le_bytes); + buf_get_impl!(self, i16::from_le); } #[inline] fn get_i16_ne(&mut self) -> i16 { - buf_get_impl!(self, i16::from_ne_bytes); + buf_get_impl!(self, i16::from); } #[inline] fn get_u32(&mut self) -> u32 { - buf_get_impl!(self, u32::from_be_bytes); + buf_get_impl!(self, u32::from_be); } #[inline] fn get_u32_le(&mut self) -> u32 { - buf_get_impl!(self, u32::from_le_bytes); + buf_get_impl!(self, u32::from_le); } #[inline] fn get_u32_ne(&mut self) -> u32 { - buf_get_impl!(self, u32::from_ne_bytes); + buf_get_impl!(self, u32::from); } #[inline] fn get_i32(&mut self) -> i32 { - buf_get_impl!(self, i32::from_be_bytes); + buf_get_impl!(self, i32::from_be); } #[inline] fn get_i32_le(&mut self) -> i32 { - buf_get_impl!(self, i32::from_le_bytes); + buf_get_impl!(self, i32::from_le); } #[inline] fn get_i32_ne(&mut self) -> i32 { - buf_get_impl!(self, i32::from_ne_bytes); + buf_get_impl!(self, i32::from); } #[inline] fn get_u64(&mut self) -> u64 { - buf_get_impl!(self, u64::from_be_bytes); + buf_get_impl!(self, u64::from_be); } #[inline] fn get_u64_le(&mut self) -> u64 { - buf_get_impl!(self, u64::from_le_bytes); + buf_get_impl!(self, u64::from_le); } #[inline] fn get_u64_ne(&mut self) -> u64 { - buf_get_impl!(self, u64::from_ne_bytes); + buf_get_impl!(self, u64::from); } #[inline] fn get_i64(&mut self) -> i64 { - buf_get_impl!(self, i64::from_be_bytes); + buf_get_impl!(self, i64::from_be); } #[inline] fn get_i64_le(&mut self) -> i64 { - buf_get_impl!(self, i64::from_le_bytes); + buf_get_impl!(self, i64::from_le); } #[inline] fn get_i64_ne(&mut self) -> i64 { - buf_get_impl!(self, i64::from_ne_bytes); + buf_get_impl!(self, i64::from); } #[inline] fn get_u128(&mut self) -> u128 { - buf_get_impl!(self, u128::from_be_bytes); + buf_get_impl!(self, u128::from_be); } #[inline] fn get_u128_le(&mut self) -> u128 { - buf_get_impl!(self, u128::from_le_bytes); + buf_get_impl!(self, u128::from_le); } #[inline] fn get_u128_ne(&mut self) -> u128 { - buf_get_impl!(self, u128::from_ne_bytes); + buf_get_impl!(self, u128::from); } #[inline] fn get_i128(&mut self) -> i128 { - buf_get_impl!(self, i128::from_be_bytes); + buf_get_impl!(self, i128::from_be); } #[inline] fn get_i128_le(&mut self) -> i128 { - buf_get_impl!(self, i128::from_le_bytes); + buf_get_impl!(self, i128::from_le); } #[inline] fn get_i128_ne(&mut self) -> i128 { - buf_get_impl!(self, i128::from_ne_bytes); + buf_get_impl!(self, i128::from); } } diff --git a/tests/test_bytes.rs b/tests/test_bytes.rs index 779c2e924..745e1ffde 100644 --- a/tests/test_bytes.rs +++ b/tests/test_bytes.rs @@ -1223,3 +1223,20 @@ fn test_get_u16_buffer_underflow() { let mut buf = Bytes::from(&b"\x21"[..]); buf.get_u16(); } + +#[test] +#[should_panic] +fn test_bytes_overread() { + let mut b = Bytes::from_static(&[0, 1, 2]); + let _ = b.get_u32(); +} + +// running this test would result in a panic without `.read_unaligned()` +// on x86 read_unaligned compiles down to a single `mov`, on platforms with no unaligned access, +// it uses rust's `copy_nonoverlapping` +#[test] +fn test_bytes_misaligned() { + let mut b = Bytes::from_static(&[0, 1, 2, 3, 4, 5, 6, 7, 8]); + b.advance(2); + let _ = b.get_u32(); +}