Undo assertion on STANDALONE_WASM not working with memory growth #9588
Conversation
…le it isn't 100% standalone (there is an import to grow the memory), it is usable for people that implement that import, so don't break them. fixes #9587
This change updates the js build to Emscripten 1.38.47. This version changes the behavior of standalone .wasm modules - instead of importing a few symbols, including memory, externally, the standard library now creates and exports the memory object. Because of this the JS embedding structure needed to change a bit. Additionally, the ChakraCore workaround is not necessary anymore because the bug isn't being hit with LLVM upstream codegen. As a result, the JS is ~2% smaller after these updates. Note that because of a regression in .47, building this locally requires patching Emscripten with this change: emscripten-core/emscripten#9588
|
Just a minor comment, "there is an import to grow the memory" - there actually isn't :) emscripten_resize_heap is implemented in standalone_wasm.c without any callouts to JS. |
|
Thanks @zeux! Actually that's more than a minor comment, it showed I missed something important here, and had to rework this PR. We don't support memory growth in pure wasm runtimes yet, because of WebAssembly/WASI#82 My goal was for this PR to make it work outside of those runtimes - that is, when standalone mode is enabled, but still running with the JS. And my assumption was that wasm runtimes would show an error on I rewrote this PR to do things properly, and added a test for growth as well:
|
|
Hey would this change also fix #9190? |
|
@VirtualTim this is separate from that, as this PR is just for standalone wasm mode - in which there is no JS. We don't have support for pthreads in that case, but on the plus side, without JS the problem in #9190 won't exist, if/when we do have standalone wasm pthreads. |
|
I also added docs for the new API. @sbc100 maybe you want to take another look here, as it changed since the lgtm? |
tools/system_libs.py
Outdated
| def get_base_name(self): | ||
| name = super(libstandalonewasm, self).get_base_name() | ||
| if self.is_mem_grow: | ||
| name += '-mem_grow' |
There was a problem hiding this comment.
Seems odd to mix hyphen and underscore here.
There was a problem hiding this comment.
Good point, I'll make it memgrow (no underscore).
Remove the assertion on memory growth not being allowed in standalone mode. Add emscripten_notify_memory_growth which is just called to notify. That will show an error in wasm runtimes that don't support it. Add a variation on libstandalone that calls that notification if growth is enabled. That lets programs not using growth still be 100% pure. Add a test for memory growth in "impure" standalone wasm (i.e., standalone mode does not emit a 100% pure wasm yet). Fix some bugs in emscripten_resize_heap and the preamble memory loading code, that the test uncovered.
While it isn't 100% standalone (there is an import to grow the memory), it is usable for people that implement that import, so don't break them.
fixes #9587