ignore() is potentially unsafe

[mitsuhiko]

I believe that the implementation of ignore in miniserde is unsafe. It casts a global static into a mutable reference which means there will be multiple references alive at once. At the same time it’s a zero sized type which can give out multiple addresses so I’m not sure how this works. I made a minimal repo example and miri complains:

use std::mem::transmute;

trait Magic {
    fn do_magic(&mut self);
}

struct Vodoo;

impl Magic for Vodoo {
    fn do_magic(&mut self) {
        println!("interesting");
    }
}

fn vodoo() -> &'static mut dyn Magic {
    unsafe {
        transmute::<_, &mut Vodoo>(&mut Vodoo)
    }
}

fn main() {
    vodoo().do_magic();
}

Produces this:

error: Undefined Behavior: type validation failed: encountered a dangling reference (use-after-free)
  [--> src/main.rs:22:5
](https://play.rust-lang.org/#)   |
22 |     vodoo().do_magic();
   |     ^^^^^^^ type validation failed: encountered a dangling reference (use-after-free)
   |
   = 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

[mitsuhiko]

I changed the comment as I’m not entirely sure if it’s indeed unsafe or if miri is wrong here.

[dtolnay]

It casts a global static into a mutable reference which means there will be multiple references alive at once

That doesn't sound at all like what miri is saying. The miri message "encountered a dangling reference", which is about Vodoo being on fn vodoo's stack instead of in a static. That would be fixed by changing it to a static. In general multiple ZST references at the same address is not an issue. For example let [a, b] = &mut [(); 2] has two mutable references a and b which have identical pointer value.

It does sound like miri is being overzealous here given that this is both zero sized and never dereferenced (either of which independently could be sufficient for this to be well defined). It seems fine though to change it to a static.

[mitsuhiko]

Indeed. It turns out that this is permitted. I will open an issue against miri.

//EDIT: I filed the issue here: rust-lang/miri#1972

[dtolnay]

Fixed by #26 + some further notes in b149874.

[RalfJung]

It does sound like miri is being overzealous here given that this is both zero sized and never dereferenced (either of which independently could be sufficient for this to be well defined).

References must not be dangling even if they are never used, and even zero-sized accesses are subject to some restrictions:

Even for operations of size zero, the pointer must not be pointing to deallocated memory, i.e., deallocation makes pointers invalid even for zero-sized operations. However, casting any non-zero integer literal to a pointer is valid for zero-sized accesses, even if some memory happens to exist at that address and gets deallocated. This corresponds to writing your own allocator: allocating zero-sized objects is not very hard. The canonical way to obtain a pointer that is valid for zero-sized accesses is NonNull::dangling.

We sadly have to require this due to the rules LLVM imposes on getelementptr inbounds.