Fix MAIN_THREAD_EM_ASM_INT + CAN_ADDRESS_2GB (emscripten-core#21292)

We were using the sign bit of an integer to distinguish between data
pointers and fixed JS function indexes, but that doesn't work once
that data address can be larger than 2^31.

Technically this is very unlikely in practice since in order to get
an EM_ASM address over 2^31 you would either need 2Gb of static data
to be using `-sGLOBAL_BASE=2gb` like we do in the tests.

An alternative approach here would be assume we have fewer than
`GLOBAL_BASE` (1024 is most cases) proxied JS library functions and then
we could assume that small integers we JS library functions and larger
ones were data pointers (EM_ASM functions).  However that seems fragile
too.  Passing an extra argument around seems like a small cost here.

Author: sbc100

.circleci/config.yml

@@ -817,6 +817,7 @@ jobs:
             browser_2gb.test_fulles2_sdlproc
             browser_2gb.test_cubegeom*
             browser_2gb.test_html5_webgl_create_context*
+            browser_2gb.test_main_thread_async_em_asm
             "
   test-browser-chrome-wasm64-4gb:
     executor: bionic

src/jsifier.js

@@ -272,7 +272,7 @@ function(${args}) {
             return `
 function(${args}) {
 if (ENVIRONMENT_IS_PTHREAD)
-  return ${proxyFunc}(${proxiedFunctionTable.length}, ${+sync}${args ? ', ' : ''}${args});
+  return ${proxyFunc}(${proxiedFunctionTable.length}, 0, ${+sync}${args ? ', ' : ''}${args});
 ${body}
 }\n`
           });

src/library.js

