Bug 840548 - GC: rooting hazards in the parser r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 12 Feb 2013 17:19:05 +0000
changeset 131598 8a66b6d3bd11fd8e57c82399f3e74a8d55fdd887
parent 131597 cbcdfbeadb35b30183a4059a4263b5faa0ecc482
child 131599 3b5018fa761febbac38837a9bef3f68d8eef9753
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs840548
milestone21.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 840548 - GC: rooting hazards in the parser r=sfink
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -57,16 +57,21 @@
 
 #include "vm/NumericConversions.h"
 #include "vm/RegExpObject-inl.h"
 
 using namespace js;
 using namespace js::gc;
 using namespace js::frontend;
 
+typedef Rooted<StaticBlockObject*> RootedStaticBlockObject;
+typedef Handle<StaticBlockObject*> HandleStaticBlockObject;
+
+typedef MutableHandle<PropertyName*> MutableHandlePropertyName;
+
 /*
  * Insist that the next token be of type tt, or report errno and return null.
  * NB: this macro uses cx and ts from its lexical environment.
  */
 #define MUST_MATCH_TOKEN_WITH_FLAGS(tt, errno, __flags)                                     \
     JS_BEGIN_MACRO                                                                          \
         if (tokenStream.getToken((__flags)) != tt) {                                        \
             reportError(NULL, errno);                                                       \
@@ -97,17 +102,17 @@ static void
 PushStatementPC(ParseContext *pc, StmtInfoPC *stmt, StmtType type)
 {
     stmt->blockid = pc->blockid();
     PushStatement(pc, stmt, type);
 }
 
 // See comment on member function declaration.
 bool
-ParseContext::define(JSContext *cx, PropertyName *name, ParseNode *pn, Definition::Kind kind)
+ParseContext::define(JSContext *cx, HandlePropertyName name, ParseNode *pn, Definition::Kind kind)
 {
     JS_ASSERT(!pn->isUsed());
     JS_ASSERT_IF(pn->isDefn(), pn->isPlaceholder());
 
     Definition *prevDef = NULL;
     if (kind == Definition::LET)
         prevDef = decls_.lookupFirst(name);
     else
@@ -779,17 +784,17 @@ Parser::functionBody(FunctionBodyType ty
     /* Check for falling off the end of a function that returns a value. */
     if (context->hasStrictOption() && pc->funHasReturnExpr &&
         !CheckFinalReturn(context, this, pn))
     {
         pn = NULL;
     }
 
     /* Time to implement the odd semantics of 'arguments'. */
-    Handle<PropertyName*> arguments = context->names().arguments;
+    HandlePropertyName arguments = context->names().arguments;
 
     /*
      * Non-top-level functions use JSOP_DEFFUN which is a dynamic scope
      * operation which means it aliases any bindings with the same name.
      * Due to the implicit declaration mechanism (below), 'arguments' will not
      * have decls and, even if it did, they will not be noted as closed in the
      * emitter. Thus, in the corner case of function-statement-overridding-
      * arguments, flag the whole scope as dynamic.
@@ -1041,17 +1046,17 @@ struct frontend::BindData {
     ParseNode       *pn;        /* name node for definition processing and
                                    error source coordinates */
     JSOp            op;         /* prolog bytecode or nop */
     Binder          binder;     /* binder, discriminates u */
 
     struct LetData {
         LetData(JSContext *cx) : blockObj(cx) {}
         VarContext varContext;
-        Rooted<StaticBlockObject*> blockObj;
+        RootedStaticBlockObject blockObj;
         unsigned   overflow;
     } let;
 
     void initLet(VarContext varContext, StaticBlockObject &blockObj, unsigned overflow) {
         this->pn = NULL;
         this->op = JSOP_NOP;
         this->binder = BindLet;
         this->let.varContext = varContext;
@@ -1135,17 +1140,17 @@ DeoptimizeUsesWithin(Definition *dn, con
 
 /*
  * Beware: this function is called for functions nested in other functions or
  * global scripts but not for functions compiled through the Function
  * constructor or JSAPI. To always execute code when a function has finished
  * parsing, use Parser::functionBody.
  */
 static bool
-LeaveFunction(ParseNode *fn, Parser *parser, PropertyName *funName = NULL,
+LeaveFunction(ParseNode *fn, Parser *parser, HandlePropertyName funName,
               FunctionSyntaxKind kind = Expression)
 {
     JSContext *cx = parser->context;
     ParseContext *funpc = parser->pc;
     ParseContext *pc = funpc->parent;
     pc->blockidGen = funpc->blockidGen;
 
     FunctionBox *funbox = fn->pn_funbox;
@@ -1998,26 +2003,26 @@ Parser::condition()
         !reportStrictWarning(NULL, JSMSG_EQUAL_AS_ASSIGN))
     {
         return NULL;
     }
     return pn;
 }
 
 static bool
-MatchLabel(JSContext *cx, TokenStream *ts, PropertyName **label)
+MatchLabel(JSContext *cx, TokenStream *ts, MutableHandlePropertyName label)
 {
     TokenKind tt = ts->peekTokenSameLine(TSF_OPERAND);
     if (tt == TOK_ERROR)
         return false;
     if (tt == TOK_NAME) {
         (void) ts->getToken();
-        *label = ts->currentToken().name();
+        label.set(ts->currentToken().name());
     } else {
-        *label = NULL;
+        label.set(NULL);
     }
     return true;
 }
 
 static bool
 ReportRedeclaration(JSContext *cx, Parser *parser, ParseNode *pn, bool isConst, JSAtom *atom)
 {
     JSAutoByteString name;
@@ -2088,49 +2093,52 @@ BindLet(JSContext *cx, BindData *data, H
 
     /* Store pn in the static block object. */
     blockObj->setDefinitionParseNode(blockCount, reinterpret_cast<Definition *>(pn));
     return true;
 }
 
 template <class Op>
 static inline bool
-ForEachLetDef(JSContext *cx, ParseContext *pc, StaticBlockObject &blockObj, Op op)
+ForEachLetDef(JSContext *cx, ParseContext *pc, HandleStaticBlockObject blockObj, Op op)
 {
-    for (Shape::Range r = blockObj.lastProperty()->all(); !r.empty(); r.popFront()) {
+    for (Shape::Range r = blockObj->lastProperty()->all(); !r.empty(); r.popFront()) {
+        Shape::Range::AutoRooter rooter(cx, &r);
         Shape &shape = r.front();
 
         /* Beware the destructuring dummy slots. */
         if (JSID_IS_INT(shape.propid()))
             continue;
 
         if (!op(cx, pc, blockObj, shape, JSID_TO_ATOM(shape.propid())))
             return false;
     }
     return true;
 }
 
 struct PopLetDecl {
-    bool operator()(JSContext *, ParseContext *pc, StaticBlockObject &, const Shape &, JSAtom *atom) {
+    bool operator()(JSContext *, ParseContext *pc, HandleStaticBlockObject, const Shape &,
+                    JSAtom *atom)
+    {
         pc->popLetDecl(atom);
         return true;
     }
 };
 
 static void
 PopStatementPC(JSContext *cx, ParseContext *pc)
 {
-    StaticBlockObject *blockObj = pc->topStmt->blockObj;
+    RootedStaticBlockObject blockObj(cx, pc->topStmt->blockObj);
     JS_ASSERT(!!blockObj == (pc->topStmt->isBlockScope));
 
     FinishPopStatement(pc);
 
     if (blockObj) {
         JS_ASSERT(!blockObj->inDictionaryMode());
-        ForEachLetDef(cx, pc, *blockObj, PopLetDecl());
+        ForEachLetDef(cx, pc, blockObj, PopLetDecl());
         blockObj->resetPrevBlockChainFromParser();
     }
 }
 
 static inline bool
 OuterLet(ParseContext *pc, StmtInfoPC *stmt, HandleAtom atom)
 {
     while (stmt->downScope) {
@@ -2592,71 +2600,76 @@ Parser::returnOrYield(bool useAssignExpr
     {
         return NULL;
     }
 
     return pn;
 }
 
 static ParseNode *
-PushLexicalScope(JSContext *cx, Parser *parser, StaticBlockObject &blockObj, StmtInfoPC *stmt)
+PushLexicalScope(JSContext *cx, Parser *parser, HandleStaticBlockObject blockObj, StmtInfoPC *stmt)
 {
+    JS_ASSERT(blockObj);
+
     ParseNode *pn = LexicalScopeNode::create(PNK_LEXICALSCOPE, parser);
     if (!pn)
         return NULL;
 
-    ObjectBox *blockbox = parser->newObjectBox(&blockObj);
+    ObjectBox *blockbox = parser->newObjectBox(blockObj);
     if (!blockbox)
         return NULL;
 
     ParseContext *pc = parser->pc;
 
     PushStatementPC(pc, stmt, STMT_BLOCK);
-    blockObj.initPrevBlockChainFromParser(pc->blockChain);
-    FinishPushBlockScope(pc, stmt, blockObj);
+    blockObj->initPrevBlockChainFromParser(pc->blockChain);
+    FinishPushBlockScope(pc, stmt, *blockObj.get());
 
     pn->setOp(JSOP_LEAVEBLOCK);
     pn->pn_objbox = blockbox;
     pn->pn_cookie.makeFree();
     pn->pn_dflags = 0;
     if (!GenerateBlockId(pc, stmt->blockid))
         return NULL;
     pn->pn_blockid = stmt->blockid;
     return pn;
 }
 
 static ParseNode *
 PushLexicalScope(JSContext *cx, Parser *parser, StmtInfoPC *stmt)
 {
-    StaticBlockObject *blockObj = StaticBlockObject::create(cx);
+    RootedStaticBlockObject blockObj(cx, StaticBlockObject::create(cx));
     if (!blockObj)
         return NULL;
 
-    return PushLexicalScope(cx, parser, *blockObj, stmt);
+    return PushLexicalScope(cx, parser, blockObj, stmt);
 }
 
 #if JS_HAS_BLOCK_SCOPE
 
 struct AddLetDecl
 {
     uint32_t blockid;
 
     AddLetDecl(uint32_t blockid) : blockid(blockid) {}
 
-    bool operator()(JSContext *cx, ParseContext *pc, StaticBlockObject &blockObj, const Shape &shape, JSAtom *)
-    {
-        ParseNode *def = (ParseNode *) blockObj.getSlot(shape.slot()).toPrivate();
+    bool operator()(JSContext *cx, ParseContext *pc, HandleStaticBlockObject blockObj,
+                    const Shape &shape, JSAtom *)
+        {
+        ParseNode *def = (ParseNode *) blockObj->getSlot(shape.slot()).toPrivate();
         def->pn_blockid = blockid;
-        return pc->define(cx, def->name(), def, Definition::LET);
+        RootedPropertyName name(cx, def->name());
+        return pc->define(cx, name, def, Definition::LET);
     }
 };
 
 static ParseNode *
-PushLetScope(JSContext *cx, Parser *parser, StaticBlockObject &blockObj, StmtInfoPC *stmt)
+PushLetScope(JSContext *cx, Parser *parser, HandleStaticBlockObject blockObj, StmtInfoPC *stmt)
 {
+    JS_ASSERT(blockObj);
     ParseNode *pn = PushLexicalScope(cx, parser, blockObj, stmt);
     if (!pn)
         return NULL;
 
     /* Tell codegen to emit JSOP_ENTERLETx (not JSOP_ENTERBLOCK). */
     pn->pn_dflags |= PND_LET;
 
     /* Populate the new scope with decls found in the head with updated blockid. */
@@ -2675,30 +2688,30 @@ ParseNode *
 Parser::letBlock(LetContext letContext)
 {
     JS_ASSERT(tokenStream.currentToken().type == TOK_LET);
 
     ParseNode *pnlet = BinaryNode::create(PNK_LET, this);
     if (!pnlet)
         return NULL;
 
-    Rooted<StaticBlockObject*> blockObj(context, StaticBlockObject::create(context));
+    RootedStaticBlockObject blockObj(context, StaticBlockObject::create(context));
     if (!blockObj)
         return NULL;
 
     MUST_MATCH_TOKEN(TOK_LP, JSMSG_PAREN_BEFORE_LET);
 
     ParseNode *vars = variables(PNK_LET, blockObj, DontHoistVars);
     if (!vars)
         return NULL;
 
     MUST_MATCH_TOKEN(TOK_RP, JSMSG_PAREN_AFTER_LET);
 
     StmtInfoPC stmtInfo(context);
-    ParseNode *block = PushLetScope(context, this, *blockObj, &stmtInfo);
+    ParseNode *block = PushLetScope(context, this, blockObj, &stmtInfo);
     if (!block)
         return NULL;
 
     pnlet->pn_left = vars;
     pnlet->pn_right = block;
 
     ParseNode *ret;
     if (letContext == LetStatement && !tokenStream.matchToken(TOK_LC, TSF_OPERAND)) {
@@ -2998,17 +3011,17 @@ Parser::forStatement()
 
     /*
      * True if we have 'for (var/let/const ...)', except in the oddball case
      * where 'let' begins a let-expression in 'for (let (...) ...)'.
      */
     bool forDecl = false;
 
     /* Non-null when forDecl is true for a 'for (let ...)' statement. */
-    Rooted<StaticBlockObject*> blockObj(context);
+    RootedStaticBlockObject blockObj(context);
 
     /* Set to 'x' in 'for (x ;... ;...)' or 'for (x in ...)'. */
     ParseNode *pn1;
 
     {
         TokenKind tt = tokenStream.peekToken(TSF_OPERAND);
         if (tt == TOK_SEMI) {
             if (pn->pn_iflags & JSITER_FOREACH) {
@@ -3180,17 +3193,17 @@ Parser::forStatement()
 
         if (blockObj) {
             /*
              * Now that the pn3 has been parsed, push the let scope. To hold
              * the blockObj for the emitter, wrap the TOK_LEXICALSCOPE node
              * created by PushLetScope around the for's initializer. This also
              * serves to indicate the let-decl to the emitter.
              */
-            ParseNode *block = PushLetScope(context, this, *blockObj, &letStmt);
+            ParseNode *block = PushLetScope(context, this, blockObj, &letStmt);
             if (!block)
                 return NULL;
             letStmt.isForLetBlock = true;
             block->pn_expr = pn1;
             block->pn_pos = pn1->pn_pos;
             pn1 = block;
         }
 
@@ -3236,17 +3249,17 @@ Parser::forStatement()
         if (!forHead)
             return NULL;
     } else {
         if (blockObj) {
             /*
              * Desugar 'for (let A; B; C) D' into 'let (A) { for (; B; C) D }'
              * to induce the correct scoping for A.
              */
-            ParseNode *block = PushLetScope(context, this, *blockObj, &letStmt);
+            ParseNode *block = PushLetScope(context, this, blockObj, &letStmt);
             if (!block)
                 return NULL;
             letStmt.isForLetBlock = true;
 
             ParseNode *let = new_<BinaryNode>(PNK_LET, JSOP_NOP, pos, pn1, block);
             if (!let)
                 return NULL;
 
@@ -3665,17 +3678,17 @@ Parser::expressionStatement()
     if (!pn2)
         return NULL;
 
     if (tokenStream.peekToken() == TOK_COLON) {
         if (!pn2->isKind(PNK_NAME)) {
             reportError(NULL, JSMSG_BAD_LABEL);
             return NULL;
         }
-        JSAtom *label = pn2->pn_atom;
+        RootedAtom label(context, pn2->pn_atom);
         for (StmtInfoPC *stmt = pc->topStmt; stmt; stmt = stmt->down) {
             if (stmt->type == STMT_LABEL && stmt->label == label) {
                 reportError(NULL, JSMSG_DUPLICATE_LABEL);
                 return NULL;
             }
         }
         ForgetUse(pn2);
 
@@ -3849,21 +3862,21 @@ Parser::statement()
 
       case TOK_FINALLY:
         reportError(NULL, JSMSG_FINALLY_WITHOUT_TRY);
         return NULL;
 
       case TOK_BREAK:
       {
         TokenPtr begin = tokenStream.currentToken().pos.begin;
-        PropertyName *label;
+        RootedPropertyName label(context);
         if (!MatchLabel(context, &tokenStream, &label))
             return NULL;
         TokenPtr end = tokenStream.currentToken().pos.end;
-        pn = new_<BreakStatement>(label, begin, end);
+        pn = new_<BreakStatement>(label.get(), begin, end);
         if (!pn)
             return NULL;
         StmtInfoPC *stmt = pc->topStmt;
         if (label) {
             for (; ; stmt = stmt->down) {
                 if (!stmt) {
                     reportError(NULL, JSMSG_LABEL_NOT_FOUND);
                     return NULL;
@@ -3882,21 +3895,21 @@ Parser::statement()
             }
         }
         break;
       }
 
       case TOK_CONTINUE:
       {
         TokenPtr begin = tokenStream.currentToken().pos.begin;
-        PropertyName *label;
+        RootedPropertyName label(context);
         if (!MatchLabel(context, &tokenStream, &label))
             return NULL;
         TokenPtr end = tokenStream.currentToken().pos.begin;
-        pn = new_<ContinueStatement>(label, begin, end);
+        pn = new_<ContinueStatement>(label.get(), begin, end);
         if (!pn)
             return NULL;
         StmtInfoPC *stmt = pc->topStmt;
         if (label) {
             for (StmtInfoPC *stmt2 = NULL; ; stmt = stmt->down) {
                 if (!stmt) {
                     reportError(NULL, JSMSG_LABEL_NOT_FOUND);
                     return NULL;
@@ -5279,17 +5292,18 @@ Parser::generatorExpr(ParseNode *kid)
 
         if (AtomDefnPtr p = genpc.lexdeps->lookup(context->names().arguments)) {
             Definition *dn = p.value();
             ParseNode *errorNode = dn->dn_uses ? dn->dn_uses : body;
             reportError(errorNode, JSMSG_BAD_GENEXP_BODY, js_arguments_str);
             return NULL;
         }
 
-        if (!LeaveFunction(genfn, this))
+        RootedPropertyName funName(context);
+        if (!LeaveFunction(genfn, this, funName))
             return NULL;
     }
 
     /*
      * Our result is a call expression that invokes the anonymous generator
      * function object.
      */
     ParseNode *result = ListNode::create(PNK_GENEXP, this);
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -111,17 +111,17 @@ struct ParseContext                 /* t
      *    non-placeholder definition.
      *  + If this is a function scope (sc->inFunction), 'pn' is bound to a
      *    particular local/argument slot.
      *  + PND_CONST is set for Definition::COSNT
      *  + Pre-existing uses of pre-existing placeholders have been linked to
      *    'pn' if they are in the scope of 'pn'.
      *  + Pre-existing placeholders in the scope of 'pn' have been removed.
      */
-    bool define(JSContext *cx, PropertyName *name, ParseNode *pn, Definition::Kind);
+    bool define(JSContext *cx, HandlePropertyName name, ParseNode *pn, Definition::Kind);
 
     /*
      * Let definitions may shadow same-named definitions in enclosing scopes.
      * To represesent this, 'decls' is not a plain map, but actually:
      *   decls :: name -> stack of definitions
      * New bindings are pushed onto the stack, name lookup always refers to the
      * top of the stack, and leaving a block scope calls popLetDecl for each
      * name in the block's scope.