Fix -sWASM_WORKERS and -sSTACK_OVERFLOW_CHECK=2 and add a test (#16497)

* Fix -sWASM_WORKERS and -sSTACK_OVERFLOW_CHECK=2 and add a test. See #16496

* Update comment

src/library_wasm_worker.js

@@ -55,13 +55,21 @@ mergeInto(LibraryManager.library, {
     assert(m['sz'] % 16 == 0);
 #endif
 
+#if STACK_OVERFLOW_CHECK >= 2
+    // _emscripten_wasm_worker_initialize() initializes the stack for this Worker,
+    // but it cannot call to extern __set_stack_limits() function, or Binaryen breaks
+    // with "Fatal: Module::addFunction: __set_stack_limits already exists".
+    // So for now, invoke this function from JS side. TODO: remove this in the future.
+    // Note that this call is not exactly correct, since this limit will include
+    // the TLS slot, that will be part of the region between m['sb'] and m['sz'],
+    // so we need to fix up the call below.
+    ___set_stack_limits(m['sb'] + m['sz'], m['sb']);
+#endif
     // Run the C side Worker initialization for stack and TLS.
     _emscripten_wasm_worker_initialize(m['sb'], m['sz']);
-    // The above function initializes the stack for this Worker, but C code cannot
-    // call to extern __set_stack_limits() function, or Binaryen breaks with
-    // "Fatal: Module::addFunction: __set_stack_limits already exists".
-    // So for now, invoke the function from JS side. TODO: remove this in the future.
 #if STACK_OVERFLOW_CHECK >= 2
+    // Fix up stack base. (TLS frame is created at the bottom address end of the stack)
+    // See https://github.com/emscripten-core/emscripten/issues/16496
     ___set_stack_limits(_emscripten_stack_get_base(), _emscripten_stack_get_end());
 #endif
 

system/lib/compiler-rt/stack_limits.S

@@ -107,6 +107,14 @@ emscripten_wasm_worker_initialize:
   PTR.add
   global.set __stack_base
 
+// TODO: We'd like to do this here to avoid JS side calls to __set_stack_limits.
+//       (or even better, we'd like to avoid duplicate versions of the stack variables)
+// See https://github.com/emscripten-core/emscripten/issues/16496
+//  global.get __stack_base
+//  global.get __stack_end
+//  .functype __set_stack_limits (PTR, PTR) -> ()
+//  call __set_stack_limits
+
   // __wasm_init_tls(stackLowestAddress);
   local.get 0
   .functype __wasm_init_tls (PTR) -> ()

tests/test_browser.py

@@ -5106,7 +5106,8 @@ def test_wasm_worker_embedded(self):
   # Tests Wasm Worker thread stack setup
   @also_with_minimal_runtime
   def test_wasm_worker_thread_stack(self):
-    self.btest(test_file('wasm_worker/thread_stack.c'), expected='0', args=['-sWASM_WORKERS'])
+    for mode in [0, 1, 2]:
+      self.btest(test_file('wasm_worker/thread_stack.c'), expected='0', args=['-sWASM_WORKERS', f'-sSTACK_OVERFLOW_CHECK={mode}'])
 
   # Tests emscripten_malloc_wasm_worker() and emscripten_current_thread_is_wasm_worker() functions
   @also_with_minimal_runtime

tools/system_libs.py

@@ -1031,16 +1031,14 @@ def can_use(self):
 
 class libwasm_workers(MTLibrary):
   def __init__(self, **kwargs):
-    self.stack_check = kwargs.pop('stack_check')
     self.debug = kwargs.pop('debug')
     super().__init__(**kwargs)
 
   name = 'libwasm_workers'
 
   def get_cflags(self):
     cflags = ['-pthread',
-              '-D_DEBUG' if self.debug else '-Oz',
-              '-DSTACK_OVERFLOW_CHECK=' + ('2' if self.stack_check else '0')]
+              '-D_DEBUG' if self.debug else '-Oz']
     if not self.debug:
       cflags += ['-DNDEBUG']
     if self.is_ww or self.is_mt:
@@ -1055,17 +1053,15 @@ def get_base_name(self):
       name += '_stub'
     if self.debug:
       name += '-debug'
-    if self.stack_check:
-      name += '-stackcheck'
     return name
 
   @classmethod
   def vary_on(cls):
-    return super().vary_on() + ['debug', 'stack_check']
+    return super().vary_on() + ['debug']
 
   @classmethod
   def get_default_variation(cls, **kwargs):
-    return super().get_default_variation(debug=settings.ASSERTIONS >= 1, stack_check=settings.STACK_OVERFLOW_CHECK == 2, **kwargs)
+    return super().get_default_variation(debug=settings.ASSERTIONS >= 1, **kwargs)
 
   def get_files(self):
     return files_in_path(