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

[MONO][Wasm] Potential function signature mismatch in m2n invoke #99514

Closed
Nemo-G opened this issue Mar 11, 2024 · 1 comment · Fixed by #101106
Closed

[MONO][Wasm] Potential function signature mismatch in m2n invoke #99514

Nemo-G opened this issue Mar 11, 2024 · 1 comment · Fixed by #101106
Assignees
Labels
arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm
Milestone

Comments

@Nemo-G
Copy link

Nemo-G commented Mar 11, 2024

Description

When collecting PInvoke in managed code and generate native m2n function, the function naming is following rules like

string FixupSymbolName(string name)

Regarding special chars, there is a table
private static readonly char[] s_charsToReplace = new[] { '.', '-', '+', '<', '>' };

With the above logic, the generated function map would only have underscore representing all special chars.
image

However in interpreter logic, either .Net9

if (key [i] == '.')

or .Net8
if (key [i] == '.')

The function name resolving logic cannot cover all cases when generating them as only '.' is replaced. There should be logic which resembles the C# FixupSymbolName. Otherwise we may resolve pinvoke function name from Assemly-CSharp-firstpass.dll to
image

Also the function length currently 128 seems not sufficient. I have doubled it to 256. Any consideration here regarding the 128 length?

Reproduction Steps

Having C# functions marked with PInvoke in assembly with - in its name (or other special chars)

Expected behavior

Resolve those chars as understore

Actual behavior

Failed to find those functions in wasm_native_to_interp_map (calling wasm_dl_get_native_to_interp )

Regression?

Yes

Known Workarounds

Implement FixupSymbolName logic in naive code when calling get_native_to_interp so as to keep align with the way it is generated(Module name in particular). And increase the key size limit to 256 for some super long generated functions.

Configuration

.Net8
.Net9

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Mar 11, 2024
@Nemo-G
Copy link
Author

Nemo-G commented Mar 11, 2024

#99241 Just saw a similar issue. Seems backqoutes can also be part of the special character list to be escaped/special handled.

@vcsjones vcsjones added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 11, 2024
@maraf maraf added area-Build-mono and removed untriaged New issue has not been triaged by the area owner area-System.Runtime.InteropServices.JavaScript labels Mar 14, 2024
@maraf maraf added this to the 9.0.0 milestone Mar 14, 2024
@mkhamoyan mkhamoyan self-assigned this Mar 25, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants