-O0 + js_library + int to boolean = error

[mighty1231]

Version of emscripten/emsdk:

docker image with emscripten/emsdk:3.1.74

file test.js

addToLibrary({
  my_js_1: function() {
    return 1;
  },
  my_js_2: function() {
    return 2;
  },
});

file test.cc

#include <emscripten/console.h>

extern "C" {
  extern bool my_js_1(void);
  extern bool my_js_2(void);
}

int main() {
  if (my_js_1()) {
    emscripten_out("my_js_1 is truthy");
  } else {
    emscripten_out("my_js_1 is falsy");
  }

  if (my_js_2()) {
    emscripten_out("my_js_2 is truthy");
  } else {
    emscripten_out("my_js_2 is falsy");
  }

  return 0;
}

command

$ em++ --js-library test.js -o build/O0.js -O0 test.cc
$ em++ --js-library test.js -o build/O1.js -O1 test.cc

What I expected

$ node build/O0.js
my_js_1 is truthy
my_js_2 is truthy

$ node build/O1.js
my_js_1 is truthy
my_js_2 is truthy

But the result for actual run is

$ node build/O0.js
my_js_1 is truthy
my_js_2 is falsy // <--- 😲

$ node build/O1.js
my_js_1 is truthy
my_js_2 is truthy

-O2, -O3 work same as -O1

Severity: flakiness on wasm worker + -O0 option

emscripten/src/library_wasm_worker.js

Line 239 in 1092ec3

emscripten_current_thread_is_wasm_worker: () => {

  emscripten_current_thread_is_wasm_worker: () => {
#if WASM_WORKERS
    return ENVIRONMENT_IS_WASM_WORKER; // <-- returns number 😭
#else
    // implicit return 0;
#endif
  },

[sbc100]

I think this is undefined behaviour in C/C++ due the valid values for bool type. I think that the compiler is within its rights to assume that bool only has two possible values (0 and 1) when you return 2 from a function that is supposed to return bool I believe that is undefined behaviour.

The C/C++ compiler (if it chose too) could just compare with 1 and then assume it is false if that check failed. This would make 2 (and all other values) appear as 0.

You can reproduce in native clang too without emscripten:

$ cat main.c 
#include <stdbool.h>
#include <stdio.h>

bool my_js_1(void);
bool my_js_2(void);

int main() {
  printf("%d\n", my_js_1());
  printf("%d\n", my_js_2());
  return 0;
}
$ cat other.c 
int my_js_1() {
  return 1;
}
int my_js_2() {
  return 2;
}
$ clang main.c other.c
$ ./a.out
1
0

[sbc100]

Are you seeing an issue with emscripten_current_thread_is_wasm_worker? ENVIRONMENT_IS_WASM_WORKER should always be 1 or 0 it shouldn't be a problem there

[oleg-derevenetz]

Are you seeing an issue with emscripten_current_thread_is_wasm_worker? ENVIRONMENT_IS_WASM_WORKER should always be 1 or 0 it shouldn't be a problem there

In C or C++, if you call a function in your code with a prototype like bool foo(), but somehow link it with the real library function with signature int foo(), there is an undefined behavior, even if int foo() returns strictly 1 or 0. I wonder if the "JS bridge" always works correctly if the JS function returns an object of an integer type instead of the boolean type?

[sbc100]

Sure, I'm playing tricks with the linker in my example above and that is another kind of undefined behaviour, but the undefined behaviour that is the root of the problem at hand is that bool can only hold 0 or 1.

[sbc100]

I wonder if the "JS bridge" always works correctly if the JS function returns an object of an integer type instead of the boolean type?

From the JS bridge POV the int and bool types are both identical. They both lower to the i32 wasm type.

Its true that returning a boolean value (JS will convert the boolean to an i32 before it returns to wasm) would help prevent this kind of error, returning a value of 1 or 0 also works just fine.

[mighty1231]

Are you seeing an issue with emscripten_current_thread_is_wasm_worker? ENVIRONMENT_IS_WASM_WORKER should always be 1 or 0 it shouldn't be a problem there

you may create two wasm workers, then the value is defined with Module['$ww']. It's value be given with the wasm worker id that increases from 1. 1, 2, 3, ...

[sbc100]

Ah, yes that does indeed look like a bug. We should probably changed: ENVIRONMENT_IS_WASM_WORKER = !!Module['$ww'].

Does that mean that emscripten_current_thread_is_wasm_worker has been returning values greater than 1 for all workers but the very first? I wonder how this has now caused tests to break!

[sbc100]

Thanks for the report. I've got a fix out in #23484. I actually think I may have broken this myself buy change EM_BOOL to use the C bool type: #22157. I think prior to that the C compiler would have seen the return value as an int and generated different code.

[mighty1231]

This would make 2 (and all other values) appear as 0.

Actually, 1, 3, 5, 7, ... gives truthy and 2, 4, 6, 8, ... gives falsy. It seems it depends on the last bit of int, with whatever settings - clang or emscripten+js.

I wonder how this has now caused tests to break!

I have found the issue with my personal project, using assert(emscripten_current_thread_is_wasm_worker()). This issue may not related to any predefined tests from emscripten. Thanks for fix.