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

chore: test more features with Miri #4397

Merged
merged 14 commits into from
Feb 9, 2022
Merged

chore: test more features with Miri #4397

merged 14 commits into from
Feb 9, 2022

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Jan 12, 2022

depend on #4396.

Thanks to rust-lang/miri#1952, more tests can be run with Miri.

The following tests still do not work.

@taiki-e taiki-e added the A-ci Area: The continuous integration setup label Jan 12, 2022
@taiki-e
Copy link
Member Author

taiki-e commented Jan 12, 2022

stacked borrows violation with -Zmiri-tag-raw-pointers:

MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-tag-raw-pointers" cargo miri test -p tokio --features full --lib -- runtime::tests::queue::overflow
output
error: Undefined Behavior: trying to reborrow for SharedReadWrite at alloc4551509+0x30, but parent tag <13554726> does not have an appropriate item in the borrow stack
   --> /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:327:18
    |
327 |         unsafe { &*self.as_ptr() }
    |                  ^^^^^^^^^^^^^^^ trying to reborrow for SharedReadWrite at alloc4551509+0x30, but parent tag <13554726> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
            
    = note: inside `std::ptr::NonNull::<runtime::task::core::Cell<std::future::from_generator::GenFuture<[static generator@tokio/src/runtime/tests/queue.rs:29:46: 29:48]>, runtime::blocking::schedule::NoopSchedule>>::as_ref` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:327:18
note: inside `runtime::task::harness::Harness::<std::future::from_generator::GenFuture<[static generator@tokio/src/runtime/tests/queue.rs:29:46: 29:48]>, runtime::blocking::schedule::NoopSchedule>::trailer` at tokio/src/runtime/task/harness.rs:34:19
   --> tokio/src/runtime/task/harness.rs:34:19
    |
34  |         unsafe { &self.cell.as_ref().trailer }
    |                   ^^^^^^^^^^^^^^^^^^
note: inside `runtime::task::harness::Harness::<std::future::from_generator::GenFuture<[static generator@tokio/src/runtime/tests/queue.rs:29:46: 29:48]>, runtime::blocking::schedule::NoopSchedule>::dealloc` at tokio/src/runtime/task/harness.rs:148:9
   --> tokio/src/runtime/task/harness.rs:148:9
    |
148 |         self.trailer().waker.with_mut(drop);
    |         ^^^^^^^^^^^^^^
note: inside `runtime::task::raw::dealloc::<std::future::from_generator::GenFuture<[static generator@tokio/src/runtime/tests/queue.rs:29:46: 29:48]>, runtime::blocking::schedule::NoopSchedule>` at tokio/src/runtime/task/raw.rs:118:5
   --> tokio/src/runtime/task/raw.rs:118:5
    |
118 |     harness.dealloc();
    |     ^^^^^^^^^^^^^^^^^
note: inside `runtime::task::raw::RawTask::dealloc` at tokio/src/runtime/task/raw.rs:76:13
   --> tokio/src/runtime/task/raw.rs:76:13
    |
76  |             (vtable.dealloc)(self.ptr);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `<runtime::task::Task<runtime::blocking::schedule::NoopSchedule> as std::ops::Drop>::drop` at tokio/src/runtime/task/mod.rs:394:13
   --> tokio/src/runtime/task/mod.rs:394:13
    |
394 |             self.raw.dealloc();
    |             ^^^^^^^^^^^^^^^^^^
    = note: inside `std::ptr::drop_in_place::<runtime::task::Task<runtime::blocking::schedule::NoopSchedule>> - shim(Some(runtime::task::Task<runtime::blocking::schedule::NoopSchedule>))` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<runtime::task::Notified<runtime::blocking::schedule::NoopSchedule>> - shim(Some(runtime::task::Notified<runtime::blocking::schedule::NoopSchedule>))` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<std::option::Option<runtime::task::Notified<runtime::blocking::schedule::NoopSchedule>>> - shim(Some(std::option::Option<runtime::task::Notified<runtime::blocking::schedule::NoopSchedule>>))` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
note: inside `runtime::tests::queue::overflow` at tokio/src/runtime/tests/queue.rs:35:32
   --> tokio/src/runtime/tests/queue.rs:35:32
    |
