diff --git a/ChangeLog.md b/ChangeLog.md index 4387128ac324..9fd723412008 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -17,6 +17,16 @@ See docs/process.md for how version tagging works. Current Trunk ------------- +- Add support for the new `ASYNCIFY_ADD_LIST`, and update existing list names + following the updates in Binaryen, so that now we have `ASYNCIFY_ADD_LIST` to + add a function, `ASYNCIFY_REMOVE_LIST` to remove one (previously this was + called `ASYNCIFY_BLACKLIST`), and `ASYNCIFY_ONLY_LIST` to set a list of the + only functions to instrument and no others (previously this was called + `ASYNCIFY_WHITELIST`). The updated lists also handle indirect calls properly, + so that if you use `ASYNCIFY_IGNORE_INDIRECT` and then add (using either the + add-list or the only-list) all the functions that are on the stack when + pausing, then things will work (for more, see + https://github.com/WebAssembly/binaryen/pull/2913). 1.39.18: 06/12/2020 ------------------- diff --git a/emcc.py b/emcc.py index a40ea20c5734..0fcabda0a81b 100755 --- a/emcc.py +++ b/emcc.py @@ -656,7 +656,6 @@ def backend_binaryen_passes(): # be able to whitelist them etc. passes += ['--fpcast-emu'] if shared.Settings.ASYNCIFY: - # TODO: allow whitelist as in asyncify passes += ['--asyncify'] if shared.Settings.ASSERTIONS: passes += ['--pass-arg=asyncify-asserts'] @@ -694,15 +693,18 @@ def check_human_readable_list(items): logger.warning('''emcc: ASYNCIFY list contains an item without balanced parentheses ("(", ")"):''') logger.warning(''' ''' + item) logger.warning('''This may indicate improper escaping that led to splitting inside your names.''') - logger.warning('''Try to quote the entire argument, like this: -s 'ASYNCIFY_WHITELIST=["foo(int, char)", "bar"]' ''') + logger.warning('''Try to quote the entire argument, like this: -s 'ASYNCIFY_ONLY_LIST=["foo(int, char)", "bar"]' ''') break - if shared.Settings.ASYNCIFY_BLACKLIST: - check_human_readable_list(shared.Settings.ASYNCIFY_BLACKLIST) - passes += ['--pass-arg=asyncify-blacklist@%s' % ','.join(shared.Settings.ASYNCIFY_BLACKLIST)] - if shared.Settings.ASYNCIFY_WHITELIST: - check_human_readable_list(shared.Settings.ASYNCIFY_WHITELIST) - passes += ['--pass-arg=asyncify-whitelist@%s' % ','.join(shared.Settings.ASYNCIFY_WHITELIST)] + if shared.Settings.ASYNCIFY_REMOVE_LIST: + check_human_readable_list(shared.Settings.ASYNCIFY_REMOVE_LIST) + passes += ['--pass-arg=asyncify-removelist@%s' % ','.join(shared.Settings.ASYNCIFY_REMOVE_LIST)] + if shared.Settings.ASYNCIFY_ADD_LIST: + check_human_readable_list(shared.Settings.ASYNCIFY_ADD_LIST) + passes += ['--pass-arg=asyncify-addlist@%s' % ','.join(shared.Settings.ASYNCIFY_ADD_LIST)] + if shared.Settings.ASYNCIFY_ONLY_LIST: + check_human_readable_list(shared.Settings.ASYNCIFY_ONLY_LIST) + passes += ['--pass-arg=asyncify-onlylist@%s' % ','.join(shared.Settings.ASYNCIFY_ONLY_LIST)] if shared.Settings.BINARYEN_IGNORE_IMPLICIT_TRAPS: passes += ['--ignore-implicit-traps'] @@ -3177,7 +3179,7 @@ def do_binaryen(target, asm_target, options, memfile, wasm_binary_target, debug_info = shared.Settings.DEBUG_LEVEL >= 2 or options.profiling_funcs # whether we need to emit -g in the intermediate binaryen invocations (but not necessarily at the very end). # this is necessary for emitting a symbol map at the end. - intermediate_debug_info = bool(debug_info or options.emit_symbol_map or shared.Settings.ASYNCIFY_WHITELIST or shared.Settings.ASYNCIFY_BLACKLIST) + intermediate_debug_info = bool(debug_info or options.emit_symbol_map or shared.Settings.ASYNCIFY_ONLY_LIST or shared.Settings.ASYNCIFY_REMOVE_LIST or shared.Settings.ASYNCIFY_ADD_LIST) emit_symbol_map = options.emit_symbol_map or shared.Settings.CYBERDWARF # finish compiling to WebAssembly, using asm2wasm, if we didn't already emit WebAssembly directly using the wasm backend. if not shared.Settings.WASM_BACKEND: diff --git a/emscripten.py b/emscripten.py index 45679cc65b7f..bdbb0f067e58 100644 --- a/emscripten.py +++ b/emscripten.py @@ -2092,7 +2092,7 @@ def finalize_wasm(temp_files, infile, outfile, memfile, DEBUG): # tell binaryen to look at the features section, and if there isn't one, to use MVP # (which matches what llvm+lld has given us) - if shared.Settings.DEBUG_LEVEL >= 2 or shared.Settings.PROFILING_FUNCS or shared.Settings.EMIT_SYMBOL_MAP or shared.Settings.ASYNCIFY_WHITELIST or shared.Settings.ASYNCIFY_BLACKLIST: + if shared.Settings.DEBUG_LEVEL >= 2 or shared.Settings.PROFILING_FUNCS or shared.Settings.EMIT_SYMBOL_MAP or shared.Settings.ASYNCIFY_ONLY_LIST or shared.Settings.ASYNCIFY_REMOVE_LIST or shared.Settings.ASYNCIFY_ADD_LIST: args.append('-g') if shared.Settings.WASM_BIGINT: args.append('--bigint') diff --git a/site/source/docs/porting/asyncify.rst b/site/source/docs/porting/asyncify.rst index 26a1b1ac3fc2..f1e24533a227 100644 --- a/site/source/docs/porting/asyncify.rst +++ b/site/source/docs/porting/asyncify.rst @@ -243,11 +243,15 @@ you can tell Asyncify to ignore indirect calls using If you know that some indirect calls matter and others do not, then you can provide a manual list of functions to Asyncify: -* ``ASYNCIFY_BLACKLIST`` is a list of functions that do not unwind the stack. - Asyncify will do it's normal whole-program analysis under the assumption - that those do not unwind. -* ``ASYNCIFY_WHITELIST`` is a list of the **only** functions that can unwind - the stack. Asyncify will instrument those and no others. +* ``ASYNCIFY_REMOVE_LIST`` is a list of functions that do not unwind the stack. + Asyncify will do its normal whole-program analysis, then remove these + functions from the list of instrumented functions. +* ``ASYNCIFY_ADD_LIST`` is a list of functions that do unwind the stack, and + are added after doing the normal whole-program analysis. This is mostly useful + if you use ``ASYNCIFY_IGNORE_INDIRECT`` but want to also mark some additional + functions that need to unwind. +* ``ASYNCIFY_ONLY_LIST`` is a list of the **only** functions that can unwind + the stack. Asyncify will instrument exactly those and no others. For more details see ``settings.js``. Note that the manual settings mentioned here are error-prone - if you don't get things exactly right, diff --git a/src/settings.js b/src/settings.js index 45818ad68a69..d669ae73cdb6 100644 --- a/src/settings.js +++ b/src/settings.js @@ -691,7 +691,7 @@ var NODEJS_CATCH_REJECTION = 1; // This allows to inject some async functions to the C code that appear to be sync // e.g. emscripten_sleep // On fastcomp this uses the Asyncify IR transform. -// On upstream this uses the Asyncify pass in Binaryen. TODO: whitelist, coroutines +// On upstream this uses the Asyncify pass in Binaryen. var ASYNCIFY = 0; // Imports which can do a sync operation, in addition to the default ones that @@ -714,7 +714,7 @@ var ASYNCIFY_IGNORE_INDIRECT = 0; // In that case, you should increase this size. var ASYNCIFY_STACK_SIZE = 4096; -// If the Asyncify blacklist is provided, then the functions in it will not +// If the Asyncify remove-list is provided, then the functions in it will not // be instrumented even if it looks like they need to. This can be useful // if you know things the whole-program analysis doesn't, like if you // know certain indirect calls are safe and won't unwind. But if you @@ -735,13 +735,22 @@ var ASYNCIFY_STACK_SIZE = 4096; // builds, etc.). You can inspect the wasm binary to look for the actual names, // either directly or using wasm-objdump or wasm-dis, etc. // Simple '*' wildcard matching is supported. -var ASYNCIFY_BLACKLIST = []; - -// If the Asyncify whitelist is provided, then *only* the functions in the list -// will be instrumented. Like the blacklist, getting this wrong will break +var ASYNCIFY_REMOVE_LIST = []; + +// Functions in the Asyncify add-list are added to the list of instrumented +// functions, that is, they will be instrumented even if otherwise asyncify +// thinks they don't need to be. As by default everything will be instrumented +// in the safest way possible, this is only useful if you use IGNORE_INDIRECT +// and use this list to fix up some indirect calls that *do* need to be +// instrumented. +// See notes on ASYNCIFY_REMOVE_LIST about the names. +var ASYNCIFY_ADD_LIST = []; + +// If the Asyncify only-list is provided, then *only* the functions in the list +// will be instrumented. Like the remove-list, getting this wrong will break // your application. -// See notes on ASYNCIFY_BLACKLIST about the names. -var ASYNCIFY_WHITELIST = []; +// See notes on ASYNCIFY_REMOVE_LIST about the names. +var ASYNCIFY_ONLY_LIST = []; // Allows lazy code loading: where emscripten_lazy_load_code() is written, we // will pause execution, load the rest of the code, and then resume. @@ -1817,4 +1826,6 @@ var LEGACY_SETTINGS = [ ['BINARYEN_MEM_MAX', 'MAXIMUM_MEMORY'], ['BINARYEN_PASSES', [''], 'Use BINARYEN_EXTRA_PASSES to add additional passes'], ['SWAPPABLE_ASM_MODULE', [0], 'Fully swappable asm modules are no longer supported'], + ['ASYNCIFY_WHITELIST', 'ASYNCIFY_ONLY_LIST'], + ['ASYNCIFY_BLACKLIST', 'ASYNCIFY_REMOVE_LIST'], ]; diff --git a/tests/browser/async_bad_whitelist.cpp b/tests/browser/async_bad_list.cpp similarity index 100% rename from tests/browser/async_bad_whitelist.cpp rename to tests/browser/async_bad_list.cpp diff --git a/tests/core/test_asyncify_indirect_lists.cpp b/tests/core/test_asyncify_indirect_lists.cpp new file mode 100644 index 000000000000..e25c5d78d93a --- /dev/null +++ b/tests/core/test_asyncify_indirect_lists.cpp @@ -0,0 +1,33 @@ +#include +#include +#include + +int x = 0; + +struct Structy { + virtual int virty() { + if (x == 1337) return virty(); // don't inline me + emscripten_sleep(1); + return 42; + } +}; + +Structy y; + +__attribute__((noinline)) +void virt() { + if (x == 1337) { + // don't inline me + virt(); + } + Structy *z = &y; + printf("virt: %d\n", z->virty()); // but the indirect call itself! +} + +int main() { + EM_ASM({ + Module.counter = (Module.counter || 0) + 1; + if (Module.counter > 10) throw "infinite loop due to asyncify bug?"; + }); + virt(); +} diff --git a/tests/core/test_asyncify_indirect_lists.txt b/tests/core/test_asyncify_indirect_lists.txt new file mode 100644 index 000000000000..a1f64d5c3236 --- /dev/null +++ b/tests/core/test_asyncify_indirect_lists.txt @@ -0,0 +1 @@ +virt: 42 diff --git a/tests/test_browser.py b/tests/test_browser.py index 7dce6c061044..513470438ead 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -3307,8 +3307,8 @@ def test_async_stack_overflow(self): self.btest('browser/async_stack_overflow.cpp', '0', args=['-s', 'ASYNCIFY', '-s', 'ASYNCIFY_STACK_SIZE=4']) @no_fastcomp('wasm backend asyncify specific') - def test_async_bad_whitelist(self): - self.btest('browser/async_bad_whitelist.cpp', '0', args=['-s', 'ASYNCIFY', '-s', 'ASYNCIFY_WHITELIST=["waka"]', '--profiling']) + def test_async_bad_list(self): + self.btest('browser/async_bad_list.cpp', '0', args=['-s', 'ASYNCIFY', '-s', 'ASYNCIFY_ONLY_LIST=["waka"]', '--profiling']) # Tests that when building with -s MINIMAL_RUNTIME=1, the build can use -s MODULARIZE=1 as well. def test_minimal_runtime_modularize(self): diff --git a/tests/test_core.py b/tests/test_core.py index 2344745286ee..5567f6703eee 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -7862,22 +7862,22 @@ def test_asyncify_unused(self): @parameterized({ 'normal': ([], True), - 'blacklist_a': (['-s', 'ASYNCIFY_BLACKLIST=["foo(int, double)"]'], False), - 'blacklist_b': (['-s', 'ASYNCIFY_BLACKLIST=["bar()"]'], True), - 'blacklist_c': (['-s', 'ASYNCIFY_BLACKLIST=["baz()"]'], False), - 'whitelist_a': (['-s', 'ASYNCIFY_WHITELIST=["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()","bar()"]'], True), - 'whitelist_b': (['-s', 'ASYNCIFY_WHITELIST=["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()"]'], True), - 'whitelist_c': (['-s', 'ASYNCIFY_WHITELIST=["main","__original_main","foo(int, double)","baz()","c_baz"]'], False), - 'whitelist_d': (['-s', 'ASYNCIFY_WHITELIST=["foo(int, double)","baz()","c_baz","Structy::funcy()"]'], False), - 'whitelist_b_response': ([], True, '["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()"]'), - 'whitelist_c_response': ([], False, '["main","__original_main","foo(int, double)","baz()","c_baz"]'), + 'removelist_a': (['-s', 'ASYNCIFY_REMOVE_LIST=["foo(int, double)"]'], False), + 'removelist_b': (['-s', 'ASYNCIFY_REMOVE_LIST=["bar()"]'], True), + 'removelist_c': (['-s', 'ASYNCIFY_REMOVE_LIST=["baz()"]'], False), + 'onlylist_a': (['-s', 'ASYNCIFY_ONLY_LIST=["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()","bar()"]'], True), + 'onlylist_b': (['-s', 'ASYNCIFY_ONLY_LIST=["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()"]'], True), + 'onlylist_c': (['-s', 'ASYNCIFY_ONLY_LIST=["main","__original_main","foo(int, double)","baz()","c_baz"]'], False), + 'onlylist_d': (['-s', 'ASYNCIFY_ONLY_LIST=["foo(int, double)","baz()","c_baz","Structy::funcy()"]'], False), + 'onlylist_b_response': ([], True, '["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()"]'), + 'onlylist_c_response': ([], False, '["main","__original_main","foo(int, double)","baz()","c_baz"]'), }) @no_asan('asan is not compatible with asyncify stack operations; may also need to not instrument asan_c_load_4, TODO') @no_fastcomp('new asyncify only') def test_asyncify_lists(self, args, should_pass, response=None): if response is not None: create_test_file('response.file', response) - self.emcc_args += ['-s', 'ASYNCIFY_WHITELIST=@response.file'] + self.emcc_args += ['-s', 'ASYNCIFY_ONLY_LIST=@response.file'] self.set_setting('ASYNCIFY', 1) self.emcc_args += args try: @@ -7889,6 +7889,25 @@ def test_asyncify_lists(self, args, should_pass, response=None): if should_pass: raise + @parameterized({ + 'normal': ([], True), + 'ignoreindirect': (['-s', 'ASYNCIFY_IGNORE_INDIRECT'], False), + 'add': (['-s', 'ASYNCIFY_IGNORE_INDIRECT', '-s', 'ASYNCIFY_ADD_LIST=["main","virt()"]'], True), + }) + @no_asan('asan is not compatible with asyncify stack operations; may also need to not instrument asan_c_load_4, TODO') + @no_fastcomp('new asyncify only') + def test_asyncify_indirect_lists(self, args, should_pass): + self.set_setting('ASYNCIFY', 1) + self.emcc_args += args + try: + self.do_run_in_out_file_test('tests', 'core', 'test_asyncify_indirect_lists', assert_identical=True) + if not should_pass: + should_pass = True + raise Exception('should not have passed') + except Exception: + if should_pass: + raise + @no_asan('asyncify stack operations confuse asan') @no_fastcomp('wasm-backend specific feature') def test_emscripten_scan_registers(self): diff --git a/tests/test_other.py b/tests/test_other.py index 7aa405bc0ddd..7a1438d073c6 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -8931,7 +8931,7 @@ def test_emcc_parsing(self): @no_fastcomp('uses new ASYNCIFY') def test_asyncify_escaping(self): - proc = run_process([EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'ASYNCIFY=1', '-s', "ASYNCIFY_WHITELIST=[DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)]"], stdout=PIPE, stderr=PIPE) + proc = run_process([EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'ASYNCIFY=1', '-s', "ASYNCIFY_ONLY_LIST=[DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)]"], stdout=PIPE, stderr=PIPE) self.assertContained('emcc: ASYNCIFY list contains an item without balanced parentheses', proc.stderr) self.assertContained(' DOS_ReadFile(unsigned short', proc.stderr) self.assertContained('Try to quote the entire argument', proc.stderr) @@ -8943,10 +8943,10 @@ def test_asyncify_response_file(self): "DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)" ] ''') - proc = run_process([EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'ASYNCIFY=1', '-s', "ASYNCIFY_WHITELIST=@a.txt"], stdout=PIPE, stderr=PIPE) + proc = run_process([EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'ASYNCIFY=1', '-s', "ASYNCIFY_ONLY_LIST=@a.txt"], stdout=PIPE, stderr=PIPE) # we should parse the response file properly, and then issue a proper warning for the missing function self.assertContained( - 'Asyncify whitelist contained a non-matching pattern: DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)', + 'Asyncify onlylist contained a non-matching pattern: DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)', proc.stderr) # Sockets and networking