Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BlobVec does not respect alignment for zero-sized types #6615

Closed
JoJoJet opened this issue Nov 14, 2022 · 0 comments
Closed

BlobVec does not respect alignment for zero-sized types #6615

JoJoJet opened this issue Nov 14, 2022 · 0 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior P-Unsound A bug that results in undefined compiler behavior

Comments

@JoJoJet
Copy link
Member

JoJoJet commented Nov 14, 2022

Bevy version

e48c05c

Problem

Like Vec, BlobVec uses a dangling pointer as the backing storage for zero-sized types. However, BlobVec does not respect alignment, which results in instant UB when using a ZST with alignment other than 1.

#[test]
fn aligned_zst() {
    #[derive(Component)]
    #[repr(align(32))]
    struct Zst;

    let mut world = World::default();
    world.spawn(Zst);
    world.spawn(Zst);
    world.spawn(Zst);

    let mut q = world.query::<&Zst>();
    for &Zst in q.iter(&world) {}
}

The offending line: it creates a dangling pointer which is well-aligned for u8, not for the type being stored.

Miri

error: Undefined Behavior: accessing memory with alignment 1, but alignment 32 is required
   --> C:\Users\joe10\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\slice\raw.rs:100:9
    |
100 |         &*ptr::slice_from_raw_parts(data, len)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory with alignment 1, but alignment 32 is required
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
    = note: inside `std::slice::from_raw_parts::<'_, std::cell::UnsafeCell<tests::aligned_zst::Zst>>` at C:\Users\joe10\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\slice\raw.rs:100:9
note: inside `storage::blob_vec::BlobVec::get_slice::<tests::aligned_zst::Zst>` at crates\bevy_ecs\src\storage\blob_vec.rs:309:9
   --> crates\bevy_ecs\src\storage\blob_vec.rs:309:9
    |
309 |         std::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell<T>, self.len)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `storage::table::Column::get_data_slice::<tests::aligned_zst::Zst>` at crates\bevy_ecs\src\storage\table.rs:191:9
   --> crates\bevy_ecs\src\storage\table.rs:191:9
    |
191 |         self.data.get_slice()
    |         ^^^^^^^^^^^^^^^^^^^^^
note: inside `<&tests::aligned_zst::Zst as query::fetch::WorldQuery>::set_table` at crates\bevy_ecs\src\query\fetch.rs:586:13
   --> crates\bevy_ecs\src\query\fetch.rs:586:13
    |
586 | /             table
587 | |                 .get_column(component_id)
588 | |                 .debug_checked_unwrap()
589 | |                 .get_data_slice()
    | |_________________________________^
note: inside `query::iter::QueryIterationCursor::<'_, '_, &tests::aligned_zst::Zst, ()>::next` at crates\bevy_ecs\src\query\iter.rs:595:21
   --> crates\bevy_ecs\src\query\iter.rs:595:21
    |
595 |                     Q::set_table(&mut self.fetch, &query_state.fetch_state, table);
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `<query::iter::QueryIter<'_, '_, &tests::aligned_zst::Zst, ()> as std::iter::Iterator>::next` at crates\bevy_ecs\src\query\iter.rs:53:13
   --> crates\bevy_ecs\src\query\iter.rs:53:13
    |
53  | /             self.cursor
54  | |                 .next(self.tables, self.archetypes, self.query_state)
    | |_____________________________________________________________________^
note: inside `tests::aligned_zst` at crates\bevy_ecs\src\lib.rs:87:21
   --> crates\bevy_ecs\src\lib.rs:87:21
    |
87  |         for &Zst in q.iter(&world) {}
    |                     ^^^^^^^^^^^^^^
note: inside closure at crates\bevy_ecs\src\lib.rs:76:5
   --> crates\bevy_ecs\src\lib.rs:76:5
    |
75  |       #[test]
    |       ------- in this procedural macro expansion
76  | /     fn aligned_zst() {
77  | |         #[derive(Component)]
78  | |         #[repr(align(32))]
79  | |         struct Zst;
...   |
87  | |         for &Zst in q.iter(&world) {}
88  | |     }
    | |_____^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

test tests::aligned_zst ... error: test failed, to rerun pass `-p bevy_ecs --lib`
@JoJoJet JoJoJet added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Nov 14, 2022
@james7132 james7132 added A-ECS Entities, components, systems, and events P-Unsound A bug that results in undefined compiler behavior and removed S-Needs-Triage This issue needs to be labelled labels Nov 14, 2022
@bors bors bot closed this as completed in 3ac06b5 Nov 14, 2022
cart pushed a commit that referenced this issue Nov 30, 2022
# Objective

Fixes #6615.

`BlobVec` does not respect alignment for zero-sized types, which results in UB whenever a ZST with alignment other than 1 is used in the world.

## Solution

Add the fn `bevy_ptr::dangling_with_align`.

---

## Changelog

+ Added the function `dangling_with_align` to `bevy_ptr`, which creates a well-aligned dangling pointer to a type whose alignment is not known at compile time.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
…e#6618)

# Objective

Fixes bevyengine#6615.

`BlobVec` does not respect alignment for zero-sized types, which results in UB whenever a ZST with alignment other than 1 is used in the world.

## Solution

Add the fn `bevy_ptr::dangling_with_align`.

---

## Changelog

+ Added the function `dangling_with_align` to `bevy_ptr`, which creates a well-aligned dangling pointer to a type whose alignment is not known at compile time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior P-Unsound A bug that results in undefined compiler behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants