Heap corruption after fetch when using WASM exceptions

[slowriot]

This is a weird one!

After calling emscripten_fetch, heap-allocated memory held by members of objects allocated on the stack (such as the test_vector in the example below) becomes corrupted with garbage values. This only happens when exception mode is WASM exceptions.

I isolated a minimal example of this with emscripten_fetch, but I believe it's not limited to the fetch functionality, as I get similar symptoms elsewhere when enabling WASM exceptions in more complex programs.

Below is a minimal example to trigger this behaviour, with a CMakeLists to aid in setting the right flags:

main.cpp

#include <iostream>
#include <emscripten/emscripten.h>
#include <emscripten/fetch.h>

struct test_struct {
  unsigned int test_int{123};
  std::vector<unsigned int> test_vector{123, 456, 789};

  test_struct() {
    std::cout << "At construction: Test int " << test_int << ", vector " << test_vector[0] << ", " << test_vector[1] << ", " << test_vector[2] << std::endl;
  }
};

void on_success(emscripten_fetch_t *fetch) {
  /// Success callback
  auto &test{*static_cast<test_struct*>(fetch->userData)};
  std::cout << "In success callback: Test int " << test.test_int << ", vector " << test.test_vector[0] << ", " << test.test_vector[1] << ", " << test.test_vector[2] << std::endl;
  emscripten_fetch_close(fetch);
};

auto main()->int {
  test_struct test;

  emscripten_fetch_attr_t attr;
  emscripten_fetch_attr_init(&attr);
  std::strcpy(attr.requestMethod, "GET");
  attr.attributes = EMSCRIPTEN_FETCH_LOAD_TO_MEMORY | EMSCRIPTEN_FETCH_REPLACE;
  attr.userData = &test;
  attr.onsuccess = on_success;
  emscripten_fetch(&attr, "test.txt");

  std::cout << "Immediately after fetch: Test int " << test.test_int << ", vector " << test.test_vector[0] << ", " << test.test_vector[1] << ", " << test.test_vector[2] << std::endl;

  emscripten_set_main_loop_arg([](void *data){
    auto &test{*reinterpret_cast<test_struct*>(data)};
    std::cout << "In main loop: Test int " << test.test_int << ", vector " << test.test_vector[0] << ", " << test.test_vector[1] << ", " << test.test_vector[2] << std::endl;
  }, &test, 0, true);

  std::unreachable();
}

CMakeLists.txt

cmake_minimum_required(VERSION 3.13)

project(client)

set(CMAKE_CXX_STANDARD 23)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

set(EXCEPTION_HANDLING wasm CACHE STRING "Exception handling mode: none, js or wasm")
if(EXCEPTION_HANDLING STREQUAL "none")
  message(STATUS "Exception handling disabled")
  set(exception_compile_definitions
    DISABLE_EXCEPTION_THROWING
    DISABLE_EXCEPTION_CATCHING
  )
  set(exception_compile_options
    -fignore-exceptions
    -fno-exceptions
  )
  set(exception_link_options
    -fno-except
    -fignore-exceptions
  )
elseif(EXCEPTION_HANDLING STREQUAL "js")
  message(STATUS "Exception handling using JS (slow)")
  set(exception_compile_definitions
    NO_DISABLE_EXCEPTION_CATCHING
  )
  set(exception_compile_options
    -fexceptions
  )
  set(exception_link_options
  -fexceptions
  )
elseif(EXCEPTION_HANDLING STREQUAL "wasm")
  message(STATUS "Exception handling using WASM)")
  set(exception_compile_definitions
    NO_DISABLE_EXCEPTION_CATCHING
  )
  set(exception_compile_options
    -fwasm-exceptions
  )
  set(exception_link_options
    -fwasm-exceptions
  )
else()
  message(FATAL_ERROR "Invalid exception handling mode \"${EXCEPTION_HANDLING}\"")
endif()

add_executable(client
  main.cpp
)

target_compile_options(client PRIVATE
  ${exception_compile_options}
)

target_link_options(client PRIVATE
  -sFETCH
  ${exception_link_options}
  -sENVIRONMENT=web
)

