Put stack first when not optimizing for size

.circleci/config.yml

@@ -551,6 +551,7 @@ jobs:
             core2.test_em_js
             core2.test_em_js_pthreads
             core2.test_unicode_js_library
+            other.test_stack_overflow
             other.test_dlmalloc_modes
             other.test_c_preprocessor
             other.test_prejs_unicode

ChangeLog.md

@@ -26,6 +26,11 @@ See docs/process.md for more on how version tagging works.
   that will allow the command to be replicated.  This can be useful for sharing
   reproduction cases with others (inspired by the lld option of the same name).
   (#18160)
+- In non-optimizing builds emscripten will now place the stack first in memory,
+  before global data.  This is to get more accurate stack overflow errors (since
+  overflow will trap rather corrupting global data first).  This should not
+  be a user-visible change (unless your program does something very odd such
+  depending on the specific location of stack data in memory). (#18154)
 
 3.1.25 - 11/08/22
 -----------------

emcc.py

@@ -1883,6 +1883,14 @@ def phase_linker_setup(options, state, newargs):
   if not settings.AUTO_JS_LIBRARIES:
     default_setting('USE_SDL', 0)
 
+  if 'GLOBAL_BASE' not in user_settings and not settings.SHRINK_LEVEL and not settings.OPT_LEVEL:
+    # When optimizing for size it helps to put static data first before
+    # the stack (sincs this makes instructions for accessing this data
+    # use a smaller LEB encoding).
+    # However, for debugability is better to have the stack come first
+    # (becuase stack overflows will trap rather than corrupting data).
+    settings.STACK_FIRST = True
+
   # Default to TEXTDECODER=2 (always use TextDecoder to decode UTF-8 strings)
   # in -Oz builds, since custom decoder for UTF-8 takes up space.
   # In pthreads enabled builds, TEXTDECODER==2 may not work, see
@@ -2074,7 +2082,8 @@ def phase_linker_setup(options, state, newargs):
     settings.REQUIRED_EXPORTS += [
       'emscripten_stack_get_end',
       'emscripten_stack_get_free',
-      'emscripten_stack_get_base'
+      'emscripten_stack_get_base',
+      'emscripten_stack_get_current',
     ]
 
     # We call one of these two functions during startup which caches the stack limits
@@ -2219,7 +2228,7 @@ def phase_linker_setup(options, state, newargs):
     exit_with_error('cannot have both DYNAMIC_EXECUTION=0 and RELOCATABLE enabled at the same time, since RELOCATABLE needs to eval()')
 
   if settings.SIDE_MODULE and 'GLOBAL_BASE' in user_settings:
-    exit_with_error('Cannot set GLOBAL_BASE when building SIDE_MODULE')
+    exit_with_error('GLOBAL_BASE is not compatible with SIDE_MODULE')
 
   if options.use_preload_plugins or len(options.preload_files) or len(options.embed_files):
     if settings.NODERAWFS:
@@ -2567,6 +2576,7 @@ def check_memory_setting(setting):
     # We start our global data after the shadow memory.
     # We don't need to worry about alignment here.  wasm-ld will take care of that.
     settings.GLOBAL_BASE = shadow_size
+    settings.STACK_FIRST = False
 
     if not settings.ALLOW_MEMORY_GROWTH:
       settings.INITIAL_MEMORY = total_mem

src/library.js

@@ -3442,6 +3442,14 @@ mergeInto(LibraryManager.library, {
     if (e instanceof ExitStatus || e == 'unwind') {
       return EXITSTATUS;
     }
+#if STACK_OVERFLOW_CHECK
+    checkStackCookie();
+    if (e instanceof WebAssembly.RuntimeError) {
+      if (_emscripten_stack_get_current() <= 0) {
+        err('Stack overflow detected.  You can try increasing -sSTACK_SIZE (currently set to ' + STACK_SIZE + ')');
+      }
+    }
+#endif
 #if MINIMAL_RUNTIME
     throw e;
 #else

src/runtime_stack_check.js

@@ -14,6 +14,12 @@ function writeStackCookie() {
 #if ASSERTIONS
   assert((max & 3) == 0);
 #endif
+  // If the stack ends at address zero we write our cookies 4 bytes into the
+  // stack.  This prevents interference with the (separate) address-zero check
+  // below.
+  if (max == 0) {
+    max += 4;
+  }
   // The stack grow downwards towards _emscripten_stack_get_end.
   // We write cookies to the final two words in the stack and detect if they are
   // ever overwritten.
@@ -33,6 +39,10 @@ function checkStackCookie() {
 #if RUNTIME_DEBUG
   dbg('checkStackCookie: ' + max.toString(16));
 #endif
+  // See writeStackCookie().
+  if (max == 0) {
+    max += 4;
+  }
   var cookie1 = {{{ makeGetValue('max', 0, 'u32') }}};
   var cookie2 = {{{ makeGetValue('max', 4, 'u32') }}};
   if (cookie1 != 0x2135467 || cookie2 != 0x89BACDFE) {

src/settings_internal.js

@@ -244,3 +244,5 @@ var ALL_INCOMING_MODULE_JS_API = [];
 // when weak symbols are undefined.  Only applies in the case of dyanmic linking
 // (MAIN_MODULE).
 var WEAK_IMPORTS = [];
+
+var STACK_FIRST = false;

test/core/stack_overflow.cpp → test/core/stack_overflow.c

@@ -6,7 +6,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
-#include <emscripten.h>
+#include <emscripten/emscripten.h>
+#include <emscripten/stack.h>
 
 void recurse(unsigned long x);
 
@@ -18,7 +19,7 @@ void act(volatile unsigned long *a) {
 }
 
 void recurse(volatile unsigned long x) {
-  printf("recurse %ld\n", x);
+  printf("recurse %ld sp=%#lx\n", x, emscripten_stack_get_current());
   volatile unsigned long a = x;
   volatile char buffer[1000*1000];
   buffer[x/2] = 0;
@@ -36,4 +37,3 @@ void recurse(volatile unsigned long x) {
 int main() {
   recurse(1000*1000);
 }
-

test/core/test_emmalloc_trim.out

@@ -1,35 +1,35 @@
 heap size: 134217728
 dynamic heap 0: 4096
 free dynamic memory 0: 4096
-unclaimed heap memory 0: 2142171136
-sbrk 0: 0x502000
+unclaimed heap memory 0: 2142175232
+sbrk 0: 0x501000
 1
 dynamic heap 1: 37752832
 free dynamic memory 1: 4096
-unclaimed heap memory 1: 2104422400
-sbrk 1: 0x2902000
+unclaimed heap memory 1: 2104426496
+sbrk 1: 0x2901000
 1st trim: 1
 dynamic heap 1: 37752832
 free dynamic memory 1: 0
-unclaimed heap memory 1: 2104422400
-sbrk 1: 0x2902000
+unclaimed heap memory 1: 2104426496
+sbrk 1: 0x2901000
 2nd trim: 0
 dynamic heap 2: 37752832
 free dynamic memory 2: 0
-unclaimed heap memory 2: 2104422400
-sbrk 2: 0x2902000
+unclaimed heap memory 2: 2104426496
+sbrk 2: 0x2901000
 3rd trim: 1
 dynamic heap 3: 33656832
 free dynamic memory 3: 102400
-unclaimed heap memory 3: 2104422400
-sbrk 3: 0x2902000
+unclaimed heap memory 3: 2104426496
+sbrk 3: 0x2901000
 4th trim: 0
 dynamic heap 4: 33656832
 free dynamic memory 4: 102400
-unclaimed heap memory 4: 2104422400
-sbrk 4: 0x2902000
+unclaimed heap memory 4: 2104426496
+sbrk 4: 0x2901000
 5th trim: 1
 dynamic heap 5: 33558528
 free dynamic memory 5: 0
-unclaimed heap memory 5: 2104422400
-sbrk 5: 0x2902000
+unclaimed heap memory 5: 2104426496
+sbrk 5: 0x2901000

test/core/test_stack_placement.c

@@ -7,6 +7,7 @@
 
 #include <stdio.h>
 #include <assert.h>
+#include <emscripten/stack.h>
 
 /* We had a regression where the stack position was not taking into account
  * the data and bss.  This test runs with 1024 byte stack which given that bug
@@ -22,7 +23,14 @@ int main(int argc, char* argv[]) {
   int* bss_address = &static_bss[BSS_BYTES-1];
   int* stack_address = &stack_var;
   printf("data: %p bss: %p stack: %p\n", data_address, bss_address, stack_address);
-  assert(stack_address > bss_address);
+  // Stack can either come after BSS or before data (In debug builds we link
+  // with --stack-first).
+  int stack_first = emscripten_stack_get_end() == 0;
+  if (stack_first) {
+    assert(stack_address < data_address);
+  } else {
+    assert(stack_address > bss_address);
+  }
   assert(bss_address > data_address);
   printf("success.\n");
   return 0;

test/other/metadce/test_metadce_hello_O0.exports

@@ -3,6 +3,7 @@ __indirect_function_table
 __wasm_call_ctors
 dynCall_jiji
 emscripten_stack_get_base
+emscripten_stack_get_current
 emscripten_stack_get_end
 emscripten_stack_get_free
 emscripten_stack_init

test/other/metadce/test_metadce_hello_O0.funcs

@@ -23,6 +23,7 @@ $__wasi_syscall_ret
 $__wasm_call_ctors
 $dynCall_jiji
 $emscripten_stack_get_base
+$emscripten_stack_get_current
 $emscripten_stack_get_end
 $emscripten_stack_get_free
 $emscripten_stack_init

test/other/metadce/test_metadce_hello_O0.jssize

@@ -1 +1 @@
-26256
+26641

test/other/metadce/test_metadce_hello_O0.size

@@ -1 +1 @@
-12058
+12172

test/other/metadce/test_metadce_minimal_O0.exports

@@ -3,6 +3,7 @@ __indirect_function_table
 __wasm_call_ctors
 add
 emscripten_stack_get_base
+emscripten_stack_get_current
 emscripten_stack_get_end
 emscripten_stack_get_free
 emscripten_stack_init

test/other/metadce/test_metadce_minimal_O0.funcs

@@ -8,6 +8,7 @@ $__unlockfile
 $__wasm_call_ctors
 $add
 $emscripten_stack_get_base
+$emscripten_stack_get_current
 $emscripten_stack_get_end
 $emscripten_stack_get_free
 $emscripten_stack_init

test/other/metadce/test_metadce_minimal_O0.jssize

@@ -1 +1 @@
-22987
+23200

test/other/metadce/test_metadce_minimal_O0.size

@@ -1 +1 @@
-865
+919

test/other/test_unoptimized_code_size.js.size

@@ -1 +1 @@
-66886
+67655

test/other/test_unoptimized_code_size.wasm.size

@@ -1 +1 @@
-12091
+12205

test/other/test_unoptimized_code_size_no_asserts.wasm.size

@@ -1 +1 @@
-11580
+11644

test/other/test_unoptimized_code_size_strict.js.size

@@ -1 +1 @@
-65752
+66521

test/other/test_unoptimized_code_size_strict.wasm.size

@@ -1 +1 @@
-12091
+12205

test/test_core.py

@@ -1074,6 +1074,7 @@ def test_emmalloc_memory_statistics(self, *args):
     self.do_core_test('test_emmalloc_memory_statistics.cpp', out_suffix=out_suffix)
 
   @no_optimize('output is sensitive to optimization flags, so only test unoptimized builds')
+  @no_wasm64('output is sensitive to absolute data size')
   @no_asan('ASan does not support custom memory allocators')
   @no_lsan('LSan does not support custom memory allocators')
   def test_emmalloc_trim(self, *args):
@@ -2949,7 +2950,7 @@ def test_bsearch(self):
 
   def test_stack_overflow(self):
     self.set_setting('ASSERTIONS', 2)
-    self.do_runf(test_file('core/stack_overflow.cpp'), 'Aborted(stack overflow', assert_returncode=NON_ZERO)
+    self.do_runf(test_file('core/stack_overflow.c'), 'Aborted(stack overflow', assert_returncode=NON_ZERO)
 
   def test_stackAlloc(self):
     self.do_core_test('stackAlloc.cpp')

test/test_other.py

@@ -12698,3 +12698,11 @@ def test_reproduce(self):
     root = os.path.splitdrive(path_from_root())[1][1:]
     response = read_file('foo/response.txt').replace(root, '<root>')
     self.assertTextDataIdentical(response, expected)
+
+  @no_mac('https://github.com/emscripten-core/emscripten/issues/18175')
+  def test_stack_overflow(self):
+    self.set_setting('STACK_OVERFLOW_CHECK', 1)
+    self.emcc_args += ['-O1', '--profiling-funcs']
+    self.do_runf(test_file('core/stack_overflow.c'),
+                 'Stack overflow detected.  You can try increasing -sSTACK_SIZE',
+                 assert_returncode=NON_ZERO)

tools/building.py

@@ -225,8 +225,11 @@ def lld_flags_for_executable(external_symbols):
       cmd.append('--max-memory=%d' % settings.INITIAL_MEMORY)
     elif settings.MAXIMUM_MEMORY != -1:
       cmd.append('--max-memory=%d' % settings.MAXIMUM_MEMORY)
-    if not settings.RELOCATABLE:
-      cmd.append('--global-base=%s' % settings.GLOBAL_BASE)
+
+  if settings.STACK_FIRST:
+    cmd.append('--stack-first')
+  elif not settings.RELOCATABLE:
+    cmd.append('--global-base=%s' % settings.GLOBAL_BASE)
 
   return cmd