Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions embuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
'libc_optz-debug',
'libc++abi',
'libc++abi-except',
'libc++abi-debug-except',
'libc++abi-noexcept',
'libc++',
'libc++-except',
Expand Down
5 changes: 5 additions & 0 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -2652,6 +2652,11 @@ 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:
settings.EXPORTED_FUNCTIONS += ['___cpp_exception']

# Make `getExceptionMessage` and other necessary functions available for use.
if settings.EXPORT_EXCEPTION_HANDLING_HELPERS:
# We also export refcount increasing and decreasing functions because if you
Expand Down
11 changes: 9 additions & 2 deletions src/library_exceptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,11 +417,18 @@ var LibraryExceptions = {
return Module['asm']['__cpp_exception'];
},

// Given an WebAssembly.Exception object, returns the actual user-thrown
// C++ object address in the Wasm memory.
// Throw a WebAssembly.Exception object with the C++ tag. If traceStack is
// true, includes the stack traces within the exception object.
// WebAssembly.Exception is a JS object representing a Wasm exception,
// provided by Wasm JS API:
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Exception
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps add something like. "In release builds this function is not needed and the native _Unwind_RaiseException is used instead"

Copy link
Member

Choose a reason for hiding this comment

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

To add to that, perhaps we can put the entire function within #if ASSERTIONS? That would both further clarify it is for debug builds, and also we'd get a link error if we try to use it by mistake in the future.

(Though from discussion below perhaps we want to allow this to be used optionally in release builds too. In that case ignore my comment here.)

Copy link
Member Author

Choose a reason for hiding this comment

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

@sbc100 Added the comment.
@kripken Put the #if ASSERTIONS guard around it.

__throwCppWebAssemblyException__deps: ['$getCppExceptionTag'],
__throwCppWebAssemblyException: function(ex, traceStack) {
throw new WebAssembly.Exception(getCppExceptionTag(), [ex], {traceStack: traceStack});
},

// Given an WebAssembly.Exception object, returns the actual user-thrown
// C++ object address in the Wasm memory.
$getCppExceptionThrownObjectFromWebAssemblyException__deps: ['$getCppExceptionTag', '__thrown_object_from_unwind_exception'],
$getCppExceptionThrownObjectFromWebAssemblyException: function(ex) {
// In Wasm EH, the value extracted from WebAssembly.Exception is a pointer
Expand Down
15 changes: 15 additions & 0 deletions system/lib/libcxxabi/src/cxa_exception.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,13 @@ handler, _Unwind_RaiseException may return. In that case, __cxa_throw
will call terminate, assuming that there was no handler for the
exception.
*/

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

void
#ifdef __USING_WASM_EXCEPTIONS__
// In wasm, destructors return their argument
Expand Down Expand Up @@ -280,6 +287,14 @@ __cxa_throw(void *thrown_object, std::type_info *tinfo, void (*dest)(void *)) {

#ifdef __USING_SJLJ_EXCEPTIONS__
_Unwind_SjLj_RaiseException(&exception_header->unwindHeader);
#elif __USING_WASM_EXCEPTIONS__
#ifdef NDEBUG
_Unwind_RaiseException(&exception_header->unwindHeader);
#else
// In debug mode, call a JS library function to use WebAssembly.Exception JS
// API, which enables us to include stack traces
__throwCppWebAssemblyException(&exception_header->unwindHeader, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about calling this _Unwind_RaiseException_WithStacktrace? Since its basically the same function as _Unwind_RaiseException? Maybe it doesn't need a second argument if its always called with true?

Copy link
Member Author

Choose a reason for hiding this comment

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

_Unwind_ functions are the naming convention of libunwind, and this function is in library_exceptions.js.. So I named it like this. On the other hand, library_exception.js's (or other library JS files') naming schemes are kind of mixed... Some are camelCase, some are snake_case, and some are with two underscores, some are with one underscore, some are without... So it was kind of not very easy to name it. Should it have leading underscores or not?

The reason I added the boolean option is in case users want to directly use that function to throw WebAssembly.Exception from JS, in which case they may or may not want the stack traces.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But we don't have such users yet do we? So maybe we can keep the code simple for now and make it more complex later if it proves useful, and also keep this function private/internal.

Also, a user who doesn't want a stacktrace could/should presumably just call the existing _Unwind_RaiseException function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, given that we decided to use this as an internal function for the moment, will rename this to __throw_exception_with_stack_trace. (Not sure about JS library naming schemes, but it looks internal functions are likely to be in snake_case...) Let me know if you have a better idea.

#endif
#else
_Unwind_RaiseException(&exception_header->unwindHeader);
#endif
Expand Down
41 changes: 41 additions & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -7904,6 +7904,47 @@ 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_v8
def test_wasm_exceptions_stack_trace(self):
src = r'''
void bar() {
throw 3;
}
void foo() {
bar();
}
int main() {
foo();
return 0;
}
'''
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
# at ___throwCppWebAssemblyException (test.js:1139:13)
# 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)
# at __original_main (wasm://wasm/009a7c9a:wasm-function[20]:0x15a6)
# at main (wasm://wasm/009a7c9a:wasm-function[56]:0x25be)
# at test.js:833:22
# 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']

# We attach stack traces to exception objects only when ASSERTIONS is set
self.set_setting('ASSERTIONS')
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)
err = self.do_run(src, emcc_args=emcc_args, assert_returncode=NON_ZERO)
for check in stack_trace_checks:
self.assertNotContained(check, err)

@requires_node
def test_jsrun(self):
print(config.NODE_JS)
Expand Down
3 changes: 1 addition & 2 deletions tools/system_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ def can_use(self):
return super().can_use() and settings.SHARED_MEMORY


class libcxxabi(NoExceptLibrary, MTLibrary):
class libcxxabi(NoExceptLibrary, MTLibrary, DebugLibrary):
name = 'libc++abi'
cflags = [
'-Oz',
Expand All @@ -1363,7 +1363,6 @@ class libcxxabi(NoExceptLibrary, MTLibrary):

def get_cflags(self):
cflags = super().get_cflags()
cflags.append('-DNDEBUG')
if not self.is_mt and not self.is_ww:
cflags.append('-D_LIBCXXABI_HAS_NO_THREADS')
if self.eh_mode == Exceptions.NONE:
Expand Down