From 633f98900384c5ccdfc369c4552b10d213e13ed7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 2 Aug 2019 11:11:17 -0700 Subject: [PATCH] Parse -s changes using the json parser where possible, allowing human-readable C++ names in -s lists, which is important for Asyncify (#9132) We have nice syntax for lists, -s LIST=[foo,bar] which works well in many cases. But sometimes the list contains items that contain ",", for example -s LIST=["foo(x, y)", "bar"]. To handle that properly, try to parse things that look like json using the json parser; if it fails go back to the old code. Together with https://github.com/WebAssembly/binaryen/pull/2275 this fixes #9128 where the lists there actually do have ","s in them. Add testing for Asyncify lists with such names, and also added extra warnings for when escaping is confusing, and a test for that as well. --- emcc.py | 20 +++++++++++++++++++- tests/core/test_asyncify_lists.cpp | 4 +++- tests/test_core.py | 10 +++++----- tests/test_other.py | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/emcc.py b/emcc.py index 01d797cfdfe52..d3dd4695da5eb 100755 --- a/emcc.py +++ b/emcc.py @@ -1591,9 +1591,22 @@ def parse_passes(string): shared.Settings.ASYNCIFY_IMPORTS += ['__call_main'] if shared.Settings.ASYNCIFY_IMPORTS: passes += ['--pass-arg=asyncify-imports@%s' % ','.join(['env.' + i for i in shared.Settings.ASYNCIFY_IMPORTS])] + + # shell escaping can be confusing; try to emit useful warnings + def check_human_readable_list(items): + for item in items: + if item.count('(') != item.count(')'): + 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"]' ''') + 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.BINARYEN_IGNORE_IMPLICIT_TRAPS: passes += ['--ignore-implicit-traps'] @@ -3494,7 +3507,12 @@ def parse_string_list(text): return parse_string_list_members(inner) if text[0] == '[': - return parse_string_list(text) + # if json parsing fails, we fall back to our own parser, which can handle a few + # simpler syntaxes + try: + return json.loads(text) + except: + return parse_string_list(text) try: return int(text) diff --git a/tests/core/test_asyncify_lists.cpp b/tests/core/test_asyncify_lists.cpp index f4812307a8dd5..181d1583d2eb4 100644 --- a/tests/core/test_asyncify_lists.cpp +++ b/tests/core/test_asyncify_lists.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -7,8 +8,9 @@ int bar(); // Foo is a function that needs asyncify support. __attribute__((noinline)) -int foo() { +int foo(int a = 0, double b = 0) { if (x == 1337) return bar(); // don't inline me + assert(a == b); emscripten_sleep(1); return 1; } diff --git a/tests/test_core.py b/tests/test_core.py index 32bf1edb6d9ae..9e07f5e1c81a3 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -7555,13 +7555,13 @@ def test_coroutine_emterpretify_async(self): @parameterized({ 'normal': ([], True), - 'blacklist_a': (['-s', 'ASYNCIFY_BLACKLIST=["foo()"]'], False), + '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()","baz()","c_baz","Structy::funcy()","bar()"]'], True), - 'whitelist_b': (['-s', 'ASYNCIFY_WHITELIST=["main","__original_main","foo()","baz()","c_baz","Structy::funcy()"]'], True), - 'whitelist_c': (['-s', 'ASYNCIFY_WHITELIST=["main","__original_main","foo()","baz()","c_baz"]'], False), - 'whitelist_d': (['-s', 'ASYNCIFY_WHITELIST=["foo()","baz()","c_baz","Structy::funcy()"]'], 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), }) @no_fastcomp('new asyncify only') def test_asyncify_lists(self, args, should_pass): diff --git a/tests/test_other.py b/tests/test_other.py index 1d6c117c52383..f05c4194ec2ba 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -8833,6 +8833,23 @@ def test_emcc_parsing(self): self.assertNotEqual(proc.returncode, 0) self.assertContained(expected, proc.stderr) + @no_fastcomp('uses new ASYNCIFY') + def test_asyncify_escaping(self): + proc = run_process([PYTHON, 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) + 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) + + @no_fastcomp('uses new ASYNCIFY') + def test_asyncify_response_file(self): + create_test_file('a.txt', r'''[ + "DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)" +] +''') + proc = run_process([PYTHON, EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'ASYNCIFY=1', '-s', "ASYNCIFY_WHITELIST=@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-existing function name: DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)', proc.stderr) + # Sockets and networking def test_inet(self):