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

This crate contains data races and ptr-int transmutes, both are UB #2

Open
saethlin opened this issue Apr 16, 2022 · 0 comments
Open

Comments

@saethlin
Copy link

I've noticed that the tests for a few ecosystem crates fail due to problems in this crate, so I figured you might want to know. The data races can be reproduced with

MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-track-alloc-id=64338" cargo miri test

I've attached the output below.

This crate also relies on transmutes between pointers and integers. These are extremely problematic for a language like Rust which does not have typed memory. If we permit such transmutes in the language, we have to treat all copies of memory as if they could be copying a transmuted pointer, and at that point you basically lose all memory-related optimizations. Therefore, such operations are not officially UB, but I cannot imagine a world in which they would be permitted. Miri also detects these with:

MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-check-number-validity" cargo miri test

The pointer-usize transmutes remind me of the code that used to be in std::sync::mpsc. I don't have the time this moment to write up a PR for this crate, but if anyone else is interested in fixing this, my PR that removed these transmutes from std::sync::mpsc might be helpful: rust-lang/rust#95621

test barrier_clone ... note: tracking was triggered
 --> /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:89:14
  |
89  |     unsafe { __rust_alloc(layout.size(), layout.align()) }
  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ created allocation with id 64338
  |
  = note: inside `std::alloc::alloc` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:89:14
  = note: inside `std::alloc::Global::alloc_impl` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:171:73
  = note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:231:9
  = note: inside `alloc::alloc::exchange_malloc` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:320:11
  = note: inside `std::boxed::Box::<pulse::Inner>::new` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/boxed.rs:200:9
note: inside `pulse::Signal::new` at /tmp/pulse-0.5.3/src/lib.rs:264:21
 --> /tmp/pulse-0.5.3/src/lib.rs:264:21
  |
264 |           let inner = Box::new(Inner {
  |  _____________________^
265 | |             state: AtomicUsize::new(2),
266 | |             waiting: Atom::empty(),
267 | |         });
  | |__________^
note: inside `barrier_clone` at tests/barrier.rs:122:18
 --> tests/barrier.rs:122:18
  |
122 |     let (p, t) = Signal::new();
  |                  ^^^^^^^^^^^^^
note: inside closure at tests/barrier.rs:121:1
 --> tests/barrier.rs:121:1
  |
120 |   #[test]
  |   ------- in this procedural macro expansion
121 | / fn barrier_clone() {
122 | |     let (p, t) = Signal::new();
123 | |     let p1 = p.clone();
124 | |     let join = thread::spawn(move || {
...   |
131 | |     join.join().unwrap();
132 | | }
  | |_^
  = note: this note originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: thread support is experimental and incomplete: weak memory effects are not emulated.

error: Undefined Behavior: Data race detected between Deallocate on Thread(id = 1) and Atomic Store on Thread(id = 0, name = "main") at alloc64338 (current vector clock = VClock([30, 27]), conflicting timestamp = VClock([32, 25]))
 --> /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:107:14
  |
107 |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between Deallocate on Thread(id = 1) and Atomic Store on Thread(id = 0, name = "main") at alloc64338 (current vector clock = VClock([30, 27]), conflicting timestamp = VClock([32, 25]))
  |
  = 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: inside `std::alloc::dealloc` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:107:14
  = note: inside `<std::alloc::Global as std::alloc::Allocator>::deallocate` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:244:22
  = note: inside `alloc::alloc::box_free::<pulse::Inner, std::alloc::Global>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/alloc.rs:342:9
  = note: inside `std::ptr::drop_in_place::<std::boxed::Box<pulse::Inner>> - shim(Some(std::boxed::Box<pulse::Inner>))` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:486:1
  = note: inside `std::mem::drop::<std::boxed::Box<pulse::Inner>>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/mem/mod.rs:974:24
note: inside `pulse::delete_inner` at /tmp/pulse-0.5.3/src/lib.rs:160:9
 --> /tmp/pulse-0.5.3/src/lib.rs:160:9
  |
160 |         drop(inner);
  |         ^^^^^^^^^^^
note: inside `<pulse::Signal as std::ops::Drop>::drop` at /tmp/pulse-0.5.3/src/lib.rs:257:9
 --> /tmp/pulse-0.5.3/src/lib.rs:257:9
  |
257 |         delete_inner(flag, self.inner);
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  = note: inside `std::ptr::drop_in_place::<pulse::Signal> - shim(Some(pulse::Signal))` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:486:1
note: inside `<pulse::ThreadScheduler as pulse::Scheduler>::wait` at /tmp/pulse-0.5.3/src/lib.rs:517:5
 --> /tmp/pulse-0.5.3/src/lib.rs:517:5
  |
517 |     }
  |     ^
note: inside `pulse::Signal::wait` at /tmp/pulse-0.5.3/src/lib.rs:364:27
 --> /tmp/pulse-0.5.3/src/lib.rs:364:27
  |
364 |                 let res = s.wait(self);
  |                           ^^^^^^^^^^^^
note: inside closure at tests/barrier.rs:125:9
 --> tests/barrier.rs:125:9
  |
125 |         p1.wait().unwrap();
  |         ^^^^^^^^^

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

error: aborting due to previous error; 2 warnings emitted

error: test failed, to rerun pass '--test barrier'
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

No branches or pull requests

1 participant