Re: [RFC PATCH v5 0/8] Add Rust support, implement ARM PL011

[Manos Pitsidianakis]

On Thu, 25 Jul 2024 18:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On Thu, Jul 25, 2024 at 4:48 PM Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> > > pl011_receive (called by qemu_chr_fe_accept_input) creates a mutable
> > > reference that *overlaps* the lifetime of the outer reference created
> > > by pl011_read. This is undefined behavior. You're effectively writing:
> >
> > There is no overlap there, sorry. Once qemu_chr_fe_accept_input
> > returns, any references it created do not exist anymore.
>
> read     |-------------|
> receive      |-----|
>
> That's the overlap. Maybe you're thinking that the outer &mut "goes to
> sleep" and is reborn when qemu_chr_fe_accept_input() returns, but
> there's no such thing in the language.

There's no overlap, because the read scope is not live while receive is 
active. References are linear types, when you give `&mut sth` as an 
argument to a function call, you can model it in your mind as "giving 
the mutual reference to the function call" and "receiving it back" when 
it returns.

There'd be an overlap if qemu_chr_fe_accept_input got two mutual 
references of the same memory location, such as calling foo(&mut self, 
&mut self) with foo being `fn foo(a: &mut T, b: &mut T)`.

> You can do it within a function:

These examples you provide are not really equivalent.

> Honestly I don't see how I can provide a proof or explanation that is
> more definitive than miri. If some specific documentation or
> discussions gives you the perception that there is no undefined
> behavior, I'm happy to check them out and provide an explanation in
> that context. But otherwise, I don't think it's useful to keep
> debating *whether* it is undefined behavior.


Agreed! but thank you for taking the time to discuss this :)

Manos

PS: I will explain why your good/bad example behaves like it does:

> // MIRIFLAGS=-Zmiri-ignore-leaks  cargo +nightly miri run
> use std::mem::MaybeUninit;
>
> struct S {
>    me: *mut S,
>    them: *mut S
> }
>
> impl S {
>    pub fn chardev_receive(&mut self) {
>        println!("in chardev_receive with &mut");
>    }
>
>    pub fn memory_write_good(&mut self) {
>        println!("in memory_write_good, calling qemu_chr_fe_accept_input()");
>        qemu_chr_fe_accept_input_good(self);
>    }
>
>    pub fn memory_write_bad(&mut self) {
>        println!("in memory_write_bad, calling qemu_chr_fe_accept_input()");
>        qemu_chr_fe_accept_input_bad(self);
>    }
> }
>
> fn qemu_chr_fe_accept_input_good(c: &S) {
>    // you can still go from *mut to &mut in _another_ instance of struct S
>    (unsafe { &mut *c.them }).chardev_receive();

`them` is an uninitialized value, so you are dereferencing it and 
passing a mutable reference to it, while

> }
>
> fn qemu_chr_fe_accept_input_bad(c: &S) {
>    // you cannot go from *mut to &mut when it points to _this_ instance;
>    // creating the &mut that is passed to memory_write_bad() has
>    // effectively made that *mut unusable!
>    (unsafe { &mut *c.me }).chardev_receive();
> }

`me` is the same memory location as `c`, which is passed as read only 
(&S) here.

> fn main() {
>    let p: &mut MaybeUninit<S> = Box::leak(Box::new(MaybeUninit::uninit()));
>    let q: &mut MaybeUninit<S> = Box::leak(Box::new(MaybeUninit::uninit()));
>
>    unsafe {
>        let p_mut_ptr = p.as_mut_ptr();
>        let q_mut_ptr = q.as_mut_ptr();
>        *(&mut *p_mut_ptr) = S { me: p_mut_ptr, them: q_mut_ptr };
>        (&mut *p_mut_ptr).memory_write_bad();
>        (&mut *p_mut_ptr).memory_write_good();
>   }
> }