Update wasm exception handling model (migration to emsdk 4-x)

[anutosh491]

Description

Hi,

Today while building xeus-cpp on emscripten-forge's emscripten-4x branch, I saw this

This was expected because

1. with emsdk 3.1.73 we are using the Emscripten EH model (which is pulls in the invoke_*** functions/symbols)
2. with emsdk 4-x, emscripten promotes using the wasm EH model. Hence emscripten-forge's emscripten-4x branch builds all hosted packages (xeus, llvm and everything involved using -fwasm-exceptions and -sSUPPORT_LONGJMP=wasm)
3. We need to replicate the same using clang's webassembly toolchain (https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/WebAssembly.cpp#L357-L452)

• using -fwasm-exceptions pulls in the following

-target-feature +exception-handling
-target-feature +multivalue
-target-feature +reference-types
-exception-model=wasm
-mllvm -wasm-enable-eh

• and -sSUPPORT_LONGJMP=wasm which is an emcc specific flag boils down to -mllvm -wasm-enable-sjlj which can be used through clang.

So basically we end up replicating the same result but with a different exception handling model.

Type of change

Please tick all options which are relevant.

• Bug fix
• New feature
• Added/removed dependencies
• Required documentation updates

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.94%. Comparing base (778e639) to head (a509dba).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #416   +/-   ##
=======================================
  Coverage   81.94%   81.94%           
=======================================
  Files          21       21           
  Lines         853      853           
  Branches       87       87           
=======================================
  Hits          699      699           
  Misses        154      154           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[anutosh491]

Once ready, add test

#if defined(XEUS_CPP_EMSCRIPTEN_WASM_BUILD)
    TEST_CASE("Emscripten Exception Handling")
    {
        std::vector<const char*> Args = {
            "-std=c++20",
            "-v",
            "-fwasm-exceptions",
            "-mllvm", "-wasm-enable-sjlj"
        };

        xcpp::interpreter interpreter((int)Args.size(), Args.data());
        std::string code = "try { throw 1; } catch (...) { 0; }";
        nl::json user_expressions = nl::json::object();
        xeus::execute_request_config config;
        config.silent = false;
        config.store_history = false;
        config.allow_stdin = false;
        nl::json header = nl::json::object();
        xeus::xrequest_context::guid_list id = {};
        xeus::xrequest_context context(header, id);

        std::promise<nl::json> promise;
        std::future<nl::json> future = promise.get_future();
        auto callback = [&promise](nl::json result) {
            promise.set_value(result);
        };

        interpreter.execute_request(
            std::move(context),
            std::move(callback),
            code,
            std::move(config),
            user_expressions
        );
        nl::json result = future.get();
        REQUIRE(result["status"] == "ok");
    }
#endif

We were somewhat checking the older emscripten EH model here

https://github.com/compiler-research/CppInterOp/blob/2df83a9bb4060e2bd7d2878f195eb757401405bf/unittests/CppInterOp/InterpreterTest.cpp#L164-L189

[mcbarton]

Once its ready will this PR fix this issue #412 given it involves mentions of -fwasm-exceptions ? If yes, can you link the PR to this issue, so it automatically closes with these changes.

[anutosh491]

Once its ready will this PR fix this issue #412 given it involves mentions of -fwasm-exceptions ? If yes, can you link the PR to this issue, so it automatically closes with these changes.

Well the changes above technically come in at runtime to generate correct wasm from the LLVM module. Feel free to compare the 2 wasms actually

1. deployment in the readme that's still using emsdk 3-x with emscripten EH
2. A deployment link I hosted https://anutosh491.github.io/xeus-cpp-demo/lab/index.html using emsdk 4-x with wasm EH

Compare something like to see the difference in symbols.

#include <stdexcept>

try {
    throw std::runtime_error("Unknown exception");
}
catch (const std::exception& e) {
    std::cout << "Caught an exception: " << e.what() << std::endl;
}

What you are talking about is build time stuff. So although relevant, not addressed through the above changes just yet. Let me address them though !

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[anutosh491]

Cc @SylvainCorlay @DerThorsten

Pinging for reviews (and then we can make a minor release I think)

[martinRenou]

