Test and fix for incompatible HEAP transforms in acorn-optimizer (#24309)

While working on #24295, I noticed I could accidentally prevent ASan and
SAFE_HEAP from finding any `HEAPxx[n]` accesses from JS when
`ALLOW_MEMORY_GROWTH` is enabled, and no tests would catch it, because
no tests verify that acorn-optimizer passes which change `HEAPxx[n]`
into something else can work in conjunction with each other.

After adding the test, I found that an existing `SUPPORT_BIG_ENDIAN`
feature was already broken in the same way, because it transforms
`HEAPxx[n]` accesses into DataView methods, and since it was running
before ASan and SAFE_HEAP, those latter transforms became no-ops. I
fixed that in the same PR by moving the big-endian transform to the end.

In the future we might want to consolidate those transforms in a more
reliable way (e.g. only run a single code transform that does
`HEAPxx[n]` -> `__loadHeap`/`__storeHeap` where those functions do all
the work using `#if`s), but at least for now this PR fixes an immediate
problem and catches potential future regressions.

Author: RReverser

src/lib/libpthread.js

@@ -978,7 +978,7 @@ var LibraryPThread = {
 
   $establishStackSpace__internal: true,
   $establishStackSpace__deps: ['$stackRestore', 'emscripten_stack_set_limits'],
-  $establishStackSpace: (pthread_ptr) => {
+  $establishStackSpace: function (pthread_ptr) {
     var stackHigh = {{{ makeGetValue('pthread_ptr', C_STRUCTS.pthread.stack, '*') }}};
     var stackSize = {{{ makeGetValue('pthread_ptr', C_STRUCTS.pthread.stack_size, '*') }}};
     var stackLow = stackHigh - stackSize;

src/runtime_asan.js

@@ -12,18 +12,23 @@
 // ASan instrumentation on them. However, until the wasm module is ready, we
 // must access things directly.
 
-function _asan_js_load(arr, index) {
+function _asan_js_check_index(arr, index, asanFn) {
+#if EXIT_RUNTIME
+  if (runtimeInitialized && !runtimeExited) {
+#else
   if (runtimeInitialized) {
+#endif
     const elemSize = arr.BYTES_PER_ELEMENT;
-    ___asan_loadN(index * elemSize, elemSize);
+    asanFn(index * elemSize, elemSize);
   }
+}
+
+function _asan_js_load(arr, index) {
+  _asan_js_check_index(arr, index, ___asan_loadN);
   return arr[index];
 }
 
 function _asan_js_store(arr, index, value) {
-  if (runtimeInitialized) {
-    const elemSize = arr.BYTES_PER_ELEMENT;
-    ___asan_storeN(index * elemSize, elemSize);
-  }
+  _asan_js_check_index(arr, index, ___asan_storeN);
   return arr[index] = value;
 }

test/other/codesize/test_codesize_minimal_pthreads.gzsize

@@ -1 +1 @@
-3785
+3786

test/other/codesize/test_codesize_minimal_pthreads.jssize

@@ -1 +1 @@
-7816
+7827

test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize

@@ -1 +1 @@
-3985
+3989

test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize

@@ -1 +1 @@
-8247
+8258

test/core/test_safe_heap_user_js.c renamed to test/other/test_safe_heap_user_js.c

@@ -9084,14 +9084,6 @@ def test_asan_modularized_with_closure(self):
     self.set_setting('INITIAL_MEMORY', '300mb')
     self.do_run_in_out_file_test('hello_world.c')
 
-  @no_asan('SAFE_HEAP cannot be used with ASan')
-  @no_2gb('asan doesnt support GLOBAL_BASE')
-  @no_esm_integration('sanitizers do not support WASM_ESM_INTEGRATION')
-  def test_safe_heap_user_js(self):
-    self.set_setting('SAFE_HEAP')
-    self.do_runf('core/test_safe_heap_user_js.c',
-                 expected_output=['Aborted(segmentation fault storing 1 bytes at address 0)'], assert_returncode=NON_ZERO)
-
   def test_safe_stack(self):
     self.set_setting('STACK_OVERFLOW_CHECK', 2)
     self.set_setting('STACK_SIZE', 1024)

test/test_core.py

@@ -12235,6 +12235,24 @@ def test_asan_strncpy(self):
     # https://github.com/emscripten-core/emscripten/issues/14618
     self.do_runf('other/test_asan_strncpy.c', emcc_args=['-fsanitize=address'])
 
+  @parameterized({
+    'asan': ['AddressSanitizer: null-pointer-dereference', '-fsanitize=address'],
+    'safe_heap': ['Aborted(segmentation fault storing 1 bytes at address 0)', '-sSAFE_HEAP'],
+  })
+  @parameterized({
+    '': [],
+    'memgrowth': ['-pthread', '-sALLOW_MEMORY_GROWTH', '-Wno-pthreads-mem-growth'],
+  })
+  def test_null_deref_via_js(self, expected_output, *args):
+    # Multiple JS transforms look for pattern like `HEAPxx[...]` and transform it.
+    # This test ensures that one of the transforms doesn't produce a pattern that
+    # another pass can't find anymore, that is that features can work in conjunction.
+    self.do_runf(
+      'other/test_safe_heap_user_js.c',
+      emcc_args=args,
+      assert_returncode=NON_ZERO,
+      expected_output=[expected_output])
+
   @node_pthreads
   def test_proxy_to_pthread_stack(self):
     # Check that the proxied main gets run with STACK_SIZE setting and not

test/test_other.py

@@ -1372,7 +1372,10 @@ function isHEAPAccess(node) {
 function asanify(ast) {
   recursiveWalk(ast, {
     FunctionDeclaration(node, c) {
-      if (node.id.type === 'Identifier' && node.id.name.startsWith('_asan_js_')) {
+      if (
+        node.id.type === 'Identifier' &&
+        (node.id.name.startsWith('_asan_js_') || node.id.name === 'establishStackSpace')
+      ) {
         // do not recurse into this js impl function, which we use during
         // startup before the wasm is ready
       } else {

tools/acorn-optimizer.mjs

@@ -2322,31 +2322,33 @@ def phase_binaryen(target, options, wasm_target):
   # after generating the wasm, do some final operations
 
   if final_js:
-    if settings.SUPPORT_BIG_ENDIAN:
-      with ToolchainProfiler.profile_block('little_endian_heap'):
-        final_js = building.little_endian_heap(final_js)
-
     # >=2GB heap support requires pointers in JS to be unsigned. rather than
     # require all pointers to be unsigned by default, which increases code size
     # a little, keep them signed, and just unsign them here if we need that.
     if settings.CAN_ADDRESS_2GB:
       with ToolchainProfiler.profile_block('use_unsigned_pointers_in_js'):
         final_js = building.use_unsigned_pointers_in_js(final_js)
 
+    if settings.USE_ASAN:
+      final_js = building.instrument_js_for_asan(final_js)
+
+    if settings.SAFE_HEAP:
+      final_js = building.instrument_js_for_safe_heap(final_js)
+
     # shared memory growth requires some additional JS fixups.
     # note that we must do this after handling of unsigned pointers. unsigning
     # adds some >>> 0 things, while growth will replace a HEAP8 with a call to
     # a method to get the heap, and that call would not be recognized by the
-    # unsigning pass
+    # unsigning pass.
+    # we also must do this after the asan or safe_heap instrumentation, as they
+    # wouldn't be able to recognize patterns produced by the growth pass.
     if settings.SHARED_MEMORY and settings.ALLOW_MEMORY_GROWTH:
       with ToolchainProfiler.profile_block('apply_wasm_memory_growth'):
         final_js = building.apply_wasm_memory_growth(final_js)
 
-    if settings.USE_ASAN:
-      final_js = building.instrument_js_for_asan(final_js)
-
-    if settings.SAFE_HEAP:
-      final_js = building.instrument_js_for_safe_heap(final_js)
+    if settings.SUPPORT_BIG_ENDIAN:
+      with ToolchainProfiler.profile_block('little_endian_heap'):
+        final_js = building.little_endian_heap(final_js)
 
     if settings.OPT_LEVEL >= 2 and settings.DEBUG_LEVEL <= 2:
       # minify the JS. Do not minify whitespace if Closure is used, so that