getditto/safer_ffi

Cannot safely convert from &std::string::String to &safer_ffi::String

TheButlah opened this issue · 3 comments

context: I have a &String or &mut String, and want to return them over ffi. I think I should not use &str or &mut str because it actually complicates my API (since my API is generic such that given a T it returns an &T), and because conceptually I may want the ability to string.reserve() and similar.

With other T's, I solved this by implementing From<&RustType> for &FfiType, where FfIType was something like this:

#[derive_ReprC]
#[ReprC::opaque]
pub struct FfiType {
    inner: RustType,
}

Note: I don't derive on the inner type because A: not all of them are my types, and B: I want to prefix the name of the FFI type with the mangled rust namespace name, and C: I need to manually monomorphize any generics in the rust type.

I then use dtolnay's ref-cast crate to manually implement (using a declarative macro) the From<&RustType> for &FfiType (as well as the &mut version).

I was hoping to skip my new-typing directly and just use safer_ffi::String, and impl From<&std::string::String> for &safer_ffi::String. This obviously wont work as it violates the orphan rule.

I looked into the implementation of safer_ffi's string, and it looks like it might not be possible to simply ref-cast or safely transmute it.

What is the best solution to my problem? Am I correct in that I cannot safely transmute the types? Should I rethink not using &mut str?

Perhaps since I only expose references to std::string::String, safer-ffi could implement #[ReprC::opaque] on it automatically, circumventing the orphan rule?

safer-ffi could implement #[ReprC::opaque] on it

🤔 That's a very interesting idea indeed! I'll "sleep on it", but implementing that trait for most stdlib types seeems quite sensible.

In that case, I could also offer a convenience generic type for people to "get it" for the others, especially considering the aforementioned ref-cast macro:

#[deriveReprC]
#[repr(opaque)] // <- new syntax, on `master`, not yet released to crates.io
#[derive(RefCast)]
struct FfiOpaque<T>(T);

so that you could cast a &String to a &FfiOpaque<String>, which would be FFI-safe (modulo the "generic opaque type name clashes" from that other issue you opened, but which ought to be solved on master).


Back to the original question, indeed casting a &String to a safer_ffi::String cannot be done directly, precisely because the concrete layout of a String is "undefined". So the usual workaround is offering a scoped API, which I did offer for a &mut String ~> &mut safer_ffi::String "conversion"; but I didn't think of a need for &Strings, so there is no API for it yet.

It would boil down to:

fn with_ffi_ref<R> (
    s: &String,
    scope: impl FnOnce(&safer_ffi::String) -> R,
) -> R
{
    let ffi_string_copy = unsafe {
        let copy: String = <*const _>::read(s); // double free danger-zone!
        let ffi_copy: safer_ffi::String = copy.into(); // still in double-free danger zone
        ::core::mem::ManuallyDrop::new(ffi_copy) // defuse the glue: we're fine!
    };
    scope(&ffi_string_copy)
}

// example:
let s: &String = …;
with_ffi_ref(s, |s: &safer_ffi::String| {});
  • (you could also return an opaque impl '_ + Deref<Target = safer_ffi::String> to avoid the scoped API, at the cost of having to sometimes explicitly use &** on it to get a &safer_ffi::String)

In case it is helpful, the solution I used in the meantime while the design in safer_ffi is workshopped is this:

use derive_more::{From, Into};
use safer_ffi::prelude::*;

#[derive_ReprC]
#[ReprC::opaque]
#[repr(transparent)]
#[derive(ref_cast::RefCast, From, Into, Debug, Clone, Eq, PartialEq, Hash)]
pub struct String {
    inner: std::string::String,
}

and then I return my new-typed opaque String instead of safer_ffi::string::String or std::string::String everywhere in my public api.

Downside is I would have to manually implement any string functions I want to support calling from C on the newtype