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

Proposal to add WasmImportAttribute to control wasm module names #93823

Merged
merged 6 commits into from
Nov 9, 2023

Conversation

yowl
Copy link
Contributor

@yowl yowl commented Oct 22, 2023

This PR proposes a new attribute to be used to control the Wasm import module and function names. In Wasm when a function is imported it can be specified as coming from a named module using this WAT

(import "hellowasi" "reverse" (func $reverse (type 4)))

The first use of this would be in the experimental NativeAOT-LLVM backend, but the desire is to have something that works for Mono and NativeAOT-LLVM. Something that we can use to proceed with Wasm Components and WIT binding via source gen for both Mono and NativeAOT-LLVM.

DllImportAttribute has a Value and EntryPoint which can be used to define the names, but to preserve existing semantics including static linking for DllImport("*") and DirectPInvoke we cannot simply say that we should always create a Wasm import if the DllImport Value is not *. The presence of this new WasmImport will distinguish these use cases and the attribute has no properties.

Please see dotnet/runtimelab#2414 for how we got here and in particular, dotnet/runtimelab#2414 (comment).

Other issues and PRs of note on this subject:

dotnet/runtimelab#1390
dotnet/runtimelab#1845
dotnet/runtimelab#2410
dotnet/runtimelab#2383

It has been noted (dotnet/runtimelab#2414 (comment)) that WasmImport is a potentially confusing name as the Import suffix can infer it's use should be as an alternative to DllImport but this is not the case, so some other alternatives that I could think of:

WasmDynamicLink
WasmImportLink
WasmModuleLink
LlvmWasmImport (not good for Mono interpreter perhaps)
WasmNamedModule

Thanks for reading.

cc @AaronRobinsonMSFT @dotnet/nativeaot-llvm @pavelsavara

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 22, 2023
@ghost
Copy link

ghost commented Oct 22, 2023

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR proposes a new attribute to be used to control the Wasm import module and function names. In Wasm when a function is imported it can be specified as coming from a named module using this WAT

(import "hellowasi" "reverse" (func $reverse (type 4)))

The first use of this would be in the experimental NativeAOT-LLVM backend, but the desire is to have something that works for Mono and NativeAOT-LLVM. Something that we can use to proceed with Wasm Components and WIT binding via source gen for both Mono and NativeAOT-LLVM.

DllImportAttribute has a Value and EntryPoint which can be used to define the names, but to preserve existing semantics including static linking for DllImport("*") and DirectPInvoke we cannot simply say that we should always create a Wasm import if the DllImport Value is not *. The presence of this new WasmImport will distinguish these use cases and the attribute has no properties.

Please see dotnet/runtimelab#2414 for how we got here and in particular, dotnet/runtimelab#2414 (comment).

Other issues and PRs of note on this subject:

dotnet/runtimelab#1390
dotnet/runtimelab#1845
dotnet/runtimelab#2410
dotnet/runtimelab#2383

It has been noted (dotnet/runtimelab#2414 (comment)) that WasmImport is a potentially confusing name as the Import suffix can infer it's use should be as an alternative to DllImport but this is not the case, so some other alternatives that I could think of:

WasmDynamicLink
WasmImportLink
WasmModuleLink
LlvmWasmImport (not good for Mono interpreter perhaps)
WasmNamedModule

Thanks for reading.

cc @AaronRobinsonMSFT @dotnet/nativeaot-llvm @pavelsavara

Author: yowl
Assignees: -
Labels:

area-System.Runtime.InteropServices, community-contribution

Milestone: -

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Porposal to add WasmImportAttribute to control wasm module names Proposal to add WasmImportAttribute to control wasm module names Oct 22, 2023
@AaronRobinsonMSFT AaronRobinsonMSFT added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 22, 2023
@lewing lewing requested review from pavelsavara and maraf October 22, 2023 00:48
@AaronRobinsonMSFT
Copy link
Member

@yowl Typically we would start with an issue with the API proposal. I will create that quickly and point to this PR. However, this PR will need a few things.

@AaronRobinsonMSFT AaronRobinsonMSFT removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 22, 2023
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 9.0.0 milestone Oct 22, 2023
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as draft October 22, 2023 00:53
@kg
Copy link
Member

kg commented Oct 22, 2023

Instead of a new attribute, could you add a new CallingConvention.Wasm or CallingConvention.WasmImport that is used to identify a specific DllImport as one that's a wasm import? So instead of
[DllImport("foo_module", EntryPoint="bar"), WasmImport]
you would have
[DllImport("foo_module", EntryPoint="bar", CallingConvention=CallingConvention.Wasm)]

@SingleAccretion
Copy link
Contributor

@kg Calling conventions are orthogonal to symbol resolution. For example, CallingConvention.WasmImport would not apply to function pointers and indirect calls.

@lambdageek
Copy link
Member

@pavelsavara

@pavelsavara
Copy link
Member

Are there limits to what the wasm import could do ?

Should we support full set of existing DllImport features from day 1 ?
(maybe it's easier to add functionality across releases than remove it later)

What would be behavior of DllImport for call conventions ? Should we explicitly limit it to particular call convention?
(as compared to usual native function with a stack, registers, ... )
In particular we have 2 "stacks" one is the wasm import arguments and the other is the shadow stack of LLVM.
Is there some issue with passing structs on stack ?

Do we expect that the imported functions could malloc, free, stackAlloc or WASI cabi_realloc ?
(that probably also assumes we export those functions)

Should we support all string marshaling options ?

Are all unsupported scenarios compile time errors ?

Is this act of opening new ABI ?

@SingleAccretion
Copy link
Contributor

@pavelsavara the attribute will not affect the ABI or calling convention (which both default to C interop, with marshalling, shadow stacks, arbitrary reentrancy, etc).

@pavelsavara
Copy link
Member

@pavelsavara the attribute will not affect the ABI or calling convention (which both default to C interop, with marshalling, shadow stacks, arbitrary reentrancy, etc).

C calling conventions are OS/CPU/compiler dependent behavior. From whatever consumes this wasm import it's "undefined" details unless they share the exact same compiler (and maybe compiler flags). Are we hand-waving it ?

I think we have this problem with incompatible behavior of different versions of emscripten between versions of dotnet already. The existing use-case is when we link static library into .wasm file. Hope I remember it correctly. @lewing ?

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Oct 25, 2023

Are we hand-waving it ?

@pavelsavara No, "the C ABI" I was referring to is https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md. It is stable.

There are unstable variations (-mmulti-value) and aspects (v128 passing/returning) of it; I don't think these are important for us right now.

I think we have this problem with incompatible behavior of different versions of emscripten between versions of dotnet already. The existing use-case is when we link static library into .wasm file.

This is true, and the reason is that Emscripten has its own ABI conventions of top of C, and does not promise ABI compatibility. Static libraries are by their nature fragile too (though I don't know the precise extent of compatibility promised by LLVM; I believe that if you communicate using "plain C", it should be more or less stable).

All that said, ABI is a higher-level concern than this attribute, which is just about linkage.

@pavelsavara
Copy link
Member

Thanks!

@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review November 7, 2023 19:12
…pServices/WasmImportLinkageAttribute.cs

Co-authored-by: Aaron Robinson <arobins@microsoft.com>
@AaronRobinsonMSFT
Copy link
Member

@dotnet/interop-contrib or @dotnet/nativeaot-llvm Can I get another sign off?

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit a68243b into dotnet:main Nov 9, 2023
180 of 185 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.InteropServices community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants