[INFER] Propagate SSA stack eagerly to targets in switch and try blocks, bug 652646.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 26 Apr 2011 14:32:52 -0700
changeset 74980 e5068d17c8e381b390938f6183d7d9ae1bd87b96
parent 74979 8f0c5e12eba9a6e0e3b95487c730c60ec2024889
child 74981 b40247ae7dd53bd1c828ceafff037afdfbf15fb8
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs652646
milestone6.0a1
[INFER] Propagate SSA stack eagerly to targets in switch and try blocks, bug 652646.
js/src/jit-test/tests/basic/bug652646.js
js/src/jsanalyze.cpp
js/src/jsanalyze.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug652646.js
@@ -0,0 +1,11 @@
+function foo() {
+  try {
+    return 0;
+  } catch (e) {
+    try {
+      return 1;
+    } catch (e) {
+    }
+  }
+}
+foo();
--- a/js/src/jsanalyze.cpp
+++ b/js/src/jsanalyze.cpp
@@ -1416,55 +1416,90 @@ ScriptAnalysis::analyzeSSA(JSContext *cx
           case JSOP_INITMETHOD:
             stack[stackDepth - 1] = code->poppedValues[1];
             break;
 
           case JSOP_INITELEM:
             stack[stackDepth - 1] = code->poppedValues[2];
             break;
 
+          /*
+           * Switch and try blocks preserve the stack between the original op
+           * and all case statements or exception/finally handlers. Even though
+           * we don't track the values of variables in scripts containing such
+           * switch and try blocks, propagate the stack now to all targets as
+           * the values on the stack may be popped by intervening cleanup ops
+           * (e.g. LEAVEBLOCK, ENDITER).
+           */
+
+          case JSOP_TABLESWITCH:
+          case JSOP_TABLESWITCHX: {
+            jsbytecode *pc2 = pc;
+            unsigned jmplen = (op == JSOP_TABLESWITCH) ? JUMP_OFFSET_LEN : JUMPX_OFFSET_LEN;
+            unsigned defaultOffset = offset + GetJumpOffset(pc, pc2);
+            pc2 += jmplen;
+            jsint low = GET_JUMP_OFFSET(pc2);
+            pc2 += JUMP_OFFSET_LEN;
+            jsint high = GET_JUMP_OFFSET(pc2);
+            pc2 += JUMP_OFFSET_LEN;
+
+            checkBranchTarget(cx, defaultOffset, branchTargets, values, stackDepth);
+
+            for (jsint i = low; i <= high; i++) {
+                unsigned targetOffset = offset + GetJumpOffset(pc, pc2);
+                if (targetOffset != offset)
+                    checkBranchTarget(cx, targetOffset, branchTargets, values, stackDepth);
+                pc2 += jmplen;
+            }
+            break;
+          }
+
+          case JSOP_LOOKUPSWITCH:
+          case JSOP_LOOKUPSWITCHX: {
+            jsbytecode *pc2 = pc;
+            unsigned jmplen = (op == JSOP_LOOKUPSWITCH) ? JUMP_OFFSET_LEN : JUMPX_OFFSET_LEN;
+            unsigned defaultOffset = offset + GetJumpOffset(pc, pc2);
+            pc2 += jmplen;
+            unsigned npairs = GET_UINT16(pc2);
+            pc2 += UINT16_LEN;
+
+            checkBranchTarget(cx, defaultOffset, branchTargets, values, stackDepth);
+
+            while (npairs) {
+                pc2 += INDEX_LEN;
+                unsigned targetOffset = offset + GetJumpOffset(pc, pc2);
+                checkBranchTarget(cx, targetOffset, branchTargets, values, stackDepth);
+                pc2 += jmplen;
+                npairs--;
+            }
+            break;
+          }
+
+          case JSOP_TRY: { 
+            JSTryNote *tn = script->trynotes()->vector;
+            JSTryNote *tnlimit = tn + script->trynotes()->length;
+            for (; tn < tnlimit; tn++) {
+                unsigned startOffset = script->main - script->code + tn->start;
+                if (startOffset == offset + 1) {
+                    unsigned catchOffset = startOffset + tn->length;
+
+                    if (tn->kind != JSTRY_ITER)
+                        checkBranchTarget(cx, catchOffset, branchTargets, values, stackDepth);
+                }
+            }
+            break;
+          }
+
           default:;
         }
 
         uint32 type = JOF_TYPE(js_CodeSpec[op].format);
         if (type == JOF_JUMP || type == JOF_JUMPX) {
             unsigned targetOffset = FollowBranch(script, offset);
-
-            unsigned targetDepth = getCode(targetOffset).stackDepth;
-            JS_ASSERT(targetDepth <= stackDepth);
-
-            /*
-             * If there is already an active branch to target, make sure its
-             * pending values reflect any changes made since the first branch.
-             * Otherwise, add a new pending branch and determine its pending
-             * values lazily.
-             */
-            Vector<SlotValue> *&pending = getCode(targetOffset).pendingValues;
-            if (pending) {
-                for (unsigned i = 0; i < pending->length(); i++) {
-                    SlotValue &v = (*pending)[i];
-                    mergeValue(cx, targetOffset, values[v.slot], &v);
-                }
-            } else {
-                JS_ASSERT(targetOffset > offset);
-                pending = cx->new_< Vector<SlotValue> >(cx);
-                if (!pending || !branchTargets.append(targetOffset)) {
-                    setOOM(cx);
-                    return;
-                }
-            }
-
-            /*
-             * Make sure there is a pending entry for each value on the stack.
-             * The number of stack entries at join points is usually zero, and
-             * we don't want to look at the active branches while popping and
-             * pushing values in each opcode.
-             */
-            for (unsigned i = 0; i < targetDepth; i++)
-                checkPendingValue(cx, stack[i], StackSlot(script, i), pending);
+            checkBranchTarget(cx, targetOffset, branchTargets, values, stackDepth);
 
             /*
              * If this is a back edge, we're done with the loop and can freeze
              * the phi values at the head now.
              */
             if (targetOffset < offset)
                 freezeNewValues(cx, targetOffset);
         }
