Changeset 186996 in webkit


Ignore:
Timestamp:
Jul 18, 2015 1:12:14 PM (9 years ago)
Author:
saambarati1@gmail.com
Message:

lexical scoping is broken with respect to "break" and "continue"
https://bugs.webkit.org/show_bug.cgi?id=147063

Reviewed by Filip Pizlo.

Bug #142944 which introduced "let" and lexical scoping
didn't properly hook into the bytecode generator's machinery
for calculating scope depth deltas for "break" and "continue". This
resulted in the bytecode generator popping an incorrect number
of scopes when lexical scopes were involved.

This patch fixes this problem and generalizes this machinery a bit.
This patch also renames old functions in a sensible way that is more
coherent in a world with lexical scoping.

  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::newLabelScope):
(JSC::BytecodeGenerator::emitProfileType):
(JSC::BytecodeGenerator::pushLexicalScope):
(JSC::BytecodeGenerator::popLexicalScope):
(JSC::BytecodeGenerator::prepareLexicalScopeForNextForLoopIteration):
(JSC::BytecodeGenerator::resolveType):
(JSC::BytecodeGenerator::emitResolveScope):
(JSC::BytecodeGenerator::emitGetFromScope):
(JSC::BytecodeGenerator::emitPutToScope):
(JSC::BytecodeGenerator::emitPushWithScope):
(JSC::BytecodeGenerator::emitGetParentScope):
(JSC::BytecodeGenerator::emitPopScope):
(JSC::BytecodeGenerator::emitPopWithOrCatchScope):
(JSC::BytecodeGenerator::emitPopScopes):
(JSC::BytecodeGenerator::calculateTargetScopeDepthForExceptionHandler):
(JSC::BytecodeGenerator::localScopeDepth):
(JSC::BytecodeGenerator::labelScopeDepth):
(JSC::BytecodeGenerator::emitThrowReferenceError):
(JSC::BytecodeGenerator::emitPushFunctionNameScope):
(JSC::BytecodeGenerator::pushScopedControlFlowContext):
(JSC::BytecodeGenerator::popScopedControlFlowContext):
(JSC::BytecodeGenerator::emitPushCatchScope):
(JSC::BytecodeGenerator::currentScopeDepth): Deleted.

  • bytecompiler/BytecodeGenerator.h:

(JSC::BytecodeGenerator::hasFinaliser):
(JSC::BytecodeGenerator::scopeDepth): Deleted.

  • bytecompiler/NodesCodegen.cpp:

(JSC::ContinueNode::trivialTarget):
(JSC::BreakNode::trivialTarget):
(JSC::ReturnNode::emitBytecode):
(JSC::WithNode::emitBytecode):
(JSC::TryNode::emitBytecode):

  • tests/stress/lexical-scoping-break-continue.js: Added.

(assert):
(.):

