emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.61k stars 3.28k forks source link

setTimeout is not defined in Spidermonkey #19264

Open cschreib opened 1 year ago

cschreib commented 1 year ago

I am developing and testing a C++ library, and I aim to have WebAssembly/Emscripten as a supported target. I have a suite of tests for the library, which normally just run headless on the command line. For Emscripten, I normally ask it to output a .js file, and run it through Node.js; this works fine.

The issue is when I try to use a Javascript shell instead. After setting the link option -s ENVIRONMENT=shell, both d8 and Spidermonkey are able to run my code, however Spidermonkey fails at the end with the following error:

Unhandled rejection: (new ReferenceError("setTimeout is not defined", "snitch_runtime_tests.js", 106))
Stack:
  quit_@snitch_runtime_tests.js:106:7
  handleException@snitch_runtime_tests.js:4369:12
  callMain@snitch_runtime_tests.js:5336:12
  doRun@snitch_runtime_tests.js:5379:31
  run@snitch_runtime_tests.js:5394:5
  runCaller@snitch_runtime_tests.js:5307:19
  removeRunDependency@snitch_runtime_tests.js:500:7
  receiveInstance@snitch_runtime_tests.js:711:24
  receiveInstantiationResult@snitch_runtime_tests.js:730:20

The offending Emscripten-generated JS code:

  if (typeof quit == 'function') {
    quit_ = (status, toThrow) => {
      // Unlike node which has process.exitCode, d8 has no such mechanism. So we
      // have no way to set the exit code and then let the program exit with
      // that code when it naturally stops running (say, when all setTimeouts
      // have completed). For that reason, we must call `quit` - the only way to
      // set the exit code - but quit also halts immediately.  To increase
      // consistency with node (and the web) we schedule the actual quit call
      // using a setTimeout to give the current stack and any exception handlers
      // a chance to run.  This enables features such as addOnPostRun (which
      // expected to be able to run code after main returns).
      setTimeout(() => {
        if (!(toThrow instanceof ExitStatus)) {
          let toLog = toThrow;
          if (toThrow && typeof toThrow == 'object' && toThrow.stack) {
            toLog = [toThrow, toThrow.stack];
          }
          err('exiting due to exception: ' + toLog);
        }
        quit(status);
      });
      throw toThrow;
    };
  }

setTimeout is called in a few other places in the Emscripten-generated JS code.

I'm not super versed in JS, but after doing some research, it seems that setTimeout is a browser/window thing, and is not normally available in a standalone JS shell. I am confused as to why Emscripten uses this function, even though I linked with -s ENVIRONMENT=shell. The Emscripten documentation seems to indicate that the Spidermonkey shell is a supported execution platform. I could not find any information about any other specific compilation/link flags that would be required for this to work.

Spidermonkey's jsshell was downloaded from here.

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.35 (a90c5abb489c516f59c0d16864d953d4bedd37da)
clang version 17.0.0 (https://github.com/llvm/llvm-project 6865cff8ea8b07d9f2385fd92cecb422404f0f35)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /home/cschreib/programming/emsdk/upstream/bin

Failing command line in full:

/home/cschreib/programming/emsdk/upstream/emscripten/em++ -s DISABLE_EXCEPTION_CATCHING=0 -g -s STACK_SIZE=8388608 -s ENVIRONMENT=shell @CMakeFiles/snitch_runtime_tests.dir/objects1.rsp -o snitch_runtime_tests.js @CMakeFiles/snitch_runtime_tests.dir/linkLibs.rsp

Full link command and output with -v appended:

 "/home/cschreib/programming/emsdk/upstream/bin/clang" --version
 "/home/cschreib/programming/emsdk/upstream/bin/wasm-ld" -o snitch_runtime_tests.wasm CMakeFiles/snitch_runtime_tests.dir/testing_event.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/type_name.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/small_vector.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/small_string.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/string_utility.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/small_function.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/matchers.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/check.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/skip.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/capture.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/section.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/cli.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/registry.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/macros.cpp.o CMakeFiles/snitch_runtime_tests.dir/runtime_tests/regressions.cpp.o CMakeFiles/snitch_runtime_tests.dir/__/src/snitch.cpp.o ../_deps/doctest-build/libdoctest_with_main.a -L/home/cschreib/programming/emsdk/upstream/emscripten/cache/sysroot/lib/wasm32-emscripten -lGL -lal -lhtml5 -lstubs-debug -lnoexit -lc-debug -ldlmalloc -lcompiler_rt -lc++ -lc++abi-debug -lsockets -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr --allow-undefined-file=/tmp/tmptlbrkzx9.undefined --export-if-defined=main --export-if-defined=__get_exception_message --export-if-defined=free --export-if-defined=__start_em_asm --export-if-defined=__stop_em_asm --export-if-defined=__start_em_lib_deps --export-if-defined=__stop_em_lib_deps --export-if-defined=__start_em_js --export-if-defined=__stop_em_js --export-if-defined=__main_argc_argv --export-if-defined=fflush --export=emscripten_stack_get_end --export=emscripten_stack_get_free --export=emscripten_stack_get_base --export=emscripten_stack_get_current --export=emscripten_stack_init --export=stackSave --export=stackRestore --export=stackAlloc --export=__errno_location --export=getTempRet0 --export=setTempRet0 --export=__get_temp_ret --export=__set_temp_ret --export=__cxa_is_pointer_type --export=__cxa_can_catch --export=setThrew --export=__cxa_free_exception --export=__wasm_call_ctors --export-table -z stack-size=8388608 --initial-memory=16777216 --no-entry --max-memory=16777216 --stack-first
 "/home/cschreib/programming/emsdk/upstream/bin/wasm-emscripten-finalize" -g --dyncalls-i64 --pass-arg=legalize-js-interface-exported-helpers --dwarf snitch_runtime_tests.wasm -o snitch_runtime_tests.wasm --detect-features
 "/home/cschreib/programming/emsdk/node/14.15.5_64bit/bin/node" /home/cschreib/programming/emsdk/upstream/emscripten/src/compiler.js /tmp/tmpiopqcenj.json
 "/home/cschreib/programming/emsdk/upstream/bin/llvm-objcopy" snitch_runtime_tests.wasm snitch_runtime_tests.wasm --remove-section=producers
sbc100 commented 1 year ago

We don't run any tests under the spidermonkey shell, so its likely that a lot of stuff won't work. We should probably make this more clear somehow.

Can I ask why you wouldn't just want to stick with node? Soooo much more stuff works under node that under either d8 or spidermonkey.

cschreib commented 1 year ago

I see, I was under the illusion that you did, because the docs say that Spidermonkey is required for running tests :) But looking more closely, it seems all the tests using it are currently disabled?

