Bug 826581 - Root RegExp source for the lifetime of RegExpShared. r=dvander
authorSean Stangl <sstangl@mozilla.com>
Mon, 07 Jan 2013 14:32:52 -0800
changeset 118359 7d905588403603834591ab29d677e6712557b5a8
parent 118358 5df4d899632389374589f46aeb7d4ab90c5e6253
child 118360 de8c4a52722e3764479e667507736dad8bdc7e42
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersdvander
bugs826581
milestone20.0a1
Bug 826581 - Root RegExp source for the lifetime of RegExpShared. r=dvander
js/src/builtin/RegExp.cpp
js/src/jit-test/tests/basic/bug826581.js
js/src/jsstr.cpp
js/src/methodjit/Compiler.cpp
js/src/methodjit/MethodJIT.cpp
js/src/vm/RegExpObject.cpp
js/src/vm/RegExpObject.h
js/src/vm/RegExpStatics.cpp
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -138,17 +138,17 @@ ExecuteRegExpImpl(JSContext *cx, RegExpS
 }
 
 /* Legacy ExecuteRegExp behavior is baked into the JSAPI. */
 bool
 js::ExecuteRegExpLegacy(JSContext *cx, RegExpStatics *res, RegExpObject &reobj,
                         Handle<JSStableString*> input, StableCharPtr chars, size_t length,
                         size_t *lastIndex, JSBool test, jsval *rval)
 {
-    RegExpGuard shared;
+    RegExpGuard shared(cx);
     if (!reobj.getShared(cx, &shared))
         return false;
 
     ScopedMatchPairs matches(&cx->tempLifoAlloc());
     MatchConduit conduit(&matches);
 
     RegExpRunStatus status =
         ExecuteRegExpImpl(cx, res, *shared, reobj, input, chars, length, lastIndex, conduit);
@@ -247,17 +247,17 @@ CompileRegExpObject(JSContext *cx, RegEx
         }
 
         /*
          * Only extract the 'flags' out of sourceObj; do not reuse the
          * RegExpShared since it may be from a different compartment.
          */
         RegExpFlag flags;
         {
-            RegExpGuard g;
+            RegExpGuard g(cx);
             if (!RegExpToShared(cx, *sourceObj, &g))
                 return false;
 
             flags = g->getFlags();
         }
 
         /*
          * 'toSource' is a permanent read-only property, so this is equivalent
@@ -551,17 +551,17 @@ js_InitRegExpClass(JSContext *cx, Handle
 }
 
 RegExpRunStatus
 js::ExecuteRegExp(JSContext *cx, HandleObject regexp, HandleString string, MatchConduit &matches)
 {
     /* Step 1 (b) was performed by CallNonGenericMethod. */
     Rooted<RegExpObject*> reobj(cx, &regexp->asRegExp());
 
-    RegExpGuard re;
+    RegExpGuard re(cx);
     if (!reobj->getShared(cx, &re))
         return RegExpRunStatus_Error;
 
     RegExpStatics *res = cx->regExpStatics();
 
     /* Step 3. */
     Rooted<JSStableString*> stableInput(cx, string->ensureStable(cx));
     if (!stableInput)
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug826581.js
@@ -0,0 +1,12 @@
+// Don't crash.
+
+try {
+    x = "          ()    ";
+    for (var y = 0; y < 19; y++) {
+        x += x;
+    }
+} catch (e) {}
+
+try {
+	"".replace(x, "", "gy");
+} catch (e) { }
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -1543,17 +1543,19 @@ class StringRegExpGuard
                 if (!sb.append(*it))
                     return NULL;
             }
         }
         return sb.finishAtom();
     }
 
   public:
