safe

paolobarbolini · paolobarbolini · commit 7a9d4bcfb848 · 2024-11-30T11:06:51.000Z
diff --git a/src/decoding/decodebuffer.rs b/src/decoding/decodebuffer.rs
@@ -107,20 +107,7 @@ impl DecodeBuffer {
                 // We need to copy in chunks.
                 self.repeat_in_chunks(offset, match_length, start_idx);
             } else {
-                // can just copy parts of the existing buffer
-                // SAFETY: Requirements checked:
-                // 1. start_idx + match_length must be <= self.buffer.len()
-                //      We know that:
-                //      1. start_idx = self.buffer.len() - offset
-                //      2. end_idx = start_idx + match_length
-                //      3. end_idx <= self.buffer.len()
-                //      Thus follows: start_idx + match_length <= self.buffer.len()
-                //
-                // 2. explicitly reserved enough memory for the whole match_length
-                unsafe {
-                    self.buffer
-                        .extend_from_within_unchecked(start_idx, match_length)
-                };
+                self.buffer.extend_from_within(start_idx, match_length);
             }
 
             self.total_output_counter += match_length as u64;
@@ -137,22 +124,7 @@ impl DecodeBuffer {
         while copied_counter_left > 0 {
             let chunksize = usize::min(offset, copied_counter_left);
 
-            // SAFETY: Requirements checked:
-            // 1. start_idx + chunksize must be <= self.buffer.len()
-            //      We know that:
-            //      1. start_idx starts at buffer.len() - offset
-            //      2. chunksize <= offset (== offset for each iteration but the last, and match_length modulo offset in the last iteration)
-            //      3. the buffer grows by offset many bytes each iteration but the last
-            //      4. start_idx is increased by the same amount as the buffer grows each iteration
-            //
-            //      Thus follows: start_idx + chunksize == self.buffer.len() in each iteration but the last, where match_length modulo offset == chunksize < offset
-            //          Meaning: start_idx + chunksize <= self.buffer.len()
-            //
-            // 2. explicitly reserved enough memory for the whole match_length
-            unsafe {
-                self.buffer
-                    .extend_from_within_unchecked(start_idx, chunksize)
-            };
+            self.buffer.extend_from_within(start_idx, chunksize);
             copied_counter_left -= chunksize;
             start_idx += chunksize;
         }
diff --git a/src/decoding/ringbuffer.rs b/src/decoding/ringbuffer.rs
@@ -1,8 +1,8 @@
 use alloc::collections::VecDeque;
-use core::{cmp, hint::unreachable_unchecked, mem::MaybeUninit, slice};
+use core::cmp;
 
 pub struct RingBuffer {
-    buf: VecDeque<MaybeUninit<u8>>,
+    buf: VecDeque<u8>,
 }
 
 impl RingBuffer {
@@ -24,12 +24,10 @@ impl RingBuffer {
     }
 
     /// Return the amount of available space (in bytes) of the buffer.
+    #[cfg(test)]
     pub fn free(&self) -> usize {
         let len = self.buf.len();
         let capacity = self.buf.capacity();
-        if len > capacity {
-            unsafe { unreachable_unchecked() }
-        }
 
         capacity - len
     }
@@ -46,41 +44,23 @@ impl RingBuffer {
 
     /// Ensure that there's space for `amount` elements in the buffer.
     pub fn reserve(&mut self, additional: usize) {
-        if self.free() < additional {
-            self.reserve_amortized(additional);
-        }
-
-        if self.free() < additional {
-            unsafe { unreachable_unchecked() }
-        }
-    }
-
-    #[inline(never)]
-    #[cold]
-    fn reserve_amortized(&mut self, additional: usize) {
         self.buf.reserve(additional);
     }
 
     #[allow(dead_code)]
     pub fn push_back(&mut self, byte: u8) {
-        self.reserve(1);
-        self.buf.push_back(MaybeUninit::new(byte));
+        self.buf.push_back(byte);
     }
 
     /// Fetch the byte stored at the selected index from the buffer, returning it, or
     /// `None` if the index is out of bounds.
     #[allow(dead_code)]
     pub fn get(&self, idx: usize) -> Option<u8> {
-        self.buf
-            .get(idx)
-            .map(|&byte| unsafe { MaybeUninit::assume_init(byte) })
+        self.buf.get(idx).copied()
     }
 
     /// Append the provided data to the end of `self`.
     pub fn extend(&mut self, data: &[u8]) {
-        let len = data.len();
-        let data = data.as_ptr().cast::<MaybeUninit<u8>>();
-        let data = unsafe { slice::from_raw_parts(data, len) };
         self.buf.extend(data);
     }
 
@@ -94,16 +74,12 @@ impl RingBuffer {
 
     /// Return references to each part of the ring buffer.
     pub fn as_slices(&self) -> (&[u8], &[u8]) {
-        let (a, b) = self.buf.as_slices();
-
-        (unsafe { slice_assume_init_ref_polyfill(a) }, unsafe {
-            slice_assume_init_ref_polyfill(b)
-        })
+        self.buf.as_slices()
     }
 
     /// Copies elements from the provided range to the end of the buffer.
     #[allow(dead_code)]
-    pub fn extend_from_within(&mut self, start: usize, len: usize) {
+    pub fn extend_from_within(&mut self, mut start: usize, len: usize) {
         if start + len > self.len() {
             panic!(
                 "Calls to this functions must respect start ({}) + len ({}) <= self.len() ({})!",
@@ -113,43 +89,26 @@ impl RingBuffer {
             );
         }
 
-        self.reserve(len);
-
-        // SAFETY: Requirements checked:
-        // 1. explicitly checked above, resulting in a panic if it does not hold
-        // 2. explicitly reserved enough memory
-        unsafe { self.extend_from_within_unchecked(start, len) }
-    }
+        // Naive and cheaper implementation (for small lengths)
+        if len <= 12 {
+            self.reserve(len);
+            for i in 0..len {
+                let byte = self.get(start + i).unwrap();
+                self.push_back(byte);
+            }
 
-    /// Copies data from the provided range to the end of the buffer, without
-    /// first verifying that the unoccupied capacity is available.
-    ///
-    /// SAFETY:
-    /// For this to be safe two requirements need to hold:
-    /// 1. start + len <= self.len() so we do not copy uninitialised memory
-    /// 2. More then len reserved space so we do not write out-of-bounds
-    #[warn(unsafe_op_in_unsafe_fn)]
-    pub unsafe fn extend_from_within_unchecked(&mut self, mut start: usize, len: usize) {
-        debug_assert!(start + len <= self.len());
-        debug_assert!(self.free() >= len);
-
-        if self.free() < len {
-            unsafe { unreachable_unchecked() }
+            return;
         }
 
         let original_len = self.len();
         let mut intermediate = {
             IntermediateRingBuffer {
                 this: self,
                 original_len,
-                disarmed: false,
             }
         };
 
-        intermediate
-            .this
-            .buf
-            .extend((0..len).map(|_| MaybeUninit::uninit()));
+        intermediate.this.buf.extend((0..len).map(|_| 0));
         debug_assert_eq!(intermediate.this.buf.len(), original_len + len);
 
         let (a, b, a_spare, b_spare) = intermediate.as_slices_spare_mut();
@@ -158,7 +117,7 @@ impl RingBuffer {
         let skip = cmp::min(a.len(), start);
         start -= skip;
         let a = &a[skip..];
-        let b = unsafe { b.get_unchecked(start..) };
+        let b = &b[start..];
 
         let mut remaining_copy_len = len;
 
@@ -168,7 +127,6 @@ impl RingBuffer {
         remaining_copy_len -= copy_at_least;
 
         if remaining_copy_len == 0 {
-            intermediate.disarmed = true;
             return;
         }
 
@@ -181,7 +139,6 @@ impl RingBuffer {
         remaining_copy_len -= copy_at_least;
 
         if remaining_copy_len == 0 {
-            intermediate.disarmed = true;
             return;
         }
 
@@ -193,7 +150,6 @@ impl RingBuffer {
         remaining_copy_len -= copy_at_least;
 
         if remaining_copy_len == 0 {
-            intermediate.disarmed = true;
             return;
         }
 
@@ -205,22 +161,17 @@ impl RingBuffer {
         remaining_copy_len -= copy_at_least;
 
         debug_assert_eq!(remaining_copy_len, 0);
-
-        intermediate.disarmed = true;
     }
 }
 
 struct IntermediateRingBuffer<'a> {
     this: &'a mut RingBuffer,
     original_len: usize,
-    disarmed: bool,
 }
 
 impl<'a> IntermediateRingBuffer<'a> {
     // inspired by `Vec::split_at_spare_mut`
-    fn as_slices_spare_mut(
-        &mut self,
-    ) -> (&[u8], &[u8], &mut [MaybeUninit<u8>], &mut [MaybeUninit<u8>]) {
+    fn as_slices_spare_mut(&mut self) -> (&[u8], &[u8], &mut [u8], &mut [u8]) {
         let (a, b) = self.this.buf.as_mut_slices();
         debug_assert!(a.len() + b.len() >= self.original_len);
 
@@ -230,26 +181,11 @@ impl<'a> IntermediateRingBuffer<'a> {
         let b_mid = remaining_init_len;
         debug_assert!(b.len() >= b_mid);
 
-        let (a, a_spare) = unsafe { a.split_at_mut_unchecked(a_mid) };
-        let (b, b_spare) = unsafe { b.split_at_mut_unchecked(b_mid) };
+        let (a, a_spare) = a.split_at_mut(a_mid);
+        let (b, b_spare) = b.split_at_mut(b_mid);
         debug_assert!(a_spare.is_empty() || b.is_empty());
 
-        (
-            unsafe { slice_assume_init_ref_polyfill(a) },
-            unsafe { slice_assume_init_ref_polyfill(b) },
-            a_spare,
-            b_spare,
-        )
-    }
-}
-
-impl<'a> Drop for IntermediateRingBuffer<'a> {
-    fn drop(&mut self) {
-        if self.disarmed {
-            return;
-        }
-
-        self.this.buf.truncate(self.original_len);
+        (a, b, a_spare, b_spare)
     }
 }
 
@@ -266,48 +202,11 @@ impl<'a> Drop for IntermediateRingBuffer<'a> {
 /// The chunk size is not part of the contract and may change depending on the target platform.
 ///
 /// If that isn't possible we just fall back to ptr::copy_nonoverlapping
-fn copy_bytes_overshooting(src: &[u8], dst: &mut [MaybeUninit<u8>], copy_at_least: usize) {
-    // this assert is required for this function to be safe
-    // the optimizer should be able to remove it given how the caller
-    // has somehow to figure out `copy_at_least <= src.len() && copy_at_least <= dst.len()`
-    assert!(src.len() >= copy_at_least && dst.len() >= copy_at_least);
-
-    type CopyType = usize;
-
-    const COPY_AT_ONCE_SIZE: usize = core::mem::size_of::<CopyType>();
-    let min_buffer_size = usize::min(src.len(), dst.len());
-
-    // this check should be removed by the optimizer thanks to the above assert
-    // if `src.len() >= copy_at_least && dst.len() >= copy_at_least` then `min_buffer_size >= copy_at_least`
-    assert!(min_buffer_size >= copy_at_least);
-
-    // these bounds checks are removed because this is guaranteed:
-    // `min_buffer_size <= src.len() && min_buffer_size <= dst.len()`
-    let src = &src[..min_buffer_size];
-    let dst = &mut dst[..min_buffer_size];
-
-    // Can copy in just one read+write, very common case
-    if min_buffer_size >= COPY_AT_ONCE_SIZE && copy_at_least <= COPY_AT_ONCE_SIZE {
-        let chunk = unsafe { src.as_ptr().cast::<CopyType>().read_unaligned() };
-        unsafe { dst.as_mut_ptr().cast::<CopyType>().write_unaligned(chunk) };
-    } else {
-        unsafe {
-            dst.as_mut_ptr()
-                .cast::<u8>()
-                .copy_from_nonoverlapping(src.as_ptr(), copy_at_least)
-        };
-    }
-
-    debug_assert_eq!(&src[..copy_at_least], unsafe {
-        slice_assume_init_ref_polyfill(&dst[..copy_at_least])
-    });
-}
+fn copy_bytes_overshooting(src: &[u8], dst: &mut [u8], copy_at_least: usize) {
+    let src = &src[..copy_at_least];
+    let dst = &mut dst[..copy_at_least];
 
-#[inline(always)]
-unsafe fn slice_assume_init_ref_polyfill(slice: &[MaybeUninit<u8>]) -> &[u8] {
-    let len = slice.len();
-    let data = slice.as_ptr().cast::<u8>();
-    slice::from_raw_parts(data, len)
+    dst.copy_from_slice(src);
 }
 
 #[cfg(test)]
diff --git a/src/lib.rs b/src/lib.rs
@@ -14,6 +14,7 @@
 //! than the original implementation.
 #![no_std]
 #![deny(trivial_casts, trivial_numeric_casts, rust_2018_idioms)]
+#![forbid(unsafe_code)]
 
 #[cfg(feature = "std")]
 extern crate std;
