[wasm] UnmanagedCallersOnly generates incorrect wasm export types

[silesmo]

Description

When building an application targeting wasm and wasi and exporting it a function using UnmanagedCallersOnly results in the incorrect wasm type to be exported in the built wasm binary.

This is blocking progress with https://github.com/bytecodealliance/wit-bindgen for the mono implementation.

It's also worth mentioning that it's not generating the _initialize function instead of _start for libraries which is what we do with native aot llvm.

Related: #96419

@AaronRobinsonMSFT @lewing @yowl @jsturtevant

Reproduction Steps

Example function:

Expected behavior

Expected export type:
Two I32 as parameters and one I32 as the result

Actual behavior

Actual export type:
Six I32 as parameters and one I32 as the result

Regression?

No response

Known Workarounds

No response

Configuration

Full example can be found here: https://github.com/silesmo/wasm-unmanaged-callers-bug

Other information

No response

[ghost]

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

Issue Details

---

Description

When building an application targeting wasm and wasi and exporting it a function using UnmanagedCallersOnly results in the incorrect wasm type to be exported in the built wasm binary.

This is blocking progress with https://github.com/bytecodealliance/wit-bindgen for the mono implementation.

It's also worth mentioning that it's not generating the _initialize function instead of _start for libraries which is what we do with native aot llvm.

@AaronRobinsonMSFT @lewing @yowl @jsturtevant

Reproduction Steps

Example function:

Expected behavior

Expected export type:
Two I32 as parameters and one I32 as the result

Actual behavior

Actual export type:
Six I32 as parameters and one I32 as the result

Regression?

No response

Known Workarounds

No response

Configuration

Full example can be found here: https://github.com/silesmo/wasm-unmanaged-callers-bug

Other information

No response

Author:	silesmo
Assignees:	-
Labels:	arch-wasm, untriaged, needs-area-label
Milestone:	-

[lambdageek]

/cc @kg

[kg]

Description

When building an application targeting wasm and wasi and exporting it a function using UnmanagedCallersOnly results in the incorrect wasm type to be exported in the built wasm binary.

This is blocking progress with https://github.com/bytecodealliance/wit-bindgen for the mono implementation.

It's also worth mentioning that it's not generating the _initialize function instead of _start for libraries which is what we do with native aot llvm.

Related: #96419

@AaronRobinsonMSFT @lewing @yowl @jsturtevant

Reproduction Steps

Example function:

Expected behavior

Expected export type: Two I32 as parameters and one I32 as the result

Actual behavior

Actual export type: Six I32 as parameters and one I32 as the result

Regression?

No response

Known Workarounds

No response

Configuration

Full example can be found here: https://github.com/silesmo/wasm-unmanaged-callers-bug

Other information

No response

In these screenshots you're comparing roundtrip-s8 and roundtrip-u8. Can you show me the definition of roundtrip-s8?

What tool are you using to disassemble the module? Are you sure the function labelled roundtrip-u8 is actually roundtrip-u8?

[silesmo]

Description

When building an application targeting wasm and wasi and exporting it a function using UnmanagedCallersOnly results in the incorrect wasm type to be exported in the built wasm binary.
This is blocking progress with https://github.com/bytecodealliance/wit-bindgen for the mono implementation.
It's also worth mentioning that it's not generating the _initialize function instead of _start for libraries which is what we do with native aot llvm.
Related: #96419
@AaronRobinsonMSFT @lewing @yowl @jsturtevant

Reproduction Steps

Example function:

Expected behavior

Expected export type: Two I32 as parameters and one I32 as the result

Actual behavior

Actual export type: Six I32 as parameters and one I32 as the result

Regression?

No response

Known Workarounds

No response

Configuration

Full example can be found here: https://github.com/silesmo/wasm-unmanaged-callers-bug

Other information

No response

In these screenshots you're comparing roundtrip-s8 and roundtrip-u8. Can you show me the definition of roundtrip-s8?

What tool are you using to disassemble the module? Are you sure the function labelled roundtrip-u8 is actually roundtrip-u8?

Oh sorry I had accidentally taken the wrong picture. But this still holds true. You can see the full definition here https://github.com/silesmo/wasm-unmanaged-callers-bug/blob/main/mono-example/wit.exports.test.numbers.TestInterop.cs.

The tool for disassembly is https://wa2.dev/ and I'm sure it's correct. I built it myself and it have gone through lots of testing and several thousand users each month.

@kg

[silesmo]

roundtrip-u8 was just an example from the example project. There are many more exports that get the wrong export type in that project.

[kg]

roundtrip-u8 was just an example from the example project. There are many more exports that get the wrong export type in that project.

OK, it's helpful to know that it's not just one specific export. I was looking through the code in the repository and couldn't come up with an explanation for why only that one would be affected.

[silesmo]

I have looked into it a bit more this morning. There might actually be something wrong happening with the export types in the tool in certain scenarios. I will get back to you on this. I will get back with the findings @kg

[silesmo]

Well that's embarrassing. There was indeed an issue with the export types not being displayed correctly @kg.
I have fixed it in the tool and the new version is deployed. Sorry about that.

[silesmo]

The module can't be initialized without enabling the threads proposal in Wasmtime which is not something that is currently supported with wasi_preview2 with the component model. Can a dependence on the threads proposal please be removed until the shared everything threads proposal have been implemented. #96629.

This makes us unable to initialize the module in the component model.
@kg

This can be tested using wasmtime run --invoke test-imports -Spreview2=n -Sthreads=n ./csharp-wasm.wasm

[kg]

Well that's embarrassing. There was indeed an issue with the export types not being displayed correctly @kg. I have fixed it in the tool and the new version is deployed. Sorry about that.

Nothing to be ashamed of, I have my own wasm decompiler that has had similar problems before. It's a tricky spec and I bear some of the responsibility for that...

The module can't be initialized without enabling the threads proposal in Wasmtime which is not something that is currently supported with wasi_preview2 with the component model. Can a dependence on the threads proposal please be removed until the shared everything threads proposal have been implemented. #96629.

This makes us unable to initialize the module in the component model. @kg

This can be tested using wasmtime run --invoke test-imports -Spreview2=n -Sthreads=n ./csharp-wasm.wasm

We can definitely look into why threads support is required. Normally, it shouldn't be required since multi-threaded builds of the runtime are not the default.

[lewing]

We can definitely look into why threads support is required. Normally, it shouldn't be required since multi-threaded builds of the runtime are not the default.

Yeah, it is very odd that there would be any thread dependency in the wasi build and last time I tested there was none. Sometime could have slipped in with the build unification. We'll look into it when we have some capacity.

[lewing]

@silesmo I wasn't seeing the threading requirement on recent builds, are you still seeing it?

[lewing]

I believe this was fixed in #100652 and I've tested locally, the test case was dependent on a particular name mangling semantic and I've updated the generator to follow the same pattern

[silesmo]

Sorry, been a bit swamped recently. I will verify if this has been resolved and get back to you, this week.

Thank you for looking into it!

[silesmo]

This issue seem to be resolved as you said @lewing. There however seems to be a few other issues related to the initialization of a module with the component model. Essentially related to this: bytecodealliance/preview2-prototyping#99

Specifically the part about reactors having an initialization function.

See this one and linked PR for how it was done for Native AOT LLVM dotnet/runtimelab#2359

I can create a separate issue about it in the coming week to start the discussion how we can tackle that for mono.