abort() on malloc() failure in C++ new with exceptions disabled + memory growth (#11079)

When libc++/libc++abi are built with exceptions disabled, the
new implementation there does not throw an exception for
an error, but it also does nothing else. So new ends up returning
a zero if malloc did, which can break programs.

Technically libc++/libc++abi are doing a reasonable thing here,
just removing all exceptions-related code when exceptions are
disabled. The assumption is likely that a user program would
set a new_handler if an error is desired. For us, we have to change
this as our default mode is to have exceptions disabled, and we
don't want users to need to know they need to do anything.

This makes it abort instead. (Note that without growth this
happened to always work, since we abort on any failing allocation.
With growth enabled, though, malloc returns 0, and we end up
in this situation.)

Fixes #11042 As discussed there I also looked at the option of
installing a set_new_handler that does an abort. That ends up
increasing code size by a little (bit less than 1%) because if adds
a global constructor, a function to the table, and some memory
operations. It seems better to just modify new itself which avoids
all that.

Author: kripken

system/lib/libcxx/new.cpp

@@ -74,8 +74,17 @@ operator new(std::size_t size) _THROW_BAD_ALLOC
         else
 #ifndef _LIBCPP_NO_EXCEPTIONS
             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;
+#endif
 #endif
     }
     return p;

system/lib/libcxxabi/src/stdlib_new_delete.cpp

@@ -38,8 +38,17 @@ operator new(std::size_t size) _THROW_BAD_ALLOC
         else
 #ifndef _LIBCXXABI_NO_EXCEPTIONS
             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;
+#endif
 #endif
     }
     return p;

tests/core/test_aborting_new.cpp

@@ -0,0 +1,23 @@
+#include <emscripten.h>
+#include <stdio.h>
+#include <vector>
+
+EMSCRIPTEN_KEEPALIVE extern "C" void allocate_too_much() {
+  std::vector<int> 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 abort happened");
+    } catch (e) {
+      assert(("" + e).indexOf("abort") >= 0, "expect an abort from new");
+      out("new aborted as expected");
+    }
+  });
+}

tests/core/test_aborting_new.txt

@@ -0,0 +1 @@
+new aborted as expected

tests/test_core.py

@@ -2169,6 +2169,16 @@ 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')
 
+  @parameterized({
+    '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', '-s', 'MAXIMUM_MEMORY=18MB'] + args
+    self.do_run_in_out_file_test('tests', 'core', 'test_aborting_new')
+
   @no_asmjs()
   @no_wasm2js('no WebAssembly.Memory()')
   @no_asan('ASan alters the memory size')