Default to STACK_OVERFLOW_CHECK=2 in debug builds

ChangeLog.md

@@ -20,6 +20,10 @@ See docs/process.md for more on how version tagging works.
 
 3.1.25 (in development)
 -----------------------
+- `STACK_OVERFLOW_CHECK` now defaults 2 in debug builds (when `ASSERTIONS` are
+  enabled). This is because we are planning on lowering the default stack size
+  and we want good reporting of stack overflow in debug builds.  See #14177.
+  This can be set of 1 or 0 if the performance overhead is a problem. (#18040)
 
 3.1.24 - 10/11/22
 -----------------

emcc.py

@@ -1774,14 +1774,20 @@ def phase_linker_setup(options, state, newargs, user_settings):
   if '_main' in settings.EXPORTED_FUNCTIONS:
     settings.EXPORT_IF_DEFINED.append('__main_argc_argv')
 
-  # -sASSERTIONS implies basic stack overflow checks, and ASSERTIONS=2
-  # implies full stack overflow checks.
-  if settings.ASSERTIONS:
-    # However, we don't set this default in PURE_WASI, or when we are linking without standard
-    # libraries because STACK_OVERFLOW_CHECK depends on emscripten_stack_get_end which is defined
-    # in libcompiler-rt.
-    if not settings.PURE_WASI and '-nostdlib' not in newargs and '-nodefaultlibs' not in newargs:
-      default_setting(user_settings, 'STACK_OVERFLOW_CHECK', max(settings.ASSERTIONS, settings.STACK_OVERFLOW_CHECK))
+  # -sASSERTIONS implies stack overflow checks
+  # However, we don't set this default when we are linking without standard
+  # libraries (because STACK_OVERFLOW_CHECK depends on emscripten_stack_get_end
+  # which is defined in compiler-rt).
+  if settings.ASSERTIONS and '-nostdlib' not in newargs and '-nodefaultlibs' not in newargs:
+    # Don't set the default to 2 in STANDALONE_WASM since it (currently)
+    # requires the host to call __set_stack_limits (which WASI runtimes
+    # don't know about).
+    # TODO(sbc): Remove this once https://github.com/emscripten-core/emscripten/pull/17564
+    # lands.
+    if settings.STANDALONE_WASM:
+      default_setting(user_settings, 'STACK_OVERFLOW_CHECK', 1)
+    else:
+      default_setting(user_settings, 'STACK_OVERFLOW_CHECK', 2)
 
   if settings.LLD_REPORT_UNDEFINED or settings.STANDALONE_WASM:
     # Reporting undefined symbols at wasm-ld time requires us to know if we have a `main` function

src/settings.js

@@ -63,10 +63,10 @@ var ASSERTIONS = 1;
 var RUNTIME_LOGGING = false;
 
 // Chooses what kind of stack smash checks to emit to generated code:
-// Building with ASSERTIONS=1 causes STACK_OVERFLOW_CHECK default to 1.
+// Building with ASSERTIONS=1 causes STACK_OVERFLOW_CHECK default to 2.
 // Since ASSERTIONS=1 is the default at -O0, which itself is the default
 // optimization level this means that this setting also effectively
-// defaults 1, absent any other settings.
+// defaults 2, absent any other settings.
 // 0: Stack overflows are not checked.
 // 1: Adds a security cookie at the top of the stack, which is checked at end of
 //    each tick and at exit (practically zero performance overhead)

test/other/metadce/test_metadce_hello_O0.exports

@@ -1,5 +1,6 @@
 __errno_location
 __indirect_function_table
+__set_stack_limits
 __wasm_call_ctors
 dynCall_jiji
 emscripten_stack_get_base

test/other/metadce/test_metadce_hello_O0.funcs

@@ -12,6 +12,7 @@ $__memcpy
 $__ofl_lock
 $__ofl_unlock
 $__original_main
+$__set_stack_limits
 $__stdio_write
 $__syscall_getpid
 $__towrite

test/other/metadce/test_metadce_hello_O0.imports

@@ -1,2 +1,3 @@
+env.__handle_stack_overflow
 env.emscripten_memcpy_big
 wasi_snapshot_preview1.fd_write

test/other/metadce/test_metadce_hello_O0.jssize

@@ -1 +1 @@
-26257
+26543

test/other/metadce/test_metadce_hello_O0.sent

@@ -1,2 +1,3 @@
+__handle_stack_overflow
 emscripten_memcpy_big
 fd_write

test/other/metadce/test_metadce_hello_O0.size

@@ -1 +1 @@
-12071
+12507

test/other/metadce/test_metadce_minimal_O0.exports

@@ -1,5 +1,6 @@
 __errno_location
 __indirect_function_table
+__set_stack_limits
 __wasm_call_ctors
 add
 emscripten_stack_get_base

test/other/metadce/test_metadce_minimal_O0.funcs

@@ -3,6 +3,7 @@ $__lock
 $__lockfile
 $__ofl_lock
 $__ofl_unlock
+$__set_stack_limits
 $__unlock
 $__unlockfile
 $__wasm_call_ctors

test/other/metadce/test_metadce_minimal_O0.imports

@@ -1 +1 @@
-
+env.__handle_stack_overflow

test/other/metadce/test_metadce_minimal_O0.jssize

@@ -1 +1 @@
-22976
+23254

test/other/metadce/test_metadce_minimal_O0.sent

@@ -1 +1 @@
-
+__handle_stack_overflow

test/other/metadce/test_metadce_minimal_O0.size

@@ -1 +1 @@
-865
+988

test/other/test_unoptimized_code_size.js.size

@@ -1 +1 @@
-66353
+66952

test/other/test_unoptimized_code_size.wasm.size

@@ -1 +1 @@
-12104
+12540

test/other/test_unoptimized_code_size_strict.js.size

@@ -1 +1 @@
-65219
+65818

test/other/test_unoptimized_code_size_strict.wasm.size

@@ -1 +1 @@
-12104
+12540

test/test_core.py

@@ -4131,6 +4131,7 @@ def test_dylink_basics_no_modify(self):
     if self.get_setting('MEMORY64') == 2:
       self.skipTest('MEMORY64=2 always requires module re-writing')
     self.set_setting('WASM_BIGINT')
+    self.set_setting('STACK_OVERFLOW_CHECK', 1)
     self.set_setting('ERROR_ON_WASM_CHANGES_AFTER_LINK')
     self.do_basic_dylink_test()
 
@@ -8324,6 +8325,7 @@ def test_asyncify_main_module(self):
   def test_emscripten_lazy_load_code(self, conditional):
     if self.get_setting('STACK_OVERFLOW_CHECK'):
       self.skipTest('https://github.com/emscripten-core/emscripten/issues/16828')
+    self.set_setting('STACK_OVERFLOW_CHECK', 0)
     self.set_setting('ASYNCIFY_LAZY_LOAD_CODE')
     self.set_setting('ASYNCIFY_IGNORE_INDIRECT')
     self.set_setting('MALLOC', 'emmalloc')

test/test_other.py

@@ -10612,8 +10612,9 @@ def ok(args, filename='hello_world.cpp', expected='hello, world!'):
       self.run_process([EMCC, test_file(filename)] + args)
       self.assertContained(expected, self.run_js('a.out.js'))
 
-    # -O0 with BigInt support (to avoid the need for legalization)
-    required_flags = ['-sWASM_BIGINT']
+    # -O0 with BigInt support (to avoid the need for legalization) and STACK_OVERFLOW_CHECK=1 (to
+    # avoid the need for the stack check pass).
+    required_flags = ['-sWASM_BIGINT', '-sSTACK_OVERFLOW_CHECK=1']
     ok(required_flags)
     # Same with DWARF
     ok(required_flags + ['-g'])