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 141048 b4f5625652fa3c5c61f3f11cb225a918bca42552
parent 141047 e90cc4d3501ad757ef077c57307fb9b05591c1ac
child 141049 d86c086e96f8804fb288621f9368ce9ecffd0d9a
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs866134
milestone23.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 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.