Skip to content

Fix soundness: use raw pointers instead of erased lifetimes#64

Merged
somethingelseentirely merged 1 commit intomainfrom
claude/formal-verification-RfheN
Mar 17, 2026
Merged

Fix soundness: use raw pointers instead of erased lifetimes#64
somethingelseentirely merged 1 commit intomainfrom
claude/formal-verification-RfheN

Conversation

@somethingelseentirely
Copy link
Contributor

Summary

This PR fixes undefined behavior in Bytes and View by replacing lifetime-erased references (&'static [u8] and &'static T) with raw pointers (*const [u8] and *const T). The previous approach violated both Stacked Borrows and Tree Borrows when Bytes/View were passed by value to functions (including drop) while holding the last strong reference to their owner.

Key Changes

  • Changed internal storage: Bytes::data and View::data now use raw pointers instead of lifetime-erased references

    • Avoids "protected" borrow violations when the struct is passed by value and the owner is deallocated
    • Raw pointers are !Send + !Sync by default, but unsafe impl Send/Sync is justified since the Arc<dyn ByteOwner> guarantees thread-safe access
  • Removed erase_lifetime helper: No longer needed with raw pointer storage; lifetime erasure now happens implicitly via pointer casting

  • Updated all data access paths:

    • get_data() now returns a raw pointer; callers dereference it with unsafe { &*self.data }
    • set_data() accepts &[u8] (not &'static) and stores it as a raw pointer
    • data_ptr() new method returns the raw pointer directly for internal use
    • All view construction methods (view_prefix, view_suffix, etc.) updated to work with raw pointers
  • Added comprehensive Miri test suite (tests/miri.rs):

    • 36 tests covering all unsafe code paths: lifetime erasure, weak reference upgrades, try_unwrap_owner, view operations
    • Tests complex drop orderings, nested slices, and pointer validity across owner lifetime
    • Includes a helper script (scripts/miri.sh) that runs tests with Tree Borrows (the recommended borrow model)
  • Updated documentation:

    • Added comments explaining why raw pointers are used instead of references
    • Documented known Stacked Borrows limitation with Box<T> (Tree Borrows handles it correctly)
    • Updated CHANGELOG and INVENTORY with soundness fix details

Implementation Details

The core insight is that when Bytes is passed by value to a function and that function call is the last use of the struct, the borrow checker "protects" any &'static references as function arguments. If the Arc owner is dropped during that function (e.g., in drop), the protected reference becomes dangling—undefined behavior under both borrow models. Raw pointers bypass this protection mechanism while maintaining safety through the Arc owner's lifetime guarantee.

https://claude.ai/code/session_01Q8DdoMzpG97eRPDx6dTwhP

…el UB

The `data` fields in `Bytes` and `View` stored `&'static [u8]` and
`&'static T` references alongside their `Arc<dyn ByteOwner>` owner.
Under both Stacked Borrows and Tree Borrows, passing these types by
value to any function (including `std::mem::drop`) while holding the
last strong reference caused undefined behavior: the reference's
"protected" tag conflicted with the Arc deallocation of the owner.

This changes the fields to raw pointers (`*const [u8]` and `*const T`),
which do not receive protection under either borrow model. The
`erase_lifetime` helper is removed as it is no longer needed.

Also adds a Miri test suite (36 tests) covering all unsafe code paths:
lifetime erasure, weak reference upgrades, try_unwrap_owner pointer
reconstruction, view operations, and complex drop orderings.

https://claude.ai/code/session_01Q8DdoMzpG97eRPDx6dTwhP
@somethingelseentirely somethingelseentirely merged commit 854468f into main Mar 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants