Skip to content

[EH] Add more manual tests for new Wasm EH #21913

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.

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 4 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
11 changes: 11 additions & 0 deletions test/other/metadce/test_metadce_cxx_except_wasm_exnref.exports
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
__indirect_function_table
__trap
__wasm_call_ctors
_emscripten_stack_alloc
dynCall_iiiiiijj
dynCall_iiiiij
dynCall_iiiiijj
dynCall_jiji
dynCall_viijii
main
memory
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9807
10 changes: 10 additions & 0 deletions test/other/metadce/test_metadce_cxx_except_wasm_exnref.imports
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
env._abort_js
env._emscripten_memcpy_js
env.emscripten_resize_heap
env.strftime_l
wasi_snapshot_preview1.environ_get
wasi_snapshot_preview1.environ_sizes_get
wasi_snapshot_preview1.fd_close
wasi_snapshot_preview1.fd_read
wasi_snapshot_preview1.fd_seek
wasi_snapshot_preview1.fd_write
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
24140
10 changes: 10 additions & 0 deletions test/other/metadce/test_metadce_cxx_except_wasm_exnref.sent
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
_abort_js
_emscripten_memcpy_js
emscripten_resize_heap
environ_get
environ_sizes_get
fd_close
fd_read
fd_seek
fd_write
strftime_l
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
141975
14 changes: 11 additions & 3 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from common import env_modify, with_env_modify, disabled, flaky, node_pthreads, also_with_wasm_bigint
from common import read_file, read_binary, requires_v8, requires_node, requires_wasm2js, requires_node_canary
from common import compiler_for, crossplatform, no_4gb, no_2gb, also_with_minimal_runtime
from common import with_all_eh_sjlj, with_all_sjlj, also_with_standalone_wasm, can_do_standalone, no_wasm64
from common import with_all_eh_sjlj, with_all_sjlj, also_with_standalone_wasm, can_do_standalone, no_wasm64, requires_wasm_exnref
from common import NON_ZERO, WEBIDL_BINDER, EMBUILDER, PYTHON
import clang_native

Expand Down Expand Up @@ -896,6 +896,7 @@ def test_longjmp_zero(self):
self.skipTest('https://github.com/emscripten-core/emscripten/issues/21533')
self.do_core_test('test_longjmp_zero.c')

@requires_wasm_exnref
def test_longjmp_with_and_without_exceptions(self):
# Emscripten SjLj with and without Emscripten EH support
self.set_setting('SUPPORT_LONGJMP', 'emscripten')
Expand All @@ -908,13 +909,15 @@ def test_longjmp_with_and_without_exceptions(self):
self.set_setting('SUPPORT_LONGJMP', 'wasm')
if self.is_wasm2js():
self.skipTest('wasm2js does not support wasm EH/SjLj')
self.require_wasm_eh()
# FIXME Temporarily disabled. Enable this later when the bug is fixed.
if '-fsanitize=address' in self.emcc_args:
self.skipTest('Wasm EH does not work with asan yet')
self.emcc_args.append('-fwasm-exceptions')
for arg in ['-fwasm-exceptions', '-fno-exceptions']:
self.do_core_test('test_longjmp.c', emcc_args=[arg])
# Wasm SjLj with and with new EH (exnref) support
self.set_setting('WASM_EXNREF')
self.do_core_test('test_longjmp.c', emcc_args=['-fwasm-exceptions'])

@with_all_sjlj
def test_longjmp2(self):
Expand Down Expand Up @@ -1031,6 +1034,7 @@ def test_exceptions(self):
self.maybe_closure()
self.do_run_in_out_file_test('core/test_exceptions.cpp', out_suffix='_caught')

@requires_wasm_exnref
def test_exceptions_with_and_without_longjmp(self):
self.set_setting('EXCEPTION_DEBUG')
self.maybe_closure()
Expand All @@ -1043,14 +1047,18 @@ def test_exceptions_with_and_without_longjmp(self):
self.clear_setting('DISABLE_EXCEPTION_CATCHING')
if self.is_wasm2js():
self.skipTest('wasm2js does not support wasm EH/SjLj')
self.require_wasm_eh()
# FIXME Temporarily disabled. Enable this later when the bug is fixed.
if '-fsanitize=address' in self.emcc_args:
self.skipTest('Wasm EH does not work with asan yet')
self.emcc_args.append('-fwasm-exceptions')
for support_longjmp in [0, 'wasm']:
self.set_setting('SUPPORT_LONGJMP', support_longjmp)
self.do_run_in_out_file_test('core/test_exceptions.cpp', out_suffix='_caught')
# Wasm new EH (exnref) with and without Wasm SjLj support
self.set_setting('WASM_EXNREF')
for support_longjmp in [0, 'wasm']:
self.set_setting('SUPPORT_LONGJMP', support_longjmp)
self.do_run_in_out_file_test('core/test_exceptions.cpp', out_suffix='_caught')

