Bug 140743 - BytecodeGenerator::initializeCapturedVariable() sets a misleading value for the 5th operand of op_put_to_scope
Summary: BytecodeGenerator::initializeCapturedVariable() sets a misleading value for t...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-01-21 14:46 PST by Mark Lam
Modified: 2015-01-22 11:05 PST (History)
8 users (show)

See Also:


Attachments
the patch. (12.12 KB, patch)
2015-01-21 15:36 PST, Mark Lam
no flags Details | Formatted Diff | Diff
patch 2: applied Geoff's feedback (12.06 KB, patch)
2015-01-21 18:33 PST, Mark Lam
no flags Details | Formatted Diff | Diff
perf results (49.93 KB, text/plain)
2015-01-21 18:36 PST, Mark Lam
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 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.
Comment 1 Mark Lam 2015-01-21 14:47:17 PST
<rdar://problem/18821173>
Comment 2 Mark Lam 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.
Comment 3 Geoffrey Garen 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.
Comment 4 Mark Lam 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.
Comment 5 Mark Lam 2015-01-21 18:36:09 PST
Created attachment 245107 [details]
perf results
Comment 6 WebKit Commit Bot 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>
Comment 7 WebKit Commit Bot 2015-01-22 11:05:45 PST
All reviewed patches have been landed.  Closing bug.