[Webkit-unassigned] [Bug 70246] REGRESSION(r96189): Cappuccino applications don't work anymore

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=70246


Filip Pizlo <fpizlo at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|webkit-unassigned at lists.web |fpizlo at apple.com
                   |kit.org                     |




--- Comment #23 from Filip Pizlo <fpizlo at apple.com>  2012-09-21 15:33:13 PST ---
Created an attachment (id=165212)
 --> (https://bugs.webkit.org/attachment.cgi?id=165212&action=review)
simple reduced case

Note that it will start to print '1,1' after about 66 iterations.  That's when we tier up to the optimizing JIT, and then the optimizing JIT mistakenly assumes that 'array.length' was invariant under 'array[1] = 42'.  It makes this mistake because of my array hole optimizations:

- 'array[1] = 42' is an in-bounds store, since the original array allocation would have created an array with enough room for 4 elements.

- 'array[1] = 42' is a holy store, since it stores to an empty value (a hole).

- We correctly infer that this does not clobber the world.  So for example accesses to unrelated things (like regular object properties, global variables, etc) would not see this array store.

- But then we incorrectly infer that this does not clobber the length.  It clearly does - in this case it changes the length from 1 to 2.

The more fundamental bug was that CSE was being way overaggressive for GetArrayLength in general. It was assuming that if in between two GetArrayLengths there wasn't anything that "clobbered the world" (function calls, crazy polymorphism, eval, etc) and the two GetArrayLengths accessed the same array, then the second one was redundant. It was saved a little bit by a separate bug where it would fall back if the butterfly CSE failed (so anything that could reallocate backing storage would seem to clobber length).  In this case, the PutByVal for 'array[1] = 42' doesn't clobber the world and doesn't lead to array storage reallocation - so GetArrayLength CSE would kick in, and boom, wrong result.

The solution is to actually write a proper interference analysis for GetArrayLength.  That will fix it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.