Bug 97836

Summary: JSC: CallFrameClosure::resetCallFrame() can corrupt the previous frame.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fix.
none
Adding regression test. ggaren: review+

Mark Lam
Reported 2012-09-27 17:30:31 PDT
Currently, CallFrameClosure::resetCallFrame() looks like this: void resetCallFrame() { newCallFrame->setScope(scope); for (int i = argumentCountIncludingThis; i < parameterCountIncludingThis; ++i) newCallFrame->setArgument(i, jsUndefined()); } However, CallFrame::setArgument() takes an arg index that starts from 0 and does not include the 'this' value. Since both argumentCountIncludingThis and parameterCountIncludingThis, we need to subtract 1 from both values in order to make the iterator i be a valid arg index for setArgument(). If this is not corrected, then the last setArgument() would be writing to the stack slot before the beginning of the current frame (i.e. corruption the last stack of the previous frame). In addition, it is also not properly initializing the 1st un-passed argument to 'undefined'.
Attachments
Fix. (2.70 KB, patch)
2012-09-27 17:49 PDT, Mark Lam
no flags
Adding regression test. (3.61 KB, patch)
2012-09-27 23:54 PDT, Mark Lam
ggaren: review+
Mark Lam
Comment 1 2012-09-27 17:49:30 PDT
Mark Lam
Comment 2 2012-09-27 17:51:38 PDT
Geoffrey Garen
Comment 3 2012-09-27 19:16:18 PDT
> In addition, it is also not properly initializing the 1st un-passed argument to 'undefined'. Please add a regression test for this.
Mark Lam
Comment 4 2012-09-27 23:53:50 PDT
Reopening to add a regression test.
Mark Lam
Comment 5 2012-09-27 23:54:28 PDT
Created attachment 166156 [details] Adding regression test.
Geoffrey Garen
Comment 6 2012-09-28 08:43:26 PDT
Comment on attachment 166156 [details] Adding regression test. r=me
Mark Lam
Comment 7 2012-09-28 08:51:50 PDT
Regression test landed in http://trac.webkit.org/changeset/129902.
Note You need to log in before you can comment on or make changes to this bug.