Skip to content

Fix from raw parts - #25277

Open
yilin0518 wants to merge 1 commit into
bevyengine:mainfrom
yilin0518:fix_from_raw_parts
Open

Fix from raw parts#25277
yilin0518 wants to merge 1 commit into
bevyengine:mainfrom
yilin0518:fix_from_raw_parts

Conversation

@yilin0518

Copy link
Copy Markdown
Contributor

Objective

AlignedVec::from_raw_parts's safety documentation has a contract gap: when capacity == 0, it only said ptr "need only be aligned" and never required align itself to be a non-zero power of two. This means a caller can construct an AlignedVec that satisfies the documented contract literally (e.g. align = 3, capacity = 0), yet later trigger undefined behavior through purely safe method calls (e.g. push), because internal methods like layout() unconditionally build a Layout via Layout::from_size_align_unchecked(self.cap, self.align), which requires align to be a non-zero power of two regardless of capacity.

The old docs were also imprecise about:

  • there being no upper bound stated on align independent of allocation
  • what "capacity ... needs to be the same size" meant when the pointer isn't required to be allocated
  • what exact byte alignment ptr must satisfy when capacity == 0

Solution

Rewrote the # Safety section of from_raw_parts to state the invariants unconditionally and precisely:

  • align must be a non-zero power of two, and less than isize::MAX — regardless of capacity, since align is used to build a Layout even when the vector currently has zero capacity.
  • capacity, rounded up to the nearest multiple of align, must not overflow isize.
  • If capacity != 0, ptr must have been allocated with a Layout using this exact align and a size of capacity (merging the two previously-separate, overlapping clauses about matching align/size on dealloc).
  • If capacity == 0, ptr need not point to allocated memory, but must still satisfy ptr.as_ptr() as usize % align == 0 (replacing the ambiguous "need only be aligned").

No functional code changed — this is a documentation-only fix to close the public unsafe contract gap.

Testing

  • Verified the change compiles: cargo doc -p bevy_platform --no-deps passes, and the existing doctest on from_raw_parts still passes.
  • To confirm the gap in the original docs was real (not just a theoretical wording nitpick), I built a standalone reproduction outside this repo:
use bevy_platform::collections::AlignedVec;
use core::ptr::NonNull;

fn test_align_not_power_of_two() {
    unsafe {
        let ptr = NonNull::new(3usize as *mut u8).unwrap();
        let mut vec = AlignedVec::from_raw_parts(ptr, 3, 0, 0);
        // Triggers UB: Layout::from_size_align_unchecked(new_cap, 3)
        // requires align to be power of 2.
        vec.push(1);
    }
}

fn main() {
    test_align_not_power_of_two();
}

I use the command cargo +nightly-x86_64-pc-windows-gnu miri run to test the above code, and it trigger the unsafe precondition(s) violation:

thread 'main' (1) panicked at D:\projects\bevy\crates\bevy_platform\src\collections\aligned_vec.rs:227:30:
unsafe precondition(s) violated: Layout::from_size_align_unchecked requires that align is a power of 2 and the rounded-up allocation size does not exceed isize::MAX

This indicates a bug in the program. This Undefined Behavior check is optional, and cannot be relied on for safety.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
thread caused non-unwinding panic. aborting.

@yilin0518
yilin0518 force-pushed the fix_from_raw_parts branch from dcc101c to 57a2a6d Compare August 3, 2026 11:46
@yilin0518
yilin0518 force-pushed the fix_from_raw_parts branch from 57a2a6d to a898242 Compare August 3, 2026 11:59
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