Improve safety docs and pass MaybeUninit<u8> to RBrotliEncCompress.

Author: veluca93

Cargo.toml

@@ -19,3 +19,6 @@ debug = true
 [workspace.lints.clippy]
 missing_safety_doc = "deny"
 undocumented_unsafe_blocks = "deny"
+
+[workspace.lints.rust]
+unsafe_op_in_unsafe_fn = "deny"

bounded-utils/src/lib.rs

@@ -183,17 +183,23 @@ impl<T, const LOWER_BOUND: usize> BoundedSlice<T, LOWER_BOUND> {
     /// # Safety
     /// Caller must guarantee slice.len() >= LOWER_BOUND.
     pub unsafe fn from_slice_unchecked(slice: &[T]) -> &BoundedSlice<T, LOWER_BOUND> {
-        // SAFETY: same layout and interpretation of metadata.
-        &*(slice as *const [T] as *const Self)
+        let ptr_to_self = slice as *const [T] as *const Self;
+        // SAFETY: `Self` is a repr(transparent) wrapper around `[T]`, so it has the same memory
+        // layout. Dereferencing the pointer is then valid, and the lifetimes in the function
+        // signature guarantee that the returned slice does not outlive the input slice.
+        unsafe { &*ptr_to_self }
     }
 
     /// Constructs a new mutable BoundedSlice without checking that its length is sufficient.
     ///
     /// # Safety
     /// Caller must guarantee slice.len() >= LOWER_BOUND.
     pub unsafe fn from_slice_unchecked_mut(slice: &mut [T]) -> &mut BoundedSlice<T, LOWER_BOUND> {
-        // SAFETY: same layout and interpretation of metadata.
-        &mut *(slice as *mut [T] as *mut Self)
+        let ptr_to_self = slice as *mut [T] as *mut Self;
+        // SAFETY: `Self` is a repr(transparent) wrapper around `[T]`, so it has the same memory
+        // layout. Dereferencing the pointer is then valid, and the lifetimes in the function
+        // signature guarantee that the returned slice does not outlive the input slice.
+        unsafe { &mut *ptr_to_self }
     }
 
     pub fn new_from_array<const ARR_SIZE: usize>(
@@ -406,7 +412,12 @@ macro_rules! impl_bounded_iterable {
 
                 #[inline(always)]
                 unsafe fn internal_make(($([< state_ $bound:lower >]),*): Self::State) -> Self {
-                    ($(BoundedUsize::new_unchecked([< state_ $bound:lower >])),*)
+                    // SAFETY: the safety invariant on `internal_make` guarantees that the `usize`
+                    // passed to `new_unchecked` is indeed bounded by the required bound, thanks to
+                    // the checks done in `iter` and `riter`.
+                    unsafe {
+                        ($(BoundedUsize::new_unchecked([< state_ $bound:lower >])),*)
+                    }
                 }
             }
         }

clib/rbrotli_enc.h

@@ -25,6 +25,9 @@ RBrotliEncoder *RBrotliEncMakeEncoder(uint32_t quality);
  *
  * `*out_len` is overwritten with the total size of the encoded data.
  *
+ * If this function returns `true`, the first `*out_len` bytes pointed at by `*out_data` will be
+ * initialized.
+ *
  * # Safety
  * `encoder` must be a valid Encoder created by RBrotliEncMakeEncoder that has not been
  * freed yet.

clib/src/lib.rs

@@ -12,7 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::ptr::{slice_from_raw_parts, slice_from_raw_parts_mut};
+use std::{
+    mem::MaybeUninit,
+    ptr::{slice_from_raw_parts, slice_from_raw_parts_mut},
+};
 
 /// cbindgen:ignore
 type RBrotliEncoder = rbrotli_enc_lib::Encoder;
@@ -31,6 +34,9 @@ pub extern "C" fn RBrotliEncMakeEncoder(quality: u32) -> *mut RBrotliEncoder {
 ///
 /// `*out_len` is overwritten with the total size of the encoded data.
 ///
+/// If this function returns `true`, the first `*out_len` bytes pointed at by `*out_data` will be
+/// initialized.
+///
 /// # Safety
 /// `encoder` must be a valid Encoder created by RBrotliEncMakeEncoder that has not been
 /// freed yet.
@@ -44,23 +50,32 @@ pub unsafe extern "C" fn RBrotliEncCompress(
     encoder: *mut RBrotliEncoder,
     data: *const u8,
     len: usize,
-    out_data: *mut *mut u8,
+    out_data: *mut *mut MaybeUninit<u8>,
     out_len: *mut usize,
 ) -> bool {
-    let Some(encoder) = encoder.as_mut() else {
+    // SAFETY: caller guarantees the pointer to be dereferenceable.
+    let encoder = unsafe { encoder.as_mut() };
+    let Some(encoder) = encoder else {
         return false;
     };
-    let Some(data) = slice_from_raw_parts(data, len).as_ref() else {
+    // SAFETY: caller guarantees `data` to point at `len` bytes of initialized memory.
+    let data = unsafe { slice_from_raw_parts(data, len).as_ref() };
+    let Some(data) = data else {
         return false;
     };
-    let Some(out) = encoder.compress(
-        data,
-        slice_from_raw_parts_mut((*out_data).cast(), *out_len).as_mut(),
-    ) else {
+
+    // SAFETY: caller guarantees that `out_data` and `out_len` are dereferenceable and either:
+    // - *out_data == ptr::null()
+    // - *out_data points at a memory region at least `*out_len` bytes.
+    let out_buf = unsafe { slice_from_raw_parts_mut(*out_data, *out_len).as_mut() };
+    let Some(out) = encoder.compress(data, out_buf) else {
         return false;
     };
-    out_data.write(out.as_ptr() as *mut u8);
-    out_len.write(out.len());
+    // SAFETY: caller guarantees that `out_data` and `out_len` are dereferenceable.
+    unsafe {
+        out_data.write(out.as_ptr() as *mut MaybeUninit<u8>);
+        out_len.write(out.len());
+    }
     true
 }
 
@@ -75,7 +90,9 @@ pub unsafe extern "C" fn RBrotliEncMaxRequiredSize(
     encoder: *mut RBrotliEncoder,
     in_size: usize,
 ) -> usize {
-    let Some(encoder) = encoder.as_mut() else {
+    // SAFETY: caller guarantees the pointer to be dereferenceable.
+    let encoder = unsafe { encoder.as_mut() };
+    let Some(encoder) = encoder else {
         return usize::MAX;
     };
     encoder.max_required_size(in_size)
@@ -88,7 +105,8 @@ pub unsafe extern "C" fn RBrotliEncMaxRequiredSize(
 /// freed yet.
 #[no_mangle]
 pub unsafe extern "C" fn RBrotliEncFreeEncoder(encoder: *mut RBrotliEncoder) {
-    drop(Box::from_raw(encoder));
+    // SAFETY: caller guarantees the pointer to be dereferenceable.
+    drop(unsafe { Box::from_raw(encoder) });
 }
 
 /// Returns true if this machine is supported by the encoder.

hugepage-buffer/src/lib.rs

@@ -42,22 +42,27 @@ unsafe fn allocate<T>(layout: Layout, zeroed: bool) -> *mut T {
             sysconf, MADV_HUGEPAGE, MAP_ANONYMOUS, MAP_FAILED, MAP_PRIVATE, PROT_READ, PROT_WRITE,
             _SC_PAGE_SIZE,
         };
-        let page_size = sysconf(_SC_PAGE_SIZE);
+        // SAFETY: `sysconf` is always safe.
+        let page_size = unsafe { sysconf(_SC_PAGE_SIZE) };
         assert!(page_size >= 0);
         let page_size = page_size as u64;
         const _: () = assert!(std::mem::size_of::<u64>() >= std::mem::size_of::<usize>());
         assert_eq!(page_size as u64 % layout.align() as u64, 0);
-        let mem = libc::mmap(
-            null_mut(),
-            layout.size(),
-            PROT_READ | PROT_WRITE,
-            MAP_PRIVATE | MAP_ANONYMOUS,
-            -1,
-            0,
-        );
-        libc::madvise(mem, layout.size(), MADV_HUGEPAGE);
+        // SAFETY: creating anonymous mappings is always safe.
+        let mem = unsafe {
+            libc::mmap(
+                null_mut(),
+                layout.size(),
+                PROT_READ | PROT_WRITE,
+                MAP_PRIVATE | MAP_ANONYMOUS,
+                -1,
+                0,
+            )
+        };
         assert_ne!(mem, MAP_FAILED);
-        // SAFETY: mmap guarantees that the returned pointer is aligned to a page size and points
+        // SAFETY: `madvise(MADV_HUGEPAGE)` is always safe.
+        unsafe { libc::madvise(mem, layout.size(), MADV_HUGEPAGE) };
+        // Safety note: mmap guarantees that the returned pointer is aligned to a page size and points
         // to an allocated region at least as long as its `len` argument. We check that the
         // required alignment is compatible with the page size before calling mmap.
         mem as *mut T
@@ -78,16 +83,23 @@ unsafe fn allocate<T>(layout: Layout, zeroed: bool) -> *mut T {
 ///
 /// Safety:
 /// - `ptr` must have been allocated with `allocate`, with the same `layout` passed to this
-///   function.
+///   function, and not have been deallocated yet.
 /// - The memory pointed by `ptr` must still be valid.
 unsafe fn deallocate<T>(ptr: *mut T, layout: Layout) {
     #[cfg(all(target_os = "linux", not(miri)))]
     {
         use libc::c_void;
-        libc::munmap(ptr as *mut c_void, layout.size());
+        // SAFETY: `ptr` comes from a call to `mmap` with the same size as passed to this call to
+        // `munmap`, and the memory was not unmapped before.
+        unsafe {
+            libc::munmap(ptr as *mut c_void, layout.size());
+        }
     }
     #[cfg(any(not(target_os = "linux"), miri))]
-    std::alloc::dealloc(ptr as *mut u8, layout)
+    // SAFETY: the memory was allocated with `std::alloc::alloc` and not deallocated yet.
+    unsafe {
+        std::alloc::dealloc(ptr as *mut u8, layout)
+    }
 }
 
 impl<T: Copy, const LEN: usize> BoxedHugePageArray<T, LEN> {

lsb-bitwriter/src/lib.rs

@@ -99,11 +99,18 @@ impl<'a> BitWriter<'a> {
         self.bit_buffer |= bits << self.bits_in_buffer;
         let shift = 64 - self.bits_in_buffer;
         self.bits_in_buffer += count;
-        copy_nonoverlapping(
-            (&mut self.bit_buffer.to_le()) as *mut u64 as *mut u8,
-            self.buf.as_mut_ptr().add(self.bytes_written).cast(),
-            8,
-        );
+        // SAFETY: It is always safe to bit-copy `u8`s to `MaybeUninit<u8>`s. `src` points to an
+        // 8-byte local buffer (as returned by `to_le_bytes`), and `dst` points to the buffer
+        // provided by the user when creating the BitWriter, which for sure does not overlap it.
+        // The caller guarantees at least `8` bytes are available in `self.buf` after
+        // `self.bytes_written`.
+        unsafe {
+            copy_nonoverlapping(
+                self.bit_buffer.to_le_bytes().as_ptr(),
+                self.buf.as_mut_ptr().add(self.bytes_written).cast(),
+                8,
+            );
+        }
         if self.bits_in_buffer >= 64 {
             self.bit_buffer = bits >> shift;
             self.bits_in_buffer -= 64;