Bug 866134 - GC: Address reported TokenStream::Position rooting hazards r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 26 Apr 2013 18:50:18 +0100
changeset 130103 b4f5625652fa3c5c61f3f11cb225a918bca42552
parent 130102 e90cc4d3501ad757ef077c57307fb9b05591c1ac
child 130104 d86c086e96f8804fb288621f9368ce9ecffd0d9a
push id1552
push userttaubert@mozilla.com
push dateSat, 27 Apr 2013 15:33:29 +0000
treeherderfx-team@40dafc376794 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs866134
milestone23.0a1
Bug 866134 - GC: Address reported TokenStream::Position rooting hazards r=sfink
js/src/devtools/rootAnalysis/annotations.js
js/src/frontend/BytecodeCompiler.cpp
js/src/frontend/Parser.cpp
js/src/frontend/TokenStream.cpp
js/src/frontend/TokenStream.h
js/src/jscntxt.h
js/src/jsfun.cpp
--- a/js/src/devtools/rootAnalysis/annotations.js
+++ b/js/src/devtools/rootAnalysis/annotations.js
@@ -112,18 +112,21 @@ function ignoreGCFunction(fun)
     // XXX modify refillFreeList<NoGC> to not need data flow analysis to understand it cannot GC.
     if (/refillFreeList/.test(fun) && /\(js::AllowGC\)0u/.test(fun))
         return true;
     return false;
 }
 
 function isRootedTypeName(name)
 {
-    if (name == "mozilla::ErrorResult")
+    if (name == "mozilla::ErrorResult" ||
+        name == "js::frontend::TokenStream::Position")
+    {
         return true;
+    }
     return false;
 }
 
 function isRootedPointerTypeName(name)
 {
     if (name.startsWith('struct '))
         name = name.substr(7);
     if (name.startsWith('class '))
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -360,17 +360,17 @@ frontend::CompileFunctionBody(JSContext 
                                                   /* staticLevel = */ 0, ss,
                                                   /* sourceStart = */ 0, length));
     if (!script)
         return false;
 
     // If the context is strict, immediately parse the body in strict
     // mode. Otherwise, we parse it normally. If we see a "use strict"
     // directive, we backup and reparse it as strict.
-    TokenStream::Position start;
+    TokenStream::Position start(parser.keepAtoms);
     parser.tokenStream.tell(&start);
     bool initiallyStrict = StrictModeFromContext(cx);
     bool becameStrict;
     FunctionBox *funbox;
     ParseNode *pn = parser.standaloneFunctionBody(fun, formals, script, fn, &funbox,
                                                   initiallyStrict, &becameStrict);
     if (!pn) {
         if (initiallyStrict || !becameStrict || parser.tokenStream.hadError())
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -378,17 +378,17 @@ Parser<ParseHandler>::reportWithOffset(P
     return result;
 }
 
 template <typename ParseHandler>
 Parser<ParseHandler>::Parser(JSContext *cx, const CompileOptions &options,
                              const jschar *chars, size_t length, bool foldConstants)
   : AutoGCRooter(cx, PARSER),
     context(cx),
-    tokenStream(cx, options, chars, length, thisForCtor()),
+    tokenStream(cx, options, chars, length, thisForCtor(), keepAtoms),
     traceListHead(NULL),
     pc(NULL),
     sct(NULL),
     keepAtoms(cx->runtime),
     foldConstants(foldConstants),
     compileAndGo(options.compileAndGo),
     selfHostingMode(options.selfHostingMode),
     unknownResult(false),
@@ -2063,34 +2063,34 @@ Parser<ParseHandler>::functionStmt()
     if (tokenStream.getToken(TSF_KEYWORD_IS_NAME) == TOK_NAME) {
         name = tokenStream.currentToken().name();
     } else {
         /* Unnamed function expressions are forbidden in statement context. */
         report(ParseError, false, null(), JSMSG_UNNAMED_FUNCTION_STMT);
         return null();
     }
 
-    TokenStream::Position start;
+    TokenStream::Position start(keepAtoms);
     tokenStream.positionAfterLastFunctionKeyword(start);
 
     /* We forbid function statements in strict mode code. */
     if (!pc->atBodyLevel() && pc->sc->needStrictChecks() &&
         !report(ParseStrictError, pc->sc->strict, null(), JSMSG_STRICT_FUNCTION_STATEMENT))
         return null();
 
     return functionDef(name, start, tokenStream.positionToOffset(start), Normal, Statement);
 }
 
 template <typename ParseHandler>
 typename ParseHandler::Node
 Parser<ParseHandler>::functionExpr()
 {
     RootedPropertyName name(context);
     JS_ASSERT(tokenStream.currentToken().type == TOK_FUNCTION);
-    TokenStream::Position start;
+    TokenStream::Position start(keepAtoms);
     tokenStream.positionAfterLastFunctionKeyword(start);
     if (tokenStream.getToken(TSF_KEYWORD_IS_NAME) == TOK_NAME)
         name = tokenStream.currentToken().name();
     else
         tokenStream.ungetToken();
     return functionDef(name, start, tokenStream.positionToOffset(start), Normal, Expression);
 }
 