def test_exceptions_off(self):
self.set_setting('DISABLE_EXCEPTION_CATCHING')
Expand Down
26 changes: 20 additions & 6 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from common import env_modify, no_mac, no_windows, only_windows, requires_native_clang, with_env_modify
from common import create_file, parameterized, NON_ZERO, node_pthreads, TEST_ROOT, test_file
from common import compiler_for, EMBUILDER, requires_v8, requires_node, requires_wasm64, requires_node_canary
from common import requires_wasm_eh, crossplatform, with_all_eh_sjlj, with_all_sjlj
from common import requires_wasm_exnref, crossplatform, with_all_eh_sjlj, with_all_sjlj
from common import also_with_standalone_wasm, also_with_env_modify, also_with_wasm2js
from common import also_with_minimal_runtime, also_with_wasm_bigint, also_with_wasm64, flaky
from common import EMTEST_BUILD_VERBOSE, PYTHON, WEBIDL_BINDER
Expand Down Expand Up @@ -3297,13 +3297,19 @@ def test_embind_tsgen_memory64(self):
self.get_emcc_args())
self.assertFileContents(test_file('other/embind_tsgen_memory64.d.ts'), read_file('embind_tsgen_memory64.d.ts'))

def test_embind_tsgen_exceptions(self):
@parameterized({
'': [0],
'wasm_exnref': [1]
})
def test_embind_tsgen_exceptions(self, wasm_exnref):
self.set_setting('WASM_EXNREF', wasm_exnref)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Presumably this should have:

if wasm_exnref: 
  self.requires_wasm_enxref()

Copy link
Member Author

Choose a reason for hiding this comment

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

This only runs compilation and does not run it. Note that there was no self.require_wasm_eh() before.

# Check that when Wasm exceptions and assertions are enabled bindings still generate.
self.run_process([EMXX, test_file('other/embind_tsgen.cpp'),
'-lembind', '-fwasm-exceptions', '-sASSERTIONS',
# Use the deprecated `--embind-emit-tsd` to ensure it
# still works until removed.
'--embind-emit-tsd', 'embind_tsgen.d.ts', '-Wno-deprecated'])
'--embind-emit-tsd', 'embind_tsgen.d.ts', '-Wno-deprecated'] +
self.get_emcc_args())
self.assertFileContents(test_file('other/embind_tsgen.d.ts'), read_file('embind_tsgen.d.ts'))

def test_embind_jsgen_method_pointer_stability(self):
Expand Down Expand Up @@ -8531,6 +8537,7 @@ def test_metadce_minimal_pthreads(self):
'-sDEMANGLE_SUPPORT', '-Wno-deprecated'], [], ['waka']), # noqa
# Wasm EH's code size increase is smaller than that of Emscripten EH
'except_wasm': (['-O2', '-fwasm-exceptions'], [], ['waka']), # noqa
'except_wasm_exnref': (['-O2', '-fwasm-exceptions', '-sWASM_EXNREF'], [], ['waka']), # noqa
# eval_ctors 1 can partially optimize, but runs into getenv() for locale
# code. mode 2 ignores those and fully optimizes out the ctors
'ctors1': (['-O2', '-sEVAL_CTORS'], [], ['waka']), # noqa
Expand Down Expand Up @@ -8830,7 +8837,8 @@ def test_lto(self, args):
@parameterized({
'noexcept': [],
'except': ['-sDISABLE_EXCEPTION_CATCHING=0'],
'except_wasm': ['-fwasm-exceptions']
'except_wasm': ['-fwasm-exceptions'],
'except_wasm_exnref': ['-fwasm-exceptions', '-sWASM_EXNREF']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same 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.

The same

})
def test_lto_libcxx(self, *args):
self.run_process([EMXX, test_file('hello_libcxx.cpp'), '-flto'] + list(args))
Expand All @@ -8849,11 +8857,14 @@ def test_lto_flags(self):

# We have LTO tests covered in 'wasmltoN' targets in test_core.py, but they
# don't run as a part of Emscripten CI, so we add a separate LTO test here.
@requires_wasm_eh
@requires_wasm_exnref
def test_lto_wasm_exceptions(self):
self.set_setting('EXCEPTION_DEBUG')
self.emcc_args += ['-fwasm-exceptions', '-flto']
self.do_run_in_out_file_test('core/test_exceptions.cpp', out_suffix='_caught')
# New Wasm EH with exnref
self.set_setting('WASM_EXNREF')
self.do_run_in_out_file_test('core/test_exceptions.cpp', out_suffix='_caught')

