Skip to content

[NativeAOT LLVM] DllImport / LibraryImport doesn't automatically produce imports on wasi-wasm #2383

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
RReverser opened this issue Aug 22, 2023 · 12 comments
Labels
area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly)

Comments

@RReverser
Copy link

As per discussion in #2235 (comment), looks like wasi-wasm currently gets passed --unresolved-symbols=ignore-all which means that you need explicit <WasmImport ...> annotations for any native imports.

@jkotas jkotas added the area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly) label Aug 22, 2023
@SingleAccretion
Copy link

I worked on this a bit. After adding in the obviously missing pthread shims, we are left with:

 - func[0] sig=0 <__cxa_allocate_exception> <- env.__cxa_allocate_exception
 - func[1] sig=8 <__cxa_throw> <- env.__cxa_throw
 - func[2] sig=15 <__cxa_end_catch> <- env.__cxa_end_catch
 - func[3] sig=6 <mono_bundled_resources_get_data_resource_values> <- env.mono_bundled_resources_get_data_resource_values
 - func[4] sig=6 <emscripten_get_callstack> <- env.emscripten_get_callstack

For HelloWasm. As can be seen, not too bad.

The challenge with importing anything unresolved on not-Browser is that the default behavior of engines is to hard fail loading the module if the imports cannot be satisfied. It means that with our limited test matrix we are practically bound to "test" this feature "in production" as some new upstream or existing dynamically unreachable runtime code decides to do something like PI into Win32 (this can be trivially seen if you try to compile a libraries test, there will be dozens of unresolved yet dynamically unreachable PIs).

@RReverser
Copy link
Author

You can define those functions with __builtin_trap() body in custom C that is passed to the linker. That way you won't get imports for them, but any user's functions will be imported as expected.

@RReverser
Copy link
Author

decides to do something like PI into Win32 (this can be trivially seen if you try to compile a libraries test, there will be dozens of unresolved yet dynamically unreachable PIs).

Ah sorry that doesn't help with this part, yeah...

@RReverser
Copy link
Author

I'd say assemblies compiled to Wasm shouldn't have those DllImport in the first place, but if it's something we really want to support, maybe the solution is to add a new property to either DllImport or LibraryImport like WasmImport = true? Not sure if platform-specific extension is ideal, but then there are already some Windows-specific properties on those attributes.

// this would be compiled to a trap
[DllImport("foo")]
public static void Bar();

// this would be propagated to the linker
[DllImport("foo", WasmImport = true)]
public static void Baz();

@RReverser
Copy link
Author

That said, wouldn't the mentioned issue be a problem for upstream Wasm support in Blazor too? If they're okay with emitting those PIs as imports, maybe best to match that behaviour?

@SingleAccretion
Copy link

Upstream art in this area is not yet established, I would expect them to hit this problem as well of course.

I'd say assemblies compiled to Wasm shouldn't have those DllImport in the first place

I think it's basically impractical to require this. .NET assemblies are by their nature cross-platform, so constructs such as below:

[DllImport("kernel32.dll")]
void win32_api();

[DllImport("*")]
void wasi_api();

public void Api()
{
    if (OperatingSystem.IsWindows()) { win32_api(); } else { wasi_api() };
}

Are both the recommended way to write code and lead to this problem.

I agree the solution lies somewhere around custom annotations and restricting the set of automatically imported code, it just needs more thought. Now we have WasmImport, but it has suboptimal ergonomics. Perhaps we could fix them by e. g. allowing one to mark an entire assembly?

@yowl
Copy link
Contributor

yowl commented Aug 23, 2023

[DllImport] is also how we are doing WIT interface imports.

@yowl
Copy link
Contributor

yowl commented Aug 23, 2023

For WIT (component model) we have more options perhaps as we have a source generator.

@RReverser
Copy link
Author

RReverser commented Aug 23, 2023

Now we have WasmImport, but it has suboptimal ergonomics. Perhaps we could fix them by e. g. allowing one to mark an entire assembly?

Hm yeah that actually sounds like a reasonable tradeoff too. But will something like <WasmImport Include="env" /> (for the default and most common Wasm namespace) be a problem for those .NET assemblies or are their import libraries always well-defined?

@SingleAccretion
Copy link

SingleAccretion commented Aug 23, 2023

I was thinking it'd work similar to UnmanagedEntryPointsAssembly, i. e. you would Include assemblies where you want the import to be auto-generated. It would have to be a separate item group (WasmImportAssembly, say) if WasmImport is to stay as-is.

WasmImport today is modeled after the Direct P/Invoke feature, where you don't have this "assembly" dimension and is useful because it allows specifying non-env module names. If we decided that module names can be specified directly via DllImport("<module name>"), it changes the equation a bit. This needs a bit more end-to-end design...

@RReverser
Copy link
Author

If we decided that module names can be specified directly via DllImport("<module name>"), it changes the equation a bit.

Yes, I believe we'd want that long-term, as also mentioned by Steve in #2266. Having to declare imports in multiple places is somewhat inconvenient.

Although if we have to add another element to csproj one way or another, and can't make DllImport always generate imports automatically, I guess it matters less for DX whether that element would specify Wasm namespace or C# assembly.

@SingleAccretion
Copy link

This has been fixed upstream and downstream via WasmImportLikageAttribute.

@jkotas jkotas closed this as completed Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly)
Projects
None yet
Development

No branches or pull requests

4 participants