From 46f8dd6c334a3ab758a8a36ea107b03ef3c6e2ef Mon Sep 17 00:00:00 2001 From: Sean Stangl Date: Fri, 1 Feb 2013 17:36:29 -0800 Subject: [PATCH] Bug 832217 - Construct RegExpShared in the same compartment as the RegExpStatics. r=billm --- js/src/gc/RootMarking.cpp | 5 ++-- js/src/vm/RegExpObject-inl.h | 38 ++++++------------------ js/src/vm/RegExpObject.cpp | 29 ++---------------- js/src/vm/RegExpObject.h | 34 ++-------------------- js/src/vm/RegExpStatics-inl.h | 55 ++++++++++++++++++----------------- js/src/vm/RegExpStatics.cpp | 15 ++++++---- 6 files changed, 55 insertions(+), 121 deletions(-) diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index 14b44d653d7..a356732d96a 100644 --- a/js/src/gc/RootMarking.cpp +++ b/js/src/gc/RootMarking.cpp @@ -633,11 +633,12 @@ RegExpStatics::AutoRooter::trace(JSTracer *trc) if (statics->matchesInput) MarkStringRoot(trc, reinterpret_cast(&statics->matchesInput), "RegExpStatics::AutoRooter matchesInput"); + if (statics->lazySource) + MarkStringRoot(trc, reinterpret_cast(&statics->lazySource), + "RegExpStatics::AutoRooter lazySource"); if (statics->pendingInput) MarkStringRoot(trc, reinterpret_cast(&statics->pendingInput), "RegExpStatics::AutoRooter pendingInput"); - if (statics->regexp.initialized()) - statics->regexp->trace(trc); } void diff --git a/js/src/vm/RegExpObject-inl.h b/js/src/vm/RegExpObject-inl.h index f4e4d55d434..ad1a8a22d54 100644 --- a/js/src/vm/RegExpObject-inl.h +++ b/js/src/vm/RegExpObject-inl.h @@ -100,12 +100,6 @@ RegExpObject::setSticky(bool enabled) setSlot(STICKY_FLAG_SLOT, BooleanValue(enabled)); } -inline void -RegExpShared::writeBarrierPre() -{ - JSString::writeBarrierPre(source); -} - /* This function should be deleted once bad Android platforms phase out. See bug 604774. */ inline bool RegExpShared::isJITRuntimeEnabled(JSContext *cx) @@ -170,32 +164,18 @@ RegExpGuard::release() } } -RegExpHeapGuard::RegExpHeapGuard(RegExpShared &re) -{ - init(re); -} - -RegExpHeapGuard::~RegExpHeapGuard() -{ - release(); -} - inline void -RegExpHeapGuard::init(RegExpShared &re) +MatchPairs::checkAgainst(size_t inputLength) { - JS_ASSERT(!initialized()); - re_ = &re; - re_->incRef(); -} - -inline void -RegExpHeapGuard::release() -{ - if (re_) { - re_->writeBarrierPre(); - re_->decRef(); - re_ = NULL; +#ifdef DEBUG + for (size_t i = 0; i < pairCount_; i++) { + const MatchPair &p = pair(i); + JS_ASSERT(p.check()); + if (p.isUndefined()) + continue; + JS_ASSERT(size_t(p.limit) <= inputLength); } +#endif } } /* namespace js */ diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index cb5e7df2fbe..d1b8fbcfe87 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -159,20 +159,6 @@ MatchPairs::displace(size_t disp) } } -inline void -MatchPairs::checkAgainst(size_t inputLength) -{ -#if DEBUG - for (size_t i = 0; i < pairCount_; i++) { - const MatchPair &p = pair(i); - JS_ASSERT(p.check()); - if (p.isUndefined()) - continue; - JS_ASSERT(size_t(p.limit) <= inputLength); - } -#endif -} - bool ScopedMatchPairs::allocOrExpandArray(size_t pairCount) { @@ -312,7 +298,7 @@ RegExpObject::assignInitialShape(JSContext *cx) return self->addDataProperty(cx, cx->names().sticky, STICKY_FLAG_SLOT, attrs); } -inline bool +bool RegExpObject::init(JSContext *cx, HandleAtom source, RegExpFlag flags) { Rooted self(cx, this); @@ -645,17 +631,6 @@ RegExpCompartment::RegExpCompartment(JSRuntime *rt) RegExpCompartment::~RegExpCompartment() { JS_ASSERT(map_.empty()); - - /* - * RegExpStatics may have prevented a single RegExpShared from - * being collected during RegExpCompartment::sweep(). - */ - for (PendingSet::Enum e(inUse_); !e.empty(); e.popFront()) { - RegExpShared *shared = e.front(); - JS_ASSERT(shared->activeUseCount == 0); - js_delete(shared); - e.removeFront(); - } JS_ASSERT(inUse_.empty()); } @@ -691,7 +666,7 @@ RegExpCompartment::sweep(JSRuntime *rt) } } -inline bool +bool RegExpCompartment::get(JSContext *cx, JSAtom *source, RegExpFlag flags, RegExpGuard *g) { Key key(source, flags); diff --git a/js/src/vm/RegExpObject.h b/js/src/vm/RegExpObject.h index 7c838c1c866..1537c388c42 100644 --- a/js/src/vm/RegExpObject.h +++ b/js/src/vm/RegExpObject.h @@ -109,6 +109,7 @@ CloneRegExpObject(JSContext *cx, JSObject *obj, JSObject *proto); class RegExpShared { friend class RegExpCompartment; + friend class RegExpStatics; friend class RegExpGuard; typedef frontend::TokenStream TokenStream; @@ -124,7 +125,7 @@ class RegExpShared /* * Source to the RegExp, for lazy compilation. * The source must be rooted while activeUseCount is non-zero - * via RegExpGuard, RegExpHeapGuard, or explicit calls to trace(). + * via RegExpGuard or explicit calls to trace(). */ JSAtom * source; @@ -156,7 +157,6 @@ class RegExpShared void trace(JSTracer *trc) { MarkStringUnbarriered(trc, &source, "regexpshared source"); } - inline void writeBarrierPre(); /* Static functions to expose some Yarr logic. */ static inline bool isJITRuntimeEnabled(JSContext *cx); @@ -233,34 +233,6 @@ class RegExpGuard RegExpShared &operator*() { return *re(); } }; -/* Equivalent of RegExpGuard, heap-allocated, with explicit tracing. */ -class RegExpHeapGuard -{ - RegExpShared *re_; - - RegExpHeapGuard(const RegExpGuard &) MOZ_DELETE; - void operator=(const RegExpHeapGuard &) MOZ_DELETE; - - public: - RegExpHeapGuard() : re_(NULL) { } - inline RegExpHeapGuard(RegExpShared &re); - inline ~RegExpHeapGuard(); - - public: - inline void init(RegExpShared &re); - inline void release(); - - void trace(JSTracer *trc) { - if (initialized()) - re_->trace(trc); - } - - bool initialized() const { return !!re_; } - RegExpShared *re() const { JS_ASSERT(initialized()); return re_; } - RegExpShared *operator->() { return re(); } - RegExpShared &operator*() { return *re(); } -}; - class RegExpCompartment { struct Key { @@ -384,7 +356,7 @@ class RegExpObject : public JSObject */ UnrootedShape assignInitialShape(JSContext *cx); - inline bool init(JSContext *cx, HandleAtom source, RegExpFlag flags); + bool init(JSContext *cx, HandleAtom source, RegExpFlag flags); /* * Precondition: the syntax for |source| has already been validated. diff --git a/js/src/vm/RegExpStatics-inl.h b/js/src/vm/RegExpStatics-inl.h index 491eb865548..42bf7df7da0 100644 --- a/js/src/vm/RegExpStatics-inl.h +++ b/js/src/vm/RegExpStatics-inl.h @@ -21,16 +21,21 @@ class RegExpStatics VectorMatchPairs matches; HeapPtr matchesInput; - /* The previous RegExp input, used to resolve lazy state. */ - RegExpHeapGuard regexp; - size_t lastIndex; + /* + * The previous RegExp input, used to resolve lazy state. + * A raw RegExpShared cannot be stored because it may be in + * a different compartment via evalcx(). + */ + HeapPtr lazySource; + RegExpFlag lazyFlags; + size_t lazyIndex; /* The latest RegExp input, set before execution. */ HeapPtr pendingInput; RegExpFlag flags; /* - * If true, |matchesInput|, |regexp|, and |lastIndex| may be used + * If true, |matchesInput| and the |lazy*| fields may be used * to replay the last executed RegExp, and |matches| is invalid. */ bool pendingLazyEvaluation; @@ -118,12 +123,12 @@ class RegExpStatics * Changes to this function must also be reflected in * RegExpStatics::AutoRooter::trace(). */ - if (pendingInput) - MarkString(trc, &pendingInput, "res->pendingInput"); if (matchesInput) MarkString(trc, &matchesInput, "res->matchesInput"); - if (regexp.initialized()) - regexp->trace(trc); + if (lazySource) + MarkString(trc, &lazySource, "res->lazySource"); + if (pendingInput) + MarkString(trc, &pendingInput, "res->pendingInput"); } /* Value creators. */ @@ -393,20 +398,16 @@ RegExpStatics::copyTo(RegExpStatics &dst) if (!pendingLazyEvaluation) dst.matches.initArrayFrom(matches); - if (regexp.initialized()) - dst.regexp.init(*regexp); - else - dst.regexp.release(); - dst.matchesInput = matchesInput; - dst.lastIndex = lastIndex; + dst.lazySource = lazySource; + dst.lazyFlags = lazyFlags; + dst.lazyIndex = lazyIndex; dst.pendingInput = pendingInput; dst.flags = flags; dst.pendingLazyEvaluation = pendingLazyEvaluation; - JS_ASSERT_IF(pendingLazyEvaluation, regexp.initialized()); + JS_ASSERT_IF(pendingLazyEvaluation, lazySource); JS_ASSERT_IF(pendingLazyEvaluation, matchesInput); - JS_ASSERT(regexp.initialized() == dst.regexp.initialized()); } inline void @@ -436,11 +437,10 @@ RegExpStatics::updateLazily(JSContext *cx, JSLinearString *input, BarrieredSetPair(cx->zone(), pendingInput, input, matchesInput, input); - if (regexp.initialized()) - regexp.release(); - regexp.init(*shared); - this->lastIndex = lastIndex; + lazySource = shared->source; + lazyFlags = shared->flags; + lazyIndex = lastIndex; pendingLazyEvaluation = true; } @@ -452,8 +452,8 @@ RegExpStatics::updateFromMatchPairs(JSContext *cx, JSLinearString *input, MatchP /* Unset all lazy state. */ pendingLazyEvaluation = false; - this->regexp.release(); - this->lastIndex = size_t(-1); + this->lazySource = NULL; + this->lazyIndex = size_t(-1); BarrieredSetPair(cx->zone(), pendingInput, input, @@ -474,8 +474,9 @@ RegExpStatics::clear() matches.forgetArray(); matchesInput = NULL; - regexp.release(); - lastIndex = size_t(-1); + lazySource = NULL; + lazyFlags = RegExpFlag(0); + lazyIndex = size_t(-1); pendingInput = NULL; flags = RegExpFlag(0); pendingLazyEvaluation = false; @@ -536,9 +537,9 @@ RegExpStatics::checkInvariants() { #ifdef DEBUG if (pendingLazyEvaluation) { - JS_ASSERT(regexp.initialized()); - JS_ASSERT(pendingInput); - JS_ASSERT(lastIndex != size_t(-1)); + JS_ASSERT(lazySource); + JS_ASSERT(matchesInput); + JS_ASSERT(lazyIndex != size_t(-1)); return; } diff --git a/js/src/vm/RegExpStatics.cpp b/js/src/vm/RegExpStatics.cpp index f28f2e212cd..6e3e3f80f32 100644 --- a/js/src/vm/RegExpStatics.cpp +++ b/js/src/vm/RegExpStatics.cpp @@ -74,9 +74,14 @@ RegExpStatics::executeLazy(JSContext *cx) if (!pendingLazyEvaluation) return true; - JS_ASSERT(regexp.initialized()); + JS_ASSERT(lazySource); JS_ASSERT(matchesInput); - JS_ASSERT(lastIndex != size_t(-1)); + JS_ASSERT(lazyIndex != size_t(-1)); + + /* Retrieve or create the RegExpShared in this compartment. */ + RegExpGuard g(cx); + if (!cx->compartment->regExps.get(cx, lazySource, lazyFlags, &g)) + return false; /* * It is not necessary to call aboutToWrite(): evaluation of @@ -87,7 +92,7 @@ RegExpStatics::executeLazy(JSContext *cx) StableCharPtr chars(matchesInput->chars(), length); /* Execute the full regular expression. */ - RegExpRunStatus status = regexp->execute(cx, chars, length, &this->lastIndex, this->matches); + RegExpRunStatus status = g->execute(cx, chars, length, &this->lazyIndex, this->matches); if (status == RegExpRunStatus_Error) return false; @@ -99,8 +104,8 @@ RegExpStatics::executeLazy(JSContext *cx) /* Unset lazy state and remove rooted values that now have no use. */ pendingLazyEvaluation = false; - regexp.release(); - lastIndex = size_t(-1); + lazySource = NULL; + lazyIndex = size_t(-1); return true; }