Implementation-defined encoding of NaN values in JS engines

[tryone144]

Related to the changes in 8277671 (#749), assertions that test for bit specific values of OTHER_NAN (either Double or Float) depend on potentially implementation specific handling of NaN 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 DataViews (and typed arrays), while SpiderMonkey (in firefox) does not and canonicalizes NaN values when reading from a DataView (and typed arrays)¹.
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 or int -> float -> int conversion which is required by the doubleToRawLongBits() and floatToRawIntBits() methods.

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:

• Float32 to Float64 (resp. Float32 to Float64) typed array constructor needs to canonicalize NaNs
• Flag code paths tainted by unexpected NaNs to help eliminate canonicalizing floats loaded from typed arrays.
• consider canonicalizing nans passed to the JSAPI
• ensure that typed arrays cannot produce non-canonical nans
  Their stance on normalizing doubles (and NaNs) is linked to internal double-boxing which uses NaN doubles as object pointers.

Footnotes

1. See TypedArrayObject.cpp and DataViewObject.cpp in SpiderMonkey. ↩

[konsoletyper]

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?

[tryone144]

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 longs to doubles and back that would overlap with TeaVM's target audience.