Skip to content

[EH] Print message for uncaught exceptions #18003

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 10, 2022
Merged

[EH] Print message for uncaught exceptions #18003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ef9c5ce
[EH] Print message for uncaught exceptions
aheejin Sep 29, 2022
8e29c3b
Changelog
aheejin Oct 6, 2022
24b5566
flake8
aheejin Oct 6, 2022
a9d2f3f
Merge branch 'main' into wasmeh_message
aheejin Oct 6, 2022
ecdef0b
Add _free to EXPORTED_FUNCTION, which was missing all along
aheejin Oct 6, 2022
e825db2
Wasm64 fix
aheejin Oct 7, 2022
8a32e1d
Merge branch 'main' into wasmeh_message
aheejin Oct 9, 2022
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: 2 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ See docs/process.md for more on how version tagging works.
3.1.24 (in development)
-----------------------
- In Wasm exception mode (`-fwasm-exceptions`), when `ASSERTIONS` is enabled,
uncaught exceptions will display stack traces and what() message. (#17979 and
#18003)
uncaught exceptions will display stack traces. (#17979)
- It is now possible to specify indirect dependencies on JS library functions
directly in C/C++ source code. For example, in the case of a EM_JS or EM_ASM
Expand Down
3 changes: 2 additions & 1 deletion emcc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2633,6 +2633,7 @@ def get_full_import_name(name):
# using the JS API, which needs this C++ tag exported.
if settings.ASSERTIONS and settings.WASM_EXCEPTIONS:
settings.EXPORTED_FUNCTIONS += ['___cpp_exception']
settings.EXPORT_EXCEPTION_HANDLING_HELPERS = True

# Make `getExceptionMessage` and other necessary functions available for use.
if settings.EXPORT_EXCEPTION_HANDLING_HELPERS:
Expand All @@ -2642,7 +2643,7 @@ def get_full_import_name(name):
# What you need to do is different depending on the kind of EH you use
# (https://github.com/emscripten-core/emscripten/issues/17115).
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$getExceptionMessage', '$incrementExceptionRefcount', '$decrementExceptionRefcount']
settings.EXPORTED_FUNCTIONS += ['getExceptionMessage', '___get_exception_message']
settings.EXPORTED_FUNCTIONS += ['getExceptionMessage', '___get_exception_message', '_free']
if settings.WASM_EXCEPTIONS:
settings.EXPORTED_FUNCTIONS += ['___cpp_exception', '___cxa_increment_exception_refcount', '___cxa_decrement_exception_refcount', '___thrown_object_from_unwind_exception']

Expand Down
13 changes: 7 additions & 6 deletions src/library_exceptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,11 +396,11 @@ var LibraryExceptions = {
$getExceptionMessageCommon__deps: ['__get_exception_message', 'free', '$withStackSave'],
$getExceptionMessageCommon: function(ptr) {
return withStackSave(function() {
var type_addr_addr = stackAlloc(4);
var message_addr_addr = stackAlloc(4);
___get_exception_message(ptr, type_addr_addr, message_addr_addr);
var type_addr = HEAP32[type_addr_addr >> 2];
var message_addr = HEAP32[message_addr_addr >> 2];
var type_addr_addr = stackAlloc({{{ POINTER_SIZE }}});
var message_addr_addr = stackAlloc({{{ POINTER_SIZE }}});
___get_exception_message({{{ to64('ptr') }}}, {{{ to64('type_addr_addr') }}}, {{{ to64('message_addr_addr') }}});
var type_addr = {{{ makeGetValue('type_addr_addr', 0, '*') }}};
var message_addr = {{{ makeGetValue('message_addr_addr', 0, '*') }}};
var type = UTF8ToString(type_addr);
_free(type_addr);
var message;
Expand Down Expand Up @@ -431,9 +431,10 @@ var LibraryExceptions = {
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Exception
// In release builds, this function is not needed and the native
// _Unwind_RaiseException in libunwind is used instead.
__throw_exception_with_stack_trace__deps: ['$getCppExceptionTag'],
__throw_exception_with_stack_trace__deps: ['$getCppExceptionTag', '$getExceptionMessage'],
__throw_exception_with_stack_trace: function(ex) {
var e = new WebAssembly.Exception(getCppExceptionTag(), [ex], {traceStack: true});
e.message = getExceptionMessage(e);
// The generated stack trace will be in the form of:
//
// Error
Expand Down
4 changes: 2 additions & 2 deletions system/lib/libcxxabi/src/cxa_exception.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ exception.

#if defined(__USING_WASM_EXCEPTIONS__) && !defined(NDEBUG)
extern "C" {
void __throw_exception_with_stack_trace(_Unwind_Exception*, bool);
void __throw_exception_with_stack_trace(_Unwind_Exception*);
} // extern "C"
#endif

Expand Down Expand Up @@ -293,7 +293,7 @@ __cxa_throw(void *thrown_object, std::type_info *tinfo, void (*dest)(void *)) {
#else
// In debug mode, call a JS library function to use WebAssembly.Exception JS
// API, which enables us to include stack traces
__throw_exception_with_stack_trace(&exception_header->unwindHeader, true);
__throw_exception_with_stack_trace(&exception_header->unwindHeader);
Copy link
Member Author

@aheejin aheejin Oct 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This second argument is what I should've deleted in #17979 and forgot. (I first added an option in the JS f unction and then deleted it after feedback, but forgot to delete the caller side, which didn't error out)

#endif
#else
_Unwind_RaiseException(&exception_header->unwindHeader);
Expand Down
17 changes: 12 additions & 5 deletions test/test_other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7905,10 +7905,12 @@ def test_wasm_nope(self):
self.assertContained('no native wasm support detected', out)

@requires_v8
def test_wasm_exceptions_stack_trace(self):
def test_wasm_exceptions_stack_trace_and_message(self):
src = r'''
#include <stdexcept>

void bar() {
throw 3;
throw std::runtime_error("my message");
}
void foo() {
bar();
Expand All @@ -7921,8 +7923,8 @@ def test_wasm_exceptions_stack_trace(self):
emcc_args = ['-g', '-fwasm-exceptions']
self.v8_args.append('--experimental-wasm-eh')

# Stack trace example for this example code:
# exiting due to exception: [object WebAssembly.Exception],Error
# Stack trace and message example for this example code:
# exiting due to exception: [object WebAssembly.Exception],Error: std::runtime_error, my message
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated, but I wonder if we could drop the [object WebAssembly.Exception], here. It seems to be just unnecessary visual noise.

Copy link
Member Author

@aheejin aheejin Oct 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think It's easy to consistently take out that part, because different engines can have different error formats. For example, d8 prints:

exiting due to exception: [object WebAssembly.Exception],Error: std::runtime_error,my message
    at __cxa_throw (wasm://wasm/00462286:wasm-function[44]:0x3b0f)
    at bar() (wasm://wasm/00462286:wasm-function[9]:0x98c)
    at foo() (wasm://wasm/00462286:wasm-function[10]:0x992)
    at __original_main (wasm://wasm/00462286:wasm-function[11]:0x9c4)
    at main (wasm://wasm/00462286:wasm-function[12]:0x9e1)
    at test.js:934:22
    at callMain (test.js:1874:15)
    at doRun (test.js:1928:23)
    at run (test.js:1943:5)

Whereas Node prints:

/usr/local/google/home/aheejin/test/stack/test.js:147
      throw ex;
      ^

WebAssembly.Exception: std::runtime_error,my message
    at __cxa_throw (wasm://wasm/00462286:wasm-function[44]:0x3b0f)
    at bar() (wasm://wasm/00462286:wasm-function[9]:0x98c)
    at foo() (wasm://wasm/00462286:wasm-function[10]:0x992)
    at __original_main (wasm://wasm/00462286:wasm-function[11]:0x9c4)
    at main (wasm://wasm/00462286:wasm-function[12]:0x9e1)
    at /usr/local/google/home/aheejin/test/stack/test.js:934:22
    at callMain (/usr/local/google/home/aheejin/test/stack/test.js:1874:15)
    at doRun (/usr/local/google/home/aheejin/test/stack/test.js:1928:23)
    at run (/usr/local/google/home/aheejin/test/stack/test.js:1943:5)

Node.js v17.5.0

So while both of them have WebAssembly.Exception, their printing formats differ, making taking it out in a consistent way difficult. We can just simply replace WebAssembly.Exception with an empty string, but not sure if that's better..?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Leaving it as is sounds fine for to me then. We can attempt to refine in a followup perhaps.

# at __cxa_throw (wasm://wasm/009a7c9a:wasm-function[1551]:0x24367)
# at bar() (wasm://wasm/009a7c9a:wasm-function[12]:0xf53)
# at foo() (wasm://wasm/009a7c9a:wasm-function[19]:0x154e)
Expand All @@ -7932,7 +7934,12 @@ def test_wasm_exceptions_stack_trace(self):
# at callMain (test.js:4567:15)
# at doRun (test.js:4621:23)
# at run (test.js:4636:5)
stack_trace_checks = ['at __cxa_throw', 'at bar', 'at foo', 'at main']
stack_trace_checks = [
'std::runtime_error,my message',
'at __cxa_throw',
'at bar',
'at foo',
'at main']

# We attach stack traces to exception objects only when ASSERTIONS is set
self.set_setting('ASSERTIONS')
Expand Down