-    StringRegExpGuard(JSContext *cx) : fm(cx) {}
+    StringRegExpGuard(JSContext *cx)
+      : re_(cx), fm(cx)
+    { }
 
     /* init must succeed in order to call tryFlatMatch or normalizeRegExp. */
     bool init(JSContext *cx, CallArgs args, bool convertVoid = false)
     {
         if (args.length() != 0 && IsObjectWithClass(args[0], ESClass_RegExp, cx)) {
             if (!RegExpToShared(cx, args[0].toObject(), &re_))
                 return false;
         } else {
@@ -1627,17 +1629,17 @@ class StringRegExpGuard
         if (optarg < args.length()) {
             opt = ToString(cx, args[optarg]);
             if (!opt)
                 return false;
         } else {
             opt = NULL;
         }
 
-        JSAtom *patstr;
+        Rooted<JSAtom *> patstr(cx);
         if (flat) {
             patstr = flattenPattern(cx, fm.patstr);
             if (!patstr)
                 return false;
         } else {
             patstr = fm.patstr;
         }
         JS_ASSERT(patstr);
@@ -2779,17 +2781,17 @@ js::str_split(JSContext *cx, unsigned ar
         if (!ToNumber(cx, args[1], &d))
             return false;
         limit = ToUint32(d);
     } else {
         limit = UINT32_MAX;
     }
 
     /* Step 8. */
-    RegExpGuard re;
+    RegExpGuard re(cx);
     JSLinearString *sepstr = NULL;
     bool sepDefined = args.hasDefined(0);
     if (sepDefined) {
         if (IsObjectWithClass(args[0], ESClass_RegExp, cx)) {
             if (!RegExpToShared(cx, args[0].toObject(), &re))
                 return false;
         } else {
             sepstr = ArgToRootedString(cx, args, 0);
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -7009,17 +7009,17 @@ mjit::Compiler::jsop_regexp()
 
     /*
      * Force creation of the RegExpShared in the script's RegExpObject so that
      * we grab it in the getNewObject template copy. A strong reference to the
      * RegExpShared will be added when the jitcode is created. Any GC activity
      * between now and construction of that jitcode could purge the shared
      * info, but such activity will also abort compilation.
      */
-    RegExpGuard g;
+    RegExpGuard g(cx);
     if (!reobj->getShared(cx, &g))
         return false;
 
     rootedRegExps.append(g.re());
 
     RegisterID result = frame.allocReg();
     Jump emptyFreeList = getNewObject(cx, result, obj);
 
--- a/js/src/methodjit/MethodJIT.cpp
+++ b/js/src/methodjit/MethodJIT.cpp
@@ -1654,16 +1654,21 @@ mjit::NativeToPC(JITScript *jit, void *n
 void
 JITChunk::trace(JSTracer *trc)
 {
     JSObject **rootedTemplates_ = rootedTemplates();
     for (size_t i = 0; i < nRootedTemplates; i++) {
         /* We use a manual write barrier in destroyChunk. */
         MarkObjectUnbarriered(trc, &rootedTemplates_[i], "jitchunk_template");
     }
+
+    /* RegExpShared objects require the RegExp source string. */
+    RegExpShared **rootedRegExps_ = rootedRegExps();
+    for (size_t i = 0; i < nRootedRegExps; i++)
+        rootedRegExps_[i]->trace(trc);
 }
 
 void
 JITChunk::purgeCaches()
 {
     ic::Repatcher repatch(this);
 
 #if defined JS_MONOIC
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -96,17 +96,17 @@ RegExpObjectBuilder::clone(Handle<RegExp
     RegExpFlag origFlags = other->getFlags();
     RegExpFlag staticsFlags = res->getFlags();
     if ((origFlags & staticsFlags) != staticsFlags) {
         RegExpFlag newFlags = RegExpFlag(origFlags | staticsFlags);
         Rooted<JSAtom *> source(cx, other->getSource());
         return build(source, newFlags);
     }
 
-    RegExpGuard g;
+    RegExpGuard g(cx);
     if (!other->getShared(cx, &g))
         return NULL;
 
     Rooted<JSAtom *> source(cx, other->getSource());
     return build(source, *g);
 }
 
 /* MatchPairs */
@@ -401,16 +401,22 @@ RegExpShared::~RegExpShared()
 #if ENABLE_YARR_JIT
     codeBlock.release();
 #endif
     if (bytecode)
         js_delete<BytecodePattern>(bytecode);
 }
 
 void
+RegExpShared::trace(JSTracer *trc)
+{
+    MarkString(trc, &source, "regexpshared source");
+}
+
+void
 RegExpShared::reportYarrError(JSContext *cx, TokenStream *ts, ErrorCode error)
 {
     switch (error) {
       case JSC::Yarr::NoError:
         JS_NOT_REACHED("Called reportYarrError with value for no error");
         return;
 #define COMPILE_EMSG(__code, __msg)                                                              \
       case JSC::Yarr::__code:                                                                    \
--- a/js/src/vm/RegExpObject.h
+++ b/js/src/vm/RegExpObject.h
@@ -115,21 +115,21 @@ class RegExpShared
     typedef JSC::Yarr::YarrPattern YarrPattern;
 #if ENABLE_YARR_JIT
     typedef JSC::Yarr::JSGlobalData JSGlobalData;
     typedef JSC::Yarr::YarrCodeBlock YarrCodeBlock;
     typedef JSC::Yarr::YarrJITCompileMode YarrJITCompileMode;
 #endif
 
     /*
-     * Source to the RegExp. Safe to hold: if the RegExpShared is active,
-     * then at least one RegExpObject must be referencing the RegExpShared,
-     * and the RegExpObject keeps alive the source JSAtom.
+     * Source to the RegExp. The RegExpShared must either be protected by a
+     * RegExpGuard, which handles rooting for stacky RegExpShareds,
+     * or trace() must be explicitly called during marking.
      */
-    JSAtom *           source;
+    HeapPtrAtom        source;
     RegExpFlag         flags;
     unsigned           parenCount;
 
 #if ENABLE_YARR_JIT
     /* Note: Native code is valid only if |codeBlock.isFallBack() == false|. */
     YarrCodeBlock   codeBlock;
 #endif
     BytecodePattern *bytecode;
@@ -144,16 +144,18 @@ class RegExpShared
 
     bool compileIfNecessary(JSContext *cx);
     bool compileMatchOnlyIfNecessary(JSContext *cx);
 
   public:
     RegExpShared(JSRuntime *rt, JSAtom *source, RegExpFlag flags);
     ~RegExpShared();
 
+    void trace(JSTracer *trc);
+
     /* Static functions to expose some Yarr logic. */
     static inline bool isJITRuntimeEnabled(JSContext *cx);
     static void reportYarrError(JSContext *cx, TokenStream *ts, ErrorCode error);
     static bool checkSyntax(JSContext *cx, TokenStream *tokenStream, JSLinearString *source);
 
     /* Called when a RegExpShared is installed into a RegExpObject. */
     inline void prepareForUse(JSContext *cx);
 
@@ -193,32 +195,51 @@ class RegExpShared
 
 /*
  * Extend the lifetime of a given RegExpShared to at least the lifetime of
  * the guard object. See Regular Expression comment at the top.
  */
 class RegExpGuard
 {
     RegExpShared *re_;
+
+    /*
+     * Prevent the RegExp source from being collected:
+     * because RegExpShared objects compile at execution time, the source
+     * must remain rooted for the active lifetime of the RegExpShared.
+     */
+    RootedAtom source_;
+
     RegExpGuard(const RegExpGuard &) MOZ_DELETE;
     void operator=(const RegExpGuard &) MOZ_DELETE;
+
   public:
-    RegExpGuard() : re_(NULL) {}
-    RegExpGuard(RegExpShared &re) : re_(&re) {
+    RegExpGuard(JSContext *cx)
+      : re_(NULL), source_(cx)
+    { }
+
+    RegExpGuard(JSContext *cx, RegExpShared &re)
+      : re_(&re), source_(cx, re.source)
+    {
         re_->incRef();
     }
+
+    ~RegExpGuard() {
+        if (re_)
+            re_->decRef();
+    }
+
+  public:
     void init(RegExpShared &re) {
         JS_ASSERT(!re_);
         re_ = &re;
         re_->incRef();
+        source_ = re.source;
     }
-    ~RegExpGuard() {
-        if (re_)
-            re_->decRef();
-    }
+
     bool initialized() const { return !!re_; }
     RegExpShared *re() const { JS_ASSERT(initialized()); return re_; }
     RegExpShared *operator->() { return re(); }
     RegExpShared &operator*() { return *re(); }
 };
 
 class RegExpCompartment
 {
--- a/js/src/vm/RegExpStatics.cpp
+++ b/js/src/vm/RegExpStatics.cpp
@@ -82,17 +82,17 @@ RegExpStatics::executeLazy(JSContext *cx
      * It is not necessary to call aboutToWrite(): evaluation of
      * implicit copies is safe.
      */
 
     size_t length = matchesInput->length();
     StableCharPtr chars(matchesInput->chars(), length);
 
     /* Execute the full regular expression. */
-    RegExpGuard shared;
+    RegExpGuard shared(cx);
     if (!regexp->getShared(cx, &shared))
         return false;
 
     RegExpRunStatus status = shared->execute(cx, chars, length, &this->lastIndex, this->matches);
     if (status == RegExpRunStatus_Error)
         return false;
 
     /* Unset lazy state and remove rooted values that now have no use. */