Raw pointer moved when crossing ffi boundaries?

[chrome linden]

I have been struggling with a bug where some trait object containing a OnceCell worked fine, but when returned from a cdylib, caused STATUS_HEAP_CORRUPTION.
I proceeded to put on github a small project that replicated the issue as closely to the actual bug I had as possible while being minimal enough.
Then by playing a bit with it, I think I figured out the problem, or at least, a fix that works, but I would like to be sure that I understood properly.

Here is the example repo: https://github.com/Samuel-B-D/dyn-plugin-test/tree/bugged

On the bugged branch, running cargo run --example hello_world works just fine.
Running cargo build -p dyn-plugin-test | cargo run --example hello_world --features="dynamic" output

Successfully created App
Loaded handler constructor
Created Handler at: 0x2576da2f4e0
Raw handler ptr at: 0x2576da2f4e0
Dynamically created handler at: 0xcf1dbbf2a8
Re-boxed handler at: 0xcf1dbbf2a8

So it seems that the object get moved when crossing the ffi boundary?

Wrapping my trait object's raw pointer into a wrapper structure, Box::into_raw()ing that wrapper, sending that between the cdylib and retrieving it in a Box::from_raw, the contained pointer is indeed still the same, and the memory appear to be untouched and accessible, as shown in the fixed branch.

(I'm pretty new to FFI and dynamic libraries in Rust. Most of the information required to put that together come from https://michael-f-bryan.github.io/rust-ffi-guide/dynamic_loading.html, although in my case I'm dynamically loading a Rust library compiled as a C dynamic library, instead of a C++ one. The extra indirection doesn't seem to be required when loading a C/C++ library instead?)

My questions are the following:

1. Did I properly understand the issue?
2. Is this a correct / reliable way to handle this?

[wise basalt]

It looks like you're returning *mut dyn Handler from _create_handler(). Fat pointers, including trait-object pointers, are not FFI-safe types, and the compiler generates a warning outside of macros:

?play warn=true

use std::fmt::Debug;
#[allow(dead_code)]
extern "C" fn test() -> *mut dyn Debug { &mut () }

[thorn hedge]

warning: `extern` fn uses type `dyn Debug`, which is not FFI-safe
 --> src/main.rs:4:25
  |
4 | extern "C" fn test() -> *mut dyn Debug { &mut () }
  |                         ^^^^^^^^^^^^^^ not FFI-safe
  |
  = note: trait objects have no C equivalent
  = note: `#[warn(improper_ctypes_definitions)]` on by default

     Running `target/debug/playground`

[wise basalt]

So you're most likely running into the unintuitive results of UB.

There are two solutions for this, if you need to use dynamic dispatch: either box the trait object again, creating a *mut Box<dyn Handler>, or if you want to avoid the extra indirection, create your own vtable type, as described in https://adventures.michaelfbryan.com/posts/ffi-safe-polymorphism-in-rust/.

[chrome linden]

So that means that the current "fix" is also UB considering a struct with a dyn trait object pointer field is probably not FFI-safe either.

theoretically then, storing the trait object's pointer into a c_void pointer in my wrapper and then typing it properly on the receiving side should work too then? 🤔

[wise basalt]

No, converting a *mut dyn Handler to a *mut c_void would remove the vtable pointer.

(Note that a *mut dyn Handler is not FFI-safe since it's a fat pointer, but a *mut Box<dyn Handler> is, since it's a thin pointer.)

(Also, Box<T> itself is FFI-safe if T is sized, so even a fn _create_handler() -> Box<Box<dyn Handler>> would be legal.)

[chrome linden]

So *mut dyn Handler and Box<dyn Handler> are not safe but *mut Box<dyn Handler> or Box<Box<dyn Handler>> are. Interesting.
And imagine that unboxing the double-boxed one on the receiving end would also make me lose my vtable?

[wise basalt]

No, the unboxing can be done entirely safely and soundly

[chrome linden]

I mean doing it once and then storing just a Box<dyn Handler> and not going through 2 indirection on each events

[wise basalt]

That's fine, the double indirection (or manual vtable) is only needed when you're transferring it through an extern "C" function

[chrome linden]

Mhm.
So basically my "fix" by wrapping it in a tuple struct was basically the same as boxing it again

[wise basalt]

Well, that fix doesn't really work, since the tuple struct still contains the fat pointer by value

[chrome linden]

I see

I'll just box it then

[wise basalt]

?play

use std::fmt::Debug;
extern "C" fn create_handler() -> Box<Box<dyn Debug>> { Box::new(Box::new("Hello") as Box<dyn Debug>) }
let handler: Box<dyn Debug> = *create_handler();
println!("{handler:?}");

[thorn hedge]

     Running `target/debug/playground`

"Hello"

[chrome linden]

I'll read more into your article, considering there must be a reason for the manual vtable stuff, which sounded like it was just to avoid double-indirection, but there must be more to it, since the boxing is just way simpler and more elegant

Wouldn't the double-boxing method still be UB if the dynamic library and the program were compiled with different rust versions since the dynamic trait object ABI is unstable?

Although as long as both are compiled together it'll always be correct I guess

[wise basalt]

Yes, they'd have to be compiled with the same compiler version

The only way to avoid that restriction would be to only use stable-layout types like integers, pointers, and repr(C) types across the boundary

[chrome linden]

Thanks for all the clarification!

[wise basalt]

Yep, I wish they'd just stabilize the layout of fat pointers, but they're worried that people will take that as permission to start trying to read the memory behind the vtable pointer

(which is UB and you shouldn't do it)

[chrome linden]

That would simplify building plugin systems though!
Although for anything that's does not require the absolute most performance a plugin system should probably be using wasmtime instead ;p

[wise basalt]

Yeah, in that case the recommendation is to make your own vtable struct

[chrome linden]

Yea, I see from the article that it's not too bad to implement and allow you to expose a stable plugin interface