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

Undo assertion on STANDALONE_WASM not working with memory growth #9588

Merged
merged 10 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1786,8 +1786,6 @@ def check_human_readable_list(items):
exit_with_error('STANDALONE_WASM does not support pthreads yet')
if shared.Settings.SIMD:
exit_with_error('STANDALONE_WASM does not support simd yet')
if shared.Settings.ALLOW_MEMORY_GROWTH:
exit_with_error('STANDALONE_WASM does not support memory growth yet')
# the wasm must be runnable without the JS, so there cannot be anything that
# requires JS legalization
shared.Settings.LEGALIZE_JS_FFI = 0
Expand Down
17 changes: 17 additions & 0 deletions site/source/docs/api_reference/emscripten.h.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1363,3 +1363,20 @@ Functions
This function requires Asyncify - it relies on that option to spill the
local state all the way up the stack. As a result, it will add overhead
to your program.

ABI functions
=============

The following functions are not declared in ``emscripten.h``, but are used
internally in our system libraries. You may care about them if you replace the
Emscripten runtime JS code, or run Emscripten binaries in your own runtime.


.. c:function:: void emscripten_notify_memory_growth(i32 index)

Called when memory has grown. In a JS runtime, this is used to know when
to update the JS views on the wasm memory, which otherwise we would need
to constantly check for after any wasm code runs. See
`this wasi discussion <https://github.com/WebAssembly/WASI/issues/82>`_.

:param i32 index: Which memory has grown.
11 changes: 11 additions & 0 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,17 @@ LibraryManager.library = {
#endif // ALLOW_MEMORY_GROWTH
},

// Called after wasm grows memory. At that time we need to update the views.
// Without this notification, we'd need to check the buffer in JS every time
// we return from any wasm, which adds overhead. See
// https://github.com/WebAssembly/WASI/issues/82
emscripten_notify_memory_growth: function(memoryIndex) {
#if ASSERTIONS
assert(memoryIndex == 0);
#endif
updateGlobalBufferAndViews(wasmMemory.buffer);
},

