Changeset 212640 in webkit

Timestamp:

Feb 20, 2017, 9:18:27 AM (8 years ago)

Author:

mark.lam@apple.com

Message:

BytecodeGenerator should not iterate its m_controlFlowScopeStack using a pointer bump.
​https://bugs.webkit.org/show_bug.cgi?id=168585

Reviewed by Yusuke Suzuki.

This is because m_controlFlowScopeStack is a SegmentedVector, and entries for
consecutive indices in the vector are not guaranteed to be consecutive in memory
layout. Instead, we should be using indexing instead.

This issue was detected by the marathon.js test from
​https://bugs.webkit.org/show_bug.cgi?id=168580.

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitJumpViaFinallyIfNeeded):
(JSC::BytecodeGenerator::emitReturnViaFinallyIfNeeded):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecompiler/BytecodeGenerator.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2017-02-19  Mark Lam  <mark.lam@apple.com>
+
+        BytecodeGenerator should not iterate its m_controlFlowScopeStack using a pointer bump.
+        https://bugs.webkit.org/show_bug.cgi?id=168585
+
+        Reviewed by Yusuke Suzuki.
+
+        This is because m_controlFlowScopeStack is a SegmentedVector, and entries for
+        consecutive indices in the vector are not guaranteed to be consecutive in memory
+        layout.  Instead, we should be using indexing instead.
+
+        This issue was detected by the marathon.js test from
+        https://bugs.webkit.org/show_bug.cgi?id=168580.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::emitJumpViaFinallyIfNeeded):
+        (JSC::BytecodeGenerator::emitReturnViaFinallyIfNeeded):
+
 2017-02-20  Manuel Rego Casasnovas  <rego@igalia.com>
 

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2008-2009, 2012-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Cameron Zwarich <cwzwarich@uwaterloo.ca>
  * Copyright (C) 2012 Igalia, S.L.
@@ -4823 +4823 @@
 {
     ASSERT(labelScopeDepth() - targetLabelScopeDepth >= 0);
-    size_t scopeDelta = labelScopeDepth() - targetLabelScopeDepth;
-    ASSERT(scopeDelta <= m_controlFlowScopeStack.size());
-    if (!scopeDelta)
-        return false; // No finallys to thread through.
-
-    ControlFlowScope* topScope = &m_controlFlowScopeStack.last();
-    ControlFlowScope* bottomScope = &m_controlFlowScopeStack.last() - scopeDelta;
+    size_t numberOfScopesToCheckForFinally = labelScopeDepth() - targetLabelScopeDepth;
+    ASSERT(numberOfScopesToCheckForFinally <= m_controlFlowScopeStack.size());
+    if (!numberOfScopesToCheckForFinally)
+        return false;
 
     FinallyContext* innermostFinallyContext = nullptr;
     FinallyContext* outermostFinallyContext = nullptr;
-    while (topScope > bottomScope) {
-        if (topScope->isFinallyScope()) {
-            FinallyContext* finallyContext = &topScope->finallyContext;
+    size_t scopeIndex = m_controlFlowScopeStack.size() - 1;
+    while (numberOfScopesToCheckForFinally--) {
+        ControlFlowScope* scope = &m_controlFlowScopeStack[scopeIndex--];
+        if (scope->isFinallyScope()) {
+            FinallyContext* finallyContext = &scope->finallyContext;
             if (!innermostFinallyContext)
                 innermostFinallyContext = finallyContext;
@@ -4841 +4840 @@
             finallyContext->incNumberOfBreaksOrContinues();
         }
-        --topScope;
     }
     if (!outermostFinallyContext)
@@ -4857 +4855 @@
 bool BytecodeGenerator::emitReturnViaFinallyIfNeeded(RegisterID* returnRegister)
 {
-    if (!m_controlFlowScopeStack.size())
-        return false; // No finallys to thread through.
-
-    ControlFlowScope* topScope = &m_controlFlowScopeStack.last();
-    ControlFlowScope* bottomScope = &m_controlFlowScopeStack.first();
+    size_t numberOfScopesToCheckForFinally = m_controlFlowScopeStack.size();
+    if (!numberOfScopesToCheckForFinally)
+        return false;
 
     FinallyContext* innermostFinallyContext = nullptr;
-    while (topScope >= bottomScope) {
-        if (topScope->isFinallyScope()) {
-            FinallyContext* finallyContext = &topScope->finallyContext;
+    while (numberOfScopesToCheckForFinally) {
+        size_t scopeIndex = --numberOfScopesToCheckForFinally;
+        ControlFlowScope* scope = &m_controlFlowScopeStack[scopeIndex];
+        if (scope->isFinallyScope()) {
+            FinallyContext* finallyContext = &scope->finallyContext;
             if (!innermostFinallyContext)
                 innermostFinallyContext = finallyContext;
             finallyContext->setHandlesReturns();
         }
-        --topScope;
     }
     if (!innermostFinallyContext)