Incorrect trap when casting big double values to int64_t

[manxorist]

The double value converted to int64_t in the following example fits into its destination type and must not invoke undefined behaviour or trap.

$ emcc --version
emcc (Emscripten gcc/clang-like replacement) 1.38.8 (commit 97564e0bbf82fe99c3021176ac3835cdbc2da687)
Copyright (C) 2014 the Emscripten authors (see AUTHORS.txt)
This is free and open source software under the MIT license.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ nodejs --version
v8.10.0
$ cat test.c
// emcc -Os -s WASM=1 -s BINARYEN_METHOD='native-wasm,asmjs' -s BINARYEN_ASYNC_COMPILATION=0 -Wall -Wextra test.c && nodejs a.out.js
#include <assert.h>
signed long long foo(double x) {
    return (signed long long)(x * 4294967296.0);
}
int main() {
    volatile double x = 9.89365;
    volatile signed long long y = foo(x);
    assert(y == 42492903188ll);
    return (int)y;
}
$ emcc -Os -s WASM=1 -s BINARYEN_METHOD='native-wasm,asmjs' -s BINARYEN_ASYNC_COMPILATION=0 -Wall -Wextra test.c
$ nodejs a.out.js
exception thrown: RuntimeError: integer result unrepresentable,RuntimeError: integer result unrepresentable
[...]

Note that I am mainly a C/C++ developer and am thus unsure whether this is a bug in emscripten, or in binaryen, or in nodejs, or in some other component involved here.

BINARYEN_METHOD='native-wasm' or BINARYEN_METHOD='asmjs' do not crash, it is just the method where both variants are generated that crashes for me.

A conforming C/C++ compiler must not generate crashing code in this case. The behaviour in this case is precisely defined in both the C and the C++ standards. Directing towards BINARYEN_TRAP_MODE as done in issue #6352 is IMHO not a viable solution. BINARYEN_TRAP_MODE='js' however fixes this particular issue for me, thus I highly suggest making the behaviour of BINARYEN_TRAP_MODE='js' the default for this particular case (floating point to 64bit integer conversions with BINARYEN_METHOD='native-wasm,asmjs') because the other trap modes are in this case very obviously wrong and lead to silent bad code generation. Performance implications are irrelevant because the supposedly fast version is just incorrect. See C11:

6.3.1.4 Real floating and integer When a finite value of real floating type is converted to an integer type other than _Bool, the fractional part is discarded (i.e., the value is truncated toward zero). If the value of the integral part cannot be represented by the integer type, the behavior is undefined.

Related question: I understand why native-wasm method is unaffected (WebAssembly has native support for 64bit integers), however I do not understand why asmjs method is also unaffected. Can someone explain this?

[kripken]

I agree this is a bug. I think what's going on is that in asm.js there are no traps on stuff like this anyhow; in wasm-only mode we lower directly to wasm native i64 operations; in "compromise mode", wasm+asm.js, we must emit code compatible with asm.js, so we emit valid asm.js and compile that to wasm. And to do the math operation here, that code path ends up doing an operation that traps in wasm - we just are not careful enough there, right now.

For correctness it would be good to fix this. However, it's probably low priority as the "compromise mode" is not recommended for other reasons: http://kripken.github.io/emscripten-site/docs/compiling/WebAssembly.html#codegen-effects (those builds are mainly useful for internal debugging)

[manxorist]

Thanks for the explanation about compromise mode. I'll probably explicitly document the wasm+asm,js compromise mode as unsupported for my project then.

[stale[bot]]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.