system__deps: ['__setErrNo'],
system: function(command) {
// int system(const char *command);
Expand Down
3 changes: 2 additions & 1 deletion src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,8 @@ function createWasm() {
// In pure wasm mode the memory is created in the wasm (not imported), and
// then exported.
// TODO: do not create a Memory earlier in JS
updateGlobalBufferAndViews(exports['memory'].buffer);
wasmMemory = exports['memory'];
updateGlobalBufferAndViews(wasmMemory.buffer);
#if ASSERTIONS
writeStackCookie();
#endif
Expand Down
20 changes: 16 additions & 4 deletions system/lib/standalone_wasm.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* found in the LICENSE file.
*/

#include <assert.h>
#include <emscripten.h>
#include <errno.h>
#include <stdio.h>
Expand Down Expand Up @@ -70,11 +71,22 @@ void *emscripten_memcpy_big(void *restrict dest, const void *restrict src, size_

static const int WASM_PAGE_SIZE = 65536;

// Note that this does not support memory growth in JS because we don't update the JS
// heaps. Wasm and wasi lack a good API for that.
extern void emscripten_notify_memory_growth(size_t memory_index);

int emscripten_resize_heap(size_t size) {
size_t result = __builtin_wasm_memory_grow(0, (size + WASM_PAGE_SIZE - 1) / WASM_PAGE_SIZE);
return result != (size_t)-1;
#ifdef __EMSCRIPTEN_MEMORY_GROWTH__
size_t old_size = __builtin_wasm_memory_size(0) * WASM_PAGE_SIZE;
assert(old_size < size);
ssize_t diff = (size - old_size + WASM_PAGE_SIZE - 1) / WASM_PAGE_SIZE;
size_t result = __builtin_wasm_memory_grow(0, diff);
if (result != (size_t)-1) {

// Success, update JS (see https://github.com/WebAssembly/WASI/issues/82)
emscripten_notify_memory_growth(0);
return 1;
}
#endif
return 0;
}

// C++ ABI
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,7 @@ _emscripten_log_js
_emscripten_longjmp
_emscripten_main_browser_thread_id
_emscripten_memcpy_big
_emscripten_notify_memory_growth
_emscripten_pause_main_loop
_emscripten_pc_get_column
_emscripten_pc_get_file
Expand Down
22 changes: 22 additions & 0 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,27 @@ def decorated(self):
return decorated


# Similar to also_with_standalone_wasm, but suitable for tests that cannot
# run in a wasm VM yet, as they are not 100% standalone. We can still
# run them with the JS code though.
def also_with_impure_standalone_wasm(func):
def decorated(self):
func(self)
# Standalone mode is only supported in the wasm backend, and not in all
# modes there.
if self.is_wasm_backend() and self.get_setting('WASM') and not self.get_setting('SAFE_STACK'):
print('standalone (impure; no wasm runtimes)')
self.set_setting('STANDALONE_WASM', 1)
wasm_engines = shared.WASM_ENGINES
try:
shared.WASM_ENGINES = []
func(self)
finally:
shared.WASM_ENGINES = wasm_engines

return decorated


# A simple check whether the compiler arguments cause optimization.
def is_optimizing(args):
return '-O' in str(args) and '-O0' not in args
Expand Down Expand Up @@ -1929,6 +1950,7 @@ def test_memorygrowth_3(self):
self.emcc_args += ['-s', 'ALLOW_MEMORY_GROWTH=0', '-s', 'ABORTING_MALLOC=0', '-s', 'SAFE_HEAP']
self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_3')

@also_with_impure_standalone_wasm
def test_memorygrowth_wasm_mem_max(self):
if self.has_changed_setting('ALLOW_MEMORY_GROWTH'):
self.skipTest('test needs to modify memory growth')
Expand Down
2 changes: 1 addition & 1 deletion tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -8104,7 +8104,7 @@ def test_binaryen_metadce_hello(self, *args):
4, [], [], 8, 0, 0, 0), # noqa; totally empty!
# we don't metadce with linkable code! other modules may want stuff
# don't compare the # of functions in a main module, which changes a lot
'main_module_1': (['-O3', '-s', 'MAIN_MODULE=1'], 1603, [], [], 226403, None, 107, None), # noqa
'main_module_1': (['-O3', '-s', 'MAIN_MODULE=1'], 1604, [], [], 226403, None, 107, None), # noqa
'main_module_2': (['-O3', '-s', 'MAIN_MODULE=2'], 13, [], [], 10017, 13, 9, 20), # noqa
})
@no_wasm_backend()
Expand Down
2 changes: 1 addition & 1 deletion tools/shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ def get_emscripten_version(path):
# change, increment EMSCRIPTEN_ABI_MINOR if EMSCRIPTEN_ABI_MAJOR == 0
# or the ABI change is backwards compatible, otherwise increment
# EMSCRIPTEN_ABI_MAJOR and set EMSCRIPTEN_ABI_MINOR = 0.
(EMSCRIPTEN_ABI_MAJOR, EMSCRIPTEN_ABI_MINOR) = (0, 16)
(EMSCRIPTEN_ABI_MAJOR, EMSCRIPTEN_ABI_MINOR) = (0, 17)


def generate_sanity():
Expand Down
27 changes: 27 additions & 0 deletions tools/system_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,33 @@ class libstandalonewasm(MuslInternalLibrary):
cflags = ['-Os']
src_dir = ['system', 'lib']

def __init__(self, **kwargs):
self.is_mem_grow = kwargs.pop('is_mem_grow')
super(libstandalonewasm, self).__init__(**kwargs)

def get_base_name(self):
name = super(libstandalonewasm, self).get_base_name()
if self.is_mem_grow:
name += '-mem_grow'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems odd to mix hyphen and underscore here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll make it memgrow (no underscore).

return name

def get_cflags(self):
cflags = super(libstandalonewasm, self).get_cflags()
if self.is_mem_grow:
cflags += ['-D__EMSCRIPTEN_MEMORY_GROWTH__=1']
return cflags

@classmethod
def vary_on(cls):
return super(libstandalonewasm, cls).vary_on() + ['is_mem_grow']

@classmethod
def get_default_variation(cls, **kwargs):
return super(libstandalonewasm, cls).get_default_variation(
is_mem_grow=shared.Settings.ALLOW_MEMORY_GROWTH,
**kwargs
)

def get_files(self):
base_files = files_in_path(
path_components=['system', 'lib'],
Expand Down