Unsound `logger` function

[Ddystopia]

Hello,

having multiple references with at least one of them being &mut to the same memory is considered undefined behavior in Rust.

Code like this:

fn main() {
    let r1 = delog::logger();
    let r2 = delog::logger();

    core::hint::black_box((r1, r2));
}

Is absolutely safe and using public interface, yet triggering UB.

Miri output:

Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
WARNING: Ignoring `RUSTC_WRAPPER` environment variable, Miri does not support wrapping.
   Compiling delog v0.1.7
   Compiling log v0.4.20
   Compiling delog_ub v0.1.0 (/home/ddystopia/code/delog_ub)
    Finished dev [unoptimized + debuginfo] target(s) in 0.18s
     Running `/home/ddystopia/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner /home/ddystopia/code/delog_ub/target/miri/x86_64-unknown-linux-gnu/debug/delog_ub`
error: Undefined Behavior: trying to retag from <1689> for Unique permission at alloc824[0x0], but that tag does not exist in the borrow stack for this location
 --> src/main.rs:5:28
  |
5 |     core::hint::black_box((r1, r2));
  |                            ^^
  |                            |
  |                            trying to retag from <1689> for Unique permission at alloc824[0x0], but that tag does not exist in the borrow stack for this location
  |                            this error occurs as part of retag at alloc824[0x0..0x10]
  |
  = 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: <1689> was created by a Unique retag at offsets [0x0..0x10]
 --> src/main.rs:2:14
  |
2 |     let r1 = delog::logger();
  |              ^^^^^^^^^^^^^^^
help: <1689> was later invalidated at offsets [0x0..0x10] by a Unique retag
 --> src/main.rs:3:14
  |
3 |     let r2 = delog::logger();
  |              ^^^^^^^^^^^^^^^
  = note: BACKTRACE (of the first span):
  = note: inside `main` at src/main.rs:5:28: 5:30

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

error: aborting due to 1 previous error

[sosthene-nitrokey]

Yes, this is not the only possible unsoundness of this crate. Technically this should only be used for debugging purposes and disabled for production use.

If we have the bandwidth we might consider a refactor that gets rid of these.

[Ddystopia]

Yes, this is not the only possible unsoundness of this crate. Technically this should only be used for debugging purposes and disabled for production use.

If we have the bandwidth we might consider a refactor that gets rid of these.

Yes, this is not the only possible unsoundness of this crate... That crate gets mutable references to logger everytime itself, and when you have multiple threads (or executions contexts in general, like interrupts), that library can obtain 2 exclusive references to logger (already ub) and cause data races (ub too).

We debugged and confirmed that and removed delog from every part of our codebase.

[Ddystopia]

This is exactly what unsafe impl Sync for ... and unsafe impl Send for ... are meant to warn about. That threading will not allow any ub, including having 2 &mut references to a piece of memory or some type at the same time and data races.