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

[NativeAOT-LLVM] map runtimeimports to their native name for ldftn #1843

Merged
merged 1 commit into from
Mar 8, 2022

Conversation

yowl
Copy link
Contributor

@yowl yowl commented Feb 1, 2022

This PR partially addresses a problem with ldftn for RuntimeImports, i.e. from

internal static unsafe IntPtr RhGetRuntimeHelperForType(MethodTable* pEEType, RuntimeHelperKind kind)

Right now these are reported as undefined as they are not translated to the native names:

warning: undefined symbol: S_P_CoreLib_System_Runtime_InternalCalls__RhpNewArray (referenced by top-level compiled C/C++ code)

Emscripten will create imports for these in the wasm module, and stub them in JS. However a WASI host like wasmtime cannot satisfy these imports and just refuses to start. This PR at least lets the module run in a WASI host, although if these function pointers were ever called, they might be treated as managed calls, get a shadow stack arg, and fail. I propose to not solve that in the IL->LLVM compilation, but leave it until it's hit in the RyuJIT compilation.

There is still one undefined symbol preventing wasmtime starting, dotnet_browser_entropy, but that can be tackled in it's own PR.

cc @SingleAccretion

@yowl
Copy link
Contributor Author

yowl commented Feb 1, 2022

Oh we are hitting the hash problem emscripten-core/emscripten#16164 We'll have to wait for that to be resolved.

@yowl yowl closed this Feb 2, 2022
@yowl yowl reopened this Feb 2, 2022
@yowl
Copy link
Contributor Author

yowl commented Feb 10, 2022

The emscripten bug is fixed so this can be reviewed now. Thanks.

@jkotas
Copy link
Member

jkotas commented Mar 8, 2022

propose to not solve that in the IL->LLVM compilation, but leave it until it's hit in the RyuJIT compilation.

Ok, sounds reasonable.

1 similar comment
@jkotas
Copy link
Member

jkotas commented Mar 8, 2022

propose to not solve that in the IL->LLVM compilation, but leave it until it's hit in the RyuJIT compilation.

Ok, sounds reasonable.

@jkotas jkotas merged commit 258c8ce into dotnet:feature/NativeAOT-LLVM Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants