Undefined behaviour inside library

[adrian-budau-ogreai]

miri reports lots undefined behaviour, and some are easy to confirm.

For example

running 13 tests
test kanal_tests::tests::mpmc_0 ... error: Undefined Behavior: trying to retag from <272286> for SharedReadWrite permission at alloc112011[0x10], but that tag does no
t exist in the borrow stack for this location
   --> src/signal.rs:294:34
    |
294 |             Signal::Sync(sig) => (**sig).recv(),
    |                                  ^^^^^^^^^^^^^^
    |                                  |
    |                                  trying to retag from <272286> for SharedReadWrite permission at alloc112011[0x10], but that tag does not exist in the borrow st
ack for this location
    |                                  this error occurs as part of retag at alloc112011[0x0..0x18]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows 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
help: <272286> was created by a SharedReadWrite retag at offsets [0x0..0x18]
   --> src/signal.rs:184:22
    |
184 |         Signal::Sync(self as *mut Self)
    |                      ^^^^
help: <272286> was later invalidated at offsets [0x10..0x11] by a write access
   --> src/state.rs:119:9
    |
119 | /         self.v
120 | |             .compare_exchange(
121 | |                 LOCKED,
122 | |                 LOCKED_STARVATION,
123 | |                 Ordering::SeqCst,
124 | |                 Ordering::Relaxed,
125 | |             )
    | |_____________^
    = note: BACKTRACE:
    = note: inside `signal::Signal::<i32>::recv` at src/signal.rs:294:34
note: inside `Receiver::<i32>::recv` at src/lib.rs:430:25
   --> src/lib.rs:430:25
    |
430 |             unsafe { Ok(p.recv()) }
    |                         ^^^^^^^^
note: inside closure at src/kanal_tests.rs:41:36
   --> src/kanal_tests.rs:41:36
    |
41  |                         assert_eq!(rx.recv().unwrap(), 1);
    |                                    ^^^^^^^^^

Which is easy to confirm:
One thread has a Send which will lock an Internal which will then try to register (sometimes) a SyncSignal which will push a *mut SyncSignal in a list.

Thread 1 still has a mutable live access to this SyncSignal (let's name it sig). From it's point of view no one else can modify it, or it's inner fields, even creating a &mut for this same object is undefined behaviour in rust. This also is valid for it's State field which has an atomic. Having a &mut reference (even to an atomic) guarantees to the compiler that the memory pointed to can not be changed by anyone.

Thread 2 (the receiver) will pop this *mut SyncSignal from the list and dereference the pointer to call recv on it (that dereference will already introduce undefined behaviour by virtue of having 2 &mut references to sig from 2 threads at the same time).

You might argue that rust rules are too restrictive, but you also end up doing a write while these 2 mutable references are live. The first thread (sender), while waiting will write it's state with self.state.upgrade_lock() effectively introducing a race condition.

[JakkuSakura]

#3 Another UB. Analyzed by hand

[fereidani]

SyncSignal is const reference now. if you think it's fixed feel free to close the thread.

[adrian-budau]

That part is fixed, I've also added a PR that fixes some other sync related issues: #4
With it all sync tests no longer trigger any UB through miri.

I'll do async separetely since it it's harder to grok through miri's error messages there.

[saethlin]

I'll do async separetely since it it's harder to grok through miri's error messages there.

Do you have any examples? I just pulled the latest commit and all I see is a straightforward aliasing issue.

[fereidani]

There is no known undefined behavior in the library anymore, Miri reports no error on both sync and async, feel free to reopen the issue if you found something new.