Summary: | DFGArrayMode::fromObserved is too liberal when it sees different Array and NonArray shapes | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Hahnenberg <mhahnenberg> | ||||||
Component: | JavaScriptCore | Assignee: | Mark Hahnenberg <mhahnenberg> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | ||||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Mark Hahnenberg
2013-05-08 08:02:55 PDT
Created attachment 201076 [details]
Patch
Comment on attachment 201076 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201076&action=review > Source/JavaScriptCore/dfg/DFGArrayMode.cpp:120 > - if (observed & (asArrayModes(ArrayWithUndecided) | asArrayModes(ArrayWithInt32) | asArrayModes(ArrayWithDouble) | asArrayModes(ArrayWithContiguous) | asArrayModes(ArrayWithArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage))) > - arrayClass = Array::Array; > - else if (observed & (asArrayModes(NonArray) | asArrayModes(NonArrayWithInt32) | asArrayModes(NonArrayWithDouble) | asArrayModes(NonArrayWithContiguous) | asArrayModes(NonArrayWithArrayStorage) | asArrayModes(NonArrayWithSlowPutArrayStorage))) > + if (observed & (asArrayModes(NonArray) | asArrayModes(NonArrayWithInt32) | asArrayModes(NonArrayWithDouble) | asArrayModes(NonArrayWithContiguous) | asArrayModes(NonArrayWithArrayStorage) | asArrayModes(NonArrayWithSlowPutArrayStorage))) > arrayClass = Array::NonArray; > + else if (observed & (asArrayModes(ArrayWithUndecided) | asArrayModes(ArrayWithInt32) | asArrayModes(ArrayWithDouble) | asArrayModes(ArrayWithContiguous) | asArrayModes(ArrayWithArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage))) > + arrayClass = Array::Array; > else > arrayClass = Array::PossiblyArray; Actually, I think the bug here is that PossiblyArray wasn't working right. PossiblyArray is supposed to represent "this could be an Array or a NonArray". So it appears that the problem here is that PossiblyArray isn't being compiled to do the right thing (i.e. accept either Array or NonArray). (In reply to comment #2) > (From update of attachment 201076 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=201076&action=review > > > Source/JavaScriptCore/dfg/DFGArrayMode.cpp:120 > > - if (observed & (asArrayModes(ArrayWithUndecided) | asArrayModes(ArrayWithInt32) | asArrayModes(ArrayWithDouble) | asArrayModes(ArrayWithContiguous) | asArrayModes(ArrayWithArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage))) > > - arrayClass = Array::Array; > > - else if (observed & (asArrayModes(NonArray) | asArrayModes(NonArrayWithInt32) | asArrayModes(NonArrayWithDouble) | asArrayModes(NonArrayWithContiguous) | asArrayModes(NonArrayWithArrayStorage) | asArrayModes(NonArrayWithSlowPutArrayStorage))) > > + if (observed & (asArrayModes(NonArray) | asArrayModes(NonArrayWithInt32) | asArrayModes(NonArrayWithDouble) | asArrayModes(NonArrayWithContiguous) | asArrayModes(NonArrayWithArrayStorage) | asArrayModes(NonArrayWithSlowPutArrayStorage))) > > arrayClass = Array::NonArray; > > + else if (observed & (asArrayModes(ArrayWithUndecided) | asArrayModes(ArrayWithInt32) | asArrayModes(ArrayWithDouble) | asArrayModes(ArrayWithContiguous) | asArrayModes(ArrayWithArrayStorage) | asArrayModes(ArrayWithSlowPutArrayStorage))) > > + arrayClass = Array::Array; > > else > > arrayClass = Array::PossiblyArray; > > Actually, I think the bug here is that PossiblyArray wasn't working right. PossiblyArray is supposed to represent "this could be an Array or a NonArray". So it appears that the problem here is that PossiblyArray isn't being compiled to do the right thing (i.e. accept either Array or NonArray). Gotcha. So we need to check if there's both NonArray and Array stuff first and if so use PossiblyArray, otherwise do what we were doing. Created attachment 201129 [details]
Patch
Comment on attachment 201129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=201129&action=review r=me > Source/JavaScriptCore/ChangeLog:11 > + instead of just shape, which can cause us to exit a lot in the case that we saw have a NonArray. Typo: Should be "...case that we saw...". Committed r149834: <http://trac.webkit.org/changeset/149834> |