Changeset 128111 in webkit


Ignore:
Timestamp:
Sep 10, 2012 2:49:25 PM (12 years ago)
Author:
ggaren@apple.com
Message:

DFG misses arguments tear-off for function.arguments if 'arguments' is used
​https://bugs.webkit.org/show_bug.cgi?id=96227

Reviewed by Gavin Barraclough.

Source/JavaScriptCore:

We've decided not to allow function.arguments to alias the local
'arguments' object, or a local var or function named 'arguments'.
Aliasing complicates the implementation (cf, this bug) and can produce
surprising behavior for web programmers.

Eliminating the aliasing has the side-effect of fixing this bug.

The compatibilty story: function.arguments is deprecated, was never
specified, and throws an exception in strict mode, so we expect it to
disappear over time. Firefox does not alias to 'arguments'; Chrome
does, but not if you use eval or with; IE does; Safari did.

  • dfg/DFGByteCodeParser.cpp: Noticed a little cleanup while verifying

this code. Use the CodeBlock method for better encapsulation.

  • interpreter/Interpreter.cpp:

(JSC::Interpreter::retrieveArgumentsFromVMCode): Behavior change: don't
alias.

  • tests/mozilla/js1_4/Functions/function-001.js:

(TestFunction_4): Updated test expectations for changed behavior.

LayoutTests:

New test, and updated expectations.

  • fast/js/script-tests/function-dot-arguments.js:
  • fast/js/function-dot-arguments-expected.txt: Updated for new behavior.
  • fast/js/dfg-tear-off-function-dot-arguments.html:
  • fast/js/script-tests/dfg-tear-off-function-dot-arguments.js: Added. New test for bug cited here.
  • fast/js/function-dot-arguments-identity-expected.txt:
  • fast/js/function-dot-arguments-identity.html: Added. New test for new behavior.
Location:
trunk
Files:
5 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r128105 r128111  
     12012-09-10  Geoffrey Garen  <ggaren@apple.com>
     2
     3        DFG misses arguments tear-off for function.arguments if 'arguments' is used
     4        https://bugs.webkit.org/show_bug.cgi?id=96227
     5
     6        Reviewed by Gavin Barraclough.
     7
     8        New test, and updated expectations.
     9 
     10        * fast/js/script-tests/function-dot-arguments.js:
     11        * fast/js/function-dot-arguments-expected.txt: Updated for new behavior.
     12
     13        * fast/js/dfg-tear-off-function-dot-arguments.html:
     14        * fast/js/script-tests/dfg-tear-off-function-dot-arguments.js: Added. New test for bug cited here.
     15
     16        * fast/js/function-dot-arguments-identity-expected.txt:
     17        * fast/js/function-dot-arguments-identity.html: Added. New test for new behavior.
     18
    1192012-09-10  Fady Samuel  <fsamuel@chromium.org>
    220

  • trunk/LayoutTests/fast/js/function-dot-arguments-expected.txt

    r76090 r128111  
    99PASS lengthTest() is '0'
    1010PASS lengthTest('From', '%E5%8C%97%E4%BA%AC', 360, '/', 'webkit.org') is '5'
    11 PASS assignTest() is true
     11PASS assignTest().toString() is '[object Arguments]'
    1212PASS assignVarUndefinedTest().toString() is '[object Arguments]'
    1313PASS assignVarUndefinedTest2().toString() is '[object Arguments]'
    14 PASS assignVarInitTest() is true
    15 PASS assignVarInitTest2() is true
     14PASS assignVarInitTest().toString() is '[object Arguments]'
     15PASS assignVarInitTest2().toString() is '[object Arguments]'
    1616PASS assignConstUndefinedTest().toString() is '[object Arguments]'
    1717PASS assignConstUndefinedTest2().toString() is '[object Arguments]'
    18 PASS assignConstInitTest() is true
    19 PASS assignConstInitTest2() is true
    20 PASS assignForInitTest() is true
    21 PASS assignForInitTest2() is true
    22 PASS assignForInInitTest() is true
     18PASS assignConstInitTest().toString() is '[object Arguments]'
     19PASS assignConstInitTest2().toString() is '[object Arguments]'
     20PASS assignForInitTest().toString() is '[object Arguments]'
     21PASS assignForInitTest2().toString() is '[object Arguments]'
     22PASS assignForInInitTest().toString() is '[object Arguments]'
    2323PASS paramInitTest(true).toString() is '[object Arguments]'
    2424PASS paramFunctionConstructorInitTest(true).toString() is '[object Arguments]'
    … …  
    2828PASS lexicalArgumentsLiveRead2(1, 0, 3) is 2
    2929PASS lexicalArgumentsLiveRead3(1, 2, 0) is 3
    30 PASS lexicalArgumentsLiveWrite1(0, 2, 3) is 1
    31 PASS lexicalArgumentsLiveWrite2(1, 0, 3) is 2
    32 PASS lexicalArgumentsLiveWrite3(1, 2, 0) is 3
     30PASS lexicalArgumentsLiveWrite1(0, 2, 3) is 0
     31PASS lexicalArgumentsLiveWrite2(1, 0, 3) is 0
     32PASS lexicalArgumentsLiveWrite3(1, 2, 0) is 0
    3333PASS argumentsNotLiveRead1(0, 2, 3) is 0
    3434PASS argumentsNotLiveRead2(1, 0, 3) is 0
    … …  
    4040PASS overwroteArgumentsInDynamicScope1() is true
    4141PASS overwroteArgumentsInDynamicScope2() is true
    42 PASS overwroteArgumentsInDynamicScope3() is true
     42PASS overwroteArgumentsInDynamicScope3().toString() is '[object Arguments]'
    4343PASS successfullyParsed is true
    4444

  • trunk/LayoutTests/fast/js/script-tests/function-dot-arguments.js

    r98407 r128111  
    2828    return g();
    2929}
    30 shouldBeTrue("assignTest()");
     30shouldBe("assignTest().toString()", "'[object Arguments]'");
    3131
    3232function assignVarUndefinedTest()
    … …  
    6464    return g();
    6565}
    66 shouldBeTrue("assignVarInitTest()");
     66shouldBe("assignVarInitTest().toString()", "'[object Arguments]'");
    6767
    6868function assignVarInitTest2()
    … …  
    7676    return g();
    7777}
    78 shouldBeTrue("assignVarInitTest2()");
     78shouldBe("assignVarInitTest2().toString()", "'[object Arguments]'");
    7979
    8080function assignConstUndefinedTest()
    … …  
    112112    return g();
    113113}
    114 shouldBeTrue("assignConstInitTest()");
     114shouldBe("assignConstInitTest().toString()", "'[object Arguments]'");
    115115
    116116function assignConstInitTest2()
    … …  
    124124    return g();
    125125}
    126 shouldBeTrue("assignConstInitTest2()");
     126shouldBe("assignConstInitTest2().toString()", "'[object Arguments]'");
    127127
    128128function assignForInitTest()
    … …  
    136136    return g();
    137137}
    138 shouldBeTrue("assignForInitTest()");
     138shouldBe("assignForInitTest().toString()", "'[object Arguments]'");
    139139
    140140function assignForInitTest2()
    … …  
    148148    return g();
    149149}
    150 shouldBeTrue("assignForInitTest2()");
     150shouldBe("assignForInitTest2().toString()", "'[object Arguments]'");
    151151
    152152function assignForInInitTest()
    … …  
    160160    return g();
    161161}
    162 shouldBeTrue("assignForInInitTest()");
     162shouldBe("assignForInInitTest().toString()", "'[object Arguments]'");
    163163
    164164function paramInitTest(arguments)
    … …  
    237237    return a;
    238238}
    239 shouldBe("lexicalArgumentsLiveWrite1(0, 2, 3)", "1");
     239shouldBe("lexicalArgumentsLiveWrite1(0, 2, 3)", "0");
    240240
    241241function lexicalArgumentsLiveWrite2(a, b, c)
    … …  
    245245    return b;
    246246}
    247 shouldBe("lexicalArgumentsLiveWrite2(1, 0, 3)", "2");
     247shouldBe("lexicalArgumentsLiveWrite2(1, 0, 3)", "0");
    248248
    249249function lexicalArgumentsLiveWrite3(a, b, c)
    … …  
    253253    return c;
    254254}
    255 shouldBe("lexicalArgumentsLiveWrite3(1, 2, 0)", "3");
     255shouldBe("lexicalArgumentsLiveWrite3(1, 2, 0)", "0");
    256256
    257257function argumentsNotLiveRead1(a, b, c)
    … …  
    322322shouldBeTrue("overwroteArgumentsInDynamicScope1()");
    323323shouldBeTrue("overwroteArgumentsInDynamicScope2()");
    324 shouldBeTrue("overwroteArgumentsInDynamicScope3()");
     324shouldBe("overwroteArgumentsInDynamicScope3().toString()", "'[object Arguments]'");

  • trunk/Source/JavaScriptCore/ChangeLog

    r128100 r128111  
     12012-09-10  Geoffrey Garen  <ggaren@apple.com>
     2
     3        DFG misses arguments tear-off for function.arguments if 'arguments' is used
     4        https://bugs.webkit.org/show_bug.cgi?id=96227
     5
     6        Reviewed by Gavin Barraclough.
     7
     8        We've decided not to allow function.arguments to alias the local
     9        'arguments' object, or a local var or function named 'arguments'.
     10        Aliasing complicates the implementation (cf, this bug) and can produce
     11        surprising behavior for web programmers.
     12
     13        Eliminating the aliasing has the side-effect of fixing this bug.
     14
     15        The compatibilty story: function.arguments is deprecated, was never
     16        specified, and throws an exception in strict mode, so we expect it to
     17        disappear over time. Firefox does not alias to 'arguments'; Chrome
     18        does, but not if you use eval or with; IE does; Safari did.
     19
     20        * dfg/DFGByteCodeParser.cpp: Noticed a little cleanup while verifying
     21        this code. Use the CodeBlock method for better encapsulation.
     22
     23        * interpreter/Interpreter.cpp:
     24        (JSC::Interpreter::retrieveArgumentsFromVMCode): Behavior change: don't
     25        alias.
     26
     27        * tests/mozilla/js1_4/Functions/function-001.js:
     28        (TestFunction_4): Updated test expectations for changed behavior.
     29
    1302012-09-10  Filip Pizlo  <fpizlo@apple.com>
    231

  • trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

    r128096 r128111  
    31923192        }
    31933193       
    3194         if (codeBlock->usesArguments() || codeBlock->needsActivation()) {
     3194        if (codeBlock->argumentsAreCaptured()) {
    31953195            for (int i = argumentCountIncludingThis; i--;)
    31963196                inlineCallFrame.capturedVars.set(argumentToOperand(i) + inlineCallFrame.stackOffset);

  • trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

    r128096 r128111  
    51065106        return jsNull();
    51075107
    5108     CodeBlock* codeBlock = functionCallFrame->someCodeBlockForPossiblyInlinedCode();
    5109     if (codeBlock->usesArguments()) {
    5110         ASSERT(codeBlock->codeType() == FunctionCode);
    5111         int argumentsRegister = codeBlock->argumentsRegister();
    5112         int realArgumentsRegister = unmodifiedArgumentsRegister(argumentsRegister);
    5113         if (JSValue arguments = functionCallFrame->uncheckedR(argumentsRegister).jsValue())
    5114             return arguments;
    5115         JSValue arguments = JSValue(Arguments::create(callFrame->globalData(), functionCallFrame));
    5116         functionCallFrame->r(argumentsRegister) = arguments;
    5117         functionCallFrame->r(realArgumentsRegister) = arguments;
    5118         return arguments;
    5119     }
    5120 
    51215108    Arguments* arguments = Arguments::create(functionCallFrame->globalData(), functionCallFrame);
    51225109    arguments->tearOff(functionCallFrame);

  • trunk/Source/JavaScriptCore/tests/mozilla/js1_4/Functions/function-001.js

    r11995 r128111  
    8383        "return function.arguments when function contains an arguments property",
    8484        "PASS",
    85         TestFunction_4( "F", "A", "I", "L" ) +"");
     85        TestFunction_4( "P", "A", "S", "S" ) +"");
    8686
    8787    test();
    … …  
    101101
    102102    function TestFunction_4( a, b, c, d, e ) {
    103         var arguments = "PASS";
    104         return TestFunction_4.arguments;
     103        var arguments = "FAIL";
     104        return Array.prototype.join.call(TestFunction_4.arguments, "");
    105105    }
    106106
Note: See TracChangeset for help on using the changeset viewer.