Attempt to correct wasm32 ABI in bindgen

[devsnek]

So rust has this issue where it doesn't match the calling convention of clang when targeting wasm32. The only difference appears to be when passing large heterogeneous aggregate types, where clang will indirect the value but rustc will still pass it by value.

Hacking a fix together in rustc would be possible (see rust-lang/rust#79998), but it was proposed that as long as we are doing hacks, why not do the hack here in bindgen. In particular, when emitting fn foo(val: T), bindgen can instead emit &T for the argument.

The relevant logic is present in rustc here (https://github.com/rust-lang/rust/blob/fa416394275d2468d104b8f72ac31b1ddf7ee52e/compiler/rustc_target/src/abi/call/wasm32.rs#L32-L41) but I'm not sure how I would port it to bindgen. I got this far at least to verify that the stragety works:

index 0d93c491..ee3c588d 100644
--- a/src/codegen/mod.rs
+++ b/src/codegen/mod.rs
@@ -3497,7 +3497,12 @@ impl TryToRustTy for Type {
                 {
                     Ok(ty)
                 } else {
-                    utils::build_path(item, ctx)
+                    let path = utils::build_path(item, ctx)?;
+                    if ctx.is_target_wasm32_not_emscripten() /* && check_size_here */ {
+                        Ok(quote!(&#path))
+                    } else {
+                        Ok(path)
+                    }
                 }
             }
             TypeKind::Comp(ref info) => {

[briansmith]

The only difference appears to be when passing large heterogeneous aggregate types, where clang will indirect the value but rustc will still pass it by value.

Consider:

Rust:

#[repr(C)]
struct Foo {
    a: u32,
    b: u8
}

extern "C" {
     fn foo_c(foo: Foo);
}

#[wasm_bindgen]
fn foo() {
    let foo = Foo { a: 1, b: 2 };
    unsafe { foo_c(foo); }
}

C:

struct Foo {
    uint32_t a;
    uint8_t b;
};

fn foo_c(struct Foo foo) { ... }

In this case, we need for the C/Rust ABI to match not just in what wasm-bindgen produces, but within native FFI calls from Rust to C (and vice versa?). I don't see how this can be done solely within wasm-bindgen.

[devsnek]

@briansmith as it says in the OP, bindgen would emit (or you would type) fn foo_c(foo: &Foo); there.

[briansmith]

@briansmith as it says in the OP, bindgen would emit (or you would type) fn foo_c(foo: &Foo); there.

I see. I didn't realize that bindgen would rewrite the extern "C" { ... } and all the users of those functions too. Thanks for explaining.

[devsnek]

It was also pointed out that bindgen could emit something like this to "match" the c api better, though I'm personally not a fan.

extern "C" {
  #[link_name="foo_c"]
  fn foo_c_inner(foo: &Foo);
}
extern "C" pub fn foo_c(foo: Foo) {
  foo_c_inner(&foo)
}

[emilio]

The main issue here is exposing different APIs to different targets being a bit annoying... The last comment "fixes" it, but not a fan either :)

The OP could be made to work checking the type's layout quite easily, fwiw. We do have that information at that stage.

[emilio]

By the way, shouldn't this more generally be considered a rustc bug? I'd expect extern "C" functions to match the clang calling convention properly...

[emilio]

Ah, I read through the rust PR now, sorry for the noise...

So, I think I prefer to add an opt-in in bindgen to generate code like the one in #1944 (comment). I don't think it should be too invasive.

[devsnek]

@emilio are you available on something like discord or zulip for some implementation related questions?

[emilio]

Sure, yeah. Zulip (either the rust-lang one or the servo one) or Element / Matrix (@emilio:mozilla.org) should work. I'm a bit less familiar with Zulip but worth a shot if you prefer that :)

[emilio]

So things that came up:

• The check needs to be somewhat subtle (handle nested structs and such).
• The check also applies to return values, which need to be turned into an outparam, which is slightly more tricky.

But the basic idea still stands, I think, which is generating a wrapper function that does the right thing.

The right place to do this isn't the one in the OP (TryToRustTy for Type), as that would also affect members.

A good place to put the hack is on the callers of utils::fnsig_arguments. We already do the outparam shenanigans for C++ constructors, so if that could be reused it'd be amazing. Objective-C is probably not super-important to handle for this hack.

So I'd expect fnsig_arguments to grow a check like:

if ctx.options().wasm32_abi_hack_enabled && arg_ty.needs_wasm32_abi_hack(ctx) {
   arg_ty_tokens = quote! { *mut #arg_ty_tokens };
}

where arg_ty_tokens is the current arg_ty which we compute. We could maybe add the outparam handling there or something, not sure if that's nicer.

needs_wasm32_abi_hack needs to match on self, and if it's a trivial / pointer or a TypeKind::Comp that matches the aggregate test rustc does, then return true. Otherwise return false.

Then fnsig_args should probably return the information of which arguments it has modified or something and the callers need to handle it. Does that make sense?