From 1e56a39e34467407e8e142d1bf9035131ee82ad9 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 12 May 2021 14:49:55 -0700 Subject: [PATCH] Add a simple one-entry-per-line file format for specifying symbol lists For options such as `EXPORTED_FUNCTIONS` that take lists we now support a simple file format that is just one line per symbol. It doesn't require any escaping or punctuation. This is much easier to use maintain and is in line with other compiler toolchain options used by gcc and ld. For example ld's `--retain-symbols-file=filename` or wasm-ld's `--undefined-file=filename`. This should be especially useful for options like `ASYNCIFY_ADD` or `ASYNCIFY_REMOVE` which often involvea lot of quoting (because they include things like spaces and commas). --- ChangeLog.md | 4 +++ docs/emcc.txt | 10 +++--- emcc.py | 22 +++++++++--- site/source/docs/tools_reference/emcc.rst | 4 +-- tests/test_browser.py | 2 +- tests/test_other.py | 42 ++++++++++++++++------- 6 files changed, 60 insertions(+), 24 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 845c515f2dfd5..991b1afd0ec52 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -20,6 +20,10 @@ See docs/process.md for more on how version tagging works. 2.0.21 ------ +- Options such as EXPORTED_FUNCTIONS that can take a response file containing + list of symbols can now use a simple one-symbol-per-line format. This new + format is much simpler and doesn't require commas between symbols, opening + or closing braces, or any kind of escaping for special characters. - The WebAssembly linker (`wasm-ld`) now performes string tail merging on any static string data in your program. This has long been part of the native ELF linker and should not be observable in well-behaved programs. This diff --git a/docs/emcc.txt b/docs/emcc.txt index 4d0227d6e476a..1a6c230191d06 100644 --- a/docs/emcc.txt +++ b/docs/emcc.txt @@ -123,16 +123,16 @@ Options that are modified or new in *emcc* are listed below: -s "EXPORTED_FUNCTIONS=['liblib.so']" You can also specify that the value of an option will be read from - a specified JSON-formatted file. For example, the following option - sets the "EXPORTED_FUNCTIONS" option with the contents of the file - at **path/to/file**. + a file. For example, the following will set "EXPORTED_FUNCTIONS" + based on the contents of the file at **path/to/file**. -s EXPORTED_FUNCTIONS=@/path/to/file Note: - * In this case the file might contain a JSON-formatted list of - functions: "["_func1", "func2"]". + * In this case the file should contain a list of symbols, one per + line. For legacy use cases JSON-formatted files are also + supported: e.g. "["_func1", "func2"]". * The specified file path must be absolute, not relative. diff --git a/emcc.py b/emcc.py index 5b76112e20b75..df838e98e3ceb 100755 --- a/emcc.py +++ b/emcc.py @@ -351,6 +351,7 @@ def standardize_setting_change(key, value): if key in MEM_SIZE_SETTINGS: value = str(expand_byte_size_suffixes(value)) + filename = None if value and value[0] == '@': filename = value[1:] if not os.path.exists(filename): @@ -362,10 +363,14 @@ def standardize_setting_change(key, value): existing = getattr(settings, user_key, None) expect_list = type(existing) == list - try: - value = parse_value(value, expect_list) - except Exception as e: - exit_with_error('a problem occurred in evaluating the content after a "-s", specifically "%s=%s": %s', key, value, str(e)) + if filename and expect_list and value.strip()[0] != '[': + # Prefer simpler one-line-per value parser + value = parse_symbol_list_file(value) + else: + try: + value = parse_value(value, expect_list) + except Exception as e: + exit_with_error('a problem occurred in evaluating the content after a "-s", specifically "%s=%s": %s', key, value, str(e)) # Do some basic type checking by comparing to the existing settings. # Sadly we can't do this generically in the SettingsManager since there are settings @@ -3529,6 +3534,15 @@ def in_directory(root, child): return False +def parse_symbol_list_file(contents): + """Parse contents of one-symbol-per-line response file. This format can by used + with, for example, -sEXPORTED_FUNCTIONS=@filename and avoids the need for any + kind of quoting or escaping. + """ + values = contents.splitlines() + return [v.strip() for v in values] + + def parse_value(text, expect_list): # Note that using response files can introduce whitespace, if the file # has a newline at the end. For that reason, we rstrip() in relevant diff --git a/site/source/docs/tools_reference/emcc.rst b/site/source/docs/tools_reference/emcc.rst index 66b6881bbd38c..53d637721cd11 100644 --- a/site/source/docs/tools_reference/emcc.rst +++ b/site/source/docs/tools_reference/emcc.rst @@ -112,7 +112,7 @@ Options that are modified or new in *emcc* are listed below: -s EXPORTED_FUNCTIONS="['liblib.so']" -s "EXPORTED_FUNCTIONS=['liblib.so']" - You can also specify that the value of an option will be read from a specified JSON-formatted file. For example, the following option sets the ``EXPORTED_FUNCTIONS`` option with the contents of the file at **path/to/file**. + You can also specify that the value of an option will be read from a file. For example, the following will set ``EXPORTED_FUNCTIONS`` based on the contents of the file at **path/to/file**. :: @@ -120,7 +120,7 @@ Options that are modified or new in *emcc* are listed below: .. note:: - - In this case the file might contain a JSON-formatted list of functions: ``["_func1", "func2"]``. + - In this case the file should contain a list of symbols, one per line. For legacy use cases JSON-formatted files are also supported: e.g. ``["_func1", "func2"]``. - The specified file path must be absolute, not relative. .. note:: Options can be specified as a single argument without a space diff --git a/tests/test_browser.py b/tests/test_browser.py index fa48ae734fd25..9831a1f6e025c 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -3298,7 +3298,7 @@ def test_async_iostream(self): }) def test_async_returnvalue(self, args): if '@' in str(args): - create_file('filey.txt', '["sync_tunnel", "sync_tunnel_bool"]') + create_file('filey.txt', 'sync_tunnel\nsync_tunnel_bool\n') self.btest('browser/async_returnvalue.cpp', '0', args=['-s', 'ASYNCIFY', '-s', 'ASYNCIFY_IGNORE_INDIRECT', '--js-library', test_file('browser', 'async_returnvalue.js')] + args + ['-s', 'ASSERTIONS']) def test_async_stack_overflow(self): diff --git a/tests/test_other.py b/tests/test_other.py index a86371ace71d5..0b2113a822e5b 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -6226,12 +6226,16 @@ def test_dash_s_link_flag(self): self.assertContained('hello, world!', self.run_js('a.out.js')) def test_dash_s_response_file_string(self): - create_file('response_file', '"MyModule"\n') - self.run_process([EMXX, test_file('hello_world.cpp'), '-s', 'EXPORT_NAME=@response_file']) + create_file('response_file.txt', 'MyModule\n') + create_file('response_file.json', '"MyModule"\n') + self.run_process([EMXX, test_file('hello_world.cpp'), '-s', 'EXPORT_NAME=@response_file.txt']) + self.run_process([EMXX, test_file('hello_world.cpp'), '-s', 'EXPORT_NAME=@response_file.json']) def test_dash_s_response_file_list(self): - create_file('response_file', '["_main", "_malloc"]\n') - self.run_process([EMXX, test_file('hello_world.cpp'), '-s', 'EXPORTED_FUNCTIONS=@response_file']) + create_file('response_file.txt', '_main\n_malloc\n') + create_file('response_file.json', '["_main", "_malloc"]\n') + self.run_process([EMXX, test_file('hello_world.cpp'), '-s', 'EXPORTED_FUNCTIONS=@response_file.txt']) + self.run_process([EMXX, test_file('hello_world.cpp'), '-s', 'EXPORTED_FUNCTIONS=@response_file.json']) def test_dash_s_response_file_misssing(self): err = self.expect_fail([EMXX, test_file('hello_world.cpp'), '-s', 'EXPORTED_FUNCTIONS=@foo']) @@ -8117,12 +8121,19 @@ def test_dash_s_list_parsing(self): void c() { printf("c\n"); } void d() { printf("d\n"); } ''') - create_file('response', r'''[ + create_file('response.json', '''\ +[ "_a", "_b", "_c", "_d" ] +''') + create_file('response.txt', '''\ +_a +_b +_c +_d ''') for export_arg, expected in [ @@ -8137,7 +8148,9 @@ def test_dash_s_list_parsing(self): # extra space at end - should be ignored ("EXPORTED_FUNCTIONS=['_a', '_b', '_c', '_d' ]", ''), # extra newline in response file - should be ignored - ("EXPORTED_FUNCTIONS=@response", ''), + ("EXPORTED_FUNCTIONS=@response.json", ''), + # Simple one-per-line response file format + ("EXPORTED_FUNCTIONS=@response.txt", ''), # stray slash ("EXPORTED_FUNCTIONS=['_a', '_b', \\'_c', '_d']", '''undefined exported symbol: "\\\\'_c'"'''), # stray slash @@ -8154,6 +8167,9 @@ def test_dash_s_list_parsing(self): print(proc.stderr) if not expected: self.assertFalse(proc.stderr) + js = open('a.out.js').read() + for sym in ('_a', '_b', '_c', '_d'): + self.assertContained(f'var {sym} = ', js) else: self.assertNotEqual(proc.returncode, 0) self.assertContained(expected, proc.stderr) @@ -8165,16 +8181,18 @@ def test_asyncify_escaping(self): self.assertContained('Try to quote the entire argument', proc.stderr) def test_asyncify_response_file(self): - return self.skipTest(' TODO remove the support for multiple binaryen versions warning output ("function name" vs "pattern" etc).') create_file('a.txt', r'''[ "DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)" ] ''') - proc = self.run_process([EMCC, test_file('hello_world.c'), '-s', 'ASYNCIFY', '-s', "ASYNCIFY_ONLY=@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 onlylist contained a non-matching pattern: DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)', - proc.stderr) + + create_file('b.txt', 'DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)') + for file in ('a.txt', 'b.txt'): + proc = self.run_process([EMCC, test_file('hello_world.c'), '-sASYNCIFY', f'-sASYNCIFY_ONLY=@{file}'], stdout=PIPE, stderr=PIPE) + # we should parse the response file properly, and then issue a proper warning for the missing function + self.assertContained( + 'Asyncify onlylist contained a non-matching pattern: DOS_ReadFile(unsigned short, unsigned char*, unsigned short*, bool)', + proc.stderr) def test_asyncify_advise(self): src = test_file('other', 'asyncify_advise.c')