Bug 149162 - REGRESSION(r189774): It made Speedometer/Full.html performance test crash
Summary: REGRESSION(r189774): It made Speedometer/Full.html performance test crash
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P1 Critical
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks: 148661
  Show dependency treegraph
 
Reported: 2015-09-15 08:54 PDT by Csaba Osztrogonác
Modified: 2015-09-15 23:16 PDT (History)
5 users (show)

See Also:


Attachments
Crash log (63.92 KB, application/octet-stream)
2015-09-15 10:47 PDT, Chris Dumez
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Csaba Osztrogonác 2015-09-15 08:55:55 PDT
I'm sure that r189774 is the culprit, I tested r189773 
manually (EFL Linux) and this test works fine on it.
Comment 2 Chris Dumez 2015-09-15 10:47:33 PDT
Created attachment 261210 [details]
Crash log
Comment 3 Michael Saboff 2015-09-15 14:44:05 PDT
We are dying trying to execute a StoreBarrier on a local register.  This is after a CallVarArgs.  The local register is being overwritten with undefined (0xa) by the fixup arity stub on entry to the called function.  When we return from the CallVarArgs, the StoreBarrier will fail trying to access a cell at 0xa.

I believe that the argument count and therefore layout and/or position of the varargs callee is wrong.  The patch made changes to call frame sizing, with code that rounds up the size of the frame to be stack aligned.

More investigation...
Comment 4 Michael Saboff 2015-09-15 22:06:46 PDT
The issue is that we are doing arity checking on a varargs call frame  That call frame has two arguments and needs a third.  With the size of the call frame header is 5 added to the 2 arguments gives a frame size that is 7 Registers big.  The code in this change assumes that call frames are rounded up to stack alignment size.  But this one is not.  The arity fix up code assumes that there is an extra slot, based on the size of the fixup being one Register slot.  It then clobbers the last caller's local which is in the slot assumed to be unused.

I have tried a couple methods to make the call frame size be stack size aligned, but these proposed fixes cause other tests to fail.

I'm going to roll out the r189774 and r189818, the C Loop fix from https://bugs.webkit.org/show_bug.cgi?id=149171 and then debug locally before landing the original patch with a fix.
Comment 5 Michael Saboff 2015-09-15 23:16:17 PDT
Landed roll out in changes r189848 <http://trac.webkit.org/changeset/189848>