@@ -4890,17 +4890,17 @@ Parser<ParseHandler>::assignExpr()
     if (tokenStream.matchToken(TOK_YIELD, TSF_OPERAND))
         return returnOrYield(true);
     if (tokenStream.hadError())
         return null();
 #endif
 
     // Save the tokenizer state in case we find an arrow function and have to
     // rewind.
-    TokenStream::Position start;
+    TokenStream::Position start(keepAtoms);
     tokenStream.tell(&start);
 
     Node lhs = condExpr1();
     if (!lhs)
         return null();
 
     ParseNodeKind kind;
     switch (tokenStream.currentToken().type) {
@@ -6418,17 +6418,17 @@ Parser<ParseHandler>::primaryExpr(TokenK
                             return null();
                         break;
                     }
 
                     handler.setListFlag(pn, PNX_NONCONST);
 
                     /* NB: Getter function in { get x(){} } is unnamed. */
                     Rooted<PropertyName*> funName(context, NULL);
-                    TokenStream::Position start;
+                    TokenStream::Position start(keepAtoms);
                     tokenStream.tell(&start);
                     pn2 = functionDef(funName, start, tokenStream.positionToOffset(start),
                                       op == JSOP_GETTER ? Getter : Setter,
                                       Expression);
                     if (!pn2)
                         return null();
                     pn2 = handler.newBinary(PNK_COLON, pn3, pn2, op);
                     goto skip;
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -238,17 +238,18 @@ TokenStream::SourceCoords::lineNumAndCol
 
 #ifdef _MSC_VER
 #pragma warning(push)
 #pragma warning(disable:4351)
 #endif
 
 /* Initialize members that aren't initialized in |init|. */
 TokenStream::TokenStream(JSContext *cx, const CompileOptions &options,
-                         const jschar *base, size_t length, StrictModeGetter *smg)
+                         const jschar *base, size_t length, StrictModeGetter *smg,
+                         AutoKeepAtoms& keepAtoms)
   : srcCoords(cx, options.lineno),
     tokens(),
     cursor(),
     lookahead(),
     lineno(options.lineno),
     flags(),
     linebase(base),
     prevLinebase(NULL),
