dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.38k stars 4.75k forks source link

[API Proposal]: `WasmImportLinkageAttribute` to control wasm module names #93824

Open AaronRobinsonMSFT opened 1 year ago

AaronRobinsonMSFT commented 1 year ago

Background and motivation

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 WasmImportLinkage will distinguish these use cases and the attribute has no properties.

Please see https://github.com/dotnet/runtimelab/issues/2414 for how we got here and in particular, https://github.com/dotnet/runtimelab/issues/2414#issuecomment-1772551025.

Other issues and PRs of note on this subject:

https://github.com/dotnet/runtimelab/issues/1390 https://github.com/dotnet/runtimelab/pull/1845 https://github.com/dotnet/runtimelab/pull/2410 https://github.com/dotnet/runtimelab/issues/2383

Proposed PR https://github.com/dotnet/runtime/pull/93823

API Proposal

namespace System.Runtime.InteropServices;

[AttributeUsage(AttributeTargets.Method, Inherited = false)]
public sealed class WasmImportLinkageAttribute : Attribute
{
    public WasmImportLinkageAttribute() { }
}

API Usage

[WasmImportLinkage]
[DllImport("library")]
static extern void Func();

Alternative Designs

It has been noted (https://github.com/dotnet/runtimelab/issues/2414#issuecomment-1773024314) that the original WasmImport suggestion was 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:

WasmDynamicLinkAttribute WasmImportLinkageAttribute - Updated this proposal WasmModuleLinkAttribute LlvmWasmImportAttribute (not good for Mono interpreter perhaps) WasmNamedModuleAttribute

Risks

No response

ghost commented 1 year ago

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

Issue Details
### Background and motivation 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 https://github.com/dotnet/runtimelab/issues/2414 for how we got here and in particular, https://github.com/dotnet/runtimelab/issues/2414#issuecomment-1772551025. Other issues and PRs of note on this subject: https://github.com/dotnet/runtimelab/issues/1390 https://github.com/dotnet/runtimelab/pull/1845 https://github.com/dotnet/runtimelab/pull/2410 https://github.com/dotnet/runtimelab/issues/2383 ### API Proposal ```csharp namespace System.Runtime.InteropServices; [AttributeUsage(AttributeTargets.Method, Inherited = false)] public sealed class WasmImportAttribute : Attribute { public WasmImportAttribute() { } } ``` ### API Usage ```csharp [WasmImport] [DllImport("library")] static extern void Func(); ``` ### Alternative Designs It has been noted (https://github.com/dotnet/runtimelab/issues/2414#issuecomment-1773024314) 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` ### Risks _No response_
Author: AaronRobinsonMSFT
Assignees: -
Labels: `api-suggestion`, `area-System.Runtime.InteropServices`
Milestone: -
AaronRobinsonMSFT commented 1 year ago

/cc @yowl

AaronRobinsonMSFT commented 1 year ago

/cc @lambdageek @pavelsavara @dotnet/nativeaot-llvm

lewing commented 1 year ago

cc @maraf @radekdoulik @ivanpovazan

AaronRobinsonMSFT commented 1 year ago

My preference would be WasmImportLinkAttribute. I like that because it has the WasmImport prefix which calls to mind DllImport, followed by the Link, which helps describe its practical utility.

jkoritzinsky commented 1 year ago

2 thoughts:

If (as mentioned in the runtimelab issue and comments) this attribute is more similar to DllImportSearchPathsAttrbute, would we want to consider a form that could be applied at a type/module/assembly level as well where the module name is specified in the attribute?

Not really API Review related, but would we want to have LibraryImport automatically propagate this attribute to the inner DllImport? My initial thought is yes, but just wanted to cover my bases.

SingleAccretion commented 1 year ago

If (as mentioned in the runtimelab issue and comments) this attribute is more similar to DllImportSearchPathsAttrbute, would we want to consider a form that could be applied at a type/module/assembly level as well where the module name is specified in the attribute?

We don't have a concrete need for that right now, I suppose.

One of the uses for the attribute would be in generated source code, where I don't think the assembly-wide version could be used due to its potential impact on other PI declarations.

Not really API Review related, but would we want to have LibraryImport automatically propagate this attribute to the inner DllImport? My initial thought is yes, but just wanted to cover my bases.

Correct, this would need to be propagated.

I don't have preferences on naming aside from "WASM import" staying as that terminology is used by the WASM specification and related tooling.

jkotas commented 1 year ago

WasmImportLinkAttribute

WasmImportLinkageAttribute? (Inspired by extern "C" linkage }

a form that could be applied at a type/module/assembly level as well where the module name is specified in the attribute

I do not think we need this. Similar interop attributes (e.g. UnmanagedCallConvAttribute) do not have it either. DefaultDllImportSearchPathsAttribute is an exception and it only allows Method and Assembly targets.

yowl commented 1 year ago

My preference would be WasmImportLinkAttribute. I like that because it has the WasmImport prefix which calls to mind DllImport, followed by the Link, which helps describe its practical utility.

I agree it is one of the better suggestions.

AaronRobinsonMSFT commented 1 year ago

WasmImportLinkAttribute

WasmImportLinkageAttribute? (Inspired by extern "C" linkage }

+1

AaronRobinsonMSFT commented 1 year ago

Unless there is any other discussion or thoughts, I am going to move this to ready for approval? Any concerns?

/cc @yowl @jkotas @pavelsavara @SingleAccretion

SingleAccretion commented 1 year ago

Semantics LGTM. Are we unified on the name - WasmImportAttribute or WasmImportLinkageAttribute?

(My preference is towards the latter)

AaronRobinsonMSFT commented 1 year ago

(My preference is towards the latter)

Same. I updated the proposal and bolded this alternative. I will be in the API review and push in that direction unless there is concern here. Also I am happy to update the proposal after I hear from @yowl.

yowl commented 1 year ago

All good here, WasmImportLinkageAttribute is good for me.

terrajobst commented 1 year ago

Video

namespace System.Runtime.InteropServices;

[AttributeUsage(AttributeTargets.Method, Inherited = false)]
public sealed class WasmImportLinkageAttribute : Attribute
{
    public WasmImportLinkageAttribute();
}
SingleAccretion commented 1 year ago

Why don't we just make (1) behave like (3)?

Unresolved WASM imports in a binary will lead to a fail fast on startup on some WASM hosts. It means that a dynamically unreachable but unresolvable (say, because it is for a different platform) PInvoke in some NuGet library would lead to a fail fast by default.

On the other hand, PNSEs are lazy, and so do not have this problem. That is the essential difference between 1 and 3.

AaronRobinsonMSFT commented 1 year ago

Unresolved WASM imports in a binary will lead to a fail fast on some WASM hosts.

So this is dependent on what WASM host is being used? Can you share an example of each?

SingleAccretion commented 1 year ago

So this is dependent on what WASM host is being used?

Correct.

Can you share an example of each?

  1. Web, aka Emscripten, is able to generate trapping stubs for unresolved imports. What they do cannot be controlled from managed code (e. g. they cannot throw managed exceptions), but they do avoid the fail fast.
  2. wasmtime: fail fast by default, has an option to generate unconditional traps similar to 1.
  3. wasmer: fail fast without the above option (to the best of my knowledge).
jkotas commented 1 year ago

According to https://github.com/dotnet/runtimelab/issues/2414#issuecomment-1772551025 it seems we decided that P/Invokes without this attribute would fail.

Also, the P/Invokes can be ambiguous. We can have situations when there is both native library and wasm module of the same name. We would not be able to tell whether the PInvoke is trying to call the native library or the wasm module.

Perksey commented 1 year ago

On the actual API for a sec, concerns about future linkage types arising do seem pertinent and I wonder whether it's just better to have e.g. [ImportLinkage(ImportLinkageOptions.WasmModule)] to alleviate concerns of type explosions/accidentally invalid combinations (at least until those appear in the enum itself) where the absence of such attribute or the presence of the attribute with no flags set is interpreted as "use the runtime default behaviour (DirectPInvoke if present, or system default if such well-defined default exists for the given environment)".