diff --git a/js/src/vm/SavedStacks.cpp b/js/src/vm/SavedStacks.cpp index 1fc48ed84889..e8edc650c2a9 100644 --- a/js/src/vm/SavedStacks.cpp +++ b/js/src/vm/SavedStacks.cpp @@ -57,6 +57,12 @@ const uint32_t ASYNC_STACK_MAX_FRAME_COUNT = 60; /* static */ Maybe LiveSavedFrameCache::getFramePtr(const FrameIter& iter) { + if (iter.done()) + return Nothing(); + + if (iter.isWasm()) + return Nothing(); + if (iter.hasUsableAbstractFramePtr()) return Some(FramePtr(iter.abstractFramePtr())); @@ -83,6 +89,7 @@ bool LiveSavedFrameCache::insert(JSContext* cx, FramePtr& framePtr, const jsbytecode* pc, HandleSavedFrame savedFrame) { + MOZ_ASSERT(savedFrame); MOZ_ASSERT(initialized()); if (!frames->emplaceBack(framePtr, pc, savedFrame)) { @@ -90,12 +97,7 @@ LiveSavedFrameCache::insert(JSContext* cx, FramePtr& framePtr, const jsbytecode* return false; } - // Safe to dereference the cache key because the stack frames are still - // live. After this point, they should never be dereferenced again. - if (framePtr.is()) - framePtr.as().setHasCachedSavedFrame(); - else - framePtr.as()->setHasCachedSavedFrame(); + framePtr.setHasCachedSavedFrame(); return true; } @@ -105,10 +107,10 @@ LiveSavedFrameCache::find(JSContext* cx, FrameIter& frameIter, MutableHandleSave { MOZ_ASSERT(initialized()); MOZ_ASSERT(!frameIter.done()); - MOZ_ASSERT(frameIter.hasCachedSavedFrame()); Maybe maybeFramePtr = getFramePtr(frameIter); MOZ_ASSERT(maybeFramePtr.isSome()); + MOZ_ASSERT(maybeFramePtr->hasCachedSavedFrame()); Key key(*maybeFramePtr); const jsbytecode* pc = frameIter.pc(); @@ -1321,6 +1323,7 @@ SavedStacks::insertFrames(JSContext* cx, FrameIter& iter, MutableHandleSavedFram RootedString asyncCause(cx, nullptr); bool parentIsInCache = false; RootedSavedFrame cachedFrame(cx, nullptr); + Maybe framePtr = LiveSavedFrameCache::getFramePtr(iter); // Accumulate the vector of Lookup objects in |stackChain|. SavedFrame::AutoLookupVector stackChain(cx); @@ -1377,12 +1380,11 @@ SavedStacks::insertFrames(JSContext* cx, FrameIter& iter, MutableHandleSavedFram // The bit set means that the next older parent (frame, pc) pair *must* // be in the cache. if (capture.is()) - parentIsInCache = iter.hasCachedSavedFrame(); + parentIsInCache = framePtr && framePtr->hasCachedSavedFrame(); auto principals = iter.compartment()->principals(); auto displayAtom = (iter.isWasm() || iter.isFunctionFrame()) ? iter.functionDisplayAtom() : nullptr; - Maybe framePtr = LiveSavedFrameCache::getFramePtr(iter); MOZ_ASSERT_IF(framePtr && !iter.isWasm(), iter.pc()); if (!stackChain->emplaceBack(location.source(), @@ -1408,10 +1410,11 @@ SavedStacks::insertFrames(JSContext* cx, FrameIter& iter, MutableHandleSavedFram } ++iter; + framePtr = LiveSavedFrameCache::getFramePtr(iter); if (parentIsInCache && - !iter.done() && - iter.hasCachedSavedFrame()) + framePtr && + framePtr->hasCachedSavedFrame()) { auto* cache = activation.getLiveSavedFrameCache(cx); if (!cache) diff --git a/js/src/vm/Stack-inl.h b/js/src/vm/Stack-inl.h index d8776318e87a..6a0687c7bbb2 100644 --- a/js/src/vm/Stack-inl.h +++ b/js/src/vm/Stack-inl.h @@ -998,34 +998,24 @@ InterpreterActivation::resumeGeneratorFrame(HandleFunction callee, HandleValue n return true; } +struct LiveSavedFrameCache::FramePtr::HasCachedMatcher { + bool match(AbstractFramePtr f) const { return f.hasCachedSavedFrame(); } + bool match(jit::CommonFrameLayout* f) const { return f->hasCachedSavedFrame(); } +}; + inline bool -FrameIter::hasCachedSavedFrame() const -{ - if (isWasm()) - return false; - - if (hasUsableAbstractFramePtr()) - return abstractFramePtr().hasCachedSavedFrame(); - - MOZ_ASSERT(jsJitFrame().isIonScripted()); - // SavedFrame caching is done at the physical frame granularity (rather than - // for each inlined frame) for ion. Therefore, it is impossible to have a - // cached SavedFrame if this frame is not a physical frame. - return isPhysicalIonFrame() && jsJitFrame().current()->hasCachedSavedFrame(); +LiveSavedFrameCache::FramePtr::hasCachedSavedFrame() const { + return ptr.match(HasCachedMatcher()); } +struct LiveSavedFrameCache::FramePtr::SetHasCachedMatcher { + void match(AbstractFramePtr f) const { f.setHasCachedSavedFrame(); } + void match(jit::CommonFrameLayout* f) const { f->setHasCachedSavedFrame(); } +}; + inline void -FrameIter::setHasCachedSavedFrame() -{ - MOZ_ASSERT(!isWasm()); - - if (hasUsableAbstractFramePtr()) { - abstractFramePtr().setHasCachedSavedFrame(); - return; - } - - MOZ_ASSERT(isPhysicalIonFrame()); - jsJitFrame().current()->setHasCachedSavedFrame(); +LiveSavedFrameCache::FramePtr::setHasCachedSavedFrame() { + ptr.match(SetHasCachedMatcher()); } } /* namespace js */ diff --git a/js/src/vm/Stack.h b/js/src/vm/Stack.h index f2d5a8448fad..cb9c08a7780d 100644 --- a/js/src/vm/Stack.h +++ b/js/src/vm/Stack.h @@ -1170,7 +1170,27 @@ struct DefaultHasher { class LiveSavedFrameCache { public: - using FramePtr = mozilla::Variant; + // The address of a live frame for which we can cache SavedFrames: it has a + // 'hasCachedSavedFrame' bit we can examine and set, and can be converted to + // a Key to index the cache. + class FramePtr { + using Ptr = mozilla::Variant; + + Ptr ptr; + + struct HasCachedMatcher; + struct SetHasCachedMatcher; + + public: + explicit FramePtr(AbstractFramePtr ptr) : ptr(ptr) { } + explicit FramePtr(jit::CommonFrameLayout* ptr) : ptr(ptr) { } + + inline bool hasCachedSavedFrame() const; + inline void setHasCachedSavedFrame(); + + bool operator==(const FramePtr& rhs) const { return rhs.ptr == this->ptr; } + bool operator!=(const FramePtr& rhs) const { return !(rhs == *this); } + }; private: // A key in the cache: the address of a frame, live or dead, for which we @@ -1958,10 +1978,6 @@ class FrameIter bool isFunctionFrame() const; bool hasArgs() const { return isFunctionFrame(); } - // These two methods may not be called with asm frames. - inline bool hasCachedSavedFrame() const; - inline void setHasCachedSavedFrame(); - ScriptSource* scriptSource() const; const char* filename() const; const char16_t* displayURL() const;