In alloc_size, ensure the calculated final allocation size satisfies size <= (isize::MAX as usize) - (alloc_align::<T>() - 1). Alternatively, replace Layout::from_size_align_unchecked with safe Layout::from_size_align(size, align) and unwrap/expect the Result.

Unsafe Rust Review: thin_vec (v0_2)

Overall Safety Assessment

The thin_vec crate provides a memory-efficient vector implementation (ThinVec<T>) that stores its length and capacity metadata directly inside the heap buffer allocation, allowing empty instances and Option<ThinVec<T>> to occupy only a single pointer width. The crate also supports FFI bridging with Gecko's nsTArray C++ class. Overall, the core architecture and pointer manipulation algorithms (including complex slicing, draining, extracting, and splicing iterators) demonstrate strong familiarity with Rust semantics and exception-safety design principles (such as leak amplification). However, we identified one Critical soundness vulnerability in layout size calculation where requesting capacities near isize::MAX triggers Undefined Behavior in Layout::from_size_align_unchecked. Additionally, the crate exhibits two Fishy findings (pointer identity comparison of static singletons across dynamic libraries and crate-wide suppression of safety documentation lints) and a very high density of undocumented unsafe blocks and functions.

Critical Findings

Soundness vulnerability in layout calculation on near-isize::MAX capacities 🔴 🚨

• Severity: 🔴 High
• Threat Vector: 🚨 Untrusted Input
• Bug Type: Integer Overflow

File: src/lib.rs (also src/lib.rs:386-410)

Description: When ThinVec::with_capacity (or reserve, reserve_exact) is called, layout<T>(cap) computes the allocation layout:

fn layout<T>(cap: usize) -> Layout {
    unsafe { Layout::from_size_align_unchecked(alloc_size::<T>(cap), alloc_align::<T>()) }
}

The helper alloc_size<T>(cap) computes the total allocation size in bytes as data_size + header_size + padding. To guard against overflow, alloc_size converts values to isize and uses checked_add/checked_mul, ensuring final_size <= isize::MAX. However, Layout::from_size_align_unchecked(size, align) requires as a fundamental safety contract that size, when rounded up to the nearest multiple of align, must not overflow isize::MAX (i.e., size <= (isize::MAX as usize) - (align - 1)). When size is close to isize::MAX
and not aligned to alloc_align::<T>() (for example, size = isize::MAX and align = 8), rounding up adds padding that causes the integer to overflow isize::MAX into negative isize::MIN. Passing invalid arguments to Layout::from_size_align_unchecked violates its safety contract and triggers immediate Undefined Behavior. Furthermore, standard Vec::with_capacity guarantees a panic if the requested allocation exceeds isize::MAX bytes.

UB Scenario: If a user requests ThinVec::<u8>::with_capacity(isize::MAX as usize - 16), alloc_size returns isize::MAX. Layout::from_size_align_unchecked(isize::MAX, 8) is invoked, violating std safety preconditions and triggering UB before allocation occurs.

Recommended Fix: In alloc_size, ensure the calculated allocation size satisfies size <= (isize::MAX as usize) - (alloc_align::<T>() - 1). Alternatively, use safe Layout::from_size_align(size, align) and unwrap the result.

Fishy Findings

1. Pointer identity comparison of static EMPTY_HEADER breaks across Rust dynamic library boundaries 🟠 🤦

• Severity: 🟠 Medium
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: Invalid Deallocation

File: src/lib.rs

Description: To determine whether a ThinVec points to the shared empty singleton rather than a heap allocation, is_singleton compares pointer identities:

    fn is_singleton(&self) -> bool {
        unsafe { self.ptr.as_ptr() as *const Header == &EMPTY_HEADER }
    }

In non-gecko mode, EMPTY_HEADER is defined as a standard Rust static. If thin_vec is linked across dynamic library boundaries (e.g. a main application binary and a dylib plugin both embedding thin_vec), each binary artifact receives its own instance of EMPTY_HEADER at a distinct memory address. If an empty ThinVec created in dylib A is passed to dylib B, is_singleton() in dylib B evaluates to false. Consequently, has_allocation() evaluates to true, and when the vector is dropped, ThinVec::drop attempts to call std::alloc::dealloc on the static symbol address of dylib A's EMPTY_HEADER, leading to allocator corruption / abort. (Note that in gecko-ffi mode, this is explicitly avoided by linking against Gecko's exported sEmptyTArrayHeader symbol).

2. Crate-wide suppression of clippy::missing_safety_doc masking undocumented unsafe contracts 🟡 🤦

• Severity: 🟡 Low
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: Lint Suppression

File: src/lib.rs