@parameterized({
'': ([],),
Expand Down Expand Up @@ -12269,13 +12280,16 @@ def test_standalone_export_main(self):
# We should consider making this a warning since the `_main` export is redundant.
self.run_process([EMCC, '-sEXPORTED_FUNCTIONS=_main', '-sSTANDALONE_WASM', test_file('core/test_hello_world.c')])

@requires_wasm_eh
@requires_wasm_exnref
def test_standalone_wasm_exceptions(self):
self.set_setting('STANDALONE_WASM')
self.set_setting('WASM_BIGINT')
self.wasm_engines = []
self.emcc_args += ['-fwasm-exceptions']
self.do_run_in_out_file_test('core/test_exceptions.cpp', out_suffix='_caught')
# New Wasm EH with exnref
self.set_setting('WASM_EXNREF')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again I wonder if that can parameterized instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can, but does this run on node 22?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can select specific tests to to run on node canary. They are listed in the .circleci config.

Copy link
Member Author

@aheejin aheejin May 10, 2024

Choose a reason for hiding this comment

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

It looks test-wasm64 and test-wasm64-4gb

test-wasm64:
# We don't use `bionic` here since its tool old to run recent node versions:
# `/lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found`
executor: linux-python
steps:
- prepare-for-tests
# The linux-python image uses /home/circleci rather than /root and jsvu
# hardcodes /root into its launcher scripts so we need to reinstall v8.
- run: rm -rf $HOME/.jsvu
- install-v8
- install-node-canary
- run-tests:
title: "wasm64"
test_targets: "
wasm64
core_2gb.test_*em_asm*
core_2gb.test_*embind*
wasm64l.test_bigswitch
other.test_memory64_proxies
other.test_failing_growth_wasm64"
- upload-test-results
test-wasm64-4gb:
environment:
LANG: "C.UTF-8"
# Only run 2 tests at a time to avoid OOM (since each tests used >4gb)
EMCC_CORES: "2"
# We don't use `bionic` here since its too old to run recent node versions:
# `/lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found`
executor: linux-python
steps:
- prepare-for-tests
# The linux-python image uses /home/circleci rather than /root and jsvu
# hardcodes /root into its launcher scripts so we need to reinstall v8.
- run: rm -rf $HOME/.jsvu
- install-v8
- install-node-canary
- run-tests:
title: "wasm64_4gb"
test_targets: "wasm64_4gb"
- upload-test-results

and some tests in test-compat
test-node-compat:
# We don't use `bionic` here since its too old to run recent node versions:
# `/lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found`
executor: linux-python
environment:
LANG: "C.UTF-8"
EMTEST_SKIP_V8: "1"
steps:
- checkout
- run:
name: submodule update
command: git submodule update --init
- pip-install
- install-emsdk
- install-node-canary
- run-tests:
title: "node (canary)"
test_targets: "
other.test_deterministic
other.test_gen_struct_info
other.test_native_call_before_init
other.test_node_unhandled_rejection
core2.test_hello_world
core0.test_pthread_join_and_asyncify
core0.test_async_ccall_promise_jspi
core0.test_async_ccall_promise_exit_runtime_jspi
core0.test_cubescript_jspi"

are running on node canary, right?

Then I wonder, why are these tests, which has self.require_wasm_exnref(), passing in test-core0 and test-other? test-core0 and test-other don't seem to run on node canary, right? They say ok instead of skipped...

test_exceptions_with_and_without_longjmp (test_core.core0) ... ok (9.40s)
test_longjmp_with_and_without_exceptions (test_core.core0) ... ok (6.01s)
test_lto_wasm_exceptions (test_other.other) ... ok (8.19s)
test_standalone_wasm_exceptions (test_other.other) ... ok (2.03s)

test-core0 log:
https://app.circleci.com/pipelines/github/emscripten-core/emscripten/35053/workflows/a5482f96-8732-4e8f-b7ba-bce7fb9a77ab/jobs/783071
test-other log:
https://app.circleci.com/pipelines/github/emscripten-core/emscripten/35053/workflows/a5482f96-8732-4e8f-b7ba-bce7fb9a77ab/jobs/783072

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that is because v8 is installed, which also satisfies the require_wasm_exnref requirement.
See https://app.circleci.com/pipelines/github/emscripten-core/emscripten/35053/workflows/a5482f96-8732-4e8f-b7ba-bce7fb9a77ab/jobs/783041

Copy link
Member Author

@aheejin aheejin May 10, 2024

Choose a reason for hiding this comment

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

Added @requires_wasm_exnref to all relevant tests I modified here, and removed self.require_wasm_eh() and self.require_wasm_exnref(). Didn't add them to the list of node-canary tests, given that they can run on v8 anyway: 6691a92

self.do_run_in_out_file_test('core/test_exceptions.cpp', out_suffix='_caught')

def test_missing_malloc_export(self):
# we used to include malloc by default. show a clear error in builds with
Expand Down
Loading