Bug 42363

Summary: [Qt] QScriptValue::equals asserts
Product: WebKit Reporter: Jędrzej Nowacki <jedrzej.nowacki>
Component: JavaScriptCoreAssignee: Jędrzej Nowacki <jedrzej.nowacki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jedrzej.nowacki, kenneth, kent.hansen
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 31863    
Attachments:
Description Flags
Fix v1
none
Fix v2 none

Jędrzej Nowacki
Reported 2010-07-15 06:12:24 PDT
There is an assert in the QScriptValue::equals benchmarks. #0 0xb7fe2424 in __kernel_vsyscall () #1 0xb69a7751 in *__GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #2 0xb69aab82 in *__GI_abort () at abort.c:92 #3 0xb6ca946b in qt_message_output (msgType=QtFatalMsg, buf=0x812ed88 "ASSERT: \"isStringBased()\" in file /home/nierob/dev/WebKit-script/JavaScriptCore/qt/api/qscriptvalue_p.h, line 705") at global/qglobal.cpp:2271 #4 0xb6ca962a in qt_message (msgType=QtFatalMsg, msg=0xb6e589c4 "ASSERT: \"%s\" in file %s, line %d", ap=0xbfffe6f4 ",h\344\267`f\344\267\301\002") at global/qglobal.cpp:2317 #5 0xb6ca9a47 in qFatal (msg=0xb6e589c4 "ASSERT: \"%s\" in file %s, line %d") at global/qglobal.cpp:2500 #6 0xb6ca8fcc in qt_assert (assertion=0xb7e4682c "isStringBased()", file=0xb7e46660 "/home/nierob/dev/WebKit-script/JavaScriptCore/qt/api/qscriptvalue_p.h", line=705) at global/qglobal.cpp:2016 #7 0xb7c8bb04 in QScriptValuePrivate::equals (this=0x80f6fe0, other=0x80f6fe0) at /home/nierob/dev/WebKit-script/JavaScriptCore/qt/api/qscriptvalue_p.h:705 #8 0xb7c89843 in QScriptValue::equals (this=0xbfffe78c, other=...) at /home/nierob/dev/WebKit-script/JavaScriptCore/qt/api/qscriptvalue.cpp:598 #9 0x0804d9b3 in tst_QScriptValue::equals (this=0xbffff16c) at /home/nierob/dev/WebKit-script/JavaScriptCore/qt/benchmarks/qscriptvalue/tst_qscriptvalue.cpp:407 #10 0x0804e123 in tst_QScriptValue::qt_metacall (this=0xbffff16c, _c=QMetaObject::InvokeMetaMethod, _id=43, _a=0xbfffe844) at ./tst_qscriptvalue.moc:179 #11 0xb6ddc6e4 in QMetaObject::metacall (object=0xbffff16c, cl=QMetaObject::InvokeMetaMethod, idx=47, argv=0xbfffe844) at kernel/qmetaobject.cpp:237 #12 0xb6dded79 in QMetaMethod::invoke (this=0xbfffebb8, object=0xbffff16c, connectionType=Qt::DirectConnection, returnValue=..., val0=..., val1=..., val2=..., val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at kernel/qmetaobject.cpp:1581 #13 0xb6dde2af in QMetaObject::invokeMethod (obj=0xbffff16c, member=0x810d978 "equals", type=Qt::DirectConnection, ret=..., val0=..., val1=..., val2=..., val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at kernel/qmetaobject.cpp:1148 #14 0xb7c3ab7c in QMetaObject::invokeMethod (obj=0xbffff16c, member=0x810d978 "equals", type=Qt::DirectConnection, val0=..., val1=..., val2=..., val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at ../../include/QtCore/../../src/corelib/kernel/qobjectdefs.h:408 #15 0xb7c3846e in qInvokeTestMethodDataEntry (slot=0x810d978 "equals") at qtestcase.cpp:1244 #16 0xb7c389ca in qInvokeTestMethod (slotName=0x804f733 "equals()", data=0x0) at qtestcase.cpp:1352 #17 0xb7c38fad in qInvokeTestMethods (testObject=0xbffff16c) at qtestcase.cpp:1497 #18 0xb7c39727 in QTest::qExec (testObject=0xbffff16c, argc=2, argv=0xbffff244) at qtestcase.cpp:1716 #19 0x0804dd4a in main (argc=2, argv=0xbffff244) at /home/nierob/dev/WebKit-script/JavaScriptCore/qt/benchmarks/qscriptvalue/tst_qscriptvalue.cpp:441
Attachments
Fix v1 (3.63 KB, patch)
2010-07-15 06:56 PDT, Jędrzej Nowacki
no flags
Fix v2 (3.01 KB, patch)
2010-07-23 06:25 PDT, Jędrzej Nowacki
no flags
Jędrzej Nowacki
Comment 1 2010-07-15 06:56:26 PDT
Created attachment 61651 [details] Fix v1 I don't provide an additional tests, the crash is in our benchmarks so in a teory it is tested. I think we should test the case using existing tests, so I created bug 42366.
Kent Hansen
Comment 2 2010-07-16 08:10:33 PDT
Comment on attachment 61651 [details] Fix v1 > +inline bool JSCompare(qsreal a, qsreal b) > +{ > + return a == b > + || (qIsInf(a) && qIsInf(b)) > + || (qIsNaN(a) && qIsNaN(b)); > +} The Inf and NaN parts aren't correct. NaN should not be equal to NaN. -Inf should not be equal to +Inf. So this helper function isn't needed, == can be used inplace. Instead of Q_ASSERT(false) in the default case, I'd prefer a Q_ASSERT_X(false, "equals()", Not all states are included in the previous switch statement."), like existing switches do. Otherwise it looks good.
Jędrzej Nowacki
Comment 3 2010-07-23 06:25:39 PDT
Created attachment 62421 [details] Fix v2 New version: - JSCompare function was removed - The assert was replaced by a more verbose one.
WebKit Commit Bot
Comment 4 2010-07-23 09:11:03 PDT
Comment on attachment 62421 [details] Fix v2 Clearing flags on attachment: 62421 Committed r63980: <http://trac.webkit.org/changeset/63980>
WebKit Commit Bot
Comment 5 2010-07-23 09:11:08 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.