34170 2010-01-26 09:28:18 -0800 [Qt] Javascript undefined > 0 returns true on Symbian 2010-02-10 03:16:28 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) S60 Emulator S60 3rd edition RESOLVED FIXED Qt P2 Normal --- 27065 1 koshuin webkit-unassigned commit-queue hausmann ivan.de.marino kent.hansen laszlo.gombos oldest_to_newest 184612 0 47416 koshuin 2010-01-26 09:28:18 -0800 Created attachment 47416 test case undefined > 0 returns true on Symbian while on other enviroments it returns false. another failing comparison is undefined == false -> false Bug is originated from http://bugreports.qt.nokia.com/browse/QTBUG-7423 . 184783 1 laszlo.gombos 2010-01-26 15:10:37 -0800 Can this be still reproduced on QtWebKit 4.6.x trunk - just to make sure that this is not another reincarnation of https://bugs.webkit.org/show_bug.cgi?id=31773 (which was not fixed in QtWebkit 4.6.0 final) ? 184989 2 koshuin 2010-01-27 00:58:11 -0800 (In reply to comment #1) > Can this be still reproduced on QtWebKit 4.6.x trunk - just to make sure that > this is not another reincarnation of > https://bugs.webkit.org/show_bug.cgi?id=31773 (which was not fixed in QtWebkit > 4.6.0 final) ? I had similar thougths that this might be NaN issue. Alas, the fix is in the build that I used and I can still reproduce this. 185022 3 hausmann 2010-01-27 03:51:31 -0800 Kent, any idea of this affects QtScript, too? 185031 4 kent.hansen 2010-01-27 04:34:05 -0800 (In reply to comment #3) > Kent, any idea of this affects QtScript, too? Yes. The comparison should be semantically equivalent to NaN > 0, since the > operator will try to convert its operands to numbers if they aren't already --> ToNumber(undefined) -> NaN. A quick (in)sanity check would be to see if replacing undefined by NaN in the test case gives the same result. It looks like this is the same as what's causing http://bugreports.qt.nokia.com/browse/QTBUG-4621, then. For undefined, JSValue::getPrimitiveNumber() will get here: ASSERT(isUndefined()); number = nonInlineNaN(); value = *this; return true; } From JSValue.cpp: NEVER_INLINE double nonInlineNaN() { return std::numeric_limits<double>::quiet_NaN(); } Again, see Janne's comment @ http://bugreports.qt.nokia.com/browse/QTBUG-4621. 185081 5 koshuin 2010-01-27 06:58:47 -0800 insanity check Number.NaN > 0 returns true. Looks like we have a winner. 185087 6 kent.hansen 2010-01-27 07:15:50 -0800 (In reply to comment #5) > insanity check Number.NaN > 0 returns true. Looks like we have a winner. So did the Open C guys respond regarding numeric_limits<double>::quiet_NaN() being buggy? Here's a quick work-around for Qt you might want to try in JSValue::nonInlineNaN: #if PLATFORM(QT) return qQNaN(); #endif qQNaN and friends are declared in src/corelib/global/qnumeric.h. I wonder why JSValue is the only place that depends on numeric_limits<double>::quiet_NaN? There's code near the top of JSCell.cpp that constructs NaN and Infinity from raw bits if necessary. It's basically what the qnumeric implementation does as well (see qnumeric_p.h). Maybe the robust fix would be to make the distinction between quiet NaN and signaling NaN in that code, and change JSValue to use that constant. 185418 7 koshuin 2010-01-28 04:04:20 -0800 (In reply to comment #6) > So did the Open C guys respond regarding numeric_limits<double>::quiet_NaN() > being buggy? Long story short. Open C has fixed their part for their next release (1.7), but there is issue with RVCT compiler that will still make this not to work. "The problem comes from the fact that the following code is not equal in results: double dnan = std::numeric_limits<double>::quiet_NaN(); int K = 13; 1. if(!(dnan <= K)) is evaluated to false case; bool temp = (dnan <= K); if(!temp) is evaluated to true case." Above is happening if -02 optimatisation is used. with -O0 both cases evaluate the same way. So unfortunately we need to do a workaround until RVCT is getting fixed. 186390 8 47838 koshuin 2010-02-01 06:39:43 -0800 Created attachment 47838 proposed fix for jsvalue.cpp Fix affects all Qt platforms. alternatively this could be flagged for PLATFORM(QT) && PLATFORM(SYMBIAN). I don't think there are any projects implementing pure Symbian ports of webkit anymore? 186419 9 47838 darin 2010-02-01 07:40:32 -0800 Comment on attachment 47838 proposed fix for jsvalue.cpp > +#if PLATFORM(QT) > +#include <qnumeric.h> > +#endif Includes with ifdefs go into a separate paragraph. Not sorted into the normal includes. The ifdef should definitely be for Symbian rather than for Qt. This patch is all about working around a Symbian bug. And I am also really surprised it's broken! > + return qQNaN(); This seems OK, but ideally we'd work around the Symbian bug without requiring Qt here. Is there any other way to do it? For example, does 0.0 / 0.0 work? Or is there an NAN macro on Symbian? review- but something like this would be fine -- just please consider my comments. 186877 10 ivan.de.marino 2010-02-02 02:36:13 -0800 Hi. Ivan De Marino. I'm the (un)lucky guy that first discovered this bug while using QtWebKit on S60. If anyone is interested I can tell you the story that goes from jQuery.ajaxSetup down to discovering the actual comparison problem. Anyway, let's go to the point. I can add some meat here saying that the problem is probably not necessarily related with RCVT. Why? Because the place were I first discovered the bug was Symbian WINSCW, not the hardware phone (of course it happens there as well). For who doesn't know, WINSCW is the Symbian SDK Simulator: everything is just running as Windows threads within the Simulator (that is the Process containing all of them). Everything there is compiled with x86 compiler, so no RCVT involved anyhow. The only thing I need to double check is which version of OpenC is inside my WINSCW, but being running the latest Qt 4.6.1 that depends on Open C 1.7 (the one supposed to solve the problem), I guess that is out of the picture as "source of problem". Unfortunately I'm not experienced enough of the internals of WebKit to just jump in and build a Symbian specific bugfix. At least for now. 186913 11 koshuin 2010-02-02 04:36:05 -0800 > Anyway, let's go to the point. > I can add some meat here saying that the problem is probably not necessarily > related with RCVT. Why? Because the place were I first discovered the bug was > Symbian WINSCW, not the hardware phone (of course it happens there as well). Yes the Open C bug is reproducable both on HW and emulator. The fix that is to be released in next Open C addresses both platforms, but RVCT has a bug that will affect the results so that it won't work even after the patch for all cases depending on optimisation level applied. Anyways.. I have another attempt coming up shortly after I've doublechecked something :) -> heads back to reading IEEE 754. 186926 12 koshuin 2010-02-02 05:03:27 -0800 > The only thing I need to double check is which version of OpenC is inside my > WINSCW, but being running the latest Qt 4.6.1 that depends on Open C 1.7 (the > one supposed to solve the problem), I guess that is out of the picture as > "source of problem". Open C 1.7 is not released yet. What you have is Open C 1.6 if you don't have internal pre-release from somewhere. 186937 13 ivan.de.marino 2010-02-02 05:40:15 -0800 (In reply to comment #12) > > The only thing I need to double check is which version of OpenC is inside my > > WINSCW, but being running the latest Qt 4.6.1 that depends on Open C 1.7 (the > > one supposed to solve the problem), I guess that is out of the picture as > > "source of problem". > > Open C 1.7 is not released yet. What you have is Open C 1.6 if you don't have > internal pre-release from somewhere. Yeah, sorry. I got confused with version numbers. 186938 14 ivan.de.marino 2010-02-02 05:44:58 -0800 I believe that for the sake of completeness, it's good to have here a link back to the originator Bug on Qt Bug Tracker: http://bugreports.qt.nokia.com/browse/QTBUG-7423. 187304 15 48007 koshuin 2010-02-03 03:05:31 -0800 Created attachment 48007 proposed fix for jsvalue.cpp A new patch using epoc32/include/stdapis/math.h double nanval() - function to get the quietNaN value. nanval() returns correct non-signalling value 0xfff8 as exp and has double type. There is NAN macro but that ends up calling nanvalf() which due to bug would also return double type quietNaN (0xfff8 instead of 0xffc0), but I opted not to use this if in future the bug will be fixed. Fix tested on s60 5.0 emulator and 5800 express music. Why the original std::numeric_limits<double>::quiet_NaN() fails is due to wrongly having big endian set for ARM and WINSCW instead of little endian in _limits.c. so instead of getting _STLP_DOUBLE_QNAN_REP { 0, 0, 0, 0xfff8 } we get _STLP_DOUBLE_QNAN_REP { 0xfff8, 0, 0, 0 }. This is the most visible bug in Open C part. Open C guys said that there was another issue to this as well. The last part is the RVCT optimisation issue which is still unresolved. A task to revert this fix should be made once all fixes in the platform are in. 188158 16 48225 koshuin 2010-02-05 05:28:16 -0800 Created attachment 48225 proposed fix for jsvalue.cpp Changed PLATFROM macro to OS macro. 188595 17 ivan.de.marino 2010-02-08 02:38:28 -0800 Something tells me that we are not going to enjoy a proper fix of ARM compiler for this issue. At least, not where we need it. Let me explain. For who doesn't know, Symbian OS requires a version of RVCT compiler that is quite old: something around 2.x. Current version of ARM navigates around 4.x (or even more, not sure). I managed to contact folks in ARM and I received an answer from Peter Lewin (ARM Technical Support) and he says that: <blockquote> ... This has been reported to us, and the problem has been fixed in the latest patch release of RVCT 4.0 (build 697). http://www.arm.com/support/downloads/info/26545.html </blockquote> So, unless my knowledge of the Symbian compilation requirements is faulty, I believe Symbian is not compatible/compilable with such "advanced" version of the ARM compiler. And, as a consequence, even if the fix arrived, is not going to applicable. BUT hopefully Symbian will get fully compilable by using "only" GCCE. :( 188878 18 48225 laszlo.gombos 2010-02-08 20:37:00 -0800 Comment on attachment 48225 proposed fix for jsvalue.cpp LGTM; r+. 188930 19 48225 commit-queue 2010-02-09 01:51:43 -0800 Comment on attachment 48225 proposed fix for jsvalue.cpp Clearing flags on attachment: 48225 Committed r54539: <http://trac.webkit.org/changeset/54539> 188931 20 commit-queue 2010-02-09 01:51:53 -0800 All reviewed patches have been landed. Closing bug. 189307 21 hausmann 2010-02-10 03:16:28 -0800 (In reply to comment #19) > (From update of attachment 48225 [details]) > Clearing flags on attachment: 48225 > > Committed r54539: <http://trac.webkit.org/changeset/54539> Cherry-picked into qtwebkit-4.6 with commit ffae5e11181a3961193fa21ea405851cad714d4b 47416 2010-01-26 09:28:18 -0800 2010-01-26 09:28:18 -0800 test case testundefined.html text/html 307 koshuin PGh0bWw+DQo8aGVhZD4NCjxzY3JpcHQgdHlwZT0idGV4dC9qYXZhc2NyaXB0Ij4NCglhbGVydCgi dW5kZWZpbmVkID4gMCA9PSAiICsgKHVuZGVmaW5lZCA+IDApKTsgLy8gJ3RydWUnIG9uIFM2MCBR dCBXZWJraXQsIHdoaWxlIHNob3VsZCBiZSAnZmFsc2UnIGFzIG9uIGFueSBvdGhlciBRdC1wbGF0 Zm9ybSBvciBhbnkgb3RoZXIgYnJvd3Nlcg0KCWFsZXJ0KCJudWxsID4gMCA9PSAiICsgKG51bGwg PiAwKSk7IC8vICdmYWxzZScgYXMgZXhwZWN0ZWQgb24gZXZlcnkgUXQtcGxhdGZvcm0NCjwvc2Ny aXB0Pg0KPC9oZWFkPg0KPC9odG1sPg== 47838 2010-02-01 06:39:43 -0800 2010-02-03 03:05:31 -0800 proposed fix for jsvalue.cpp jsvalue_qqnan.diff text/plain 1307 koshuin ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCA4Y2M5Y2UzLi43NTk0NWE0IDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTAtMDItMDEgIEphbm5lIEtvc2tpbmVuICA8amFubmUucC5rb3NraW5lbkBkaWdpYS5j b20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW1F0 XSB1c2UgcVFOYU4gZm9yIFF0IGFzIG5vbklubGluZU5hTgorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzQxNzAKKworICAgICAgICBudW1lcmljX2xpbWl0 czxkb3VibGU+OjpxdWlldF9OYU4gaXMgYnJva2VuIGluIFN5bWJpYW4KKyAgICAgICAgY2F1c2lu ZyBOYU4gdG8gYmUgZXZhbHVhdGVkIGFzIGEgbnVtYmVyLgorCisgICAgICAgICogcnVudGltZS9K U1ZhbHVlLmNwcDoKKyAgICAgICAgKEpTQzo6bm9uSW5saW5lTmFOKToKKwogMjAxMC0wMi0wMSAg T2xpdmVyIEh1bnQgIDxvbGl2ZXJAYXBwbGUuY29tPgogCiAgICAgICAgIFdpbmRvd3MgYnVpbGQg Zml4LCB1cGRhdGUgZXhwb3J0cyBmaWxlIChhZ2FpbikKZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRD b3JlL3J1bnRpbWUvSlNWYWx1ZS5jcHAgYi9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTVmFsdWUu Y3BwCmluZGV4IDY5OWMxY2QuLmJlODYzNTQgMTAwNjQ0Ci0tLSBhL0phdmFTY3JpcHRDb3JlL3J1 bnRpbWUvSlNWYWx1ZS5jcHAKKysrIGIvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU1ZhbHVlLmNw cApAQCAtMzAsNiArMzAsOSBAQAogI2luY2x1ZGUgIkpTRnVuY3Rpb24uaCIKICNpbmNsdWRlICJK U05vdEFuT2JqZWN0LmgiCiAjaW5jbHVkZSAiTnVtYmVyT2JqZWN0LmgiCisjaWYgUExBVEZPUk0o UVQpCisjaW5jbHVkZSA8cW51bWVyaWMuaD4KKyNlbmRpZgogI2luY2x1ZGUgPHd0Zi9NYXRoRXh0 cmFzLmg+CiAjaW5jbHVkZSA8d3RmL1N0cmluZ0V4dHJhcy5oPgogCkBAIC0xNzQsNyArMTc3LDEx IEBAIHVpbnQzMl90IHRvVUludDMyU2xvd0Nhc2UoZG91YmxlIGQsIGJvb2wmIG9rKQogCiBORVZF Ul9JTkxJTkUgZG91YmxlIG5vbklubGluZU5hTigpCiB7CisjaWYgUExBVEZPUk0oUVQpCisgICAg cmV0dXJuIHFRTmFOKCk7CisjZWxzZQogICAgIHJldHVybiBzdGQ6Om51bWVyaWNfbGltaXRzPGRv dWJsZT46OnF1aWV0X05hTigpOworI2VuZGlmCiB9CiAKIH0gLy8gbmFtZXNwYWNlIEpTQwo= 48007 2010-02-03 03:05:31 -0800 2010-02-05 05:28:16 -0800 proposed fix for jsvalue.cpp jsvalue_noninlinenan.diff text/plain 1112 koshuin ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCA4Y2M5Y2UzLi4xYzkxN2JjIDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTAtMDItMDEgIEphbm5lIEtvc2tpbmVuICA8amFubmUucC5rb3NraW5lbkBkaWdpYS5j b20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW1F0 XSB1c2UgbmFudmFsKCkgZm9yIFN5bWJpYW4gYXMgbm9uSW5saW5lTmFOCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zNDE3MAorCisgICAgICAgIG51bWVy aWNfbGltaXRzPGRvdWJsZT46OnF1aWV0X05hTiBpcyBicm9rZW4gaW4gU3ltYmlhbgorICAgICAg ICBjYXVzaW5nIE5hTiB0byBiZSBldmFsdWF0ZWQgYXMgYSBudW1iZXIuCisKKyAgICAgICAgKiBy dW50aW1lL0pTVmFsdWUuY3BwOgorICAgICAgICAoSlNDOjpub25JbmxpbmVOYU4pOgorCiAyMDEw LTAyLTAxICBPbGl2ZXIgSHVudCAgPG9saXZlckBhcHBsZS5jb20+CiAKICAgICAgICAgV2luZG93 cyBidWlsZCBmaXgsIHVwZGF0ZSBleHBvcnRzIGZpbGUgKGFnYWluKQpkaWZmIC0tZ2l0IGEvSmF2 YVNjcmlwdENvcmUvcnVudGltZS9KU1ZhbHVlLmNwcCBiL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUv SlNWYWx1ZS5jcHAKaW5kZXggNjk5YzFjZC4uYmFmYjg1YiAxMDA2NDQKLS0tIGEvSmF2YVNjcmlw dENvcmUvcnVudGltZS9KU1ZhbHVlLmNwcAorKysgYi9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pT VmFsdWUuY3BwCkBAIC0xNzQsNyArMTc0LDExIEBAIHVpbnQzMl90IHRvVUludDMyU2xvd0Nhc2Uo ZG91YmxlIGQsIGJvb2wmIG9rKQogCiBORVZFUl9JTkxJTkUgZG91YmxlIG5vbklubGluZU5hTigp CiB7CisjaWYgUExBVEZPUk0oU1lNQklBTikKKyAgICByZXR1cm4gbmFudmFsKCk7CisjZWxzZQog ICAgIHJldHVybiBzdGQ6Om51bWVyaWNfbGltaXRzPGRvdWJsZT46OnF1aWV0X05hTigpOworI2Vu ZGlmCiB9CiAKIH0gLy8gbmFtZXNwYWNlIEpTQwo= 48225 2010-02-05 05:28:16 -0800 2010-02-09 01:51:43 -0800 proposed fix for jsvalue.cpp jsvalue_noninlinenan.diff text/plain 1106 koshuin ZGlmZiAtLWdpdCBhL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL0phdmFTY3JpcHRDb3JlL0No YW5nZUxvZwppbmRleCA4Y2M5Y2UzLi4xYzkxN2JjIDEwMDY0NAotLS0gYS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTAtMDItMDEgIEphbm5lIEtvc2tpbmVuICA8amFubmUucC5rb3NraW5lbkBkaWdpYS5j b20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgW1F0 XSB1c2UgbmFudmFsKCkgZm9yIFN5bWJpYW4gYXMgbm9uSW5saW5lTmFOCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zNDE3MAorCisgICAgICAgIG51bWVy aWNfbGltaXRzPGRvdWJsZT46OnF1aWV0X05hTiBpcyBicm9rZW4gaW4gU3ltYmlhbgorICAgICAg ICBjYXVzaW5nIE5hTiB0byBiZSBldmFsdWF0ZWQgYXMgYSBudW1iZXIuCisKKyAgICAgICAgKiBy dW50aW1lL0pTVmFsdWUuY3BwOgorICAgICAgICAoSlNDOjpub25JbmxpbmVOYU4pOgorCiAyMDEw LTAyLTAxICBPbGl2ZXIgSHVudCAgPG9saXZlckBhcHBsZS5jb20+CiAKICAgICAgICAgV2luZG93 cyBidWlsZCBmaXgsIHVwZGF0ZSBleHBvcnRzIGZpbGUgKGFnYWluKQpkaWZmIC0tZ2l0IGEvSmF2 YVNjcmlwdENvcmUvcnVudGltZS9KU1ZhbHVlLmNwcCBiL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUv SlNWYWx1ZS5jcHAKaW5kZXggNjk5YzFjZC4uYmFmYjg1YiAxMDA2NDQKLS0tIGEvSmF2YVNjcmlw dENvcmUvcnVudGltZS9KU1ZhbHVlLmNwcAorKysgYi9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pT VmFsdWUuY3BwCkBAIC0xNzQsNyArMTc0LDExIEBAIHVpbnQzMl90IHRvVUludDMyU2xvd0Nhc2Uo ZG91YmxlIGQsIGJvb2wmIG9rKQogCiBORVZFUl9JTkxJTkUgZG91YmxlIG5vbklubGluZU5hTigp CiB7CisjaWYgT1MoU1lNQklBTikKKyAgICByZXR1cm4gbmFudmFsKCk7CisjZWxzZQogICAgIHJl dHVybiBzdGQ6Om51bWVyaWNfbGltaXRzPGRvdWJsZT46OnF1aWV0X05hTigpOworI2VuZGlmCiB9 CiAKIH0gLy8gbmFtZXNwYWNlIEpTQwo=