Bug 96227

Summary: DFG misses arguments tear-off for function.arguments if 'arguments' is used
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, dglazkov, fpizlo, oliver, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
webkit.review.bot: commit-queue-
compatibility test
none
compatibility test
none
compatibility test
none
compatibility test
none
compatibility test none

Geoffrey Garen
Reported 2012-09-09 22:08:18 PDT
DFG misses arguments tear-off for function.arguments if 'arguments' is used
Attachments
Patch (5.09 KB, patch)
2012-09-09 22:09 PDT, Geoffrey Garen 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
compatibility test (1.07 KB, text/html)
2012-09-10 13:47 PDT, Geoffrey Garen 		no flags
Details
compatibility test (1.07 KB, text/html)
2012-09-10 13:49 PDT, Geoffrey Garen 		no flags
Details
compatibility test (1.07 KB, text/html)
2012-09-10 13:58 PDT, Geoffrey Garen 		no flags
Details
compatibility test (1.19 KB, text/html)
2012-09-10 14:01 PDT, Geoffrey Garen 		no flags
Details
compatibility test (1.20 KB, text/html)
2012-09-10 14:02 PDT, Geoffrey Garen 		no flags
Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2012-09-09 22:09:14 PDT
Created attachment 163039 [details] Patch
Geoffrey Garen
Comment 2 2012-09-09 22:11:19 PDT
Comment on attachment 163039 [details] Patch Uploading for discussion. This patch eliminates aliasing of the local 'arguments' property, and makes function.arguments always tear off. Another way to fix this bug, and keep our current behavior without regression, is to use watchpoints.
Geoffrey Garen
Comment 3 2012-09-09 22:14:04 PDT
Here's what I know about the state of function.arguments: - It's non-standard: Mozilla invented and then deprecated it; ES4 never mentioned it; and ES5 explicitly poisons it in strict mode. - Compatibility matrix of arguments == function.arguments: Firefox: never IE: never, but function.arguments == function.arguments Chrome: if caller uses 'arguments' Safari: if caller uses 'arguments' Compatibility matrix of what function.arguments contains: Firefox: current snapshot Chrome: current snapshot Safari: current snapshot IE: original snapshot Based on this matrix, the best compatibility story by market share seems to be: - function.arguments doesn't alias 'arguments' (IE + Firefox) - function.arguments snapshots current argument values (Chrome + Firefox + Safari)
Filip Pizlo
Comment 4 2012-09-09 22:17:42 PDT
Comment on attachment 163039 [details] Patch I think that your change implies that using 'arguments' should not cause the DFG to assume that arguments (i.e. the actual arguments, not the 'arguments' local) are captured. You should probably make that part of this change, as well - or at least file a separate bug indicating that we should kill off that code in the future. Other than that, I have no objections to this change.
WebKit Review Bot
Comment 5 2012-09-09 23:55:10 PDT
Comment on attachment 163039 [details] Patch Attachment 163039 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13795743 New failing tests: fast/js/dfg-tear-off-function-dot-arguments.html
Geoffrey Garen
Comment 6 2012-09-10 12:24:01 PDT
Notions of "captured arguments" that may need fixup: CodeBlock capturedVars bitvector needs to match variableAccessData isCaptured flag ByteCodeParser initialization All isCaptured() queries other than VariableAccessData queries should just forward to the CodeBlock.
Geoffrey Garen
Comment 7 2012-09-10 13:47:54 PDT
Created attachment 163198 [details] compatibility test
Geoffrey Garen
Comment 8 2012-09-10 13:49:28 PDT
Created attachment 163199 [details] compatibility test
Geoffrey Garen
Comment 9 2012-09-10 13:58:00 PDT
Created attachment 163203 [details] compatibility test
Geoffrey Garen
Comment 10 2012-09-10 14:01:22 PDT
Created attachment 163204 [details] compatibility test
Geoffrey Garen
Comment 11 2012-09-10 14:02:53 PDT
Created attachment 163205 [details] compatibility test
Geoffrey Garen
Comment 12 2012-09-10 14:48:53 PDT
Committed r128111: <http://trac.webkit.org/changeset/128111>
Note You need to log in before you can comment on or make changes to this bug.