Summary: | Crash in DFGFrozenValue | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Han Choongwoo <cwhan.tunz> | ||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Major | CC: | barraclough, benjamin, fpizlo, ggaren, mark.lam, mhahnenb, mmirman, msaboff, nrotem, oliver, saam, sam | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Han Choongwoo
2015-02-22 17:58:51 PST
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. |