Add new proxying modes foo__proxy: 'abort' and foo__proxy: 'abort_deb…

…ug', which will add compile time checks to verify that given JS function is not called in pthreads or Wasm Workers.
juj · Sep 27, 2024 · 184e8d0 · 184e8d0
1 parent f508b43
commit 184e8d0
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 28 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -29,6 +29,12 @@ See docs/process.md for more on how version tagging works.
 - The number of arguments passed to Embind function calls is now only verified
   with ASSERTIONS enabled. (#22591)
 - Optional arguments can now be omitted from Embind function calls. (#22591)
+- Added two new proxying directives. foo__proxy: 'abort' will abort program
+  execution if JS function foo is called from a pthread or a Wasm Worker.
+  foo__proxy: 'abort_debug' will do the same, but only in ASSERTIONS builds, and
+  in non-ASSERTIONS builds will be no-op.
+  Use these proxying directives to annotate JS functions that should not be
+  getting called from Workers.
 
 3.1.67 - 09/17/24
 -----------------

diff --git a/src/jsifier.mjs b/src/jsifier.mjs
@@ -319,39 +319,40 @@ function(${args}) {
 
     const proxyingMode = LibraryManager.library[symbol + '__proxy'];
     if (proxyingMode) {
-      if (proxyingMode !== 'sync' && proxyingMode !== 'async' && proxyingMode !== 'none') {
-        throw new Error(`Invalid proxyingMode ${symbol}__proxy: '${proxyingMode}' specified!`);
+      const possibleProxyingModes = ['sync', 'async', 'none', 'abort', 'abort_debug'];
+      if (!possibleProxyingModes.includes(proxyingMode)) {
+        throw new Error(`Invalid proxyingMode ${symbol}__proxy: '${proxyingMode}' specified! Possible modes: ${possibleProxyingModes.join(',')}`);
       }
       if (SHARED_MEMORY) {
-        const sync = proxyingMode === 'sync';
-        if (PTHREADS) {
-          snippet = modifyJSFunction(snippet, (args, body, async_, oneliner) => {
-            if (oneliner) {
-              body = `return ${body}`;
+        snippet = modifyJSFunction(snippet, (args, body, async_, oneliner) => {
+          if (oneliner) {
+            body = `return ${body}`;
+          }
+          const rtnType = sig && sig.length ? sig[0] : null;
+          const proxyFunc =
+            MEMORY64 && rtnType == 'p' ? 'proxyToMainThreadPtr' : 'proxyToMainThread';
+          var prefix = '';
+
+          if (proxyingMode == 'sync' || proxyingMode == 'async') {
+            if (PTHREADS) {
+              deps.push('$' + proxyFunc);
+              prefix = `if (ENVIRONMENT_IS_PTHREAD) return ${proxyFunc}(${proxiedFunctionTable.length}, 0, ${+(proxyingMode === 'sync')}${args ? ', ' : ''}${args});`;
             }
-            const rtnType = sig && sig.length ? sig[0] : null;
-            const proxyFunc =
-              MEMORY64 && rtnType == 'p' ? 'proxyToMainThreadPtr' : 'proxyToMainThread';
-            deps.push('$' + proxyFunc);
-            return `
-function(${args}) {
-if (ENVIRONMENT_IS_PTHREAD)
-  return ${proxyFunc}(${proxiedFunctionTable.length}, 0, ${+sync}${args ? ', ' : ''}${args});
+            if (WASM_WORKERS && ASSERTIONS) {
+              prefix += `\nassert(!ENVIRONMENT_IS_WASM_WORKER, "Attempted to call proxied function '${mangled}' in a Wasm Worker, but in Wasm Worker enabled builds, proxied function architecture is not available!");`;
+            }
+          } else if (proxyingMode == 'abort' || (proxyingMode == 'abort_debug' && ASSERTIONS)) {
+            const insideWorker = (PTHREADS && WASM_WORKERS)
+              ? '(ENVIRONMENT_IS_PTHREAD || ENVIRONMENT_IS_WASM_WORKER)'
+              : (PTHREADS ? 'ENVIRONMENT_IS_PTHREAD' : 'ENVIRONMENT_IS_WASM_WORKER');
+            prefix = `assert(!${insideWorker}, "Attempted to call function '${mangled}' inside a pthread/Wasm Worker, but this function has been declared to only be callable from the main browser thread");`;
+          }
+
+          return `function(${args}) {
+${prefix}
 ${body}
 }\n`;
-          });
-        } else if (WASM_WORKERS && ASSERTIONS) {
-          // In ASSERTIONS builds add runtime checks that proxied functions are not attempted to be called in Wasm Workers
-          // (since there is no automatic proxying architecture available)
-          snippet = modifyJSFunction(
-            snippet,
-            (args, body) => `
-function(${args}) {
-  assert(!ENVIRONMENT_IS_WASM_WORKER, "Attempted to call proxied function '${mangled}' in a Wasm Worker, but in Wasm Worker enabled builds, proxied function architecture is not available!");
-  ${body}
-}\n`,
-          );
-        }
+        });
         proxiedFunctionTable.push(mangled);
       }
     }

diff --git a/test/pthread/proxy_abort.c b/test/pthread/proxy_abort.c
@@ -0,0 +1,53 @@
+#include <emscripten.h>
+#include <pthread.h>
+#include <assert.h>
+
+// Test that a JS function with __proxy: 'abort' cannot be called in Wasm Workers.
+
+void proxied_js_function(void);
+
+int might_throw(void(*func)())
+{
+  int threw = EM_ASM_INT({
+    // Patch over assert() so that it does not abort execution on assert failure, but instead
+    // throws a catchable exception.
+    assert = function(condition, text) {
+      if (!condition) {
+        throw 'Assertion failed' + (text ? ": " + text : "");
+      }
+    };
+
+    try {
+      getWasmTableEntry($0)();
+    } catch(e) {
+      console.error('Threw an exception: ' + e);
+      return e.toString().includes('this function has been declared to only be callable from the main browser thread');
+    }
+    console.error('Function did not throw');
+    return 0;
+  }, func);
+  return threw;
+}
+
+void test()
+{
+  proxied_js_function();
+}
+
+void *thread_main(void *arg)
+{
+  REPORT_RESULT(might_throw(test));
+  return 0;
+}
+
+char stack[1024];
+
+int main()
+{
+  proxied_js_function(); // Should be callable from main thread
+
+  pthread_t thread;
+  pthread_attr_t attr;
+  pthread_attr_init(&attr);
+  pthread_create(&thread, &attr, thread_main, 0);
+}
diff --git a/test/test_browser.py b/test/test_browser.py
@@ -5107,6 +5107,18 @@ def test_wasm_worker_no_proxied_js_functions(self):
     self.btest('wasm_worker/no_proxied_js_functions.c', expected='0',
                args=['--js-library', test_file('wasm_worker/no_proxied_js_functions.js')])
 
+  # Tests that the proxying directives foo__proxy: 'abort' and foo__proxy: 'abort_debug' work.
+  @parameterized({
+    '': ('1', ['--js-library', test_file('wasm_worker/proxy_abort.js')],),
+    'debug': ('1', ['--js-library', test_file('wasm_worker/proxy_abort_debug.js'), '-sASSERTIONS'],),
+    'debug_no_assertions': ('0', ['--js-library', test_file('wasm_worker/proxy_abort_debug.js'), '-sASSERTIONS=0'],),
+  })
+  def test_proxy_abort(self, expect_result, args):
+    self.btest('pthread/proxy_abort.c', expected=expect_result, args=args + ['-pthread'])
+
+    self.set_setting('WASM_WORKERS')
+    self.btest('wasm_worker/proxy_abort.c', expected=expect_result, args=args)
+
   # Tests emscripten_semaphore_init(), emscripten_semaphore_waitinf_acquire() and emscripten_semaphore_release()
   @also_with_minimal_runtime
   def test_wasm_worker_semaphore_waitinf_acquire(self):

diff --git a/test/wasm_worker/proxy_abort.c b/test/wasm_worker/proxy_abort.c
@@ -0,0 +1,48 @@
+#include <emscripten.h>
+#include <emscripten/wasm_worker.h>
+#include <assert.h>
+
+// Test that a JS function with __proxy: 'abort' cannot be called in Wasm Workers.
+
+void proxied_js_function(void);
+
+int might_throw(void(*func)())
+{
+  int threw = EM_ASM_INT({
+    // Patch over assert() so that it does not abort execution on assert failure, but instead
+    // throws a catchable exception.
+    assert = function(condition, text) {
+      if (!condition) {
+        throw 'Assertion failed' + (text ? ": " + text : "");
+      }
+    };
+
+    try {
+      getWasmTableEntry($0)();
+    } catch(e) {
+      console.error('Threw an exception: ' + e);
+      return e.toString().includes('this function has been declared to only be callable from the main browser thread');
+    }
+    console.error('Function did not throw');
+    return 0;
+  }, func);
+  return threw;
+}
+
+void test()
+{
+  proxied_js_function();
+}
+
+void worker_main()
+{
+  REPORT_RESULT(might_throw(test));
+}
+
+char stack[1024];
+
+int main()
+{
+  proxied_js_function(); // Should be callable from main thread
+  emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024), worker_main);
+}
diff --git a/test/wasm_worker/proxy_abort.js b/test/wasm_worker/proxy_abort.js
@@ -0,0 +1,6 @@
+addToLibrary({
+  proxied_js_function__proxy: 'abort',
+  proxied_js_function: function() {
+  	console.log('In proxied_js_function');
+  }
+});
diff --git a/test/wasm_worker/proxy_abort_debug.js b/test/wasm_worker/proxy_abort_debug.js
@@ -0,0 +1,6 @@
+addToLibrary({
+  proxied_js_function__proxy: 'abort_debug',
+  proxied_js_function: function() {
+  	console.log('In proxied_js_function');
+  }
+});