set(CMAKE_EXECUTABLE_SUFFIX ".html")

Build and run:

emcmake cmake -B build
emmake cmake --build build -j$(nproc)
emrun build/client.html

Example console output in Chromium 131.0.6778.108:

Example output in Firefox Nightly:

Note that we expect the test_vector contents to be 123, 456, 789 but these values get replaced after the emscripten_fetch call.

Switch EXCEPTION_HANDLING in CMakeLists.txt from wasm to js or none to observe that this issue only happens when using WASM exceptions.

Comment out emscripten_fetch to observe the heap corruption goes away, even in WASM exception mode.

You can create a test.txt file to trigger the on_success callback, but the issue occurs even if the download doesn't succeed.

[sbc100]

IIUC the simulate_infinite_loop argument to emscripten_set_main_loop_arg does not work as expected when wasm exceptions are enabled.

This is because the old way that simulate_infinite_loop worked to unwind the stack without destroying on-stack objects no longer works. @aheejin do you think there is some way we can fix this, or should we perhaps error out when emscripten_set_main_loop_arg is set and wasm exceptions are used?

[slowriot]

That's interesting, I've just revisited the documentation and noticed the note to the same effect as what you said, at the bottom of https://emscripten.org/docs/api_reference/emscripten.h.html#c.emscripten_set_main_loop. I hadn't noticed it before as it's only mentioned there, and not for emscripten_set_main_loop_arg.

So does this mean that with wasm exceptions turned on, current behaviour is just as if simulate_infinite_loop is false? Or is there more nuance to it than that?

In that case, what's the current best practice approach for maintaining state while running an infinite loop, if I want to use wasm exceptions? Is it just to allocate new objects on the heap in main, and passing the raw pointer to emscripten_set_main_loop, with the appearance of leaking the memory from main?

[sbc100]

So does this mean that with wasm exceptions turned on, current behaviour is just as if simulate_infinite_loop is false? Or is there more nuance to it than that?

There is a little of of nuance. simulate_infinite_loop will continue to cause the stack to unwind at the point of the call. IIUC the only difference is that any C++ objects on the stack will have their desctructor run.

So if your current behaviour relies on C++ heap objects (with destructors) outliving the call to simulate_infinite_loop then this will no longer work, and I do't know if we have any proposed solution in this that. If you fall into this catagory then you would either need to stick to emscripten-exceptions or restructure your code to avoid relying on stack objects in your loop callback.

[slowriot]

I see, thanks for the clarification.

In that case it's clear how to work around this; I think just a warning about this behaviour would be useful, for example when simulate_infinite_loop is true and wasm exceptions are enabled. It doesn't need to be an error necessarily, as long as the user is somehow alerted to the change in behaviour.

[sbc100]

Agreed. @aheejin @dschuff @kripken, do you think we just stuck with this feature degradation here? Or is there some way we could conditionally disable the running of destructors during unwind? If we are stuck with this when we should either have a warning or just a hard error when simulate_infinite_loop is used I think.

[kripken]

Hmm, I can't think of a good way to fix this myself, but others know more about that area.

I agree a warning or error makes sense, when using wasm EH + simulating an infinite loop.

[dschuff]

The default behavior of ensuring that C++ destructors run when JS exceptions unwind through wasm frames is implemented by using catch_all/catch_ref (which catches all wasm and JS exceptions) to run the destructors. I think it's the right default generally, but it's not the only possible behavior. We could add an option to LLVM that makes it only catch C++ tags (and maybe longjmp tags?). This would mean that JS exceptions unwinding through wasm would not clean up the objects and would instead leak (which is in this case the desired result). I could imagine that there might be other possible use cases, although any use case like this is probably flirting with danger and UB in C++.

The other option might be to instead of implementing the return to event loop with a JS throw, it could be implemented with a wasm trap. This would also have the effect of uncatchably unwinding all the wasm frames (and would propagate through any JS frames as WebAssembly.RuntimeError (IIRC) exceptions. This seems like a pretty bad abuse of what traps should be for, but I think it would work. And it's maybe no worse than just throw 'unwind'?

[kripken]

Interesting idea, yeah, a trap would work here. We do already have an export for that, too.