Bug 916612 - Move the too-many args+vars checks (r=wingo)
authorLuke Wagner <luke@mozilla.com>
Thu, 16 Jan 2014 11:02:01 -0600
changeset 163755 a15cee5da933490eb4a88895f0d4e1e1e4c44a84
parent 163754 9845c94f44ffe5068478b595bb313540a1cdb04c
child 163756 556ddac71fd823521e8daa0b495ab7f2b292bf0a
push id38563
push userlwagner@mozilla.com
push dateThu, 16 Jan 2014 19:51:55 +0000
treeherdermozilla-inbound@d2eca1d56402 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswingo
bugs916612
milestone29.0a1
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
Bug 916612 - Move the too-many args+vars checks (r=wingo)
js/src/frontend/Parser.cpp
js/src/jsopcode.h
js/src/jsscript.cpp
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -144,31 +144,39 @@ ParseContext<FullParseHandler>::define(T
       case Definition::ARG:
         JS_ASSERT(sc->isFunctionBox());
         dn->setOp(JSOP_GETARG);
         dn->pn_dflags |= PND_BOUND;
         if (!dn->pn_cookie.set(ts, staticLevel, args_.length()))
             return false;
         if (!args_.append(dn))
             return false;
+        if (args_.length() >= ARGNO_LIMIT) {
+            ts.reportError(JSMSG_TOO_MANY_FUN_ARGS);
+            return false;
+        }
         if (name == ts.names().empty)
             break;
         if (!decls_.addUnique(name, dn))
             return false;
         break;
 
       case Definition::CONST:
       case Definition::VAR:
         if (sc->isFunctionBox()) {
             dn->setOp(JSOP_GETLOCAL);
             dn->pn_dflags |= PND_BOUND;
             if (!dn->pn_cookie.set(ts, staticLevel, vars_.length()))
                 return false;
             if (!vars_.append(dn))
                 return false;
+            if (vars_.length() >= SLOTNO_LIMIT) {
+                ts.reportError(JSMSG_TOO_MANY_LOCALS);
+                return false;
+            }
         }
         if (!decls_.addUnique(name, dn))
             return false;
         break;
 
       case Definition::LET:
         dn->setOp(JSOP_GETLOCAL);
         dn->pn_dflags |= (PND_LET | PND_BOUND);
@@ -190,18 +198,24 @@ ParseContext<SyntaxParseHandler>::define
                                          Definition::Kind kind)
 {
     JS_ASSERT(!decls_.lookupFirst(name));
 
     if (lexdeps.lookupDefn<SyntaxParseHandler>(name))
         lexdeps->remove(name);
 
     // Keep track of the number of arguments in args_, for fun->nargs.
-    if (kind == Definition::ARG && !args_.append((Definition *) nullptr))
-        return false;
+    if (kind == Definition::ARG) {
+        if (!args_.append((Definition *) nullptr))
+            return false;
+        if (args_.length() >= ARGNO_LIMIT) {
+            ts.reportError(JSMSG_TOO_MANY_FUN_ARGS);
+            return false;
+        }
+    }
 
     return decls_.addUnique(name, kind);
 }
 
 template <typename ParseHandler>
 void
 ParseContext<ParseHandler>::prepareToAddDuplicateArg(HandlePropertyName name, DefinitionNode prevDecl)
 {
@@ -286,16 +300,18 @@ AppendPackedBindings(const ParseContext<
 }
 
 template <typename ParseHandler>
 bool
 ParseContext<ParseHandler>::generateFunctionBindings(ExclusiveContext *cx, LifoAlloc &alloc,
                                                      InternalHandle<Bindings*> bindings) const
 {
     JS_ASSERT(sc->isFunctionBox());
+    JS_ASSERT(args_.length() < ARGNO_LIMIT);
+    JS_ASSERT(vars_.length() < SLOTNO_LIMIT);
 
     unsigned count = args_.length() + vars_.length();
     Binding *packedBindings = alloc.newArrayUninitialized<Binding>(count);
     if (!packedBindings) {
         js_ReportOutOfMemory(cx);
         return false;
     }
 
--- a/js/src/jsopcode.h
+++ b/js/src/jsopcode.h
@@ -186,23 +186,21 @@ SET_UINT32_INDEX(jsbytecode *pc, uint32_
                                  (pc)[2] = (jsbytecode)(uint32_t(i) >> 16),   \
                                  (pc)[3] = (jsbytecode)(uint32_t(i) >> 8),    \
                                  (pc)[4] = (jsbytecode)uint32_t(i))
 
 /* Index limit is determined by SN_3BYTE_OFFSET_FLAG, see frontend/BytecodeEmitter.h. */
 #define INDEX_LIMIT_LOG2        23
 #define INDEX_LIMIT             (uint32_t(1) << INDEX_LIMIT_LOG2)
 
-/* Actual argument count operand format helpers. */
 #define ARGC_HI(argc)           UINT16_HI(argc)
 #define ARGC_LO(argc)           UINT16_LO(argc)
 #define GET_ARGC(pc)            GET_UINT16(pc)
 #define ARGC_LIMIT              UINT16_LIMIT
 
-/* Synonyms for quick JOF_QARG and JOF_LOCAL bytecodes. */
 #define GET_ARGNO(pc)           GET_UINT16(pc)
 #define SET_ARGNO(pc,argno)     SET_UINT16(pc,argno)
 #define ARGNO_LEN               2
 #define ARGNO_LIMIT             UINT16_LIMIT
 
 #define GET_SLOTNO(pc)          GET_UINT16(pc)
 #define SET_SLOTNO(pc,varno)    SET_UINT16(pc,varno)
 #define SLOTNO_LEN              2
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -65,28 +65,21 @@ Bindings::argumentsVarIndex(ExclusiveCon
 
 bool
 Bindings::initWithTemporaryStorage(ExclusiveContext *cx, InternalBindingsHandle self,
                                    unsigned numArgs, unsigned numVars,
                                    Binding *bindingArray)
 {
     JS_ASSERT(!self->callObjShape_);
     JS_ASSERT(self->bindingArrayAndFlag_ == TEMPORARY_STORAGE_BIT);
-
-    if (numArgs > UINT16_MAX || numVars > UINT16_MAX) {
-        if (cx->isJSContext()) {
-            JS_ReportErrorNumber(cx->asJSContext(), js_GetErrorMessage, nullptr,
-                                 self->numArgs_ > self->numVars_ ?
-                                 JSMSG_TOO_MANY_FUN_ARGS :
-                                 JSMSG_TOO_MANY_LOCALS);
-        }
-        return false;
-    }
-
     JS_ASSERT(!(uintptr_t(bindingArray) & TEMPORARY_STORAGE_BIT));
+    JS_ASSERT(numArgs <= ARGC_LIMIT);
+    JS_ASSERT(numVars <= SLOTNO_LIMIT);
+    JS_ASSERT(UINT32_MAX - numArgs >= numVars);
+
     self->bindingArrayAndFlag_ = uintptr_t(bindingArray) | TEMPORARY_STORAGE_BIT;
     self->numArgs_ = numArgs;
     self->numVars_ = numVars;
 
     /*
      * Get the initial shape to use when creating CallObjects for this script.
      * Since unaliased variables are, by definition, only accessed by local
      * operations and never through the scope chain, only give shapes to