I'll release 4.3.1

[martinRenou]

It's baking, it will be installable with pip in a couple of minutes. Published on conda-forge later today/tomorrow.

[anutosh491]

Thanks again for the quick fix 😁

[anutosh491]

Arghh I still see this

cc @martinRenou

The error message says this : Unknown conda channel for package cppinterop. Known channels are [object Object]

Is this somehow related to how we've built cppinterop on emscripten-forge 🤔 ?
https://github.com/emscripten-forge/recipes/tree/emscripten-4x/recipes/recipes_emscripten/cppinterop

The build is here : https://prefix.dev/channels/emscripten-forge-4x/packages/cppinterop

[martinRenou]

Is this a local jupyterlite build? Can you share me the setup you have? Especially the environment.yml that you use for your emscripten-wasm32 environment

[anutosh491]

Is this a local jupyterlite build?

Nope, just following the readme (where we setup the xeus-lite-host env)

1. So the changes in this PR are also added as patches on emscripten-forge's 4-x branch. So can fetch xcpp.wasm/js/data from here https://prefix.dev/channels/emscripten-forge-4x/packages/xeus-cp I assume

2. But otherwise check the environment-wasm-build.yml and environment-wasm-host.yml in this PR. No change apart from
   from changing the channel and the emscripten version

3. And yeah then

micromamba create -n xeus-lite-host jupyter_server jupyterlite-xeus -c conda-forge
micromamba activate xeus-lite-host
jupyter lite serve --XeusAddon.prefix=$PREFIX

[anutosh491]

Also out of curiosity I tried mamba list and I don't see the channel (4-x) being mentioned

Instead of something like the following on our main deployment

[martinRenou]

Ok, so I think it has to do booting from a locally built prefix.

Also out of curiosity I tried mamba list and I don't see the channel (4-x) being mentioned

The packages that don't have channels, you built those locally, right?

[martinRenou]

Mmh no that's not what you're saying sorry

[mcbarton]

@anutosh491 Not sure if this will help you debug the issue, but CppInterOps 4.0.9 xeus-cpp build (demo available here https://mcbarton.github.io/xeus-cpp-demo/lab/index.html ) does reference the correct channel as can be seen here

Its not able to install symengine however due to channel priority. Its packages get excluded to the presence of the dev channel.

[martinRenou]

The bug is that jupyterlite-xeus is wrongfully injecting default channels "emscripten-forge-dev". For quite some time this was the main used channel so it was fine.

I'll mitigate this in jupyterlite/xeus#328

[mcbarton]

The smallpt notebook doesn't work for CppInterOps 4.0.9 xeus-cpp build here https://mcbarton.github.io/xeus-cpp-demo/lab/index.html , but does for the jupyter lite preview for this PR. @anutosh491 any idea what may be causing the difference? I get the above error message.

The xeus-cpp-lite-demo notebook works completely fine.

[anutosh491]

Hey @mcbarton

Probably let's not pollute the discussion here with what's happening in cppinterop ? Maybe let's discuss somewhere else (github/discord) ?

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[anutosh491]

As discussed with @martinRenou, we would need to pass the default channels/channels of interest while creating the host env/Prefix .... so that mambajs can make use of it while conducting a %mamba magic. This has been done in the latest commit.

1. Without passing the channels

2. With the channels

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[mcbarton]

You've linked this PR to this issue #387 . In that issue you say you'll close it during the migration, adding a test to show it now works. I cannot see the test. Can you add one?

[anutosh491]

You've linked this PR to this issue #387 . In that issue you say you'll close it during the migration, adding a test to show it now works. I cannot see the test. Can you add one?

I realized adding a test doesn't help (unless we can test the notebook directly ... which your is trying to achieve I think)

Cause we just have a wrong output, it's not a failure. So framing a test with the execute_request would just pass cause technically the execute_repl is okay and we have an output (it's just the wrong output)

On 3-x

On 4-x

EDIT : This is technically an emscripten 3-x bug as such (which they fixed in 4-x and hopefully also are testing it) , not something we should test . We're just the frontend here ;)

So every emscripten standard library (libc etc etc) based bug is a xeus-cpp-lite bug haha and we just need to report it upstream