From 2f0eb94ce0413ab6f7d6fdb5f4f2fd82a9f57d75 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 30 Jan 2023 17:11:52 -0800 Subject: [PATCH 1/6] [EH] Add EXCEPTION_STACK_TRACES option This adds `EXCEPTION_STACK_TRACES` option, which embeds stack traces into exception objects even when `ASSERTIONS` is not set. This is for the users who wants exception stack traces but don't want to incur the full overhead of `ASSERTIONS`. Exception stack traces are enabled when either of `ASSERTIONS` or `EXCEPTION_STACK_TRACES` is true. This currently works only for Wasm EH. The reason this option's name is not `WASM_EXCEPTION_STACK_TRACES` is I think this can work for Emscripten EH later if something like #18535 lands. Fixes #18533. --- ChangeLog.md | 6 ++++++ emcc.py | 7 ++++--- src/library_exceptions.js | 2 +- src/settings.js | 8 ++++++++ src/shell.js | 2 +- test/test_other.py | 24 +++++++++++++++++++++--- tools/system_libs.py | 4 ++++ 7 files changed, 45 insertions(+), 8 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 5b4f139b0ed64..ff2389edcef70 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -20,6 +20,12 @@ See docs/process.md for more on how version tagging works. 3.1.32 (in development) ----------------------- +- In Wasm exception mode (`-fwasm-exceptions`), when + `EXCEPTION_STACK_TRACES` is enabled, uncaught exceptions will display stack + traces. Note that stack traces are enabled when either of `ASSERTIONS` or this + `EXCEPTION_STACK_TRACES` is enabled, so this option is mainly for the users + who want only exceptions' stack traces without turning `ASSERTIONS` on. This + option currently works only for Wasm exceptions (-fwasm-exceptions). (#????) 3.1.31 - 01/26/23 ----------------- diff --git a/emcc.py b/emcc.py index 512f5fa5e4171..0e73c43bd4082 100755 --- a/emcc.py +++ b/emcc.py @@ -2785,9 +2785,10 @@ def get_full_import_name(name): if settings.WASM_EXCEPTIONS: settings.REQUIRED_EXPORTS += ['__trap'] - # When ASSERTIONS is set, we include stack traces in Wasm exception objects - # using the JS API, which needs this C++ tag exported. - if settings.ASSERTIONS and settings.WASM_EXCEPTIONS: + # When ASSERTIONS or EXCEPTION_STACK_TRACES is set, we include stack traces in + # Wasm exception objects using the JS API, which needs this C++ tag exported. + if (settings.ASSERTIONS or settings.EXCEPTION_STACK_TRACES) and \ + settings.WASM_EXCEPTIONS: settings.EXPORTED_FUNCTIONS += ['___cpp_exception'] settings.EXPORT_EXCEPTION_HANDLING_HELPERS = True diff --git a/src/library_exceptions.js b/src/library_exceptions.js index 107c2c3be8300..49e835d94e6c7 100644 --- a/src/library_exceptions.js +++ b/src/library_exceptions.js @@ -401,7 +401,7 @@ var LibraryExceptions = { #endif }, -#if ASSERTIONS +#if ASSERTIONS || EXCEPTION_STACK_TRACES // Throw a WebAssembly.Exception object with the C++ tag with a stack trace // embedded. WebAssembly.Exception is a JS object representing a Wasm // exception, provided by Wasm JS API: diff --git a/src/settings.js b/src/settings.js index 52f384dfd9dea..cf6f894033abd 100644 --- a/src/settings.js +++ b/src/settings.js @@ -708,6 +708,14 @@ var EXCEPTION_CATCHING_ALLOWED = []; // example usage. var EXPORT_EXCEPTION_HANDLING_HELPERS = false; +// When this is enabled, exceptions will contain stack traces and uncaught +// exceptions will display stack traces upon exiting. Note that stack traces +// will be shown when either of EXCEPTION_STACK_TRACES or ASSERTIONS is enabled. +// This option is for users who want exceptions' stack traces but do not want +// other overheads ASSERTIONS can incur. This currently works only for Wasm +// exceptions (-fwasm-exceptions). +var EXCEPTION_STACK_TRACES = false; + // Internal: Tracks whether Emscripten should link in exception throwing (C++ // 'throw') support library. This does not need to be set directly, but pass // -fno-exceptions to the build disable exceptions support. (This is basically diff --git a/src/shell.js b/src/shell.js index f43547e574a5e..9017d886638a0 100644 --- a/src/shell.js +++ b/src/shell.js @@ -173,7 +173,7 @@ var read_, function logExceptionOnExit(e) { if (e instanceof ExitStatus) return; let toLog = e; -#if ASSERTIONS +#if ASSERTIONS || EXCEPTION_STACK_TRACES if (e && typeof e == 'object' && e.stack) { toLog = [e, e.stack]; } diff --git a/test/test_other.py b/test/test_other.py index a9baee0543d9a..143fc467f55c3 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -8062,7 +8062,12 @@ def test_wasm_nope(self): out = self.run_js('a.out.js', assert_returncode=NON_ZERO) self.assertContained('no native wasm support detected', out) - @requires_wasm_eh + # FIXME Node v18.13 (LTS as of Jan 2023) has not yet implemented the new + # optional 'traceStack' option in WebAssembly.Exception constructor + # (https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/Exception/Exception) + # and embeds stack traces unconditionally. Change this back to + # @requires_wasm_eh if this issue is fixed later. + @requires_v8 def test_wasm_exceptions_stack_trace_and_message(self): src = r''' #include @@ -8098,12 +8103,25 @@ def test_wasm_exceptions_stack_trace_and_message(self): 'at foo', 'at main'] - # We attach stack traces to exception objects only when ASSERTIONS is set - self.set_setting('ASSERTIONS') + # Stack traces are enabled when either of ASSERTIONS or + # EXCEPTION_STACK_TRACES is enabled. + self.set_setting('ASSERTIONS', 1) + self.set_setting('EXCEPTION_STACK_TRACES', 1) + self.do_run(src, emcc_args=emcc_args, assert_all=True, + assert_returncode=NON_ZERO, expected_output=stack_trace_checks) + + self.set_setting('ASSERTIONS', 0) + self.set_setting('EXCEPTION_STACK_TRACES', 1) + self.do_run(src, emcc_args=emcc_args, assert_all=True, + assert_returncode=NON_ZERO, expected_output=stack_trace_checks) + + self.set_setting('ASSERTIONS', 1) + self.set_setting('EXCEPTION_STACK_TRACES', 0) self.do_run(src, emcc_args=emcc_args, assert_all=True, assert_returncode=NON_ZERO, expected_output=stack_trace_checks) self.set_setting('ASSERTIONS', 0) + self.set_setting('EXCEPTION_STACK_TRACES', 0) err = self.do_run(src, emcc_args=emcc_args, assert_returncode=NON_ZERO) for check in stack_trace_checks: self.assertNotContained(check, err) diff --git a/tools/system_libs.py b/tools/system_libs.py index 0c8394935561a..8b0e0245d1b73 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -1376,6 +1376,10 @@ class libcxxabi(NoExceptLibrary, MTLibrary, DebugLibrary): ] includes = ['system/lib/libcxx/src'] + def __init__(self, **kwargs): + super().__init__(**kwargs) + self.is_debug |= settings.EXCEPTION_STACK_TRACES + def get_cflags(self): cflags = super().get_cflags() if not self.is_mt and not self.is_ww: From 14035e952ecc82eb416d3caca938929404325121 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 31 Jan 2023 17:16:08 -0800 Subject: [PATCH 2/6] Address comments --- ChangeLog.md | 8 ++++---- emcc.py | 5 +++-- src/library_exceptions.js | 2 +- src/settings.js | 9 ++++----- src/shell.js | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index ff2389edcef70..3a96486f7d375 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -22,10 +22,10 @@ See docs/process.md for more on how version tagging works. ----------------------- - In Wasm exception mode (`-fwasm-exceptions`), when `EXCEPTION_STACK_TRACES` is enabled, uncaught exceptions will display stack - traces. Note that stack traces are enabled when either of `ASSERTIONS` or this - `EXCEPTION_STACK_TRACES` is enabled, so this option is mainly for the users - who want only exceptions' stack traces without turning `ASSERTIONS` on. This - option currently works only for Wasm exceptions (-fwasm-exceptions). (#????) + traces. This defaults to true when `ASSERTIONS` is enabled. This option is + mainly for the users who want only exceptions' stack traces without turning + `ASSERTIONS` on. This option currently works only for Wasm exceptions + (-fwasm-exceptions). (#18642) 3.1.31 - 01/26/23 ----------------- diff --git a/emcc.py b/emcc.py index 0e73c43bd4082..8533ad22307ef 100755 --- a/emcc.py +++ b/emcc.py @@ -2787,8 +2787,9 @@ def get_full_import_name(name): # When ASSERTIONS or EXCEPTION_STACK_TRACES is set, we include stack traces in # Wasm exception objects using the JS API, which needs this C++ tag exported. - if (settings.ASSERTIONS or settings.EXCEPTION_STACK_TRACES) and \ - settings.WASM_EXCEPTIONS: + if settings.ASSERTIONS: + settings.EXCEPTION_STACK_TRACES = 1 + if settings.EXCEPTION_STACK_TRACES and settings.WASM_EXCEPTIONS: settings.EXPORTED_FUNCTIONS += ['___cpp_exception'] settings.EXPORT_EXCEPTION_HANDLING_HELPERS = True diff --git a/src/library_exceptions.js b/src/library_exceptions.js index 49e835d94e6c7..c9f0a7625aba8 100644 --- a/src/library_exceptions.js +++ b/src/library_exceptions.js @@ -401,7 +401,7 @@ var LibraryExceptions = { #endif }, -#if ASSERTIONS || EXCEPTION_STACK_TRACES +#if EXCEPTION_STACK_TRACES // Throw a WebAssembly.Exception object with the C++ tag with a stack trace // embedded. WebAssembly.Exception is a JS object representing a Wasm // exception, provided by Wasm JS API: diff --git a/src/settings.js b/src/settings.js index cf6f894033abd..c236c919cc5d1 100644 --- a/src/settings.js +++ b/src/settings.js @@ -709,11 +709,10 @@ var EXCEPTION_CATCHING_ALLOWED = []; var EXPORT_EXCEPTION_HANDLING_HELPERS = false; // When this is enabled, exceptions will contain stack traces and uncaught -// exceptions will display stack traces upon exiting. Note that stack traces -// will be shown when either of EXCEPTION_STACK_TRACES or ASSERTIONS is enabled. -// This option is for users who want exceptions' stack traces but do not want -// other overheads ASSERTIONS can incur. This currently works only for Wasm -// exceptions (-fwasm-exceptions). +// exceptions will display stack traces upon exiting. This defaults to true when +// ASSERTIONS is enabled. This option is for users who want exceptions' stack +// traces but do not want other overheads ASSERTIONS can incur. This currently +// works only for Wasm exceptions (-fwasm-exceptions). var EXCEPTION_STACK_TRACES = false; // Internal: Tracks whether Emscripten should link in exception throwing (C++ diff --git a/src/shell.js b/src/shell.js index 9017d886638a0..6f74e47aac3f2 100644 --- a/src/shell.js +++ b/src/shell.js @@ -173,7 +173,7 @@ var read_, function logExceptionOnExit(e) { if (e instanceof ExitStatus) return; let toLog = e; -#if ASSERTIONS || EXCEPTION_STACK_TRACES +#if EXCEPTION_STACK_TRACES if (e && typeof e == 'object' && e.stack) { toLog = [e, e.stack]; } From 5859ff892220bb69601e812e883f5f1a0b6e2171 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 1 Feb 2023 13:38:03 -0800 Subject: [PATCH 3/6] Comment --- tools/system_libs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/system_libs.py b/tools/system_libs.py index 8b0e0245d1b73..7a3a069a79eb1 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -1378,6 +1378,7 @@ class libcxxabi(NoExceptLibrary, MTLibrary, DebugLibrary): def __init__(self, **kwargs): super().__init__(**kwargs) + # EXCEPTION_STACK_TRACES currently requires the debug version of libc++abi self.is_debug |= settings.EXCEPTION_STACK_TRACES def get_cflags(self): From b7e04366c4f15089947d0aadfd71ac8e303cf0e1 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 1 Feb 2023 23:46:45 -0800 Subject: [PATCH 4/6] Ban the combination + test + TODO --- emcc.py | 4 +++- test/test_other.py | 15 ++++++++++----- tools/system_libs.py | 6 +++++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/emcc.py b/emcc.py index 8533ad22307ef..920fcb4c01548 100755 --- a/emcc.py +++ b/emcc.py @@ -2788,7 +2788,9 @@ def get_full_import_name(name): # When ASSERTIONS or EXCEPTION_STACK_TRACES is set, we include stack traces in # Wasm exception objects using the JS API, which needs this C++ tag exported. if settings.ASSERTIONS: - settings.EXCEPTION_STACK_TRACES = 1 + if 'EXCEPTION_STACK_TRACES' in user_settings and not settings.EXCEPTION_STACK_TRACES: + exit_with_error('EXCEPTION_STACK_TRACES cannot be disabled when ASSSERTIONS are enabled') + default_setting('EXCEPTION_STACK_TRACES', 1) if settings.EXCEPTION_STACK_TRACES and settings.WASM_EXCEPTIONS: settings.EXPORTED_FUNCTIONS += ['___cpp_exception'] settings.EXPORT_EXCEPTION_HANDLING_HELPERS = True diff --git a/test/test_other.py b/test/test_other.py index 143fc467f55c3..4cbc78cca7b30 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -8104,22 +8104,27 @@ def test_wasm_exceptions_stack_trace_and_message(self): 'at main'] # Stack traces are enabled when either of ASSERTIONS or - # EXCEPTION_STACK_TRACES is enabled. + # EXCEPTION_STACK_TRACES is enabled. You can't disable + # EXCEPTION_STACK_TRACES when ASSERTIONS is enabled. + + # Prints stack traces self.set_setting('ASSERTIONS', 1) self.set_setting('EXCEPTION_STACK_TRACES', 1) self.do_run(src, emcc_args=emcc_args, assert_all=True, assert_returncode=NON_ZERO, expected_output=stack_trace_checks) + # Prints stack traces self.set_setting('ASSERTIONS', 0) self.set_setting('EXCEPTION_STACK_TRACES', 1) self.do_run(src, emcc_args=emcc_args, assert_all=True, assert_returncode=NON_ZERO, expected_output=stack_trace_checks) - self.set_setting('ASSERTIONS', 1) - self.set_setting('EXCEPTION_STACK_TRACES', 0) - self.do_run(src, emcc_args=emcc_args, assert_all=True, - assert_returncode=NON_ZERO, expected_output=stack_trace_checks) + # Not allowed + create_file('src.cpp', src) + err = self.expect_fail([EMCC, 'src.cpp', '-sASSERTIONS=1', '-sEXCEPTION_STACK_TRACES=0']) + self.assertContained('EXCEPTION_STACK_TRACES cannot be disabled when ASSSERTIONS are enabled', err) + # Doesn't print stack traces self.set_setting('ASSERTIONS', 0) self.set_setting('EXCEPTION_STACK_TRACES', 0) err = self.do_run(src, emcc_args=emcc_args, assert_returncode=NON_ZERO) diff --git a/tools/system_libs.py b/tools/system_libs.py index 7a3a069a79eb1..7b118a852c12c 100644 --- a/tools/system_libs.py +++ b/tools/system_libs.py @@ -1378,7 +1378,11 @@ class libcxxabi(NoExceptLibrary, MTLibrary, DebugLibrary): def __init__(self, **kwargs): super().__init__(**kwargs) - # EXCEPTION_STACK_TRACES currently requires the debug version of libc++abi + # TODO EXCEPTION_STACK_TRACES currently requires the debug version of + # libc++abi, causing the debug version of libc++abi to be linked, which + # increases code size. libc++abi is not a big library to begin with, but if + # this becomes a problem, consider making EXCEPTION_STACK_TRACES work with + # the non-debug version of libc++abi. self.is_debug |= settings.EXCEPTION_STACK_TRACES def get_cflags(self): From 446981d17fdbbcda929ffa52e754ac3d22a12276 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 2 Feb 2023 11:21:45 -0800 Subject: [PATCH 5/6] Update emcc.py Co-authored-by: Alon Zakai --- emcc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/emcc.py b/emcc.py index 920fcb4c01548..b8b5feecc19c4 100755 --- a/emcc.py +++ b/emcc.py @@ -2789,7 +2789,7 @@ def get_full_import_name(name): # Wasm exception objects using the JS API, which needs this C++ tag exported. if settings.ASSERTIONS: if 'EXCEPTION_STACK_TRACES' in user_settings and not settings.EXCEPTION_STACK_TRACES: - exit_with_error('EXCEPTION_STACK_TRACES cannot be disabled when ASSSERTIONS are enabled') + exit_with_error('EXCEPTION_STACK_TRACES cannot be disabled when ASSERTIONS are enabled') default_setting('EXCEPTION_STACK_TRACES', 1) if settings.EXCEPTION_STACK_TRACES and settings.WASM_EXCEPTIONS: settings.EXPORTED_FUNCTIONS += ['___cpp_exception'] From fa96c39fdb1e7d8484df981545db6b6bf3a11118 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Thu, 2 Feb 2023 12:33:07 -0800 Subject: [PATCH 6/6] Typo fix --- test/test_other.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_other.py b/test/test_other.py index 4cbc78cca7b30..1952e7019b7d0 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -8122,7 +8122,7 @@ def test_wasm_exceptions_stack_trace_and_message(self): # Not allowed create_file('src.cpp', src) err = self.expect_fail([EMCC, 'src.cpp', '-sASSERTIONS=1', '-sEXCEPTION_STACK_TRACES=0']) - self.assertContained('EXCEPTION_STACK_TRACES cannot be disabled when ASSSERTIONS are enabled', err) + self.assertContained('EXCEPTION_STACK_TRACES cannot be disabled when ASSERTIONS are enabled', err) # Doesn't print stack traces self.set_setting('ASSERTIONS', 0)