[MIPS] "(-1) % NaN" returns -1 instead of NaN with LLInt

• Component: JavaScriptCore
• Status: NEW ---

[Guillaume Emont]

The spec[1] says it should return NaN. It doesn't because fmod() on MIPS returns an impure NaN.

[1]http://www.ecma-international.org/ecma-262/6.0/#sec-applying-the-mod-operator

[Guillaume Emont]

Created attachment 263727 [details]
Patch

[Michael Catanzaro]

My preference is to approve this patch, since it's such a small #ifdef.

But I will defer for a few days to Filip, as he wants to drop support for MIPS; he might not want to add this since it is a cross-platform file that otherwise has no arch-specific #ifdefs, and this smells like a workaround for a bug in glibc and ulibc.

[Michael Catanzaro]

(In reply to comment #2)
> But I will defer for a few days to Filip, as he wants to drop support for
> MIPS; he might not want to add this since it is a cross-platform file that
> otherwise has no arch-specific #ifdefs, and this smells like a workaround
> for a bug in glibc and ulibc.

Just please wait a week before committing.

[Filip Pizlo]

Comment on attachment 263727 [details]
Patch

I think that this is the wrong approach. You should create a new header (JSMathExtras.h maybe) that provides an abstraction for fmod, and probably other things in the future. Then inside that abstraction you can do the purifyNaN based on some condition, like maybe a global switch like MATH_PURIFIES_NAN, which can be false on MIPS. This is a better approach because there will be other callers of fmod and there will be other math functions that have the same issue.

[Konstantin Tokarev]

Is there a test case which can reliably reproduce this bug? Seems to work fine here with glibc 2.18 without this patch.

[Konstantin Tokarev]

By chance, isn't it softfp-only issue? In your similar patch[1] you mention sotfp, and I'm testiong on hardfp.

https://github.com/Metrological/WebKitForWayland/commit/bcb5650161a0e93db180663d909b2c10f5c81c12

[Guillaume Emont]

(In reply to comment #6)
> By chance, isn't it softfp-only issue? In your similar patch[1] you mention
> sotfp, and I'm testiong on hardfp.
> 
> https://github.com/Metrological/WebKitForWayland/commit/
> bcb5650161a0e93db180663d909b2c10f5c81c12

Yes, I don't have a softfp build right now to test the theory, but I think you are right and that I encountered the issue when using glibc's softfp implementation. I will try to cook a patch following Filip's recommendation, though it's not the highest priority for me right now as I switched to a hardfp build.