150412 2015-10-21 13:46:28 -0700 [MIPS] "(-1) % NaN" returns -1 instead of NaN with LLInt 2016-01-18 18:26:43 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified NEW P2 Normal --- 108664 1 guijemont webkit-unassigned annulen fpizlo mcatanzaro oldest_to_newest 1135364 0 guijemont 2015-10-21 13:46:28 -0700 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 1135367 1 263727 guijemont 2015-10-21 13:51:37 -0700 Created attachment 263727 Patch 1151636 2 mcatanzaro 2015-12-30 15:20:17 -0800 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. 1151996 3 mcatanzaro 2016-01-02 10:43:03 -0800 (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. 1152016 4 263727 fpizlo 2016-01-02 12:53:49 -0800 Comment on attachment 263727 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. 1156315 5 annulen 2016-01-17 12:47:50 -0800 Is there a test case which can reliably reproduce this bug? Seems to work fine here with glibc 2.18 without this patch. 1156372 6 annulen 2016-01-18 02:38:18 -0800 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 1156502 7 guijemont 2016-01-18 18:26:43 -0800 (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. 263727 2015-10-21 13:51:37 -0700 2016-01-02 12:53:49 -0800 Patch bug-150412-20151021155105.patch text/plain 1550 guijemont U3VidmVyc2lvbiBSZXZpc2lvbjogMTkxMzk5CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk ZjZjOTI3YTVhMWI2YWJjZjJiYmU3MzlkYWFiMDg1YmFiZTVmMWVjLi41YjFiYzlmNjczZTZmODI2 OTZlY2Q3NDk5OTIwZGRhOGExZTg1YTg0IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNCBAQAorMjAxNS0xMC0yMSAgR3VpbGxhdW1lIEVtb250ICA8Z3VpamVtb250QGlnYWxp YS5jb20+CisKKyAgICAgICAgW01JUFNdICIoLTEpICUgTmFOIiByZXR1cm5zIC0xIGluc3RlYWQg b2YgTmFOIHdpdGggTExJbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE1MDQxMgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgICogcnVudGltZS9Db21tb25TbG93UGF0aHMuY3BwOgorICAgICAgICAoSlNDOjpT TE9XX1BBVEhfREVDTCk6CisgICAgICAgIFBhc3MgdGhlIHJlc3VsdCBvZiBmbW9kKCkgdGhyb3Vn aCBKU0M6OnB1cmlmeU5hTigpLgorCiAyMDE1LTEwLTIxICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNv cmFyb0BhcHBsZS5jb20+CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogQXJyYXkgcHJldmlld3Mg d2l0aCBTeW1ib2wgb2JqZWN0cyBoYXZlIHRvbyBmZXcgcHJldmlldyB2YWx1ZXMKZGlmZiAtLWdp dCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0NvbW1vblNsb3dQYXRocy5jcHAgYi9T b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9Db21tb25TbG93UGF0aHMuY3BwCmluZGV4IDM0 ZjY0OWEwMzQ4ZjkzMzRlMTUyZDA3OTNmNjIwNGM1MTVlNTAyZmIuLjNiNjY5NGQ0NWIzYTNkMjk4 M2I1ZGU3ODdiNmM5MzM4MDU3Y2Q5YWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9ydW50aW1lL0NvbW1vblNsb3dQYXRocy5jcHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3Jl L3J1bnRpbWUvQ29tbW9uU2xvd1BhdGhzLmNwcApAQCAtNDAyLDcgKzQwMiwxMyBAQCBTTE9XX1BB VEhfREVDTChzbG93X3BhdGhfbW9kKQogICAgIEJFR0lOKCk7CiAgICAgZG91YmxlIGEgPSBPUF9D KDIpLmpzVmFsdWUoKS50b051bWJlcihleGVjKTsKICAgICBkb3VibGUgYiA9IE9QX0MoMykuanNW YWx1ZSgpLnRvTnVtYmVyKGV4ZWMpOworI2lmIENQVShNSVBTKQorICAgIC8vIFdoZW4gcGFzc2Vk IGEgTmFOIGFzIHBhcmFtZXRlciwgZm1vZCgpIHRlbmRzIHRvIHJldHVybiBhbiBpbXB1cmUgTmFO IG9uCisgICAgLy8gTUlQUyAoYm90aCB3aXRoIHVjbGliYyBhbmQgZ2xpYmMpLgorICAgIFJFVFVS Tihqc051bWJlcihwdXJpZnlOYU4oZm1vZChhLCBiKSkpKTsKKyNlbHNlCiAgICAgUkVUVVJOKGpz TnVtYmVyKGZtb2QoYSwgYikpKTsKKyNlbmRpZgogfQogCiBTTE9XX1BBVEhfREVDTChzbG93X3Bh dGhfbHNoaWZ0KQo=