Handle extended indexes around JSOP_*BLOCKCHAIN (610026, r=billm).
authorBrendan Eich <brendan@mozilla.org>
Fri, 05 Nov 2010 15:03:39 -0700
changeset 57755 805c1a5d5cc690aecad5ce81a09522ee7a34fa9a
parent 57754 e2f64f43c7e1ec2c0328b18e7644192da297a29d
child 57756 2fd60328c2b0d40107aa29f09ada785a4b44d6d4
push id17032
push userrsayre@mozilla.com
push dateWed, 17 Nov 2010 21:55:39 +0000
treeherdermozilla-central@78a42f77bb90 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs610026
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Handle extended indexes around JSOP_*BLOCKCHAIN (610026, r=billm).
js/src/jsemit.cpp
js/src/jsinterp.cpp
js/src/jsopcode.cpp
js/src/jsopcode.tbl
js/src/jsopcodeinlines.h
js/src/jstracer.cpp
js/src/methodjit/Compiler.cpp
js/src/tests/js1_8_5/regress/jstests.list
js/src/tests/js1_8_5/regress/regress-610026.js
--- a/js/src/jsemit.cpp
+++ b/js/src/jsemit.cpp
@@ -1524,18 +1524,17 @@ EmitNonLocalJumpFixup(JSContext *cx, JSC
 #undef FLUSH_POPS
 }
 
 static JSBool
 EmitKnownBlockChain(JSContext *cx, JSCodeGenerator *cg, JSObjectBox *box)
 {
     if (box)
         return EmitIndexOp(cx, JSOP_BLOCKCHAIN, box->index, cg);
-    else
-        return js_Emit1(cx, cg, JSOP_NULLBLOCKCHAIN) >= 0;
+    return js_Emit1(cx, cg, JSOP_NULLBLOCKCHAIN) >= 0;
 }
 
 static JSBool
 EmitBlockChain(JSContext *cx, JSCodeGenerator *cg)
 {
     return EmitKnownBlockChain(cx, cg, cg->blockChainBox);
 }
 
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -197,46 +197,47 @@ js::GetScopeChain(JSContext *cx)
  * require bytecode scanning appears below.
  */
 JSObject *
 js::GetBlockChain(JSContext *cx, JSStackFrame *fp)
 {
     if (!fp->isScriptFrame())
         return NULL;
 
+    /* Assume that imacros don't affect blockChain */
+    jsbytecode *target = fp->hasImacropc() ? fp->imacropc() : fp->pc(cx);
+
     JSScript *script = fp->script();
     jsbytecode *start = script->code;
-    /* Assume that imacros don't affect blockChain */
-    jsbytecode *pc = fp->hasImacropc() ? fp->imacropc() : fp->pc(cx);
-
-    JS_ASSERT(pc >= start && pc < script->code + script->length);
+    JS_ASSERT(target >= start && target < start + script->length);
 
     JSObject *blockChain = NULL;
-    if (*pc == JSOP_BLOCKCHAIN) {
-        blockChain = script->getObject(GET_INDEX(pc));
-    } else if (*pc == JSOP_NULLBLOCKCHAIN) {
-        blockChain = NULL;
-    } else {
-        ptrdiff_t oplen;
-        for (jsbytecode *p = start; p < pc; p += oplen) {
-            JSOp op = js_GetOpcode(cx, script, p);
-            const JSCodeSpec *cs = &js_CodeSpec[op];
-            oplen = cs->length;
-            if (oplen < 0)
-                oplen = js_GetVariableBytecodeLength(p);
-
-            if (op == JSOP_ENTERBLOCK)
-                blockChain = script->getObject(GET_INDEX(p));
-            else if (op == JSOP_LEAVEBLOCK || op == JSOP_LEAVEBLOCKEXPR)
-                blockChain = blockChain->getParent();
-            else if (op == JSOP_BLOCKCHAIN)
-                blockChain = script->getObject(GET_INDEX(p));
-            else if (op == JSOP_NULLBLOCKCHAIN)
-                blockChain = NULL;
-        }
+    uintN indexBase = 0;
+    ptrdiff_t oplen;
+    for (jsbytecode *pc = start; pc < target; pc += oplen) {
+        JSOp op = js_GetOpcode(cx, script, pc);
+        const JSCodeSpec *cs = &js_CodeSpec[op];
+        oplen = cs->length;
+        if (oplen < 0)
+            oplen = js_GetVariableBytecodeLength(pc);
+
+        if (op == JSOP_INDEXBASE)
+            indexBase = GET_INDEXBASE(pc);
+        else if (op == JSOP_INDEXBASE1 || op == JSOP_INDEXBASE2 || op == JSOP_INDEXBASE3)
+            indexBase = (op - JSOP_INDEXBASE1 + 1) << 16;
+        else if (op == JSOP_RESETBASE || op == JSOP_RESETBASE0)
+            indexBase = 0;
+        else if (op == JSOP_ENTERBLOCK)
+            blockChain = script->getObject(indexBase + GET_INDEX(pc));
+        else if (op == JSOP_LEAVEBLOCK || op == JSOP_LEAVEBLOCKEXPR)
+            blockChain = blockChain->getParent();
+        else if (op == JSOP_BLOCKCHAIN)
+            blockChain = script->getObject(indexBase + GET_INDEX(pc));
+        else if (op == JSOP_NULLBLOCKCHAIN)
+            blockChain = NULL;
     }
 
     return blockChain;
 }
 
 /*
  * This function computes the current blockChain, but only in
  * the special case where a BLOCKCHAIN or NULLBLOCKCHAIN
@@ -244,39 +245,29 @@ js::GetBlockChain(JSContext *cx, JSStack
  * We ensure this happens for a few important ops like DEFFUN.
  * |oplen| is the length of opcode at the current PC.
  */
 JSObject *
 js::GetBlockChainFast(JSContext *cx, JSStackFrame *fp, JSOp op, size_t oplen)
 {
     /* Assume that we're in a script frame. */
     jsbytecode *pc = fp->pc(cx);
-
-    /* The fast path. */
-    if (pc[oplen] == JSOP_NULLBLOCKCHAIN) {
-        JS_ASSERT(js_GetOpcode(cx, fp->script(), pc) == op);
+    JS_ASSERT(js_GetOpcode(cx, fp->script(), pc) == op);
+
+    pc += oplen;
+    op = JSOp(*pc);
+    JS_ASSERT(js_GetOpcode(cx, fp->script(), pc) == op);
+
+    /* The fast paths assume no JSOP_RESETBASE/INDEXBASE noise. */
+    if (op == JSOP_NULLBLOCKCHAIN)
         return NULL;
-    }
-
-    JSScript *script = fp->script();
-
-    JS_ASSERT(js_GetOpcode(cx, script, pc) == op);
-
-    JSObject *blockChain;
-    JSOp opNext = js_GetOpcode(cx, script, pc + oplen);
-    if (opNext == JSOP_BLOCKCHAIN) {
-        blockChain = script->getObject(GET_INDEX(pc + oplen));
-    } else if (opNext == JSOP_NULLBLOCKCHAIN) {
-        blockChain = NULL;
-    } else {
-        blockChain = NULL; /* appease gcc */
-        JS_NOT_REACHED("invalid opcode for fast block chain access");
-    }
-
-    return blockChain;
+    if (op == JSOP_BLOCKCHAIN)
+        return fp->script()->getObject(GET_INDEX(pc));
+
+    return GetBlockChain(cx, fp);
 }
 
 /*
  * We can't determine in advance which local variables can live on the stack and
  * be freed when their dynamic scope ends, and which will be closed over and
  * need to live in the heap.  So we place variables on the stack initially, note
  * when they are closed over, and copy those that are out to the heap when we
  * leave their dynamic scope.
@@ -5634,17 +5625,17 @@ BEGIN_CASE(JSOP_LAMBDA)
 
     /* do-while(0) so we can break instead of using a goto. */
     do {
         JSObject *parent;
         if (FUN_NULL_CLOSURE(fun)) {
             parent = &regs.fp->scopeChain();
 
             if (obj->getParent() == parent) {
-                jsbytecode *pc2 = js_AdvanceOverBlockchain(regs.pc + JSOP_LAMBDA_LENGTH);
+                jsbytecode *pc2 = AdvanceOverBlockchainOp(regs.pc + JSOP_LAMBDA_LENGTH);
                 JSOp op2 = JSOp(*pc2);
 
                 /*
                  * Optimize var obj = {method: function () { ... }, ...},
                  * this.method = function () { ... }; and other significant
                  * single-use-of-null-closure bytecode sequences.
                  *
                  * WARNING: code in TraceRecorder::record_JSOP_LAMBDA must
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -4104,20 +4104,19 @@ Decompile(SprintStack *ss, jsbytecode *p
 
                     /*
                      * Advance over this op and its global |this| push, and
                      * arrange to advance over the call to this lambda.
                      */
                     pc += len;
                     if (*pc == JSOP_BLOCKCHAIN) {
                         pc += JSOP_BLOCKCHAIN_LENGTH;
-                    } else if (*pc == JSOP_NULLBLOCKCHAIN) {
+                    } else {
+                        LOCAL_ASSERT(*pc == JSOP_NULLBLOCKCHAIN);
                         pc += JSOP_NULLBLOCKCHAIN_LENGTH;
-                    } else {
-                        JS_NOT_REACHED("should see block chain operation");
                     }
                     LOCAL_ASSERT(*pc == JSOP_PUSH);
                     pc += JSOP_PUSH_LENGTH;
                     LOCAL_ASSERT(*pc == JSOP_CALL);
                     LOCAL_ASSERT(GET_ARGC(pc) == 0);
                     len = JSOP_CALL_LENGTH;
 
                     /*
--- a/js/src/jsopcode.tbl
+++ b/js/src/jsopcode.tbl
@@ -450,17 +450,17 @@ OPDEF(JSOP_DELDESC,       187,"deldesc",
 OPDEF(JSOP_UINT24,        188,"uint24",     NULL,     4,  0,  1, 16,  JOF_UINT24)
 
 /*
  * Opcodes to allow 24-bit atom or object indexes. Whenever an index exceeds
  * the 16-bit limit, the index-accessing bytecode must be bracketed by
  * JSOP_INDEXBASE and JSOP_RESETBASE to provide the upper bits of the index.
  * See jsemit.c, EmitIndexOp.
  */
-OPDEF(JSOP_INDEXBASE,     189,"atombase",   NULL,     2,  0,  0,  0,  JOF_UINT8|JOF_INDEXBASE)
+OPDEF(JSOP_INDEXBASE,     189,"indexbase",  NULL,     2,  0,  0,  0,  JOF_UINT8|JOF_INDEXBASE)
 OPDEF(JSOP_RESETBASE,     190,"resetbase",  NULL,     1,  0,  0,  0,  JOF_BYTE)
 OPDEF(JSOP_RESETBASE0,    191,"resetbase0", NULL,     1,  0,  0,  0,  JOF_BYTE)
 
 /*
  * Opcodes to help the decompiler deal with XML.
  */
 OPDEF(JSOP_STARTXML,      192,"startxml",    NULL,    1,  0,  0,  0,  JOF_BYTE)
 OPDEF(JSOP_STARTXMLEXPR,  193,"startxmlexpr",NULL,    1,  0,  0,  0,  JOF_BYTE)
@@ -527,19 +527,19 @@ OPDEF(JSOP_LEAVEBLOCKEXPR,208,"leavebloc
 OPDEF(JSOP_GETTHISPROP,   209,"getthisprop",   NULL,  3,  0,  1, 18,  JOF_ATOM|JOF_VARPROP)
 OPDEF(JSOP_GETARGPROP,    210,"getargprop",    NULL,  5,  0,  1, 18,  JOF_SLOTATOM|JOF_VARPROP)
 OPDEF(JSOP_GETLOCALPROP,  211,"getlocalprop",  NULL,  5,  0,  1, 18,  JOF_SLOTATOM|JOF_VARPROP)
 
 /*
  * Optimize atom segments 1-3.  These must be followed by JSOP_RESETBASE0 after
  * the opcode that they prefix.
  */
-OPDEF(JSOP_INDEXBASE1,    212,"atombase1",     NULL,  1,  0,  0,  0,  JOF_BYTE |JOF_INDEXBASE)
-OPDEF(JSOP_INDEXBASE2,    213,"atombase2",     NULL,  1,  0,  0,  0,  JOF_BYTE |JOF_INDEXBASE)
-OPDEF(JSOP_INDEXBASE3,    214,"atombase3",     NULL,  1,  0,  0,  0,  JOF_BYTE |JOF_INDEXBASE)
+OPDEF(JSOP_INDEXBASE1,    212,"indexbase1",    NULL,  1,  0,  0,  0,  JOF_BYTE |JOF_INDEXBASE)
+OPDEF(JSOP_INDEXBASE2,    213,"indexbase2",    NULL,  1,  0,  0,  0,  JOF_BYTE |JOF_INDEXBASE)
+OPDEF(JSOP_INDEXBASE3,    214,"indexbase3",    NULL,  1,  0,  0,  0,  JOF_BYTE |JOF_INDEXBASE)
 
 OPDEF(JSOP_CALLGNAME,     215, "callgname",    NULL,  3,  0,  2, 19,  JOF_ATOM|JOF_NAME|JOF_CALLOP|JOF_GNAME)
 OPDEF(JSOP_CALLLOCAL,     216, "calllocal",    NULL,  3,  0,  2, 19,  JOF_LOCAL|JOF_NAME|JOF_CALLOP)
 OPDEF(JSOP_CALLARG,       217, "callarg",      NULL,  3,  0,  2, 19,  JOF_QARG |JOF_NAME|JOF_CALLOP)
 OPDEF(JSOP_BINDGNAME,     218, "bindgname",    NULL,  3,  0,  1,  0,  JOF_ATOM|JOF_NAME|JOF_SET|JOF_GNAME)
 
 /*
  * Opcodes to hold 8-bit and 32-bit immediate integer operands.
--- a/js/src/jsopcodeinlines.h
+++ b/js/src/jsopcodeinlines.h
@@ -8,22 +8,21 @@
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
  * Software distributed under the License is distributed on an "AS IS" basis,
  * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
  * for the specific language governing rights and limitations under the
  * License.
  *
- * The Original Code is Mozilla Communicator client code, released
- * March 31, 1998.
+ * The Original Code is the Mozilla SpiderMonkey JaegerMonkey implementation
  *
  * The Initial Developer of the Original Code is
- * Netscape Communications Corporation.
- * Portions created by the Initial Developer are Copyright (C) 1998
+ *   Mozilla Foundation
+ * Portions created by the Initial Developer are Copyright (C) 2002-2010
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
  * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
@@ -34,18 +33,25 @@
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "jsautooplen.h"
 
+namespace js {
+
+/* 
+ * Warning: this does not skip JSOP_RESETBASE* or JSOP_INDEXBASE* ops, so it is
+ * useful only when checking for optimization opportunities.
+ */
 JS_ALWAYS_INLINE jsbytecode *
-js_AdvanceOverBlockchain(jsbytecode *pc)
+AdvanceOverBlockchainOp(jsbytecode *pc)
 {
     if (*pc == JSOP_NULLBLOCKCHAIN)
         return pc + JSOP_NULLBLOCKCHAIN_LENGTH;
-    else if (*pc == JSOP_BLOCKCHAIN)
+    if (*pc == JSOP_BLOCKCHAIN)
         return pc + JSOP_BLOCKCHAIN_LENGTH;
-    else
-        return pc;
+    return pc;
 }
+
+}
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -14842,17 +14842,17 @@ TraceRecorder::record_JSOP_LAMBDA()
      * See jsinterp.cpp, the JSOP_LAMBDA null closure case. The JSOP_SETMETHOD and
      * JSOP_INITMETHOD logic governing the early ARECORD_CONTINUE returns below
      * must agree with the corresponding break-from-do-while(0) logic there.
      */
     if (FUN_NULL_CLOSURE(fun)) {
         if (FUN_OBJECT(fun)->getParent() != globalObj)
             RETURN_STOP_A("Null closure function object parent must be global object");
 
-        jsbytecode *pc2 = js_AdvanceOverBlockchain(cx->regs->pc + JSOP_LAMBDA_LENGTH);
+        jsbytecode *pc2 = AdvanceOverBlockchainOp(cx->regs->pc + JSOP_LAMBDA_LENGTH);
         JSOp op2 = JSOp(*pc2);
 
         if (op2 == JSOP_INITMETHOD) {
             stack(0, w.immpObjGC(FUN_OBJECT(fun)));
             return ARECORD_CONTINUE;
         }
 
         if (op2 == JSOP_SETMETHOD) {
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -1714,17 +1714,17 @@ mjit::Compiler::generateMethod()
 
           BEGIN_CASE(JSOP_LAMBDA)
           {
             JSFunction *fun = script->getFunction(fullAtomIndex(PC));
 
             JSObjStubFun stub = stubs::Lambda;
             uint32 uses = 0;
 
-            jsbytecode *pc2 = js_AdvanceOverBlockchain(PC + JSOP_LAMBDA_LENGTH);
+            jsbytecode *pc2 = AdvanceOverBlockchainOp(PC + JSOP_LAMBDA_LENGTH);
             JSOp next = JSOp(*pc2);
             
             if (next == JSOP_INITMETHOD) {
                 stub = stubs::LambdaForInit;
             } else if (next == JSOP_SETMETHOD) {
                 stub = stubs::LambdaForSet;
                 uses = 1;
             } else if (fun->joinable()) {
--- a/js/src/tests/js1_8_5/regress/jstests.list
+++ b/js/src/tests/js1_8_5/regress/jstests.list
@@ -45,9 +45,10 @@ script regress-597870.js
 fails-if(!xulRuntime.shell) script regress-597945-1.js
 script regress-597945-2.js
 script regress-598176.js
 script regress-600067.js
 script regress-600137.js
 script regress-601399.js
 script regress-602621.js
 fails-if(!xulRuntime.shell) script regress-607863.js
+script regress-610026.js
 script regress-609617.js
new file mode 100644
--- /dev/null
+++ b/js/src/tests/js1_8_5/regress/regress-610026.js
@@ -0,0 +1,65 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var expect = "pass";
+var actual;
+
+/*
+ * We hardcode here that GenerateBlockId limits a program to 2^20 blocks. Start
+ * with 2^19 blocks, then test 2^20 - 1 blocks, finally test the limit.
+ */
+var s = "{}";
+for (var i = 0; i < 19; i++)
+    s += s;
+
+try {
+    eval(s);
+    actual = "pass";
+} catch (e) {
+    actual = "fail: " + e;
+}
+
+assertEq(actual, expect);
+
+s += s.slice(0, -2);
+
+try {
+    eval(s);
+    actual = "pass";
+} catch (e) {
+    actual = "fail: " + e;
+}
+
+assertEq(actual, expect);
+
+s += "{}";
+
+try {
+    eval(s);
+    actual = "fail: expected InternalError: program too large";
+} catch (e) {
+    actual = (e.message == "program too large") ? "pass" : "fail: " + e;
+}
+
+assertEq(actual, expect);
+
+/* Make 64K blocks in a row, each with two vars, the second one named x. */
+s = "{let y, x;}";
+for (i = 0; i < 16; i++)
+    s += s;
+
+/* Now append code to alias block 0 and botch a JS_NOT_REACHED or get the wrong x. */
+s += "var g; { let x = 42; g = function() { return x; }; x = x; }";
+
+try {
+    eval(s);
+    actual = g();
+} catch (e) {
+    actual = e;
+}
+assertEq(actual, 42);
+
+reportCompare(0, 0, "ok");