pthread + simd + fexceptions generates SIMD opcodes incompatible with Node.js in emsdk

[RReverser]

Please include the following in your bug report:

Version of emscripten/emsdk: emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.24 (68a9f990429e0bcfb63b1cde68bad792554350a5) clang version 16.0.0 (https://github.com/llvm/llvm-project 277c382760bf9575cfa2eac73d5ad1db91466d3f) Target: wasm32-unknown-emscripten Thread model: posix InstalledDir: /home/rreverser/emsdk/upstream/bin

Failing command line in full:

I've noticed this due to CMake detecting size of size_t as... 7:

Input

check_type_size("size_t" SIZE_T)

Output

//CHECK_TYPE_SIZE: sizeof(size_t)
SIZE_T:INTERNAL=7

That, in turn, caused all sorts of weird problems.

This is the C file CMake generated for the check:

#include <sys/types.h>
#include <stdint.h>
#include <stddef.h>

#undef KEY
#if defined(__i386)
# define KEY '_','_','i','3','8','6'
#elif defined(__x86_64)
# define KEY '_','_','x','8','6','_','6','4'
#elif defined(__ppc__)
# define KEY '_','_','p','p','c','_','_'
#elif defined(__ppc64__)
# define KEY '_','_','p','p','c','6','4','_','_'
#endif

#define SIZE (sizeof(size_t))
char info_size[] =  {'I', 'N', 'F', 'O', ':', 's','i','z','e','[',
  ('0' + ((SIZE / 10000)%10)),
  ('0' + ((SIZE / 1000)%10)),
  ('0' + ((SIZE / 100)%10)),
  ('0' + ((SIZE / 10)%10)),
  ('0' +  (SIZE    % 10)),
  ']',
#ifdef KEY
  ' ','k','e','y','[', KEY, ']',
#endif
  '\0'};

#ifdef __CLASSIC_C__
int main(argc, argv) int argc; char *argv[];
#else
int main(int argc, char *argv[])
#endif
{
  int require = 0;
  require += info_size[argc];
  (void)argv;
  return SIZE;
}

When building it with the following reduced set of flags, and then running with the Node.js v14.18.2 included in latest emsdk, this is what I'm getting:

$ emcc -O3 -pthread -fexceptions -msimd128 SIZE_T.c
$ node --experimental-wasm-threads --experimental-wasm-bulk-memory --experimental-wasm-simd a.out.js
failed to asynchronously prepare wasm: CompileError: WebAssembly.instantiate(): Compiling function #50:"n" failed: invalid simd opcode @+16117
Aborted(CompileError: WebAssembly.instantiate(): Compiling function #50:"n" failed: invalid simd opcode @+16117)
[...long output...]
$ echo $?
7 # this is where CMake takes what it assumes to be `sizeof(size_t)`

There are couple of interesting things going on:

1. Removing any of those 3 emcc flags fixes the issue, so there's some weird combo issue going on. (replacing -fexceptions with -fwasm-exceptions works too, but not something I can do here)
2. Running with newer Node.js I have on my system works fine and returns 4 as expected, so this might be due to old Node.js having old SIMD opcodes. In that case, I'd argue it's time to upgrade Node.js on the emsdk side.
3. Apparently the upstream CMake performs size check differently, in a way where exit code should never silently propagate as a valid result. It's worth looking into improving error handling here too.

[kleisauke]

The Wasm SIMD opcodes were finalized in V8 9.1.54, which corresponds to Node.js 16.4.0. So, indeed, emsdk needs to update its Node version (https://github.com/emscripten-core/emsdk/issues/1064).

After PR https://github.com/emscripten-core/emscripten/pull/18070, Emscripten's test suite only adds those --experimental-wasm-* flags for older Node versions, so that's no longer a blocker. I would also argue for setting NODEJS_CATCH_REJECTION to 0 by default, when the bundled Node version is updated (to avoid issues like #17228).

FWIW, I noticed the commit linked above, another workaround would be to do something similar to this in the Dockerfile: https://github.com/emscripten-core/emscripten/blob/00daf403cd09467c6dab841b873710bb878535b2/.circleci/config.yml#L488-L492

[RReverser]

FWIW, I noticed the commit linked above, another workaround would be to do something similar to this in the Dockerfile:

Might as well install Node from NodeSources apt :) Either way, it's only used for CMake checks so which workaround to use doesn't matter too much, but the issues above still need to be fixed on Emscripten side for other users.

[dschuff]

It would be nice to update node in emsdk. See https://github.com/emscripten-core/emsdk/pull/877 and https://github.com/emscripten-core/emsdk/issues/947 for some context on why we haven't recently. I wonder if those reasons still apply now.

[RReverser]

Looking at the first link, @juj said:

Currently it looks like we need Windows 7 support until April 2022. After that I expect we won't need it anymore, since our LTS versions will rotate, and the next LTS won't support Windows 7 anymore.

Sounds like it should be safe to upgrade by now?

I still wonder why pthread and exception flags matter for even this simple C program that uses neither of those features, but that's just curiosity and doesn't matter much since the Node.js fixes it anyway.

What about part (3) of my issue, about not using exit code for Cmake check, should I extract that into a separate issue? Having sizeof SIZE_T detected as 7 was preeetty weird to debug 😅

[sbc100]

Yes I think the exit code issue might be worth digger deeper into. I think the separate issue would make sense.

I imagine that what we want to do is make this kind of failure look like a crash to the host OS, rather than an exit(7)?

[abram]

What about part (3) of my issue, about not using exit code for Cmake check, should I extract that into a separate issue? Having sizeof SIZE_T detected as 7 was preeetty weird to debug 😅

+1 to this. I've run into this issue twice recently and it was painful (see https://github.com/emscripten-core/emscripten/discussions/17811). I don't see an issue for this yet, are you still planning to file one @RReverser?

[RReverser]

@abram Sorry, no, didn't get around to it. Created one now: https://github.com/emscripten-core/emscripten/issues/18278