@@ -2896,7 +2896,7 @@ addToLibrary({
     '$proxyToMainThread'
 #endif
   ],
-  $runMainThreadEmAsm: (code, sigPtr, argbuf, sync) => {
+  $runMainThreadEmAsm: (emAsmAddr, sigPtr, argbuf, sync) => {
     var args = readEmAsmArgs(sigPtr, argbuf);
 #if PTHREADS
     if (ENVIRONMENT_IS_PTHREAD) {
@@ -2909,29 +2909,23 @@ addToLibrary({
       // of using __proxy. (And dor simplicity, do the same in the sync
       // case as well, even though it's not strictly necessary, to keep the two
       // code paths as similar as possible on both sides.)
-      // -1 - code is the encoding of a proxied EM_ASM, as a negative number
-      // (positive numbers are non-EM_ASM calls).
-      return proxyToMainThread(-1 - code, sync, ...args);
+      return proxyToMainThread(0, emAsmAddr, sync, ...args);
     }
 #endif
 #if ASSERTIONS
-    assert(ASM_CONSTS.hasOwnProperty(code), `No EM_ASM constant found at address ${code}.  The loaded WebAssembly file is likely out of sync with the generated JavaScript.`);
+    assert(ASM_CONSTS.hasOwnProperty(emAsmAddr), `No EM_ASM constant found at address ${emAsmAddr}.  The loaded WebAssembly file is likely out of sync with the generated JavaScript.`);
 #endif
-    return ASM_CONSTS[code](...args);
+    return ASM_CONSTS[emAsmAddr](...args);
   },
   emscripten_asm_const_int_sync_on_main_thread__deps: ['$runMainThreadEmAsm'],
-  emscripten_asm_const_int_sync_on_main_thread: (code, sigPtr, argbuf) => {
-    return runMainThreadEmAsm(code, sigPtr, argbuf, 1);
-  },
+  emscripten_asm_const_int_sync_on_main_thread: (emAsmAddr, sigPtr, argbuf) => runMainThreadEmAsm(emAsmAddr, sigPtr, argbuf, 1),
 
   emscripten_asm_const_ptr_sync_on_main_thread__deps: ['$runMainThreadEmAsm'],
-  emscripten_asm_const_ptr_sync_on_main_thread: (code, sigPtr, argbuf) => {
-    return runMainThreadEmAsm(code, sigPtr, argbuf, 1);
-  },
+  emscripten_asm_const_ptr_sync_on_main_thread: (emAsmAddr, sigPtr, argbuf) => runMainThreadEmAsm(emAsmAddr, sigPtr, argbuf, 1),
 
   emscripten_asm_const_double_sync_on_main_thread: 'emscripten_asm_const_int_sync_on_main_thread',
   emscripten_asm_const_async_on_main_thread__deps: ['$runMainThreadEmAsm'],
-  emscripten_asm_const_async_on_main_thread: (code, sigPtr, argbuf) => runMainThreadEmAsm(code, sigPtr, argbuf, 0),
+  emscripten_asm_const_async_on_main_thread: (emAsmAddr, sigPtr, argbuf) => runMainThreadEmAsm(emAsmAddr, sigPtr, argbuf, 0),
 #endif
 
 #if !DECLARE_ASM_MODULE_EXPORTS

src/library_pthread.js

@@ -962,8 +962,12 @@ var LibraryPThread = {
 
   $proxyToMainThread__deps: ['$withStackSave', '_emscripten_run_on_main_thread_js'].concat(i53ConversionDeps),
   $proxyToMainThread__docs: '/** @type{function(number, (number|boolean), ...number)} */',
-  $proxyToMainThread: (index, sync, ...callArgs) => {
-    // Additional arguments are passed after those two, which are the actual
+  $proxyToMainThread: (funcIndex, emAsmAddr, sync, ...callArgs) => {
+    // EM_ASM proxying is done by passing a pointer to the address of the EM_ASM
+    // contant as `emAsmAddr`.  JS library proxying is done by passing an index
+    // into `proxiedJSCallArgs` as `funcIndex`. If `emAsmAddr` is non-zero then
+    // `funcIndex` will be ignored.
+    // Additional arguments are passed after the first three are the actual
     // function arguments.
     // The serialization buffer contains the number of call params, and then
     // all the args here.
@@ -995,7 +999,7 @@ var LibraryPThread = {
         HEAPF64[b + i] = arg;
 #endif
       }
-      return __emscripten_run_on_main_thread_js(index, serializedNumCallArgs, args, sync);
+      return __emscripten_run_on_main_thread_js(funcIndex, emAsmAddr, serializedNumCallArgs, args, sync);
     });
   },
 
@@ -1005,7 +1009,7 @@ var LibraryPThread = {
   _emscripten_receive_on_main_thread_js__deps: [
     '$proxyToMainThread',
     '$proxiedJSCallArgs'],
-  _emscripten_receive_on_main_thread_js: (index, callingThread, numCallArgs, args) => {
+  _emscripten_receive_on_main_thread_js: (funcIndex, emAsmAddr, callingThread, numCallArgs, args) => {
     // Sometimes we need to backproxy events to the calling thread (e.g.
     // HTML5 DOM events handlers such as
     // emscripten_set_mousemove_callback()), so keep track in a globally
@@ -1028,15 +1032,17 @@ var LibraryPThread = {
       proxiedJSCallArgs[i] = HEAPF64[b + i];
 #endif
     }
-    // Proxied JS library funcs are encoded as positive values, and
-    // EM_ASMs as negative values (see include_asm_consts)
+    // Proxied JS library funcs use funcIndex and EM_ASM functions use emAsmAddr
 #if HAVE_EM_ASM
-    var isEmAsmConst = index < 0;
-    var func = !isEmAsmConst ? proxiedFunctionTable[index] : ASM_CONSTS[-index - 1];
+    var func = emAsmAddr ? ASM_CONSTS[emAsmAddr] : proxiedFunctionTable[funcIndex];
 #else
-    var func = proxiedFunctionTable[index];
+#if ASSERTIONS
+    assert(!emAsmAddr);
+#endif
+    var func = proxiedFunctionTable[funcIndex];
 #endif
 #if ASSERTIONS
+    assert(!(funcIndex && emAsmAddr));
     assert(func.length == numCallArgs, 'Call args mismatch in _emscripten_receive_on_main_thread_js');
 #endif
     PThread.currentProxiedOperationCallerThread = callingThread;

src/library_sdl.js

@@ -1606,7 +1606,7 @@ var LibrarySDL = {
       var data = surfData.image.data;
       var buffer = surfData.buffer;
       assert(buffer % 4 == 0, 'Invalid buffer offset: ' + buffer);
-      var src = buffer >> 2;
+      var src = {{{ getHeapOffset('buffer', 'i32') }}};
       var dst = 0;
       var isScreen = surf == SDL.screen;
       var num;

src/library_sigs.js

@@ -328,7 +328,7 @@ sigs = {
   _emscripten_notify_mailbox_postmessage__sig: 'vppp',
   _emscripten_push_main_loop_blocker__sig: 'vppp',
   _emscripten_push_uncounted_main_loop_blocker__sig: 'vppp',
-  _emscripten_receive_on_main_thread_js__sig: 'dipip',
+  _emscripten_receive_on_main_thread_js__sig: 'dippip',
   _emscripten_runtime_keepalive_clear__sig: 'v',
   _emscripten_set_offscreencanvas_size__sig: 'ipii',
   _emscripten_system__sig: 'ip',

system/lib/pthread/proxying.c

@@ -585,6 +585,7 @@ em_promise_t emscripten_proxy_promise(em_proxying_queue* q,
 
 typedef struct proxied_js_func_t {
   int funcIndex;
+  void* emAsmAddr;
   pthread_t callingThread;
   int numArgs;
   double* argBuffer;
@@ -595,19 +596,21 @@ typedef struct proxied_js_func_t {
 static void run_js_func(void* arg) {
   proxied_js_func_t* f = (proxied_js_func_t*)arg;
   f->result = _emscripten_receive_on_main_thread_js(
-    f->funcIndex, f->callingThread, f->numArgs, f->argBuffer);
+    f->funcIndex, f->emAsmAddr, f->callingThread, f->numArgs, f->argBuffer);
   if (f->owned) {
     free(f->argBuffer);
     free(f);
   }
 }
 
-double _emscripten_run_on_main_thread_js(int index,
+double _emscripten_run_on_main_thread_js(int func_index,
+                                         void* em_asm_addr,
                                          int num_args,
                                          double* buffer,
                                          int sync) {
   proxied_js_func_t f = {
-    .funcIndex = index,
+    .funcIndex = func_index,
+    .emAsmAddr = em_asm_addr,
     .callingThread = pthread_self(),
     .numArgs = num_args,
     .argBuffer = buffer,

system/lib/pthread/threading_internal.h

@@ -95,7 +95,7 @@ int __pthread_create_js(struct __pthread *thread, const pthread_attr_t *attr, vo
 int _emscripten_default_pthread_stack_size();
 void __set_thread_state(pthread_t ptr, int is_main, int is_runtime, int can_block);
 
-double _emscripten_receive_on_main_thread_js(int functionIndex, pthread_t callingThread, int numCallArgs, double* args);
+double _emscripten_receive_on_main_thread_js(int funcIndex, void* emAsmAddr, pthread_t callingThread, int numCallArgs, double* args);
 
 // Return non-zero if the calling thread supports Atomic.wait (For example
 // if called from the main browser thread, this function will return zero

test/test_core.py

@@ -1892,15 +1892,19 @@ def test_em_asm_2(self):
   # test_em_asm_2, just search-replaces EM_ASM to MAIN_THREAD_EM_ASM on the test
   # file. That way if new test cases are added to test_em_asm_2.cpp for EM_ASM,
   # they will also get tested in MAIN_THREAD_EM_ASM form.
-  def test_main_thread_em_asm(self):
+  @parameterized({
+    '': ([],),
+    'pthread': (['-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'],),
+  })
+  def test_main_thread_em_asm(self, args):
     src = read_file(test_file('core/test_em_asm_2.cpp'))
     create_file('test.cpp', src.replace('EM_ASM', 'MAIN_THREAD_EM_ASM'))
 
     expected_result = read_file(test_file('core/test_em_asm_2.out'))
     create_file('test.out', expected_result.replace('EM_ASM', 'MAIN_THREAD_EM_ASM'))
 
-    self.do_run_in_out_file_test('test.cpp')
-    self.do_run_in_out_file_test('test.cpp', force_c=True)
+    self.do_run_in_out_file_test('test.cpp', emcc_args=args)
+    self.do_run_in_out_file_test('test.cpp', emcc_args=args, force_c=True)
 
   @needs_dylink
   @parameterized({

tools/emscripten.py

@@ -940,7 +940,7 @@ def create_pointer_conversion_wrappers(metadata):
     '_wasmfs_identify': '_p',
     '_wasmfs_read_file': 'pp',
     '__dl_seterr': '_pp',
-    '_emscripten_run_on_main_thread_js': '___p_',
+    '_emscripten_run_on_main_thread_js': '__p_p_',
     '_emscripten_proxy_execute_task_queue': '_p',
     '_emscripten_thread_exit': '_p',
     '_emscripten_thread_init': '_p_____',