Location:
trunk/Source/JavaScriptCore
Files:
1 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r186986 r186996  
     12015-07-18  Saam barati  <saambarati1@gmail.com>
     2
     3        lexical scoping is broken with respect to "break" and "continue"
     4        https://bugs.webkit.org/show_bug.cgi?id=147063
     5
     6        Reviewed by Filip Pizlo.
     7
     8        Bug #142944 which introduced "let" and lexical scoping
     9        didn't properly hook into the bytecode generator's machinery
     10        for calculating scope depth deltas for "break" and "continue". This
     11        resulted in the bytecode generator popping an incorrect number
     12        of scopes when lexical scopes were involved.
     13
     14        This patch fixes this problem and generalizes this machinery a bit.
     15        This patch also renames old functions in a sensible way that is more
     16        coherent in a world with lexical scoping.
     17
     18        * bytecompiler/BytecodeGenerator.cpp:
     19        (JSC::BytecodeGenerator::BytecodeGenerator):
     20        (JSC::BytecodeGenerator::newLabelScope):
     21        (JSC::BytecodeGenerator::emitProfileType):
     22        (JSC::BytecodeGenerator::pushLexicalScope):
     23        (JSC::BytecodeGenerator::popLexicalScope):
     24        (JSC::BytecodeGenerator::prepareLexicalScopeForNextForLoopIteration):
     25        (JSC::BytecodeGenerator::resolveType):
     26        (JSC::BytecodeGenerator::emitResolveScope):
     27        (JSC::BytecodeGenerator::emitGetFromScope):
     28        (JSC::BytecodeGenerator::emitPutToScope):
     29        (JSC::BytecodeGenerator::emitPushWithScope):
     30        (JSC::BytecodeGenerator::emitGetParentScope):
     31        (JSC::BytecodeGenerator::emitPopScope):
     32        (JSC::BytecodeGenerator::emitPopWithOrCatchScope):
     33        (JSC::BytecodeGenerator::emitPopScopes):
     34        (JSC::BytecodeGenerator::calculateTargetScopeDepthForExceptionHandler):
     35        (JSC::BytecodeGenerator::localScopeDepth):
     36        (JSC::BytecodeGenerator::labelScopeDepth):
     37        (JSC::BytecodeGenerator::emitThrowReferenceError):
     38        (JSC::BytecodeGenerator::emitPushFunctionNameScope):
     39        (JSC::BytecodeGenerator::pushScopedControlFlowContext):
     40        (JSC::BytecodeGenerator::popScopedControlFlowContext):
     41        (JSC::BytecodeGenerator::emitPushCatchScope):
     42        (JSC::BytecodeGenerator::currentScopeDepth): Deleted.
     43        * bytecompiler/BytecodeGenerator.h:
     44        (JSC::BytecodeGenerator::hasFinaliser):
     45        (JSC::BytecodeGenerator::scopeDepth): Deleted.
     46        * bytecompiler/NodesCodegen.cpp:
     47        (JSC::ContinueNode::trivialTarget):
     48        (JSC::BreakNode::trivialTarget):
     49        (JSC::ReturnNode::emitBytecode):
     50        (JSC::WithNode::emitBytecode):
     51        (JSC::TryNode::emitBytecode):
     52        * tests/stress/lexical-scoping-break-continue.js: Added.
     53        (assert):
     54        (.):
     55
    1562015-07-17  Filip Pizlo  <fpizlo@apple.com>
    257
  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r186860 r186996  
    523523    }
    524524
     525    if (m_lexicalEnvironmentRegister)
     526        pushScopedControlFlowContext();
    525527    m_symbolTableStack.append(SymbolTableStackEntry{ Strong<SymbolTable>(*m_vm, m_symbolTable), m_lexicalEnvironmentRegister, false, symbolTableConstantIndex });
    526528    m_TDZStack.append(std::make_pair(*parentScopeTDZVariables, false));
     
    628630
    629631    // Allocate new label scope.
    630     LabelScope scope(type, name, scopeDepth(), newLabel(), type == LabelScope::Loop ? newLabel() : PassRefPtr<Label>()); // Only loops have continue targets.
     632    LabelScope scope(type, name, labelScopeDepth(), newLabel(), type == LabelScope::Loop ? newLabel() : PassRefPtr<Label>()); // Only loops have continue targets.
    631633    m_labelScopes.append(scope);
    632634    return LabelScopePtr(m_labelScopes, m_labelScopes.size() - 1);
     
    12111213    emitOpcode(op_profile_type);
    12121214    instructions().append(registerToProfile->index());
    1213     instructions().append(currentScopeDepth());
     1215    instructions().append(localScopeDepth());
    12141216    instructions().append(flag);
    12151217    instructions().append(identifier ? addConstant(*identifier) : 0);
     
    13191321
    13201322        emitMove(scopeRegister(), newScope);
     1323
     1324        pushScopedControlFlowContext();
    13211325    }
    13221326
     
    13661370    if (hasCapturedVariables) {
    13671371        RELEASE_ASSERT(stackEntry.m_scope);
    1368         RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), scopeRegister());
    1369         emitMove(scopeRegister(), parentScope.get());
     1372        emitPopScope(scopeRegister(), stackEntry.m_scope);
     1373        popScopedControlFlowContext();
    13701374        stackEntry.m_scope->deref();
    13711375    }
     
    14241428    // the assumption that the scope's register index is constant even
    14251429    // though the value in that register will change on each loop iteration.
    1426     RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), scopeRegister());
     1430    RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), loopScope);
    14271431    emitMove(scopeRegister(), parentScope.get());
    14281432
     
    15731577ResolveType BytecodeGenerator::resolveType()
    15741578{
    1575     if (m_localScopeDepth)
    1576         return Dynamic;
     1579    for (unsigned i = m_symbolTableStack.size(); i--; ) {
     1580        if (m_symbolTableStack[i].m_isWithOrCatch)
     1581            return Dynamic;
     1582    }
     1583
    15771584    if (m_symbolTable && m_symbolTable->usesNonStrictEval())
    15781585        return GlobalPropertyWithVarInjectionChecks;
     
    16331640        instructions().append(addConstant(variable.ident()));
    16341641        instructions().append(resolveType());
    1635         instructions().append(currentScopeDepth());
     1642        instructions().append(localScopeDepth());
    16361643        instructions().append(0);
    16371644        return dst;
     
    16671674        instructions().append(addConstant(variable.ident()));
    16681675        instructions().append(ResolveModeAndType(resolveMode, variable.offset().isScope() ? LocalClosureVar : resolveType()).operand());
    1669         instructions().append(currentScopeDepth());
     1676        instructions().append(localScopeDepth());
    16701677        instructions().append(variable.offset().isScope() ? variable.offset().scopeOffset().offset() : 0);
    16711678        instructions().append(profile);
     
    17071714            ASSERT(resolveType() != LocalClosureVar);
    17081715            instructions().append(ResolveModeAndType(resolveMode, resolveType()).operand());
    1709             instructions().append(currentScopeDepth());
     1716            instructions().append(localScopeDepth());
    17101717        }
    17111718        instructions().append(!!offset ? offset.offset() : 0);
     
    24752482RegisterID* BytecodeGenerator::emitPushWithScope(RegisterID* dst, RegisterID* scope)
    24762483{
    2477     ControlFlowContext context;
    2478     context.isFinallyBlock = false;
    2479     m_scopeContextStack.append(context);
    2480     m_localScopeDepth++;
     2484    pushScopedControlFlowContext();
    24812485
    24822486    RegisterID* result = emitUnaryOp(op_push_with_scope, dst, scope);
     
    24932497}
    24942498
    2495 void BytecodeGenerator::emitPopScope(RegisterID* srcDst)
    2496 {
    2497     ASSERT(m_scopeContextStack.size());
    2498     ASSERT(!m_scopeContextStack.last().isFinallyBlock);
    2499 
    2500     RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), srcDst);
    2501     emitMove(srcDst, parentScope.get());
    2502 
    2503     m_scopeContextStack.removeLast();
    2504     m_localScopeDepth--;
     2499void BytecodeGenerator::emitPopScope(RegisterID* dst, RegisterID* scope)
     2500{
     2501    RefPtr<RegisterID> parentScope = emitGetParentScope(newTemporary(), scope);
     2502    emitMove(dst, parentScope.get());
     2503}
     2504
     2505void BytecodeGenerator::emitPopWithOrCatchScope(RegisterID* srcDst)
     2506{
     2507    emitPopScope(srcDst, srcDst);
     2508    popScopedControlFlowContext();
    25052509    SymbolTableStackEntry stackEntry = m_symbolTableStack.takeLast();
    25062510    RELEASE_ASSERT(stackEntry.m_isWithOrCatch);
     
    28162820void BytecodeGenerator::emitPopScopes(RegisterID* scope, int targetScopeDepth)
    28172821{
    2818     ASSERT(scopeDepth() - targetScopeDepth >= 0);
    2819 
    2820     size_t scopeDelta = scopeDepth() - targetScopeDepth;
     2822    ASSERT(labelScopeDepth() - targetScopeDepth >= 0);
     2823
     2824    size_t scopeDelta = labelScopeDepth() - targetScopeDepth;
    28212825    ASSERT(scopeDelta <= m_scopeContextStack.size());
    28222826    if (!scopeDelta)
     
    28772881int BytecodeGenerator::calculateTargetScopeDepthForExceptionHandler() const
    28782882{
    2879     int depth = m_localScopeDepth;
    2880 
    2881     for (unsigned i = m_symbolTableStack.size(); i--; ) {
    2882         RegisterID* scope = m_symbolTableStack[i].m_scope;
    2883         if (scope)
    2884             depth++;
    2885     }
     2883    int depth = localScopeDepth();
    28862884
    28872885    // Currently, we're maintaing compatibility with how things are done and letting the exception handling
    2888     // code take into consideration the base activation of the function. There is no reason we shouldn't 
     2886    // code take into consideration the base activation of the function. There is no reason we shouldn't
    28892887    // be able to calculate the exact depth here and let the exception handler not worry if there is a base
    28902888    // activation or not.
     
    28962894}
    28972895
    2898 int BytecodeGenerator::currentScopeDepth() const
    2899 {
    2900     // This is the current number of JSScope descendents that would be allocated
    2901     // in this function/program if this code were running.
    2902     int depth = 0;
    2903     for (unsigned i = m_symbolTableStack.size(); i--; ) {
    2904         if (m_symbolTableStack[i].m_scope || m_symbolTableStack[i].m_isWithOrCatch)
    2905             depth++;
    2906     }
    2907     return depth;
     2896int BytecodeGenerator::localScopeDepth() const
     2897{
     2898    return m_localScopeDepth;
     2899}
     2900
     2901int BytecodeGenerator::labelScopeDepth() const
     2902{
     2903    return localScopeDepth() + m_finallyDepth;
    29082904}
    29092905
     
    29312927}
    29322928
    2933 void BytecodeGenerator::emitPushCatchScope(RegisterID* dst, const Identifier& property, RegisterID* value, unsigned attributes)
     2929void BytecodeGenerator::pushScopedControlFlowContext()
    29342930{
    29352931    ControlFlowContext context;
     
    29372933    m_scopeContextStack.append(context);
    29382934    m_localScopeDepth++;
     2935}
     2936
     2937void BytecodeGenerator::popScopedControlFlowContext()
     2938{
     2939    ASSERT(m_scopeContextStack.size());
     2940    ASSERT(!m_scopeContextStack.last().isFinallyBlock);
     2941    m_scopeContextStack.removeLast();
     2942    m_localScopeDepth--;
     2943}
     2944
     2945void BytecodeGenerator::emitPushCatchScope(RegisterID* dst, const Identifier& property, RegisterID* value, unsigned attributes)
     2946{
     2947    pushScopedControlFlowContext();
    29392948
    29402949    emitOpcode(op_push_name_scope);
  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r186860 r186996  
    586586        void emitGetScope();
    587587        RegisterID* emitPushWithScope(RegisterID* dst, RegisterID* scope);
    588         void emitPopScope(RegisterID* srcDst);
     588        void emitPopScope(RegisterID* dst, RegisterID* scope);
     589        void emitPopWithOrCatchScope(RegisterID* srcDst);
    589590        RegisterID* emitGetParentScope(RegisterID* dst, RegisterID* scope);
    590591
    591592        void emitDebugHook(DebugHookID, unsigned line, unsigned charOffset, unsigned lineStart);
    592593
    593         int scopeDepth() { return m_localScopeDepth + m_finallyDepth; }
    594594        bool hasFinaliser() { return m_finallyDepth != 0; }
    595595
     
    625625        void popLexicalScope(VariableEnvironmentNode*);
    626626        void prepareLexicalScopeForNextForLoopIteration(VariableEnvironmentNode*, RegisterID* loopSymbolTable);
     627        int labelScopeDepth() const;
    627628
    628629    private:
     
    762763
    763764        int calculateTargetScopeDepthForExceptionHandler() const;
    764         int currentScopeDepth() const;
     765        int localScopeDepth() const;
     766        void pushScopedControlFlowContext();
     767        void popScopedControlFlowContext();
    765768
    766769        Vector<ControlFlowContext, 0, UnsafeVectorOverflow> m_scopeContextStack;
  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r186959 r186996  
    26282628    ASSERT(scope);
    26292629
    2630     if (generator.scopeDepth() != scope->scopeDepth())
     2630    if (generator.labelScopeDepth() != scope->scopeDepth())
    26312631        return 0;
    26322632
     
    26572657    ASSERT(scope);
    26582658
    2659     if (generator.scopeDepth() != scope->scopeDepth())
     2659    if (generator.labelScopeDepth() != scope->scopeDepth())
    26602660        return 0;
    26612661
     
    26912691        generator.emitTypeProfilerExpressionInfo(divotStart(), divotEnd());
    26922692    }
    2693     if (generator.scopeDepth()) {
     2693    if (generator.labelScopeDepth()) {
    26942694        returnRegister = generator.emitMove(generator.newTemporary(), returnRegister.get());
    26952695        generator.emitPopScopes(generator.scopeRegister(), 0);
     
    27152715    generator.emitPushWithScope(generator.scopeRegister(), scope.get());
    27162716    generator.emitNode(dst, m_statement);
    2717     generator.emitPopScope(generator.scopeRegister());
     2717    generator.emitPopWithOrCatchScope(generator.scopeRegister());
    27182718}
    27192719
     
    29682968        generator.emitProfileControlFlow(m_tryBlock->endOffset() + 1);
    29692969        generator.emitNode(dst, m_catchBlock);
    2970         generator.emitPopScope(generator.scopeRegister());
     2970        generator.emitPopWithOrCatchScope(generator.scopeRegister());
    29712971        generator.emitLabel(catchEndLabel.get());
    29722972    }
Note: See TracChangeset for help on using the changeset viewer.