pinned_init vs. miri?

pinned_init vs. miri?

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

Hi all,

tl;dr: while trying to learn more about pinned_init, I tried running a
simple example through miri, and I found out that it breaks.

The attached Rust code mimics a C component that calls a series of
callbacks with a void * opaque argument; plus Rust bindings for it,
that use pin_init!() to place a pointer to self in the opaque
argument.

The use of pin_init! is pretty simple, the only notable thing is that
in main() I "inlined" Box::pin_init() to clarify the miri output.

When I try to run this under miri, I get the following:

error: Undefined Behavior: trying to retag from <1750> for SharedReadOnly
permission at alloc877[0x0], but that tag does not exist in the borrow stack
for this location
   --> src/main.rs:25:37
    |
25  |         let owner: &'a T = unsafe { &*opaque.cast_const().cast::<T>() };
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                                     |
    |                                     trying to retag from <1750>
for SharedReadOnly
permission at alloc877[0x0], but that tag does not exist in the borrow stack
for this location
    |                                     this error occurs as part of
retag at alloc877[0x0..0x20]
    |
help: <1750> was created by a SharedReadWrite retag at offsets [0x0..0x20]
   --> src/main.rs:99:16
    |
99  |     let slot = backend.as_mut_ptr();
    |                ^^^^^^^^^^^^^^^^^^^^
help: <1750> was later invalidated at offsets [0x0..0x20] by a Unique retag
   --> src/main.rs:101:32
    |
101 |     let mut backend = unsafe { backend.assume_init() };
    |                                ^^^^^^^


In other words, the pointer to the Box<MaybeUninit<PrintlnLogger>>,
which was stored to be used later as the opaque pointer, became
invalid when assume_init() changed the box to a Box<PrintlnLogger>. It
is then invalid to dereference it.

The cause of this is basically that assume_init() consumes the
MaybeUninit<>; which also means that the same thing happens with
std::mem::forget(), Box::leak() or Box::into_raw().

This is not _that_ surprising in the end, but I was wondering if it is
a fundamental incompatibility between the stacked borrows model and
interior pointers, or if there are ways to fix it (I can't think of
any).

To test the attached pinned_init.rs, stick it as src/main.rs with a
Cargo.toml file like this:

[package]
name = "logger"
version = "0.1.0"
edition = "2021"

[dependencies]
pinned-init = "0.0"

and compile/run it with nightly Rust.

Thanks,

Paolo

[-- Attachment #2: pinned_init.rs --]
[-- Type: text/rust, Size: 2867 bytes --]

use std::mem::MaybeUninit;
use std::os::raw::c_void;

use pinned_init::*;

/// A callback to a logging function `f`.
/// This would ordinarily be provided by C code.
pub struct LogRegistration {
    f: fn(&str, *mut c_void),
    opaque: *mut c_void,
}

/// The Rust trait that is implemented by a logging backend
/// It is wrapped by [`LogRegistration::log_fn`] to convert
/// `&self` into an opaque pointer.
pub trait LogBackendOps<'a> {
    fn log(&self, s: &str);
}

impl LogRegistration {
    fn log_fn<'a, T: 'a + LogBackendOps<'a>>(s: &str, opaque: *mut c_void) {
        // miri failure here.  The pointer was created by
        // backend.as_mut_ptr(), but then invalidated by
        // backend.assume_init()
        let owner: &'a T = unsafe { &*opaque.cast_const().cast::<T>() };
        owner.log(s);
    }

    pub fn new<'a, T: 'a + LogBackendOps<'a>>(
        owner: *const T,
    ) -> impl Init<Self> + use<'a, T> {
        init!(LogRegistration {
            f: Self::log_fn::<T>,
            opaque: owner.cast_mut().cast(),
        })
    }
}

/// A hypothetical logging component that accepts multiple backends in
/// the form of callbacks.  This would ordinarily be provided by C code.
#[derive(Default)]
pub struct Logger<'a> {
    vec: Vec<&'a LogRegistration>,
}

impl<'a> Logger<'a> {
    pub fn add_callback(&mut self, cb: &'a LogRegistration) {
        self.vec.push(cb);
    }

    pub fn log(&self, s: &str) {
        for cb in &self.vec {
            (cb.f)(s, cb.opaque);
        }
    }
}

