From d2acfa82240d17fba6c31f1ac402a523cf35db52 Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 03:30:52 +0000 Subject: [PATCH 01/11] update UB dep --- crates/bevy_tasks/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_tasks/Cargo.toml b/crates/bevy_tasks/Cargo.toml index 555aad5b34055..1c3117c181500 100644 --- a/crates/bevy_tasks/Cargo.toml +++ b/crates/bevy_tasks/Cargo.toml @@ -10,7 +10,7 @@ keywords = ["bevy"] [dependencies] futures-lite = "1.4.0" -event-listener = "2.4.0" +event-listener = "2.5.2" async-executor = "1.3.0" async-channel = "1.4.2" num_cpus = "1.0.1" From 0a78960a9cefaca8fe5278ff6fab9e960faf354b Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 04:34:45 +0000 Subject: [PATCH 02/11] do not set thread name in miri --- crates/bevy_tasks/src/task_pool.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index 597ebc334c872..c82778c341e54 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -121,13 +121,22 @@ impl TaskPool { let ex = Arc::clone(&executor); let shutdown_rx = shutdown_rx.clone(); + #[cfg(not(miri))] let thread_name = if let Some(thread_name) = thread_name { format!("{} ({})", thread_name, i) } else { format!("TaskPool ({})", i) }; + #[cfg(miri)] + let _ = i; + #[cfg(miri)] + let _ = thread_name; + // miri does not support setting thread names `https://github.com/rust-lang/miri/issues/1717` + #[cfg(not(miri))] let mut thread_builder = thread::Builder::new().name(thread_name); + #[cfg(miri)] + let mut thread_builder = thread::Builder::new(); if let Some(stack_size) = stack_size { thread_builder = thread_builder.stack_size(stack_size); From 84b9aa8cd80834be03c93da0d66bac4e5b10abff Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 04:34:57 +0000 Subject: [PATCH 03/11] make test less big --- crates/bevy_ecs/src/lib.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index cb559641d6a16..f42e3e4e89a8e 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -637,8 +637,22 @@ mod tests { #[test] fn table_add_remove_many() { let mut world = World::default(); - let mut entities = Vec::with_capacity(10_000); - for _ in 0..1000 { + #[cfg(miri)] + let mut entities = Vec::with_capacity(10); + #[cfg(not(miri))] + let mut entities = Vec::with_capacity(1_000); + + let to; + #[cfg(miri)] + { + to = 10; + } + #[cfg(not(miri))] + { + to = 1000; + } + + for _ in 0..to { entities.push(world.spawn().insert(B(0)).id()); } From 80356445588d02fc75ecb5060d0ff893af9de6ec Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 05:43:15 +0000 Subject: [PATCH 04/11] Run in CI --- .github/workflows/ci.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c49157a7fd824..e5e1fe30cec40 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,6 +70,32 @@ jobs: # See tools/ci/src/main.rs for the commands this runs run: cargo run -p ci -- nonlocal + miri: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/cache@v2 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + key: ${{ runner.os }}-cargo-ci-${{ hashFiles('**/Cargo.toml') }} + - uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + components: miri + override: true + - name: Install alsa and udev + run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev libwayland-dev libxkbcommon-dev + - name: CI job + run: cargo miri test -p bevy_ecs + env: + RUSTFLAGS: -Zrandomize-layout + MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-tag-raw-pointers + check-benches: runs-on: ubuntu-latest needs: ci From daedf382a57dfe820dfc5e6b7d869148aa47627e Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 24 Mar 2022 13:12:01 +0000 Subject: [PATCH 05/11] make comment a TODO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: François --- crates/bevy_tasks/src/task_pool.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index c82778c341e54..0166d597d078c 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -132,7 +132,8 @@ impl TaskPool { #[cfg(miri)] let _ = thread_name; - // miri does not support setting thread names `https://github.com/rust-lang/miri/issues/1717` + // miri does not support setting thread names + // TODO: change back when https://github.com/rust-lang/miri/issues/1717 is fixed #[cfg(not(miri))] let mut thread_builder = thread::Builder::new().name(thread_name); #[cfg(miri)] From 6e3f611e81f84b68ddf4a4edf5da85fb20ed0562 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 24 Mar 2022 13:12:41 +0000 Subject: [PATCH 06/11] squish code together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: François --- crates/bevy_ecs/src/lib.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index f42e3e4e89a8e..f58deb2a0babf 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -637,20 +637,16 @@ mod tests { #[test] fn table_add_remove_many() { let mut world = World::default(); - #[cfg(miri)] - let mut entities = Vec::with_capacity(10); - #[cfg(not(miri))] - let mut entities = Vec::with_capacity(1_000); - - let to; - #[cfg(miri)] - { - to = 10; - } - #[cfg(not(miri))] - { - to = 1000; - } + #[cfg(miri)] + let (mut entities, to) = { + let to = 10; + (Vec::with_capacity(to), to) + }; + #[cfg(not(miri))] + let (mut entities, to) = { + let to = 10_000; + (Vec::with_capacity(to), to) + }; for _ in 0..to { entities.push(world.spawn().insert(B(0)).id()); From a0bd9bb545aabdba634839b978536691cbf858a4 Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 13:18:34 +0000 Subject: [PATCH 07/11] comments --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5e1fe30cec40..ad6e21bcdf55a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,7 +93,13 @@ jobs: - name: CI job run: cargo miri test -p bevy_ecs env: + # -Zrandomize-layout makes sure we dont rely on the layout of anything that might change RUSTFLAGS: -Zrandomize-layout + # -Zmiri-disable-isolation is needed because our executor uses `fastrand` which accesses system time. + # -Zmiri-ignore-leaks is needed because running bevy_ecs tests finds a memory leak but its impossible + # to track down because allocids are nondeterministic. + # -Zmiri-tag-raw-pointers is not strictly "necessary" but enables a lot of extra UB checks relating + # to raw pointer aliasing rules that we should be tryign to uphold. MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-tag-raw-pointers check-benches: From f52f30e6dff92ce61b14a1bdebf34fcf225aee36 Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 13:20:58 +0000 Subject: [PATCH 08/11] bors dot toml --- .github/bors.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/bors.toml b/.github/bors.toml index 4e8d0332954d4..7d7911af1b40f 100644 --- a/.github/bors.toml +++ b/.github/bors.toml @@ -13,6 +13,7 @@ status = [ "check-missing-examples-in-docs", "check-unused-dependencies", "ci", + "miri", "check-benches", ] From ba6d95a4eb787e05ef0f6e282a36f40150c4aace Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 13:27:28 +0000 Subject: [PATCH 09/11] SOMEONE :eyes: --- crates/bevy_ecs/src/lib.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index f58deb2a0babf..1183e71cd6e39 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -637,16 +637,16 @@ mod tests { #[test] fn table_add_remove_many() { let mut world = World::default(); - #[cfg(miri)] - let (mut entities, to) = { - let to = 10; - (Vec::with_capacity(to), to) - }; - #[cfg(not(miri))] - let (mut entities, to) = { - let to = 10_000; - (Vec::with_capacity(to), to) - }; + #[cfg(miri)] + let (mut entities, to) = { + let to = 10; + (Vec::with_capacity(to), to) + }; + #[cfg(not(miri))] + let (mut entities, to) = { + let to = 10_000; + (Vec::with_capacity(to), to) + }; for _ in 0..to { entities.push(world.spawn().insert(B(0)).id()); From fb3b8cf49ac3832256fc29baa51343758b25724b Mon Sep 17 00:00:00 2001 From: Ellen Date: Thu, 24 Mar 2022 23:52:35 +0000 Subject: [PATCH 10/11] reviews --- .github/workflows/ci.yml | 2 +- crates/bevy_ecs/src/lib.rs | 8 ++++++++ crates/bevy_tasks/src/task_pool.rs | 26 +++++++++++++------------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad6e21bcdf55a..05382f5ff7a69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,7 +99,7 @@ jobs: # -Zmiri-ignore-leaks is needed because running bevy_ecs tests finds a memory leak but its impossible # to track down because allocids are nondeterministic. # -Zmiri-tag-raw-pointers is not strictly "necessary" but enables a lot of extra UB checks relating - # to raw pointer aliasing rules that we should be tryign to uphold. + # to raw pointer aliasing rules that we should be trying to uphold. MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-tag-raw-pointers check-benches: diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 1183e71cd6e39..3e36c82ef9a02 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1614,3 +1614,11 @@ mod tests { ); } } + +#[test] +fn ub() { + #[allow(deref_nullptr)] + unsafe { + *std::ptr::null::() + }; +} diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index 0166d597d078c..ebd6ba6b41f4c 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -121,23 +121,23 @@ impl TaskPool { let ex = Arc::clone(&executor); let shutdown_rx = shutdown_rx.clone(); - #[cfg(not(miri))] - let thread_name = if let Some(thread_name) = thread_name { - format!("{} ({})", thread_name, i) - } else { - format!("TaskPool ({})", i) - }; - #[cfg(miri)] - let _ = i; - #[cfg(miri)] - let _ = thread_name; - // miri does not support setting thread names // TODO: change back when https://github.com/rust-lang/miri/issues/1717 is fixed #[cfg(not(miri))] - let mut thread_builder = thread::Builder::new().name(thread_name); + let mut thread_builder = { + let thread_name = if let Some(thread_name) = thread_name { + format!("{} ({})", thread_name, i) + } else { + format!("TaskPool ({})", i) + }; + thread::Builder::new().name(thread_name) + }; #[cfg(miri)] - let mut thread_builder = thread::Builder::new(); + let mut thread_builder = { + let _ = i; + let _ = thread_name; + thread::Builder::new() + }; if let Some(stack_size) = stack_size { thread_builder = thread_builder.stack_size(stack_size); From f0df0ed867b00962e914b402ce815e90ceb0e03b Mon Sep 17 00:00:00 2001 From: Ellen Date: Fri, 25 Mar 2022 00:08:22 +0000 Subject: [PATCH 11/11] remove ub --- crates/bevy_ecs/src/lib.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 3e36c82ef9a02..1183e71cd6e39 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1614,11 +1614,3 @@ mod tests { ); } } - -#[test] -fn ub() { - #[allow(deref_nullptr)] - unsafe { - *std::ptr::null::() - }; -}