From 95c174ed1cabcb6dbaefcc22f2b73a6bead3b9d7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 4 May 2020 09:04:53 -0700 Subject: [PATCH 1/8] fix --- system/lib/libcxx/new.cpp | 4 ++++ system/lib/libcxxabi/src/stdlib_new_delete.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/system/lib/libcxx/new.cpp b/system/lib/libcxx/new.cpp index 901e78565857b..1f4df8ed94709 100644 --- a/system/lib/libcxx/new.cpp +++ b/system/lib/libcxx/new.cpp @@ -72,11 +72,15 @@ operator new(std::size_t size) _THROW_BAD_ALLOC if (nh) nh(); else +#ifdef __EMSCRIPTEN__ + abort(); +#else #ifndef _LIBCPP_NO_EXCEPTIONS throw std::bad_alloc(); #else break; #endif +#endif // __EMSCRIPTEN__ } return p; } diff --git a/system/lib/libcxxabi/src/stdlib_new_delete.cpp b/system/lib/libcxxabi/src/stdlib_new_delete.cpp index 698c5f7c290c0..3a3052befcd87 100644 --- a/system/lib/libcxxabi/src/stdlib_new_delete.cpp +++ b/system/lib/libcxxabi/src/stdlib_new_delete.cpp @@ -36,11 +36,15 @@ operator new(std::size_t size) _THROW_BAD_ALLOC if (nh) nh(); else +#ifdef __EMSCRIPTEN__ + abort(); +#else #ifndef _LIBCXXABI_NO_EXCEPTIONS throw std::bad_alloc(); #else break; #endif +#endif // __EMSCRIPTEN__ } return p; } From d93658ce39b1d3c271caa417d1e83c20151a7e1d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 4 May 2020 09:35:14 -0700 Subject: [PATCH 2/8] test --- tests/core/test_memorygrowth_new_error.cpp | 23 ++++++++++++++++++++++ tests/core/test_memorygrowth_new_error.txt | 1 + tests/test_core.py | 5 +++++ 3 files changed, 29 insertions(+) create mode 100644 tests/core/test_memorygrowth_new_error.cpp create mode 100644 tests/core/test_memorygrowth_new_error.txt diff --git a/tests/core/test_memorygrowth_new_error.cpp b/tests/core/test_memorygrowth_new_error.cpp new file mode 100644 index 0000000000000..ea972906020fb --- /dev/null +++ b/tests/core/test_memorygrowth_new_error.cpp @@ -0,0 +1,23 @@ +#include +#include +#include + +EMSCRIPTEN_KEEPALIVE extern "C" void allocate_too_much() { + std::vector x; + puts("allocating more than TOTAL_MEMORY; this will fail."); + x.resize(20 * 1024 * 1024); + puts("oh no, it didn't fail!"); +} + +int main() { + EM_ASM({ + // Catch the failure here so we can report it. + try { + _allocate_too_much(); + out("no error happened"); + } catch (e) { + assert(("" + e).indexOf("abort") >= 0, "expect an abort"); + out("an expected error occurred"); + } + }); +} diff --git a/tests/core/test_memorygrowth_new_error.txt b/tests/core/test_memorygrowth_new_error.txt new file mode 100644 index 0000000000000..89be86abf01aa --- /dev/null +++ b/tests/core/test_memorygrowth_new_error.txt @@ -0,0 +1 @@ +an expected error occurred diff --git a/tests/test_core.py b/tests/test_core.py index a32399f8cd478..1e11f9eec4d07 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -2169,6 +2169,11 @@ def test_memorygrowth_3_force_fail_reallocBuffer(self): self.emcc_args += ['-Wno-almost-asm', '-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'TEST_MEMORY_GROWTH_FAILS=1'] self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_3') + def test_memorygrowth_new_error(self): + # test that C++ new properly errors if we fail to malloc + self.emcc_args += ['-Wno-almost-asm', '-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'MAXIMUM_MEMORY=18MB'] + self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_new_error') + @no_asmjs() @no_wasm2js('no WebAssembly.Memory()') @no_asan('ASan alters the memory size') From 45403cca1fd5a9e20141aa3d8239ecf61b86e64c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 4 May 2020 10:25:48 -0700 Subject: [PATCH 3/8] fix --- system/lib/libcxx/new.cpp | 8 ++++---- system/lib/libcxxabi/src/stdlib_new_delete.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/system/lib/libcxx/new.cpp b/system/lib/libcxx/new.cpp index 1f4df8ed94709..c6d2a1e6d30ac 100644 --- a/system/lib/libcxx/new.cpp +++ b/system/lib/libcxx/new.cpp @@ -72,15 +72,15 @@ operator new(std::size_t size) _THROW_BAD_ALLOC if (nh) nh(); else -#ifdef __EMSCRIPTEN__ - abort(); -#else #ifndef _LIBCPP_NO_EXCEPTIONS throw std::bad_alloc(); +#else +#ifdef __EMSCRIPTEN__ + abort(); #else break; #endif -#endif // __EMSCRIPTEN__ +#endif } return p; } diff --git a/system/lib/libcxxabi/src/stdlib_new_delete.cpp b/system/lib/libcxxabi/src/stdlib_new_delete.cpp index 3a3052befcd87..2cfd1aa111899 100644 --- a/system/lib/libcxxabi/src/stdlib_new_delete.cpp +++ b/system/lib/libcxxabi/src/stdlib_new_delete.cpp @@ -36,15 +36,15 @@ operator new(std::size_t size) _THROW_BAD_ALLOC if (nh) nh(); else -#ifdef __EMSCRIPTEN__ - abort(); -#else #ifndef _LIBCXXABI_NO_EXCEPTIONS throw std::bad_alloc(); +#else +#ifdef __EMSCRIPTEN__ + abort(); #else break; #endif -#endif // __EMSCRIPTEN__ +#endif } return p; } From 9484ba9acb09d186b2000da2508213632fb2715a Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 4 May 2020 15:34:25 -0700 Subject: [PATCH 4/8] comments --- system/lib/libcxx/new.cpp | 5 +++++ system/lib/libcxxabi/src/stdlib_new_delete.cpp | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/system/lib/libcxx/new.cpp b/system/lib/libcxx/new.cpp index c6d2a1e6d30ac..bd0b42f7cb7c0 100644 --- a/system/lib/libcxx/new.cpp +++ b/system/lib/libcxx/new.cpp @@ -76,6 +76,11 @@ operator new(std::size_t size) _THROW_BAD_ALLOC throw std::bad_alloc(); #else #ifdef __EMSCRIPTEN__ + // Abort here so that when exceptions are disabled, we do not just + // return 0 when malloc returns 0. + // We could also do this with set_new_handler, but that adds a + // global constructor and a table entry, overhead that we can avoid + // by doing it this way. abort(); #else break; diff --git a/system/lib/libcxxabi/src/stdlib_new_delete.cpp b/system/lib/libcxxabi/src/stdlib_new_delete.cpp index 2cfd1aa111899..0fcb486ec663a 100644 --- a/system/lib/libcxxabi/src/stdlib_new_delete.cpp +++ b/system/lib/libcxxabi/src/stdlib_new_delete.cpp @@ -40,6 +40,11 @@ operator new(std::size_t size) _THROW_BAD_ALLOC throw std::bad_alloc(); #else #ifdef __EMSCRIPTEN__ + // Abort here so that when exceptions are disabled, we do not just + // return 0 when malloc returns 0. + // We could also do this with set_new_handler, but that adds a + // global constructor and a table entry, overhead that we can avoid + // by doing it this way. abort(); #else break; From 27b2ef1b7bc84c5dc1d98f1c6e590f1b0019a089 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 4 May 2020 15:38:25 -0700 Subject: [PATCH 5/8] rename and comment --- ..._new_error.cpp => test_memorygrowth_erroring_new.cpp} | 0 ..._new_error.txt => test_memorygrowth_erroring_new.txt} | 0 tests/test_core.py | 9 ++++++--- 3 files changed, 6 insertions(+), 3 deletions(-) rename tests/core/{test_memorygrowth_new_error.cpp => test_memorygrowth_erroring_new.cpp} (100%) rename tests/core/{test_memorygrowth_new_error.txt => test_memorygrowth_erroring_new.txt} (100%) diff --git a/tests/core/test_memorygrowth_new_error.cpp b/tests/core/test_memorygrowth_erroring_new.cpp similarity index 100% rename from tests/core/test_memorygrowth_new_error.cpp rename to tests/core/test_memorygrowth_erroring_new.cpp diff --git a/tests/core/test_memorygrowth_new_error.txt b/tests/core/test_memorygrowth_erroring_new.txt similarity index 100% rename from tests/core/test_memorygrowth_new_error.txt rename to tests/core/test_memorygrowth_erroring_new.txt diff --git a/tests/test_core.py b/tests/test_core.py index e16a59eccc58e..4a8e9d501d8da 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -2169,10 +2169,13 @@ def test_memorygrowth_3_force_fail_reallocBuffer(self): self.emcc_args += ['-Wno-almost-asm', '-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'TEST_MEMORY_GROWTH_FAILS=1'] self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_3') - def test_memorygrowth_new_error(self): - # test that C++ new properly errors if we fail to malloc + def test_memorygrowth_erroring_new(self): + # test that C++ new properly errors if we fail to malloc when growth is + # enabled. (if growth is not enabled we go down a different - simpler - + # code path that always aborts on any growth attemp; this test checks the + # case where growth is enabled but despite that we fail to grow). self.emcc_args += ['-Wno-almost-asm', '-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'MAXIMUM_MEMORY=18MB'] - self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_new_error') + self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_erroring_new') @no_asmjs() @no_wasm2js('no WebAssembly.Memory()') From d3342ef57c71b8813b8e9a31219b2e14e6ecb6a9 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 4 May 2020 15:39:59 -0700 Subject: [PATCH 6/8] rename messages as suggested --- tests/core/test_memorygrowth_erroring_new.cpp | 6 +++--- tests/core/test_memorygrowth_erroring_new.txt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/core/test_memorygrowth_erroring_new.cpp b/tests/core/test_memorygrowth_erroring_new.cpp index ea972906020fb..a4d4c8b77cbf4 100644 --- a/tests/core/test_memorygrowth_erroring_new.cpp +++ b/tests/core/test_memorygrowth_erroring_new.cpp @@ -14,10 +14,10 @@ int main() { // Catch the failure here so we can report it. try { _allocate_too_much(); - out("no error happened"); + out("no abort happened"); } catch (e) { - assert(("" + e).indexOf("abort") >= 0, "expect an abort"); - out("an expected error occurred"); + assert(("" + e).indexOf("abort") >= 0, "expect an abort from new"); + out("new aborted as expected"); } }); } diff --git a/tests/core/test_memorygrowth_erroring_new.txt b/tests/core/test_memorygrowth_erroring_new.txt index 89be86abf01aa..9ff167b0e499b 100644 --- a/tests/core/test_memorygrowth_erroring_new.txt +++ b/tests/core/test_memorygrowth_erroring_new.txt @@ -1 +1 @@ -an expected error occurred +new aborted as expected From 0948f76ee29bbc51a1fde69f4da3d90fd238075d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 4 May 2020 16:27:22 -0700 Subject: [PATCH 7/8] rename test and run on non-growth too --- ...owth_erroring_new.cpp => test_aborting_new.cpp} | 0 ...owth_erroring_new.txt => test_aborting_new.txt} | 0 tests/test_core.py | 14 ++++++++------ 3 files changed, 8 insertions(+), 6 deletions(-) rename tests/core/{test_memorygrowth_erroring_new.cpp => test_aborting_new.cpp} (100%) rename tests/core/{test_memorygrowth_erroring_new.txt => test_aborting_new.txt} (100%) diff --git a/tests/core/test_memorygrowth_erroring_new.cpp b/tests/core/test_aborting_new.cpp similarity index 100% rename from tests/core/test_memorygrowth_erroring_new.cpp rename to tests/core/test_aborting_new.cpp diff --git a/tests/core/test_memorygrowth_erroring_new.txt b/tests/core/test_aborting_new.txt similarity index 100% rename from tests/core/test_memorygrowth_erroring_new.txt rename to tests/core/test_aborting_new.txt diff --git a/tests/test_core.py b/tests/test_core.py index 4a8e9d501d8da..83e69e191d6d8 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -2169,13 +2169,15 @@ def test_memorygrowth_3_force_fail_reallocBuffer(self): self.emcc_args += ['-Wno-almost-asm', '-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'TEST_MEMORY_GROWTH_FAILS=1'] self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_3') - def test_memorygrowth_erroring_new(self): + @parameterized({ + 'nogrow': ([],), + 'grow': (['-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'MAXIMUM_MEMORY=18MB'],) + }) + def test_aborting_new(self, args): # test that C++ new properly errors if we fail to malloc when growth is - # enabled. (if growth is not enabled we go down a different - simpler - - # code path that always aborts on any growth attemp; this test checks the - # case where growth is enabled but despite that we fail to grow). - self.emcc_args += ['-Wno-almost-asm', '-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'MAXIMUM_MEMORY=18MB'] - self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_erroring_new') + # enabled, with or without growth + self.emcc_args += ['-Wno-almost-asm'] + args + self.do_run_in_out_file_test('tests', 'core', 'test_aborting_new') @no_asmjs() @no_wasm2js('no WebAssembly.Memory()') From 02ac4569ad1093d77749d60048953e2e85f42814 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Mon, 4 May 2020 19:31:15 -0700 Subject: [PATCH 8/8] fix test on fastcomp --- tests/test_core.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index 83e69e191d6d8..775e99d1dc7e9 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -2170,13 +2170,13 @@ def test_memorygrowth_3_force_fail_reallocBuffer(self): self.do_run_in_out_file_test('tests', 'core', 'test_memorygrowth_3') @parameterized({ - 'nogrow': ([],), - 'grow': (['-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'MAXIMUM_MEMORY=18MB'],) + 'nogrow': (['-s', 'ALLOW_MEMORY_GROWTH=0'],), + 'grow': (['-s', 'ALLOW_MEMORY_GROWTH=1'],) }) def test_aborting_new(self, args): # test that C++ new properly errors if we fail to malloc when growth is # enabled, with or without growth - self.emcc_args += ['-Wno-almost-asm'] + args + self.emcc_args += ['-Wno-almost-asm', '-s', 'MAXIMUM_MEMORY=18MB'] + args self.do_run_in_out_file_test('tests', 'core', 'test_aborting_new') @no_asmjs()