---- (function() { var b=!2; for(i = 0; i< 1e5; i++) { b[b=this]; for (var i = 0; i < 1e5; i++) { if (a = b*3) { } } } })() ---- This script crashes. Assertion fail on DFGFrozenValue.h:53 -> RELEASE_ASSERT(!value || !value.isCell()); found with afl fuzz * thread #2: tid = 0x535a38, 0x000000010054372e JavaScriptCore`WTFCrash + 62 at Assertions.cpp:321, name = 'DFG Worklist Worker Thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) frame #0: 0x000000010054372e JavaScriptCore`WTFCrash + 62 at Assertions.cpp:321 318 globalHook(); 319 320 WTFReportBacktrace(); -> 321 *(int *)(uintptr_t)0xbbadbeef = 0; 322 // More reliable, but doesn't say BBADBEEF. 323 #if COMPILER(CLANG) 324 __builtin_trap(); (lldb) bt * thread #2: tid = 0x535a38, 0x000000010054372e JavaScriptCore`WTFCrash + 62 at Assertions.cpp:321, name = 'DFG Worklist Worker Thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) * frame #0: 0x000000010054372e JavaScriptCore`WTFCrash + 62 at Assertions.cpp:321 frame #1: 0x00000001000d1462 JavaScriptCore`JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node*) [inlined] JSC::DFG::FrozenValue::FrozenValue(JSC::JSValue) + 1522 at DFGFrozenValue.h:53 frame #2: 0x00000001000d1454 JavaScriptCore`JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node*) [inlined] JSC::DFG::FrozenValue::FrozenValue(JSC::JSValue) at DFGFrozenValue.h:54 frame #3: 0x00000001000d1454 JavaScriptCore`JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(this=0x0000000105609260, clobberLimit=<unavailable>, node=0x00000001043e2d00) + 1508 at DFGAbstractInterpreterInlines.h:311 frame #4: 0x00000001000cfddd JavaScriptCore`JSC::DFG::CFAPhase::performBlockCFA(JSC::DFG::BasicBlock*) [inlined] JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::execute(this=<unavailable>) + 637 at DFGAbstractInterpreterInlines.h:2048 frame #5: 0x00000001000cfd90 JavaScriptCore`JSC::DFG::CFAPhase::performBlockCFA(this=0x0000000105608fc8, block=0x0000000102041140) + 560 at DFGCFAPhase.cpp:125 frame #6: 0x00000001000cfa05 JavaScriptCore`JSC::DFG::CFAPhase::run() [inlined] JSC::DFG::CFAPhase::performForwardCFA(this=0x0000000105608fc8) + 93 at DFGCFAPhase.cpp:152 frame #7: 0x00000001000cf9a8 JavaScriptCore`JSC::DFG::CFAPhase::run(this=0x0000000105608fc8) + 120 at DFGCFAPhase.cpp:79 frame #8: 0x00000001000cf879 JavaScriptCore`bool JSC::DFG::runPhase<JSC::DFG::CFAPhase>(JSC::DFG::Graph&) [inlined] bool JSC::DFG::runAndLog<JSC::DFG::CFAPhase>(phase=0x0000000105609750) + 8 at DFGPhase.h:77 frame #9: 0x00000001000cf871 JavaScriptCore`bool JSC::DFG::runPhase<JSC::DFG::CFAPhase>(graph=<unavailable>) + 33 at DFGPhase.h:87 frame #10: 0x00000001000cf849 JavaScriptCore`JSC::DFG::performCFA(graph=<unavailable>) + 9 at DFGCFAPhase.cpp:168 frame #11: 0x000000010016b6cf JavaScriptCore`JSC::DFG::Plan::compileInThreadImpl(this=0x00000001037fdc00, longLivedState=<unavailable>) + 719 at DFGPlan.cpp:263 frame #12: 0x000000010016b19d JavaScriptCore`JSC::DFG::Plan::compileInThread(this=0x00000001037fdc00, longLivedState=0x0000000105609e18, threadData=<unavailable>) + 493 at DFGPlan.cpp:164 frame #13: 0x00000001001f38b2 JavaScriptCore`JSC::DFG::Worklist::runThread(this=0x00000001037e6000, data=0x0000000100d01160) + 546 at DFGWorklist.cpp:358 frame #14: 0x000000010056e283 JavaScriptCore`WTF::threadEntryPoint(void*) [inlined] std::__1::function<void (this=0x00000001005e7340)>::operator()() const + 179 at functional:1755 frame #15: 0x000000010056e279 JavaScriptCore`WTF::threadEntryPoint(contextData=<unavailable>) + 169 at Threading.cpp:58 frame #16: 0x000000010056e76f JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x0000000103ff81c0) + 15 at ThreadingPthreads.cpp:170 frame #17: 0x00007fff8290a268 libsystem_pthread.dylib`_pthread_body + 131 frame #18: 0x00007fff8290a1e5 libsystem_pthread.dylib`_pthread_start + 176 frame #19: 0x00007fff8290841d libsystem_pthread.dylib`thread_start + 13
Yup, I see it. Looking now.
I have a fix.
Created attachment 247103 [details] the patch
Comment on attachment 247103 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=247103&action=review I am a bit confused by the test, but the patch looks correct :) > Source/JavaScriptCore/tests/stress/regress-141883.js:6 > +for (var i = 0; i < n; i++) { Did you intend to reuse "i" here? > Source/JavaScriptCore/tests/stress/regress-141883.js:7 > + if (a = b*3) { Any reason for the if() here?
(In reply to comment #4) > Comment on attachment 247103 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=247103&action=review > > I am a bit confused by the test, but the patch looks correct :) This is the presumably fuzz-generated test that the bug reporter (Han) provided. It's a 100% repro case. I only changed the number of iterations to ensure it runs fast. > > > Source/JavaScriptCore/tests/stress/regress-141883.js:6 > > +for (var i = 0; i < n; i++) { > > Did you intend to reuse "i" here? No idea. ;-) > > > Source/JavaScriptCore/tests/stress/regress-141883.js:7 > > + if (a = b*3) { > > Any reason for the if() here? Lol, no idea. ;-)
Landed in http://trac.webkit.org/changeset/180505
Comment on attachment 247103 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=247103&action=review > Source/JavaScriptCore/ChangeLog:8 > + If a value might be a cell, then we have to have Graph freeze it rather than trying to Is the second "have" redundant? > Source/JavaScriptCore/tests/stress/regress-141883.js:11 > +(function() { > +var b=!2; > +var n = 1e4; > +for(i = 0; i< n; i++) { > +b[b=this]; > +for (var i = 0; i < n; i++) { > + if (a = b*3) { > + } > +} > +} > +})() Is there a reason this isn't indented properly? Also, the formatting wrt spaces is inconsistent. Did you intend to reuse "i" as the loop variable for the inner loop? Could you add comments as to what the test is doing?
(In reply to comment #7) > Comment on attachment 247103 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=247103&action=review > > > Source/JavaScriptCore/ChangeLog:8 > > + If a value might be a cell, then we have to have Graph freeze it rather than trying to > > Is the second "have" redundant? I don't think so. "I have to have a cheeseburger" is a valid sentence, isn't it? > > > Source/JavaScriptCore/tests/stress/regress-141883.js:11 > > +(function() { > > +var b=!2; > > +var n = 1e4; > > +for(i = 0; i< n; i++) { > > +b[b=this]; > > +for (var i = 0; i < n; i++) { > > + if (a = b*3) { > > + } > > +} > > +} > > +})() > > Is there a reason this isn't indented properly? Also, the formatting wrt > spaces is inconsistent. > Did you intend to reuse "i" as the loop variable for the inner loop? > Could you add comments as to what the test is doing? I don't know what this test is doing. I do know that it's a 100% repro case. It was provided by the bug reporter, see the bug description.
Out of curiosity, I played with it and it's reproducible like this: (function() { var b = false; var n = 1e4; for(i = 0; i< n; i++) { b[{}]; b = {}; for (var j = 0; j < n; j++) b*3; } })() For it to reproduce, "i" needed to be global, "b" needed to start of as bool and switch to object, and multiplication with "b" needed to be in an inner loop.
(In reply to comment #9) > Out of curiosity, I played with it and it's reproducible like this: > > (function() { > var b = false; > var n = 1e4; > for(i = 0; i< n; i++) { > b[{}]; > b = {}; > for (var j = 0; j < n; j++) > b*3; > } > })() > > For it to reproduce, "i" needed to be global, "b" needed to start of as bool > and switch to object, and multiplication with "b" needed to be in an inner > loop. May I suggest to submit your test too? Having more tests is great :) r=me for your test above.