The pragmatic reason I wasn't sticking with Node.js is that my library is currently hitting a bug in the V8 wasm JIT optimizer (so d8 is affected too). I wanted to see if there was another runtime I could use in the meantime, while this gets fixed. My code is simple and headless, I never talk to JS or the DOM. So in principle even targetting a standalone wasm runtime would be enough for me (if only they supported exceptions).

There is also a more philosophical reason. Generally, I dislike the concept of being tied to the Chrome ecosystem: it is so widespread that people confuse "works in Chrome" and "standard compliant". People start building against Chrome's behaviour, rather than against the standard specs, and we end up with all these websites that work only with Chrome. It stiffles healthy competition from other browsers, and reinforces the Chrome monopoly. If Emcscripten emits JS and WASM, that code (unless otherwise requested) should be able to run on any standard-compliant platform that can execute JS and WASM. I was under the impression that it was the case, but apparently not fully.

I understand that you have limited resources, so don't take this as an annoyed rant. It's more a disappointment :) I think Emcscripten and everyone using it would benefit from being usable with as many runtimes / shell / browser / etc as possible.

sbc100 commented 1 year ago

We do run a lot of tests under Firefox so we know that the output of emscripten does run there, so in general we know that we are compatible with the spidermonkey engine.. just maybe not the standalone shell, which IIUC, is like d8 and not really designed for use in production.

If you would like to send a patch to make the output more compatible with the standalone spider monkey that would would be more welcome. You would also suggest to the spider monkey folks that implementing setTimeout might be good idea there... I'm not sure if they have any real users outside of their own unit tests. Do they simply not include an event loop at all? Or is there some internal version of setTimeout for use with thier event loop?

cschreib commented 1 year ago

We do run a lot of tests under Firefox so we know that the output of emscripten does run there, so in general we know that we are compatible with the spidermonkey engine.. just maybe not the standalone shell, which IIUC, is like d8 and not really designed for use in production.

True! I should have acknowledged that in my comment. I guess I'm a bit surprised by how little support d8 and SpiderMonkey's JSShell seem to receive from their owners (I had to compile d8 from source since there's no binary distribution, fun times, and hunting for binaries for JSShell wasn't straightforward either). These lightweight shells seem perfect for running simple WASM code though. Although I guess long term they will be replaced by the pure WASM runtimes. Anyway, if the JS shells aren't meant to be used in production, do you know the rationale for supporting them in Emscripten with the -s ENVIRONMENT=shell option? I think that option led me to believe that shells were first-class citizens, on the same level as Node.js. I still have much to learn about this ecosystem, clearly!

You would also suggest to the spider monkey folks that implementing setTimeout might be good idea there... I'm not sure if they have any real users outside of their own unit tests.

I don't know; setTimeout isn't part of the ECMAScript specs for JS as far as I understand. So it definitely doesn't sit with SpiderMonkey, but perhaps with JSShell (in my understanding, JSShell is to SpiderMonkey what d8 is to V8; the former is the shell, the latter is the engine).

Do they simply not include an event loop at all? Or is there some internal version of setTimeout for use with thier event loop?

I tried looking at their source code, and I didn't see an event loop in JSShell; it's just an REPL interpreter that stops after each command. But honestly I can't be sure.

If you would like to send a patch to make the output more compatible with the standalone spider monkey that would would be more welcome.

I'd love to, but I have never written a single line of JS in my life; I'm a C++ guy at heart :) Maybe it's not so bad; I can give it a shot but I don't know that I'll catch all the subtleties. Especially if JSShell isn't currently part of the test suite.

sbc100 commented 1 year ago

I had to compile d8 from source since there's no binary distribution, fun times, and hunting for binaries for JSShell wasn't straightforward either

We just use jsvu to install the shell binaries: https://github.com/GoogleChromeLabs/jsvu. Its super simpler and quick.

sbc100 commented 1 year ago

Anyway, if the JS shells aren't meant to be used in production, do you know the rationale for supporting them in Emscripten with the -s ENVIRONMENT=shell option? I think that option led me to believe that shells were first-class citizens, on the same level as Node.js

Internally, within emscripten, we have used the JS shells historically to test ToT (tip-of-tree) features in V8 and spidermonkey, and don't yet exist in node.

d8 is officially not supported by the v8 team for anything other then internal testing purposes. I imagine spidermonkey have the similar policy.

We should perhaps better document that ENVIRONMENT=shell is really meant for internal testing only, and you should really prefer node in almost all cases.