@@ -1536,31 +1571,19 @@ ScriptAnalysis::insertPhi(JSContext *cx,
     node->options = newOptions;
     node->options[node->length++] = v;
 }
 
 inline void
 ScriptAnalysis::mergeValue(JSContext *cx, uint32 offset, const SSAValue &v, SlotValue *pv)
 {
     /* Make sure that v is accounted for in the pending value or phi value at pv. */
+    JS_ASSERT(v.kind() != SSAValue::EMPTY && pv->value.kind() != SSAValue::EMPTY);
 
-    /*
-     * Note: it would be nice to assert that v is also non-empty, but this can
-     * crop up when handling switch and try blocks. We don't track SSA values
-     * for variables in scripts containing these opcodes, and don't propagate
-     * the stack to all targets of the switch or try, under the assumption that
-     * code within the switch/try won't modify the stack. This assumption is
-     * not valid, however, when the script contains return statements within
-     * 'for in' blocks which then pop the stack with an ENDITER and clear out
-     * its contents before we get to the switch/try branch targets. This should
-     * be cleaned up.
-     */
-    JS_ASSERT(pv->value.kind() != SSAValue::EMPTY);
-
-    if (v.equals(pv->value) || v.kind() == SSAValue::EMPTY)
+    if (v.equals(pv->value))
         return;
 
     if (pv->value.kind() != SSAValue::PHI || pv->value.phiOffset() < offset) {
         SSAValue ov = pv->value;
         if (makePhi(cx, pv->slot, offset, &pv->value)) {
             insertPhi(cx, pv->value, v);
             insertPhi(cx, pv->value, ov);
         }
@@ -1575,23 +1598,60 @@ void
 ScriptAnalysis::checkPendingValue(JSContext *cx, const SSAValue &v, uint32 slot,
                                   Vector<SlotValue> *pending)
 {
     for (unsigned i = 0; i < pending->length(); i++) {
         if ((*pending)[i].slot == slot)
             return;
     }
 
-    JS_ASSERT(v.kind() != SSAValue::EMPTY);
-
     if (!pending->append(SlotValue(slot, v)))
         setOOM(cx);
 }
 
 void
+ScriptAnalysis::checkBranchTarget(JSContext *cx, uint32 targetOffset,
+                                  Vector<uint32> &branchTargets,
+                                  SSAValue *values, uint32 stackDepth)
+{
+    unsigned targetDepth = getCode(targetOffset).stackDepth;
+    JS_ASSERT(targetDepth <= stackDepth);
+
+    /*
+     * If there is already an active branch to target, make sure its pending
+     * values reflect any changes made since the first branch. Otherwise, add a
+     * new pending branch and determine its pending values lazily.
+     */
+    Vector<SlotValue> *&pending = getCode(targetOffset).pendingValues;
+    if (pending) {
+        for (unsigned i = 0; i < pending->length(); i++) {
+            SlotValue &v = (*pending)[i];
+            mergeValue(cx, targetOffset, values[v.slot], &v);
+        }
+    } else {
+        pending = cx->new_< Vector<SlotValue> >(cx);
+        if (!pending || !branchTargets.append(targetOffset)) {
+            setOOM(cx);
+            return;
+        }
+    }
+
+    /*
+     * Make sure there is a pending entry for each value on the stack.
+     * The number of stack entries at join points is usually zero, and
+     * we don't want to look at the active branches while popping and
+     * pushing values in each opcode.
+     */
+    for (unsigned i = 0; i < targetDepth; i++) {
+        uint32 slot = StackSlot(script, i);
+        checkPendingValue(cx, values[slot], slot, pending);
+    }
+}
+
+void
 ScriptAnalysis::mergeBranchTarget(JSContext *cx, const SSAValue &value, uint32 slot,
                                   const Vector<uint32> &branchTargets)
 {
     if (slot >= numSlots) {
         /*
          * There is no need to lazily check that there are pending values at
          * branch targets for slots on the stack, these are added to pending
          * eagerly.
--- a/js/src/jsanalyze.h
+++ b/js/src/jsanalyze.h
@@ -1034,16 +1034,18 @@ class ScriptAnalysis
     inline void extendVariable(JSContext *cx, LifetimeVariable &var, unsigned start, unsigned end);
 
     /* SSA helpers */
     bool makePhi(JSContext *cx, uint32 slot, uint32 offset, SSAValue *pv);
     void insertPhi(JSContext *cx, SSAValue &phi, const SSAValue &v);
     void mergeValue(JSContext *cx, uint32 offset, const SSAValue &v, SlotValue *pv);
     void checkPendingValue(JSContext *cx, const SSAValue &v, uint32 slot,
                            Vector<SlotValue> *pending);
+    void checkBranchTarget(JSContext *cx, uint32 targetOffset, Vector<uint32> &branchTargets,
+                           SSAValue *values, uint32 stackDepth);
     void mergeBranchTarget(JSContext *cx, const SSAValue &value, uint32 slot,
                            const Vector<uint32> &branchTargets);
     void removeBranchTarget(Vector<uint32> &branchTargets, uint32 offset);
     void freezeNewValues(JSContext *cx, uint32 offset);
 
     struct TypeInferenceState {
         Vector<SSAPhiNode *> phiNodes;
         bool hasGetSet;