Open tryone144 opened 3 months ago
I don't think there's a solution here. The only potential solution is to use WebAssembly. Though this would complicate the whole thing for users: instead of having single self-sufficient file, they'll end up with some non-trivial setup, including some HTTP server (WebAssembly does not work from local file system) and which is different for browser, Node.js, etc. Also, I have a use-case where I run TeaVM-generated JS in Rhino and it works just fine.
Having doubleToRawLongBits
and floatToRawIntBits
not implemented at all is also a bad solution. I believe, the distinction is so subtle and there are so few chances to get real bug in real software because of this issue, that simply not having these methods unimplemented is a really bad solution.
So I suggest to leave this as is, until there's an update in JS standard which allows to convert without loss of data. Perhaps, add a note to the contributor's documentation that in Firefox some tests may fail. @tryone144 are you agree? Do you have another solution in mind?
I personally think we are in a bad position to require anything. This is more of an issue with how the JavaScript engines are designed and how the specification does not enforce either behavior.
We can't really do anything about this other than to roll our own double
emulation. Which is totally overkill for this feature (not to mention the performance penalty). WebAssembly is only an option when using the WASM backend of TeaVM; otherwise we would have the issues you mentioned. From a performance perspective, switching contexts for double-operations in the runtime will most likely result in reduced performance compared to the current native JS implementation.
So I suggest to leave this as is, until there's an update in JS standard which allows to convert without loss of data. Perhaps, add a note to the contributor's documentation that in Firefox some tests may fail. @tryone144 are you agree? Do you have another solution in mind?
Yes, I totally agree. I don't have a practical workaround. This issue itself was meant to at least document this behavior for the future. Adding a comment to the relevant functions/tests is a good idea.
I can't think of legitimate reasons to convert non-canonical NaN
encodings in long
s to double
s and back that would overlap with TeaVM's target audience.
Related to the changes in https://github.com/konsoletyper/teavm/commit/8277671376b7745f9f33a1dc1206cbb8d66a0cb2 (https://github.com/konsoletyper/teavm/pull/749), assertions that test for bit specific values of
OTHER_NAN
(either Double or Float) depend on potentially implementation specific handling ofNaN
values in the underlying JS engine (and fail in firefox).Notably, V8 (in chrome) appears to keep the bit-pattern of different NaN values intact when writing to and reading from
DataView
s (and typed arrays), while SpiderMonkey (in firefox) does not and canonicalizes NaN values when reading from aDataView
(and typed arrays)[^1]. The ECMAScript specification is not worded explicitly enough for either behaviour to be against the spec (while reading). The write case explicitly mentions that an implementation must keep the encoding of distinguishable NaN values in a way that they stay distinguishable. It appears there is no consensus on what the correct behaviour is (from a performance and security perspective).Given the current state, I don't think we can rely on the internal bit-encoding of NaN values to survive the
long -> double -> long
orint -> float -> int
conversion which is required by thedoubleToRawLongBits()
andfloatToRawIntBits()
methods.[^1]: See TypedArrayObject.cpp and DataViewObject.cpp in SpiderMonkey.
Edit: JavaScriptCore in Webkit (or Nitro as it is called in Safari) seem to follow SpiderMonkey in canonicalizing
NaN
values.Potentially related discussions in the spidermonkey case:
NaN
s) is linked to internal double-boxing which usesNaN
doubles as object pointers.