-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[wasm] Suppress export name minification #95613
Conversation
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsPossible fix for #94939.
|
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
It seems to me that this could also fix #92664 (comment) @kg could you please elaborate about the alternative solution "Rather than export those functions, could you add them to the table at compile time" mentioned in the emscripten discussion ? We need the raw exports for the jiterp trace .wasm, right ? I don't understand how that would be possible without export from dotnet.wasm |
Probably unrelated
|
It would be a single exported function that contains a massive switch statement that returns references to all of the exports, so they end up having pre-reserved slots in the function pointer table. It's kind of a pain to build this |
So you get back pointer, but in the trace import, you need the index into the export table, no ? |
The function pointer is an index |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be worth capturing the potential alternate work in an issue here but this fixes known issues directly.
Red is #89409, and we have confirmation from a user that this fixes the issue for them, so merging |
are we going to backport it to Net8 ? |
/backport to release/8.0-staging |
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9063189552 |
@maraf backporting to release/8.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Suppress export name minification
Using index info to reconstruct a base tree...
M src/mono/wasm/build/WasmApp.Native.targets
M src/mono/wasm/wasm.proj
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/wasm.proj
Auto-merging src/mono/wasm/build/WasmApp.Native.targets
CONFLICT (content): Merge conflict in src/mono/wasm/build/WasmApp.Native.targets
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Suppress export name minification
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
@maraf an error occurred while backporting to release/8.0-staging, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
Possible fix for #94939.
See emscripten-core/emscripten#20762