35  |     while inject.pop().is_some() {
    |                                ^
note: inside closure at tokio/src/runtime/tests/queue.rs:24:1
   --> tokio/src/runtime/tests/queue.rs:24:1
    |
23  |   #[test]
    |   ------- in this procedural macro expansion
24  | / fn overflow() {
25  | |     let (_, mut local) = queue::local();
26  | |     let inject = Inject::new();
27  | |
...   |
43  | |     assert_eq!(n, 257);
44  | | }
    | |_^
    = note: inside `<[closure@tokio/src/runtime/tests/queue.rs:24:1: 44:2] as std::ops::FnOnce<()>>::call_once - shim` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `test::__rust_begin_short_backtrace::<fn()>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:574:5
    = note: inside closure at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:565:30
    = note: inside `<[closure@test::run_test::{closure#1}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send> as std::ops::FnOnce<()>>::call_once` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1854:9
    = note: inside `<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>> as std::ops::FnOnce<()>>::call_once` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:271:9
    = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<(), std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside `test::run_test_in_process` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:597:18
    = note: inside closure at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:491:39
    = note: inside `test::run_test::run_test_inner` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:529:13
    = note: inside `test::run_test` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:561:28
    = note: inside `test::run_tests::<[closure@test::run_tests_console::{closure#2}]>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:304:17
    = note: inside `test::run_tests_console` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/console.rs:290:5
    = note: inside `test::test_main` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:115:15
    = note: inside `test::test_main_static` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:134:5
    = note: inside `main`
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:123:18
    = note: inside closure at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:145:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside closure at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside `std::rt::lang_start_internal` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:20
    = note: inside `std::rt::lang_start::<()>` at /home/runner/.rustup/toolchains/nightly-2022-01-12-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:144:17
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

If run without -Zmiri-tag-raw-pointers, the test succeeds.

@taiki-e
Copy link
Member Author

taiki-e commented Jan 12, 2022

Hopefully, I know how to fix that SB violation.

Nah, it seems so complicated...

@github-actions github-actions bot added the R-loom Run loom tests on this PR label Jan 28, 2022
run: cargo miri test --features rt,rt-multi-thread,sync task
# Many of tests in tokio/tests and doctests use #[tokio::test] or
# #[tokio::main] that calls epoll_create1 that Miri does not support.
run: cargo miri test --features fs,io-util,io-std,net,parking_lot,rt,rt-multi-thread,sync --lib --no-fail-fast
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is opt-ing out of some things that use tokio::sync::Notified. Getting intrusive linked lists on the stack to pass with miri will probably be difficult.

Copy link
Contributor

@Darksonn Darksonn Jan 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe not .. it seems like the problem was elsewhere entirely.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to fix the timer, but Notified works now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, I figured it out: The issue for timers was futures::executor::block_on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue for timers was futures::executor::block_on.

Ah, futures has a few unreleased fixes of stacked borrows violation with -Zmiri-tag-raw-pointers: rust-lang/futures-rs#2550

@taiki-e taiki-e changed the title [WIP] chore: test more features with Miri chore: test more features with Miri Jan 29, 2022
@taiki-e taiki-e marked this pull request as ready for review January 29, 2022 04:37
@Darksonn Darksonn requested a review from hawkw January 30, 2022 11:49
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as far as my understanding of stacked borrows goes, this looks legit. i didn't notice any serious issues with these changes.

i commented on a couple of minor style nits. also, i did notice that in some cases, some safety invariants seem to have changed, now that we access all linked list pointers via pointer casts. it would be nice if this was added to the existing safety documentation.

tokio/src/runtime/tests/queue.rs Show resolved Hide resolved
tokio/src/sync/notify.rs Show resolved Hide resolved
tokio/src/time/driver/entry.rs Show resolved Hide resolved
tokio/src/signal/registry.rs Show resolved Hide resolved
tokio/src/util/slab.rs Outdated Show resolved Hide resolved
tokio/src/runtime/task/waker.rs Show resolved Hide resolved
@Darksonn Darksonn merged commit 1be8e9d into master Feb 9, 2022
@Darksonn Darksonn deleted the taiki-e/miri branch February 9, 2022 10:11
hawkw added a commit that referenced this pull request Feb 15, 2022
# 1.16.2 (February 15, 2022)

This release updates the minimum supported Rust version (MSRV) to 1.49,
the `mio` dependency to v0.8, and the (optional) `parking_lot`
dependency to v0.12. Additionally, it contains several bug fixes, as
well as internal refactoring and performance improvements.

### Fixed

- time: prevent panicking in `sleep` with large durations ([#4495])
- time: eliminate potential panics in `Instant` arithmetic on platforms
  where `Instant::now` is not monotonic ([#4461])
- io: fix `DuplexStream` not participating in cooperative yielding
  ([#4478])
- rt: fix potential double panic when dropping a `JoinHandle` ([#4430])

### Changed

- update minimum supported Rust version to 1.49 ([#4457])
- update `parking_lot` dependency to v0.12.0 ([#4459])
- update `mio` dependency to v0.8 ([#4449])
- rt: remove an unnecessary lock in the blocking pool ([#4436])
- rt: remove an unnecessary enum in the basic scheduler ([#4462])
- time: use bit manipulation instead of modulo to improve performance
  ([#4480])
- net: use `std::future::Ready` instead of our own `Ready` future
  ([#4271])
- replace deprecated `atomic::spin_loop_hint` with `hint::spin_loop`
  ([#4491])
- fix miri failures in intrusive linked lists ([#4397])

### Documented

- io: add an example for `tokio::process::ChildStdin` ([#4479])

### Unstable

The following changes only apply when building with `--cfg
tokio_unstable`:

- task: fix missing location information in `tracing` spans generated by
  `spawn_local` ([#4483])
- task: add `JoinSet` for managing sets of tasks ([#4335])
- metrics: fix compilation error on MIPS ([#4475])
- metrics: fix compilation error on arm32v7 ([#4453])

[#4495]: #4495
[#4461]: #4461
[#4478]: #4478
[#4430]: #4430
[#4457]: #4457
[#4459]: #4459
[#4449]: #4449
[#4462]: #4462
[#4436]: #4436
[#4480]: #4480
[#4271]: #4271
[#4491]: #4491
[#4397]: #4397
[#4479]: #4479
[#4483]: #4483
[#4335]: #4335
[#4475]: #4475
[#4453]: #4453
hawkw added a commit that referenced this pull request Feb 16, 2022
# 1.17.0 (February 16, 2022)

This release updates the minimum supported Rust version (MSRV) to 1.49,
the `mio` dependency to v0.8, and the (optional) `parking_lot`
dependency to v0.12. Additionally, it contains several bug fixes, as
well as internal refactoring and performance improvements.

### Fixed

- time: prevent panicking in `sleep` with large durations ([#4495])
- time: eliminate potential panics in `Instant` arithmetic on platforms
  where `Instant::now` is not monotonic ([#4461])
- io: fix `DuplexStream` not participating in cooperative yielding
  ([#4478])
- rt: fix potential double panic when dropping a `JoinHandle` ([#4430])

### Changed

- update minimum supported Rust version to 1.49 ([#4457])
- update `parking_lot` dependency to v0.12.0 ([#4459])
- update `mio` dependency to v0.8 ([#4449])
- rt: remove an unnecessary lock in the blocking pool ([#4436])
- rt: remove an unnecessary enum in the basic scheduler ([#4462])
- time: use bit manipulation instead of modulo to improve performance
  ([#4480])
- net: use `std::future::Ready` instead of our own `Ready` future
  ([#4271])
- replace deprecated `atomic::spin_loop_hint` with `hint::spin_loop`
  ([#4491])
- fix miri failures in intrusive linked lists ([#4397])

### Documented

- io: add an example for `tokio::process::ChildStdin` ([#4479])

### Unstable

The following changes only apply when building with `--cfg
tokio_unstable`:

- task: fix missing location information in `tracing` spans generated by
  `spawn_local` ([#4483])
- task: add `JoinSet` for managing sets of tasks ([#4335])
- metrics: fix compilation error on MIPS ([#4475])
- metrics: fix compilation error on arm32v7 ([#4453])

[#4495]: #4495
[#4461]: #4461
[#4478]: #4478
[#4430]: #4430
[#4457]: #4457
[#4459]: #4459
[#4449]: #4449
[#4462]: #4462
[#4436]: #4436
[#4480]: #4480
[#4271]: #4271
[#4491]: #4491
[#4397]: #4397
[#4479]: #4479
[#4483]: #4483
[#4335]: #4335
[#4475]: #4475
[#4453]: #4453
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ci Area: The continuous integration setup R-loom Run loom tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants