Fix the stack pointer in wasm backend+threads mode (#8811)

* Set STACK_MAX properly in the wasm backend, it should point to the lowest address since the wasm stack grows downwards.

* Fix the stack overflow test to work with the wasm backend.

Author: vargaz

AUTHORS

@@ -414,3 +414,4 @@ a license to everyone to use it as detailed in LICENSE.)
 * Ferris Kwaijtaal <[email protected]>
 * Konstantin Podsvirov <[email protected]>
 * Eduardo Bart <[email protected]>
+* Zoltan Varga <[email protected]> (copyright owned by Microsoft, Inc.)

src/library_pthread.js

@@ -414,6 +414,8 @@ var LibraryPThread = {
       {{{ makeSetValue('tlsMemory', 'i*4', 0, 'i32') }}};
     }
 
+    var stackHigh = threadParams.stackBase + threadParams.stackSize;
+
     var pthread = PThread.pthreads[threadParams.pthread_ptr] = { // Create a pthread info object to represent this thread.
       worker: worker,
       stackBase: threadParams.stackBase,
@@ -433,8 +435,8 @@ var LibraryPThread = {
 
     Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.attr }}}) >> 2, threadParams.stackSize);
     Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.stack_size }}}) >> 2, threadParams.stackSize);
-    Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.stack }}}) >> 2, threadParams.stackBase);
-    Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.attr }}} + 8) >> 2, threadParams.stackBase);
+    Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.stack }}}) >> 2, stackHigh);
+    Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.attr }}} + 8) >> 2, stackHigh);
     Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.attr }}} + 12) >> 2, threadParams.detached);
     Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.attr }}} + 20) >> 2, threadParams.schedPolicy);
     Atomics.store(HEAPU32, (pthread.threadInfoStruct + {{{ C_STRUCTS.pthread.attr }}} + 24) >> 2, threadParams.schedPrio);
@@ -1169,8 +1171,15 @@ var LibraryPThread = {
 
 #if MODULARIZE
   $establishStackSpaceInJsModule: function(stackBase, stackMax) {
-    STACK_BASE = STACKTOP = stackBase;
+    STACK_BASE = stackBase;
+#if WASM_BACKEND
+    // The stack grows downwards
+    STACKTOP = stackMax;
+    STACK_MAX = stackBase;
+#else
+    STACKTOP = stackBase;
     STACK_MAX = stackMax;
+#endif
   },
 #endif
 };

src/runtime_stack_check.js

@@ -2,13 +2,26 @@
 // Initializes the stack cookie. Called at the startup of main and at the startup of each thread in pthreads mode.
 function writeStackCookie() {
   assert((STACK_MAX & 3) == 0);
+#if WASM_BACKEND
+  // The stack grows downwards
+  HEAPU32[(STACK_MAX >> 2)+1] = 0x02135467;
+  HEAPU32[(STACK_MAX >> 2)+2] = 0x89BACDFE;
+#else
   HEAPU32[(STACK_MAX >> 2)-1] = 0x02135467;
   HEAPU32[(STACK_MAX >> 2)-2] = 0x89BACDFE;
+#endif
 }
 
 function checkStackCookie() {
-  if (HEAPU32[(STACK_MAX >> 2)-1] != 0x02135467 || HEAPU32[(STACK_MAX >> 2)-2] != 0x89BACDFE) {
-    abort('Stack overflow! Stack cookie has been overwritten, expected hex dwords 0x89BACDFE and 0x02135467, but received 0x' + HEAPU32[(STACK_MAX >> 2)-2].toString(16) + ' ' + HEAPU32[(STACK_MAX >> 2)-1].toString(16));
+#if WASM_BACKEND
+  var cookie1 = HEAPU32[(STACK_MAX >> 2)+1];
+  var cookie2 = HEAPU32[(STACK_MAX >> 2)+2];
+#else
+  var cookie1 = HEAPU32[(STACK_MAX >> 2)-1];
+  var cookie2 = HEAPU32[(STACK_MAX >> 2)-2];
+#endif
+  if (cookie1 != 0x02135467 || cookie2 != 0x89BACDFE) {
+    abort('Stack overflow! Stack cookie has been overwritten, expected hex dwords 0x89BACDFE and 0x02135467, but received 0x' + cookie2.toString(16) + ' ' + cookie1.toString(16));
   }
   // Also test the global address 0 for integrity.
   if (HEAP32[0] !== 0x63736d65 /* 'emsc' */) abort('Runtime error: The application has corrupted its heap memory area (address zero)!');

src/support.js

@@ -966,7 +966,7 @@ GLOBAL_BASE = alignMemory(GLOBAL_BASE, {{{ MAX_GLOBAL_ALIGN || 1 }}});
 // The wasm backend path does not have a way to set the stack max, so we can
 // just implement this function in a trivial way
 function establishStackSpace(base, max) {
-  stackRestore(base);
+  stackRestore(max);
 }
 
 // JS library code refers to Atomics in the manner used from asm.js, provide

src/worker.js

@@ -175,14 +175,25 @@ this.onmessage = function(e) {
       selfThreadId = e.data.selfThreadId;
       parentThreadId = e.data.parentThreadId;
       // Establish the stack frame for this thread in global scope
-      {{{ makeAsmExportAndGlobalAssignTargetInPthread('STACK_BASE') }}} = {{{ makeAsmExportAndGlobalAssignTargetInPthread('STACKTOP') }}} = e.data.stackBase;
-      {{{ makeAsmExportAndGlobalAssignTargetInPthread('STACK_MAX') }}} = STACK_BASE + e.data.stackSize;
+#if WASM_BACKEND
+      // The stack grows downwards
+      var max = e.data.stackBase;
+      var top = e.data.stackBase + e.data.stackSize;
+#else
+      var max = e.data.stackBase + e.data.stackSize;
+      var top = e.data.stackBase;
+#endif
+      {{{ makeAsmExportAndGlobalAssignTargetInPthread('STACK_BASE') }}} = e.data.stackBase;
+      {{{ makeAsmExportAndGlobalAssignTargetInPthread('STACKTOP') }}} = top;
+      {{{ makeAsmExportAndGlobalAssignTargetInPthread('STACK_MAX') }}} = max;
 #if ASSERTIONS
       assert(threadInfoStruct);
       assert(selfThreadId);
       assert(parentThreadId);
       assert(STACK_BASE != 0);
+#if !WASM_BACKEND
       assert(STACK_MAX > STACK_BASE);
+#endif
 #endif
       // Call inside asm.js/wasm module to set up the stack frame for this pthread in asm.js/wasm module scope
       Module['establishStackSpace'](e.data.stackBase, e.data.stackBase + e.data.stackSize);

tests/pthread/test_pthread_attr_getstack.cpp

@@ -0,0 +1,71 @@
+// Copyright 2015 The Emscripten Authors.  All rights reserved.
+// Emscripten is available under two separate licenses, the MIT license and the
+// University of Illinois/NCSA Open Source License.  Both these licenses can be
+// found in the LICENSE file.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <emscripten.h>
+#include <emscripten/threading.h>
+#include <assert.h>
+
+void *ThreadMain(void *arg)
+{
+	pthread_attr_t attr;
+	int rc;
+	void *stbase;
+	size_t stsize;
+	int dummy, result;
+
+	rc = pthread_attr_init(&attr);
+	assert(rc == 0);
+
+	rc = pthread_getattr_np(pthread_self(), &attr);
+	assert(rc == 0);
+
+	rc = pthread_attr_getstack(&attr, &stbase, &stsize);
+	assert(rc == 0);
+
+	//printf ("%p %p %p\n", stbase, (char*)stbase + stsize, &dummy);
+
+	if (&dummy < stbase)
+		result = 1;
+	else if ((char*)&dummy > (char*)stbase + stsize)
+		result = 2;
+	else
+		result = 0;
+
+	pthread_exit((void*)result);
+}
+
+int main()
+{
+	if (!emscripten_has_threading_support())
+	{
+#ifdef REPORT_RESULT
+		REPORT_RESULT(0);
+#endif
+		printf("Skipped: Threading is not supported.\n");
+		return 0;
+	}
+
+	pthread_t thread;
+	pthread_attr_t attr;
+	int rc, result;
+
+	pthread_attr_init(&attr);
+	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);
+	rc = pthread_create(&thread, &attr, ThreadMain, NULL);
+	assert(rc == 0);
+	pthread_attr_destroy(&attr);
+
+	rc = pthread_join(thread, (void**)&result);
+	assert(rc == 0);
+
+#ifdef REPORT_RESULT
+	REPORT_RESULT(result);
+#endif
+	return result;
+}

tests/test_browser.py

@@ -3658,6 +3658,10 @@ def test_pthread_mutex(self):
     for arg in [[], ['-DSPINLOCK_TEST']]:
       self.btest(path_from_root('tests', 'pthread', 'test_pthread_mutex.cpp'), expected='50', args=['-s', 'TOTAL_MEMORY=64MB', '-O3', '-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=8'] + arg)
 
+  @requires_threads
+  def test_pthread_attr_getstack(self):
+    self.btest(path_from_root('tests', 'pthread', 'test_pthread_attr_getstack.cpp'), expected='0', args=['-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=2'])
+
   # Test that memory allocation is thread-safe.
   @requires_threads
   def test_pthread_malloc(self):