@@ -257,16 +258,17 @@ TokenStream::TokenStream(JSContext *cx, 
     sourceMap(NULL),
     listenerTSData(),
     tokenbuf(cx),
     version(options.version),
     cx(cx),
     originPrincipals(JSScript::normalizeOriginPrincipals(options.principals,
                                                          options.originPrincipals)),
     strictModeGetter(smg),
+    lastFunctionKeyword(keepAtoms),
     tokenSkip(cx, &tokens),
     linebaseSkip(cx, &linebase),
     prevLinebaseSkip(cx, &prevLinebase)
 {
     if (originPrincipals)
         JS_HoldPrincipals(originPrincipals);
 
     JSSourceHandler listener = cx->runtime->debugHooks.sourceHandler;
--- a/js/src/frontend/TokenStream.h
+++ b/js/src/frontend/TokenStream.h
@@ -448,17 +448,18 @@ class TokenStream
                                                        to power of 2 to avoid divmod by 3 */
     static const unsigned maxLookahead = 2;
     static const unsigned ntokensMask = ntokens - 1;
 
   public:
     typedef Vector<jschar, 32> CharBuffer;
 
     TokenStream(JSContext *cx, const CompileOptions &options,
-                const jschar *base, size_t length, StrictModeGetter *smg);
+                const jschar *base, size_t length, StrictModeGetter *smg,
+                AutoKeepAtoms& keepAtoms);
 
     ~TokenStream();
 
     /* Accessors. */
     JSContext *getContext() const { return cx; }
     bool onCurrentLine(const TokenPos &pos) const { return srcCoords.isOnThisLine(pos.end, lineno); }
     const Token &currentToken() const { return tokens[cursor]; }
     bool isCurrentTokenType(TokenKind type) const {
@@ -629,17 +630,30 @@ class TokenStream
         Flagger flagger(this, withFlags);
         return matchToken(tt);
     }
 
     void consumeKnownToken(TokenKind tt) {
         JS_ALWAYS_TRUE(matchToken(tt));
     }
 
-    class Position {
+    class MOZ_STACK_CLASS Position {
+      public:
+        /*
+         * The Token fields may contain pointers to atoms, so for correct
+         * rooting we must ensure collection of atoms is disabled while objects
+         * of this class are live.  Do this by requiring a dummy AutoKeepAtoms
+         * reference in the constructor.
+         *
+         * This class is explicity ignored by the analysis, so don't add any
+         * more pointers to GC things here!
+         */
+        Position(AutoKeepAtoms&) { }
+      private:
+        Position(const Position&) MOZ_DELETE;
         friend class TokenStream;
         const jschar *buf;
         unsigned flags;
         unsigned lineno;
         const jschar *linebase;
         const jschar *prevLinebase;
         Token currentToken;
         unsigned lookahead;
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -1935,17 +1935,17 @@ class AutoUnlockGC
 #endif
     {
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
         JS_UNLOCK_GC(rt);
     }
     ~AutoUnlockGC() { JS_LOCK_GC(rt); }
 };
 
-class AutoKeepAtoms
+class MOZ_STACK_CLASS AutoKeepAtoms
 {
     JSRuntime *rt;
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
   public:
     explicit AutoKeepAtoms(JSRuntime *rt
                            MOZ_GUARD_OBJECT_NOTIFIER_PARAM)
       : rt(rt)
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -553,17 +553,18 @@ JS_FRIEND_DATA(Class) js::FunctionClass 
 static bool
 FindBody(JSContext *cx, HandleFunction fun, StableCharPtr chars, size_t length,
          size_t *bodyStart, size_t *bodyEnd)
 {
     // We don't need principals, since those are only used for error reporting.
     CompileOptions options(cx);
     options.setFileAndLine("internal-findBody", 0)
            .setVersion(fun->nonLazyScript()->getVersion());
-    TokenStream ts(cx, options, chars.get(), length, NULL);
+    AutoKeepAtoms keepAtoms(cx->runtime);
+    TokenStream ts(cx, options, chars.get(), length, NULL, keepAtoms);
     int nest = 0;
     bool onward = true;
     // Skip arguments list.
     do {
         switch (ts.getToken()) {
           case TOK_NAME:
             if (nest == 0)
                 onward = false;
@@ -1369,17 +1370,18 @@ js::Function(JSContext *cx, unsigned arg
 
         /*
          * Initialize a tokenstream that reads from the given string.  No
          * StrictModeGetter is needed because this TokenStream won't report any
          * strict mode errors.  Any strict mode errors which might be reported
          * here (duplicate argument names, etc.) will be detected when we
          * compile the function body.
          */
-        TokenStream ts(cx, options, collected_args.get(), args_length, /* strictModeGetter = */ NULL);
+        TokenStream ts(cx, options, collected_args.get(), args_length,
+                       /* strictModeGetter = */ NULL, keepAtoms);
 
         /* The argument string may be empty or contain no tokens. */
         TokenKind tt = ts.getToken();
         if (tt != TOK_EOF) {
             for (;;) {
                 /*
                  * Check that it's a name.  This also implicitly guards against
                  * TOK_ERROR, which was already reported.