Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm][mono] conflicting naming for wasm import/export #102844

Closed
lewing opened this issue May 29, 2024 · 4 comments · Fixed by #103881
Closed

[wasm][mono] conflicting naming for wasm import/export #102844

lewing opened this issue May 29, 2024 · 4 comments · Fixed by #103881
Assignees
Labels
arch-wasm WebAssembly architecture area-Interop-mono os-wasi Related to WASI variant of arch-wasm
Milestone

Comments

@lewing
Copy link
Member

lewing commented May 29, 2024

        [UnmanagedCallersOnly(EntryPoint = "test:variants/test#variant-zeros")]
        public static unsafe nint wasmExportVariantZeros(int p0, int p1, int p2, long p3, int p4, float p5, int p6, double p7) {

            [DllImport("test:variants/test", EntryPoint = "variant-zeros"), WasmImportLinkage]
            internal static extern void wasmImportVariantZeros(int p0, int p1, int p2, long p3, int p4, float p5, int p6, double p7, nint p8);

is generating code that doesn't match the assumptions wit-bindgen is making about the UCO signatures, we will need to verify correctness here

__attribute__((import_module("test:variants/test"),import_name("variant-zeros")))
extern void test_3A_variants_2F_test_23_variant_zeros (int32_t, int32_t, int32_t, int64_t, int32_t, float, int32_t, double, void *);


typedef void (*WasmInterpEntrySig_10) (int*, int*, int*, int*, int*, int*, int*, int*, int*, int*);
__attribute__((export_name("test:variants/test#variant-zeros")))
void * test_3A_variants_2F_test_23_variant_zeros (int32_t arg0, int32_t arg1, int32_t arg2, int64_t arg3, int32_t arg4, float arg5, int32_t arg6, double arg7) { 
  void * res;
  if (!(WasmInterpEntrySig_10)wasm_native_to_interp_ftndescs [10].func) {
   mono_wasm_marshal_get_managed_wrapper ("csharp-wasm", "VariantsWorld.wit.exports.test.variants.TestInterop", "wasmExportVariantZeros", 8);
  }
  ((WasmInterpEntrySig_10)wasm_native_to_interp_ftndescs [10].func) ((int*)&res, (int*)&arg0, (int*)&arg1, (int*)&arg2, (int*)&arg3, (int*)&arg4, (int*)&arg5, (int*)&arg6, (int*)&arg7, wasm_native_to_interp_ftndescs [10].arg);
  return res;
}

It looks like there is an import and an export with different signatures because the return value is being marshaled differently?

@lewing lewing added arch-wasm WebAssembly architecture area-Interop-mono os-wasi Related to WASI variant of arch-wasm labels May 29, 2024
@lewing lewing added this to the 9.0.0 milestone May 29, 2024
Copy link
Contributor

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

@mkhamoyan
Copy link
Contributor

@lewing I've reviewed our code, and the signature appears to be correct. However, I'm uncertain why wit-bindgen chooses to move the return type as a parameter. Is this behavior expected?

@lewing
Copy link
Member Author

lewing commented May 30, 2024

For some background I was running the wit-bindgen tests against the interpreter using bytecodealliance/wit-bindgen#958 and cargo test -p wit-bindgen-cli --no-default-features -F csharp-mono it looks at first glance like we're handling the nint return type differently than nativeaot but I haven't had time to look more closely

cc @lambdageek @maraf @kg

@lewing
Copy link
Member Author

lewing commented May 30, 2024

Looking closer this appears to be an issue in bindgen not mono

@lewing lewing closed this as completed May 30, 2024
@lewing lewing reopened this May 31, 2024
@lewing lewing changed the title [wasm] conflicting signatures for UCO [wasm][mono] conflicting naming for UCO May 31, 2024
@lewing lewing changed the title [wasm][mono] conflicting naming for UCO [wasm][mono] conflicting naming for wasm import/export May 31, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Interop-mono os-wasi Related to WASI variant of arch-wasm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants