Changeset 78727 in webkit


Ignore:
Timestamp:
Feb 16, 2011 11:31:16 AM (13 years ago)
Author:
oliver@apple.com
Message:

2011-02-16 Oliver Hunt <​oliver@apple.com>

Reviewed by Geoff Garen.

Incorrect handling of global writes in dynamic contexts
​https://bugs.webkit.org/show_bug.cgi?id=49383

Add a few tests to ensure that global writes are actually
allowed inside dynamic scopes.

  • fast/js/basic-strict-mode-expected.txt:
  • fast/js/script-tests/basic-strict-mode.js:

2011-02-16 Oliver Hunt <​oliver@apple.com>

Reviewed by Geoff Garen.

Incorrect handling of global writes in dynamic contexts
​https://bugs.webkit.org/show_bug.cgi?id=49383

  • interpreter/Interpreter.cpp: (JSC::Interpreter::privateExecute): Can't use the existing callframe to return an uncaught exception as by definition that callframe has already been torn down.
  • parser/ASTBuilder.h: (JSC::ASTBuilder::ASTBuilder): (JSC::ASTBuilder::varDeclarations): (JSC::ASTBuilder::funcDeclarations): (JSC::ASTBuilder::features): (JSC::ASTBuilder::numConstants): (JSC::ASTBuilder::createFuncDeclStatement): (JSC::ASTBuilder::addVar): (JSC::ASTBuilder::incConstants): (JSC::ASTBuilder::usesThis): (JSC::ASTBuilder::usesCatch): (JSC::ASTBuilder::usesClosures): (JSC::ASTBuilder::usesArguments): (JSC::ASTBuilder::usesAssignment): (JSC::ASTBuilder::usesWith): (JSC::ASTBuilder::usesEval): Don't need a vector of scopes in the ASTBuilder
  • runtime/Operations.h: (JSC::resolveBase): In strict mode the optimisation that we use to skip a lookup on the global object is incorrect and lead to us always disallowing global writes when we needed to do a dynamic slot lookup. Now the strict mode path actually checks for the property.
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r78722 r78727  
     12011-02-16  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Geoff Garen.
     4
     5        Incorrect handling of global writes in dynamic contexts
     6        https://bugs.webkit.org/show_bug.cgi?id=49383
     7
     8        Add a few tests to ensure that global writes are actually
     9        allowed inside dynamic scopes.
     10
     11        * fast/js/basic-strict-mode-expected.txt:
     12        * fast/js/script-tests/basic-strict-mode.js:
     13
    1142011-02-16  Nikolas Zimmermann  <nzimmermann@rim.com>
    215

  • trunk/LayoutTests/fast/js/basic-strict-mode-expected.txt

    r75896 r78727  
    193193PASS 'use strict';var a=(arguments=1); threw exception SyntaxError: Parse error.
    194194PASS (function(){'use strict';var a=(arguments=1);}) threw exception SyntaxError: Parse error.
     195PASS 'use strict'; try { throw 1; } catch (e) { aGlobal = true; } is true
     196PASS 'use strict'; (function () { try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal; is true
     197PASS (function () {'use strict';  try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal; is true
     198PASS try { throw 1; } catch (e) { aGlobal = true; } is true
     199PASS (function () { try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal; is true
     200PASS (function () {try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal; is true
    195201PASS successfullyParsed is true
    196202

  • trunk/LayoutTests/fast/js/script-tests/basic-strict-mode.js

    r75896 r78727  
    172172shouldBeSyntaxError("'use strict';var a=(eval=1);");
    173173shouldBeSyntaxError("'use strict';var a=(arguments=1);");
     174
     175var aGlobal = false;
     176shouldBeTrue("'use strict'; try { throw 1; } catch (e) { aGlobal = true; }");
     177aGlobal = false;
     178shouldBeTrue("'use strict'; (function () { try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal;");
     179aGlobal = false;
     180shouldBeTrue("(function () {'use strict';  try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal;");
     181aGlobal = false;
     182shouldBeTrue("try { throw 1; } catch (e) { aGlobal = true; }");
     183aGlobal = false;
     184shouldBeTrue("(function () { try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal;");
     185aGlobal = false;
     186shouldBeTrue("(function () {try { throw 1; } catch (e) { aGlobal = true; }})(); aGlobal;");
     187
    174188var successfullyParsed = true;

  • trunk/Source/JavaScriptCore/ChangeLog

    r78654 r78727  
     12011-02-16  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Geoff Garen.
     4
     5        Incorrect handling of global writes in dynamic contexts
     6        https://bugs.webkit.org/show_bug.cgi?id=49383
     7
     8        * interpreter/Interpreter.cpp:
     9        (JSC::Interpreter::privateExecute):
     10          Can't use the existing callframe to return an uncaught exception
     11          as by definition that callframe has already been torn down.
     12        * parser/ASTBuilder.h:
     13        (JSC::ASTBuilder::ASTBuilder):
     14        (JSC::ASTBuilder::varDeclarations):
     15        (JSC::ASTBuilder::funcDeclarations):
     16        (JSC::ASTBuilder::features):
     17        (JSC::ASTBuilder::numConstants):
     18        (JSC::ASTBuilder::createFuncDeclStatement):
     19        (JSC::ASTBuilder::addVar):
     20        (JSC::ASTBuilder::incConstants):
     21        (JSC::ASTBuilder::usesThis):
     22        (JSC::ASTBuilder::usesCatch):
     23        (JSC::ASTBuilder::usesClosures):
     24        (JSC::ASTBuilder::usesArguments):
     25        (JSC::ASTBuilder::usesAssignment):
     26        (JSC::ASTBuilder::usesWith):
     27        (JSC::ASTBuilder::usesEval):
     28          Don't need a vector of scopes in the ASTBuilder
     29        * runtime/Operations.h:
     30        (JSC::resolveBase):
     31          In strict mode the optimisation that we use to skip a lookup
     32          on the global object is incorrect and lead to us always
     33          disallowing global writes when we needed to do a dynamic slot
     34          lookup.  Now the strict mode path actually checks for the
     35          property.
     36
    1372011-02-15  Jon Honeycutt  <jhoneycutt@apple.com>
    238

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

    r78634 r78727  
    24502450    }
    24512451    DEFINE_OPCODE(op_resolve_base) {
    2452         /* resolve_base dst(r) property(id)
     2452        /* resolve_base dst(r) property(id) isStrict(bool)
    24532453
    24542454           Searches the scope chain for an object containing
    24552455           identifier property, and if one is found, writes it to
    2456            register dst. If none is found, the outermost scope (which
    2457            will be the global object) is stored in register dst.
     2456           register dst. If none is found and isStrict is false, the
     2457           outermost scope (which will be the global object) is
     2458           stored in register dst.
    24582459        */
    24592460        resolveBase(callFrame, vPC);
     2461        CHECK_FOR_EXCEPTION();
    24602462
    24612463        vPC += OPCODE_LENGTH(op_resolve_base);
    … …  
    47774779            exceptionValue = createInterruptedExecutionException(globalData);
    47784780        }
     4781        JSGlobalObject* globalObject = callFrame->lexicalGlobalObject();
    47794782        handler = throwException(callFrame, exceptionValue, vPC - codeBlock->instructions().begin());
    4780         if (!handler)
    4781             return throwError(callFrame, exceptionValue);
     4783        if (!handler) {
     4784            // Can't use the callframe at this point as the scopechain, etc have
     4785            // been released.
     4786            return throwError(globalObject->globalExec(), exceptionValue);
     4787        }
    47824788
    47834789        codeBlock = callFrame->codeBlock();

  • trunk/Source/JavaScriptCore/parser/ASTBuilder.h

    r76177 r78727  
    7777        : m_globalData(globalData)
    7878        , m_lexer(lexer)
     79        , m_scope(globalData)
    7980        , m_evalCount(0)
    8081    {
    81         m_scopes.append(Scope(globalData));
    8282    }
    8383   
    … …  
    116116    JSC::SourceElements* createSourceElements() { return new (m_globalData) JSC::SourceElements(m_globalData); }
    117117
    118     ParserArenaData<DeclarationStacks::VarStack>* varDeclarations() { return m_scopes.last().m_varDeclarations; }
    119     ParserArenaData<DeclarationStacks::FunctionStack>* funcDeclarations() { return m_scopes.last().m_funcDeclarations; }
    120     int features() const { return m_scopes.last().m_features; }
    121     int numConstants() const { return m_scopes.last().m_numConstants; }
     118    ParserArenaData<DeclarationStacks::VarStack>* varDeclarations() { return m_scope.m_varDeclarations; }
     119    ParserArenaData<DeclarationStacks::FunctionStack>* funcDeclarations() { return m_scope.m_funcDeclarations; }
     120    int features() const { return m_scope.m_features; }
     121    int numConstants() const { return m_scope.m_numConstants; }
    122122
    123123    void appendToComma(CommaNode* commaNode, ExpressionNode* expr) { commaNode->append(expr); }
    … …  
    301301        if (*name == m_globalData->propertyNames->arguments)
    302302            usesArguments();
    303         m_scopes.last().m_funcDeclarations->data.append(decl->body());
     303        m_scope.m_funcDeclarations->data.append(decl->body());
    304304        body->setLoc(bodyStartLine, bodyEndLine);
    305305        return decl;
    … …  
    495495        if (m_globalData->propertyNames->arguments == *ident)
    496496            usesArguments();
    497         m_scopes.last().m_varDeclarations->data.append(std::make_pair(ident, attrs));
     497        m_scope.m_varDeclarations->data.append(std::make_pair(ident, attrs));
    498498    }
    499499
    … …  
    612612    }
    613613
    614     void incConstants() { m_scopes.last().m_numConstants++; }
    615     void usesThis() { m_scopes.last().m_features |= ThisFeature; }
    616     void usesCatch() { m_scopes.last().m_features |= CatchFeature; }
    617     void usesClosures() { m_scopes.last().m_features |= ClosureFeature; }
    618     void usesArguments() { m_scopes.last().m_features |= ArgumentsFeature; }
    619     void usesAssignment() { m_scopes.last().m_features |= AssignFeature; }
    620     void usesWith() { m_scopes.last().m_features |= WithFeature; }
     614    void incConstants() { m_scope.m_numConstants++; }
     615    void usesThis() { m_scope.m_features |= ThisFeature; }
     616    void usesCatch() { m_scope.m_features |= CatchFeature; }
     617    void usesClosures() { m_scope.m_features |= ClosureFeature; }
     618    void usesArguments() { m_scope.m_features |= ArgumentsFeature; }
     619    void usesAssignment() { m_scope.m_features |= AssignFeature; }
     620    void usesWith() { m_scope.m_features |= WithFeature; }
    621621    void usesEval()
    622622    {
    623623        m_evalCount++;
    624         m_scopes.last().m_features |= EvalFeature;
     624        m_scope.m_features |= EvalFeature;
    625625    }
    626626    ExpressionNode* createNumber(double d)
    … …  
    631631    JSGlobalData* m_globalData;
    632632    Lexer* m_lexer;
    633     Vector<Scope> m_scopes;
     633    Scope m_scope;
    634634    Vector<BinaryOperand, 10> m_binaryOperandStack;
    635635    Vector<AssignmentInfo, 10> m_assignmentInfoStack;

  • trunk/Source/JavaScriptCore/runtime/Operations.h

    r77151 r78727  
    473473        while (true) {
    474474            base = iter->get();
    475             if (next == end)
    476                 return isStrictPut ? JSValue() : base;
     475            if (next == end) {
     476                if (isStrictPut && !base->getPropertySlot(callFrame, property, slot))
     477                    return JSValue();
     478                return base;
     479            }
    477480            if (base->getPropertySlot(callFrame, property, slot))
    478481                return base;
Note: See TracChangeset for help on using the changeset viewer.