Skip to content

create WASM module import from any [DLLImport] which doesn't match known static native symbols #86984

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

Closed
Tracked by #65895
lewing opened this issue May 31, 2023 · 8 comments
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 31, 2023

From @SteveSandersonMS link

I've been trying out the pinvoke generation and found it can't yet generate real WebAssembly imports. There are two main reasons:

It ignores pinvokes unless they correspond to an entry on _WasmNativeFileForLinking (even though when you want to generate an import, you won't be linking with any module for it)
When generating pinvoke-table.h, it never adds any import_module/import_name clang attributes, which means:
The module is always compiled as env (so it's impossible to reference most real-world libraries)
The symbol name is mangled
I was able to work around it via some MSBuild hackery, so can be fairly confident what would be the steps to solve this:

Change the pinvoke generator so that when a DllImport's module name does not correspond to an entry in _WasmNativeFileForLinking, it does still generate the pinvoke-table.h entry
... but those pinvoke-table.h entries must also be annotated with import_module/import_name
For example, given these two DllImports:

[LibraryImport("libSystem.Native")]
private static partial int Method1(int a);

[LibraryImport("mylib", EntryPoint = "this:should:not:be:mangled")]
private static partial int Method2(int a);
... the pinvoke-table.h definition should look like:

int Method1 (int);

attribute((import_module("mylib"), import_name("this:should:not:be:mangled"))) int Method2 (int);
The first one does not need any attribute because libSystem.Native does match up with one of the linker inputs (i.e., _WasmNativeFileForLinking entries). The existing codegen is correct.

The second one does need the clang import annotations because it doesn't match a _WasmNativeFileForLinking entry, so it has to become a real wasm import, and it's important to retain the real module name (mylib) and the real import name (this:should:not:be:mangled) even though it's not a legal C function name.

From @lewing
The goal of this issue is making sure that once the build completes the wasm module is able to have imports of <module>.<method> with unmangled names for missing symbols, not to solve wasi component linking more generally.

@ghost ghost added area-Interop-coreclr untriaged New issue has not been triaged by the area owner labels May 31, 2023
@lewing lewing added this to the 8.0.0 milestone May 31, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 31, 2023
@lewing lewing added arch-wasm WebAssembly architecture untriaged New issue has not been triaged by the area owner labels May 31, 2023
@ghost
Copy link

ghost commented May 31, 2023

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

Issue Details

From @SteveSandersonMS

I've been trying out the pinvoke generation and found it can't yet generate real WebAssembly imports. There are two main reasons:

It ignores pinvokes unless they correspond to an entry on _WasmNativeFileForLinking (even though when you want to generate an import, you won't be linking with any module for it)
When generating pinvoke-table.h, it never adds any import_module/import_name clang attributes, which means:
The module is always compiled as env (so it's impossible to reference most real-world libraries)
The symbol name is mangled
I was able to work around it via some MSBuild hackery, so can be fairly confident what would be the steps to solve this:

Change the pinvoke generator so that when a DllImport's module name does not correspond to an entry in _WasmNativeFileForLinking, it does still generate the pinvoke-table.h entry
... but those pinvoke-table.h entries must also be annotated with import_module/import_name
For example, given these two DllImports:

[LibraryImport("libSystem.Native")]
private static partial int Method1(int a);

[LibraryImport("mylib", EntryPoint = "this:should:not:be:mangled")]
private static partial int Method2(int a);
... the pinvoke-table.h definition should look like:

int Method1 (int);

attribute((import_module("mylib"), import_name("this:should:not:be:mangled"))) int Method2 (int);
The first one does not need any attribute because libSystem.Native does match up with one of the linker inputs (i.e., _WasmNativeFileForLinking entries). The existing codegen is correct.

The second one does need the clang import annotations because it doesn't match a _WasmNativeFileForLinking entry, so it has to become a real wasm import, and it's important to retain the real module name (mylib) and the real import name (this:should:not:be:mangled) even though it's not a legal C function name.

Author: lewing
Assignees: maraf
Labels:

arch-wasm, area-Interop-coreclr, untriaged

Milestone: 8.0.0

@lewing lewing removed the untriaged New issue has not been triaged by the area owner label May 31, 2023
@jkotas
Copy link
Member

jkotas commented Jun 1, 2023

cc @yowl

@pavelsavara
Copy link
Member

I understand that we are aiming at simple data types like numbers and string (with encoding dictated by the ABI).
I think we should not try to implement WIT on [DllImport] nor marshaling of data structures, right ?

Should we support passing simple arrays by value ?
Should we support passing pointers ?

@jkotas
Copy link
Member

jkotas commented Jun 8, 2023

like numbers and string (with encoding dictated by the ABI).

I expect that managed strings are going require some marshalling. What does that marshaling look like?

I think we should not try to implement WIT on [DllImport] nor marshaling of data structures, right ?

Yes, we should avoid any runtime provided marshalling. Instead, depend on source generators to provide the marshalling code.

@silesmo
Copy link

silesmo commented Nov 21, 2023

@AaronRobinsonMSFT This is something that would make our lives significantly easier when adding mono support to wit-bindgen and the same is true for #86985. Is this something that could be looked into relatively soon? If we don't have this we will essentially end up with two very different implementations for mono compared to NativeAOT-LLVM.

Related to: bytecodealliance/wit-bindgen#713

@AaronRobinsonMSFT
Copy link
Member

@silesmo Is this the point of the new WasmImportLinkage type? Is the source generator dropping this attribute when generating the target? My assumption here would be for users to do something akin to the following:

[WasmImportLinkage]
[LibraryImport("mylib", EntryPoint = "this:should:not:be:mangled")]
private static partial int Method2(int a);

@lewing
Copy link
Member Author

lewing commented Nov 21, 2023

Fixed in #94615

@lewing lewing closed this as completed Nov 21, 2023
@AaronRobinsonMSFT
Copy link
Member

Thanks @lewing!

@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2023
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

No branches or pull requests

7 participants