// ----------------------------------------

/// Rust client for `Logger`.  Uses `pin_init!` to construct a
/// callback+opaque pair that points to itself and that can be
/// added to a `Logger`.
#[repr(C)]
#[pin_data]
pub struct PrintlnLogger {
    #[pin]
    backend: LogRegistration,
    pub prefix: &'static str,
}

impl LogBackendOps<'_> for PrintlnLogger {
    fn log(&self, s: &str) {
        println!("{}{}", self.prefix, s);
    }
}

impl PrintlnLogger {
    pub fn new() -> impl PinInit<Self> {
        pin_init!(&this in PrintlnLogger {
            backend <- LogRegistration::new::<Self>(this.as_ptr()),
            prefix: "test",
        })
    }

    pub fn add_to<'a>(&'a self, l: &mut Logger<'a>) {
        l.add_callback(&self.backend);
    }
}

// ----------------------------------------

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut l = Logger::default();

    // let mut backend = Box::pin_init(PrintlnLogger::new())?;
    //
    // expanded as follows for clearer miri output:
    let mut backend: Box<MaybeUninit<PrintlnLogger>> = Box::new_uninit();
    let slot = backend.as_mut_ptr();
    unsafe { PrintlnLogger::new().__pinned_init(slot)? };
    let mut backend = unsafe { backend.assume_init() };

    backend.prefix = "test: ";
    backend.add_to(&mut l);
    l.log("hello world");
    Ok(())
}

Re: pinned_init vs. miri?

[Alice Ryhl]

On Tue, Nov 5, 2024 at 1:50 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Hi all,
>
> tl;dr: while trying to learn more about pinned_init, I tried running a
> simple example through miri, and I found out that it breaks.
>
> The attached Rust code mimics a C component that calls a series of
> callbacks with a void * opaque argument; plus Rust bindings for it,
> that use pin_init!() to place a pointer to self in the opaque
> argument.
>
> The use of pin_init! is pretty simple, the only notable thing is that
> in main() I "inlined" Box::pin_init() to clarify the miri output.
>
> When I try to run this under miri, I get the following:
>
> error: Undefined Behavior: trying to retag from <1750> for SharedReadOnly
> permission at alloc877[0x0], but that tag does not exist in the borrow stack
> for this location
>    --> src/main.rs:25:37
>     |
> 25  |         let owner: &'a T = unsafe { &*opaque.cast_const().cast::<T>() };
>     |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     |                                     |
>     |                                     trying to retag from <1750>
> for SharedReadOnly
> permission at alloc877[0x0], but that tag does not exist in the borrow stack
> for this location
>     |                                     this error occurs as part of
> retag at alloc877[0x0..0x20]
>     |
> help: <1750> was created by a SharedReadWrite retag at offsets [0x0..0x20]
>    --> src/main.rs:99:16
>     |
> 99  |     let slot = backend.as_mut_ptr();
>     |                ^^^^^^^^^^^^^^^^^^^^
> help: <1750> was later invalidated at offsets [0x0..0x20] by a Unique retag
>    --> src/main.rs:101:32
>     |
> 101 |     let mut backend = unsafe { backend.assume_init() };
>     |                                ^^^^^^^
>
>
> In other words, the pointer to the Box<MaybeUninit<PrintlnLogger>>,
> which was stored to be used later as the opaque pointer, became
> invalid when assume_init() changed the box to a Box<PrintlnLogger>. It
> is then invalid to dereference it.
>
> The cause of this is basically that assume_init() consumes the
> MaybeUninit<>; which also means that the same thing happens with
> std::mem::forget(), Box::leak() or Box::into_raw().
>
> This is not _that_ surprising in the end, but I was wondering if it is
> a fundamental incompatibility between the stacked borrows model and
> interior pointers, or if there are ways to fix it (I can't think of
> any).
>
> To test the attached pinned_init.rs, stick it as src/main.rs with a
> Cargo.toml file like this:
>
> [package]
> name = "logger"
> version = "0.1.0"
> edition = "2021"
>
> [dependencies]
> pinned-init = "0.0"
>
> and compile/run it with nightly Rust.
>
> Thanks,
>
> Paolo

You need to mark the type `!Unpin` so that `Box`/`&mut` are not
considered unique, which avoids breaking the provenance.

Alice