Description: At the root of src/lib.rs, the crate specifies #![allow(..., clippy::missing_safety_doc)]. This crate-wide lint suppression masks the complete omission of # Safety documentation across numerous internal and public unsafe fn definitions (header_mut, set_len_non_singleton, reallocate, shrink_to_fit_known_singleton, fill, move_tail). Under rigorous safety review principles (unsafe-rust-review-joshlf), every unsafe fn must explicitly state its preconditions in a # Safety doc comment so callers can discharge their proof obligations.

Missing Safety Comments

Virtually all unsafe blocks, trait implementations, and functions in the crate lack // SAFETY: comments or # Safety docstrings. Below are key locations and rigorous proposed proof comments:

1. layout unchecked constructor (src/lib.rs:441) 🔴

    // SAFETY: `alloc_size::<T>(cap)` computes a non-zero size that (with recommended overflow checks)
    // does not exceed `isize::MAX - (align - 1)`. `alloc_align::<T>()` returns a valid power-of-two alignment.
    unsafe { Layout::from_size_align_unchecked(alloc_size::<T>(cap), alloc_align::<T>()) }

2. header_with_capacity allocation (src/lib.rs:451) 🔴

    // SAFETY:
    // - `layout` is valid and non-zero.
    // - `alloc(layout)` returns either null or an aligned pointer valid for writes of `Header`.
    // - If null, `handle_alloc_error` diverges. Thus `header` is valid and non-null for `NonNull::new_unchecked`.
    unsafe {
        let layout = layout::<T>(cap);
        let header = alloc(layout) as *mut Header;
        ...
    }

3. Sync and Send unsafe trait impls (src/lib.rs:483-484) 🔴

// SAFETY: `ThinVec<T>` uniquely owns its underlying heap buffer and elements (or points to a static immutable singleton).
// Sharing `&ThinVec<T>` across threads only permits immutable access to `&T`, which is safe if `T: Sync`.
unsafe impl<T: Sync> Sync for ThinVec<T> {}

// SAFETY: Transferring `ThinVec<T>` across threads transfers ownership of `T` across threads, safe if `T: Send`.
unsafe impl<T: Send> Send for ThinVec<T> {}

4. header and data_raw pointer derivation (src/lib.rs:630, src/lib.rs:663) 🟡

    fn header(&self) -> &Header {
        // SAFETY: `self.ptr` is either a valid heap allocation or `EMPTY_HEADER`. Both are non-null and properly aligned.
        unsafe { self.ptr.as_ref() }
    }

        // SAFETY:
        // - If empty singleton and header unaligned, returns `NonNull::dangling()`.
        // - Otherwise, `self.ptr` points to an allocation of at least `header_size + padding` bytes.
        // - Offset addition remains within allocation bounds, yielding an aligned pointer to element storage.
        unsafe { ... }

5. header_mut, set_len_non_singleton, reallocate unsafe functions (src/lib.rs:677, 825, 1743) 🟡

These functions require # Safety docstrings explaining their exact preconditions:

    /// # Safety
    /// `self` must not point to the read-only static `EMPTY_HEADER` (`!self.is_singleton()`).
    unsafe fn header_mut(&mut self) -> &mut Header {

    /// # Safety
    /// - `self` must not point to `EMPTY_HEADER`.
    /// - `len` must be <= `capacity()`, and elements `0..len` must be initialized.
    unsafe fn set_len_non_singleton(&mut self, len: usize) {

    /// # Safety
    /// `new_cap` must be > 0 and resulting layout must not overflow `isize::MAX`.
    unsafe fn reallocate(&mut self, new_cap: usize) {

6. Collection mutations (push_unchecked, pop, insert, remove, swap_remove, truncate, clear) (src/lib.rs:866, 892, 923, 959, 999, 1056, 1083) 🔴

All element reads, writes, copies, and drops inside unsafe blocks require safety comments proving in-bounds access and initialization:

        unsafe {
            // SAFETY: `old_len > 0`, so vector is non-singleton. Decrementing length is within bounds.
            self.set_len_non_singleton(old_len - 1);
            // SAFETY: Element at `old_len - 1` was initialized and is now excluded from length, preventing double-drop.
            Some(ptr::read(self.data_raw().add(old_len - 1)))
        }

7. Slice extraction (as_slice, as_mut_slice, IntoIter::as_slice) (src/lib.rs:1112, 1128, 2485) 🔴

        // SAFETY: `data_raw()` returns a non-null aligned pointer. Elements `0..len()` are initialized and contiguous.
        unsafe { slice::from_raw_parts(self.data_raw(), self.len()) }

8. Iterators (IntoIter, Drain, ExtractIf, Splice) (src/lib.rs:2518, 2697, 2958, 3015, 3045) 🔴

All pointer reads and backshifts inside iterator stepping and drop cleanup require safety comments demonstrating adherence to ownership and aliasing rules.