RESOLVED FIXED 140743
BytecodeGenerator::initializeCapturedVariable() sets a misleading value for the 5th operand of op_put_to_scope
https://bugs.webkit.org/show_bug.cgi?id=140743
Summary BytecodeGenerator::initializeCapturedVariable() sets a misleading value for t...
Mark Lam
Reported 2015-01-21 14:46:39 PST
Steps to reproduce: 1. Go to https://bugreport.apple.com/problem/viewproblem?problemID=18702537 2. Log in. BytecodeGenerator::initializeCapturedVariable() is setting the 5th operand to op_put_to_scope to an inappropriate value. As a result, the execution of put_to_scope could store a wrong inferred value into the VariableWatchpointSet for which ever captured variable is at local index 0. In practice, this turns out to be the local for the Arguments object. In this example, the wrong inferred value written there is the boolean true. Subsequently, DFG compilation occurs and CreateArguments is emitted to first do a check of the local for the Arguments object. But because that local has a wrong inferred value, the check always discovers a non-null value and we never actually create the Arguments object. Immediately after this, an OSR exit occurs leaving the Arguments object local uninitialized. Later on at arguments tear off, we run into a boolean true where we had expected to find an Arguments object, which in turn, leads to the crash.
Attachments
the patch. (12.12 KB, patch)
2015-01-21 15:36 PST, Mark Lam
no flags
patch 2: applied Geoff's feedback (12.06 KB, patch)
2015-01-21 18:33 PST, Mark Lam
no flags
perf results (49.93 KB, text/plain)
2015-01-21 18:36 PST, Mark Lam
no flags
Mark Lam
Comment 1 2015-01-21 14:47:17 PST
Mark Lam
Comment 2 2015-01-21 15:36:04 PST
Created attachment 245095 [details] the patch. I need to rerun perf numbers but this patch has already passed the jsc stress tests and layout tests.
Geoffrey Garen
Comment 3 2015-01-21 17:24:18 PST
Comment on attachment 245095 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=245095&action=review > LayoutTests/js/script-tests/dfg-osr-exit-between-create-and-tearoff-arguments.js:11 > + // Loop ridiculously many times here to get this function to DFG compile. Can you use the predicate that dfgShouldBe uses instead? 20000 is probably much more than is necessary.
Mark Lam
Comment 4 2015-01-21 18:33:53 PST
Created attachment 245106 [details] patch 2: applied Geoff's feedback Thanks for the feedback. The test has been updated to use dfgCompiled() instead. The perf numbers came back neutral. I'll upload the results shortly.
Mark Lam
Comment 5 2015-01-21 18:36:09 PST
Created attachment 245107 [details] perf results
WebKit Commit Bot
Comment 6 2015-01-22 11:05:39 PST
Comment on attachment 245106 [details] patch 2: applied Geoff's feedback Clearing flags on attachment: 245106 Committed r178926: <http://trac.webkit.org/changeset/178926>
WebKit Commit Bot
Comment 7 2015-01-22 11:05:45 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.