Bug 150412 - [MIPS] "(-1) % NaN" returns -1 instead of NaN with LLInt
Summary: [MIPS] "(-1) % NaN" returns -1 instead of NaN with LLInt
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 108664
  Show dependency treegraph
 
Reported: 2015-10-21 13:46 PDT by Guillaume Emont
Modified: 2016-01-18 18:26 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.51 KB, patch)
2015-10-21 13:51 PDT, Guillaume Emont
fpizlo: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Guillaume Emont 2015-10-21 13:46:28 PDT
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
Comment 1 Guillaume Emont 2015-10-21 13:51:37 PDT
Created attachment 263727 [details]
Patch
Comment 2 Michael Catanzaro 2015-12-30 15:20:17 PST
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.
Comment 3 Michael Catanzaro 2016-01-02 10:43:03 PST
(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.
Comment 4 Filip Pizlo 2016-01-02 12:53:49 PST
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.
Comment 5 Konstantin Tokarev 2016-01-17 12:47:50 PST
Is there a test case which can reliably reproduce this bug? Seems to work fine here with glibc 2.18 without this patch.
Comment 6 Konstantin Tokarev 2016-01-18 02:38:18 PST
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
Comment 7 Guillaume Emont 2016-01-18 18:26:43 PST
(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.