[Bug] The plugin example in the tutorial reports a WARNING: "Result<Plugin, String>" is not FFI-safe.

[xuchaoqian]

Stabby is a great crate, especially because it supports futures & trait objects. I have made a demo to show this.
See here: stabby-plugin-demo.

Now I get some weird issues. It seems that the implementation of "Result and Option" has some bugs now. @p-avital

Reproduction steps for "Result" issue:

1. git clone https://github.com/xuchaoqian/stabby-example.git
2. cd stabby-example/plugin-a && cargo build
3. then we will get the WARNING as the following:

Reproduction steps for "Option" issue:

if we change "Result<Plugin, String>" to "stabby::option::Option<Plugin>" like following:

#[stabby::stabby]
pub extern "C" fn my_text_editor_init_plugin(_host: Host) -> stabby::option::Option<Plugin> {
    stabby::option::Option::Some(Box::new(MyPlugin).into())
}

then we will get a similar WARNING:

[p-avital]

Hi,

Thanks for pointing out that issue.

rustc is concerned about the structure containing a Zero-Sized-Type (ZST) which replaces the usual tag for the union when a niche optimization is found.

Technically, this is not supported by the C ABI as ZSTs are illegal constructs in C. While this shouldn't cause issues due to the entire enum being non-ZST, it is possible that a poor implementation of the ABI may behave weirdly if it tries to decompose the type into its fields.

More worrying was that the previous implementation let rustc keep thinking of padding byte repurposed into determinants as padding bytes. Since reading/writing in padding bytes is UB, this was a big soundness hole in stabby.

Luckily, the solution to that issue also addresses yours: rustc now sees repr(stabby) enums as a blob of data with the appropriate size and alignment.

I'm still investigating some errors that miri complains about in unrelated aspects, in order to avoid publishing multiple ABI breaks back to back should one become necessary to fix the issues miri has with them.

[xuchaoqian]

Many thanks for your insightful response and great work on this project!
I will retry Stabby immediately if that PR gets merged.

[p-avital]

You can already give that version a try by specifying a git dependency:

stabby = {git="https://github.com/ZettaScaleLabs/stabby.git", branch="enum-improvements"}

I'll probably release it as 5.0.0 sometime this weekend :)

[xuchaoqian]

It works fine! All warnings have disappeared!

Now I have confidence to use Stabby in a real project :) I think Stabby is the best crate for ABI stability according to my preferences because of its clean and intuitive API design and especially good support for async functions.

I'm looking forward to the new release that resolves the UB issues.

[xuchaoqian]

I found that Stabby cannot be compiled in Rust 1.78 (stable channel). Perhaps this information can help you in addressing the UB issues.

Related comment: rust-lang/rust#121250 (comment)

[p-avital]

Ohhhh nooo...

I was very afraid of that. A few months ago stabby stopped compiling on nightly because they added a constraint to associated consts that wasn't here before.

Namely, if an interiorly mutable value were to be referenced in a const, things can break, and Rust ensures that this will never happen from 1.78 onwards by forbidding references to generics in consts.

This is only a problem if that referenced value is interiorly mutable, which stabby's vtables cannot be. However, the way to tell the compiler that something isn't internally mutable, a trait called core::marker::Freeze, has been left nightly-only.

This means that as of today, there is no known way for stabby to generate vtables that will compile on 1.78 without changing the model completely (which would in turn make it impossible to do multi-trait-objects)...

Now is the time for bargaining with the Core Team: either they revert the interior mutability check until core::marker::Freeze is stabilized, or (I would prefer), they stabilize it ASAP. In the meantime, I'll try coming up with new solutions, but they've really painted us into a corner here...

[p-avital]

Of note: if you use nightly, core::marker::Freeze is available and detected automatically by stabby, so you'll be able to use stabby and all of 1.78's features (and more if you'd like).

I'll start experimenting with potential alternatives in the meantime; but having already fought this issue in the past, I can't promise I'll find a solution that works on stable

[p-avital]

Hiya @xuchaoqian,

Just letting you know that if you cargo update, you'll find the latest commit of the branch supports 1.78.

Note that this workaround is way costlier than the implements in <1.78 or nightly, as the later were able to just reference the vtables from the binary, while the workaround essentially copies them in a global set.

The current implementation of the workaround is "trivial": the set is implemented as an unordered vector, where:

• Lookup is performed through iteration.
• If lookup fails, the set is copied in a new vector, which is then itself hidden behind a pointer such that the set can be replaced atomically.

I'll try to make a better implementation for this thread-safe global set by the release this weekend. I'm thinking of an atomic BTreeMap which would make lookups latencies more predictable and reduce the risk of insertion conflicts in concurrent situations.

[xuchaoqian]

Oh, I really appreciate your efforts in solving this problem.

If the cost is too expensive, I'll just use the nightly until core::marker::Freeze is stabilized. I'll test this on my use case.

[p-avital]

Great :)

I'd really appreciate any feedback you can provide on the performance :)

I'll make a new issue to track this 1.78 breakage, as this issue has crept out of scope completely :)