diff --git a/js/src/jit-test/tests/ion/gc-during-bailout.js b/js/src/jit-test/tests/ion/gc-during-bailout.js deleted file mode 100644 index c791ca37a767..000000000000 --- a/js/src/jit-test/tests/ion/gc-during-bailout.js +++ /dev/null @@ -1,80 +0,0 @@ -setJitCompilerOption("ion.warmup.trigger", 30); - -function f(i) { - var obj_1 = { a: 0 }; - var obj_2 = { a: 0 }; - var obj_3 = { a: 0 }; - var obj_4 = { a: 0 }; - var obj_5 = { a: 0 }; - var obj_6 = { a: 0 }; - var obj_7 = { a: 0 }; - var obj_8 = { a: 0 }; - var obj_9 = { a: 0 }; - var obj_10 = { a: 0 }; - var obj_11 = { a: 0 }; - var obj_12 = { a: 0 }; - var obj_13 = { a: 0 }; - var obj_14 = { a: 0 }; - var obj_15 = { a: 0 }; - var obj_16 = { a: 0 }; - var obj_17 = { a: 0 }; - var obj_18 = { a: 0 }; - var obj_19 = { a: 0 }; - var obj_20 = { a: 0 }; - var obj_21 = { a: 0 }; - var obj_22 = { a: 0 }; - var obj_23 = { a: 0 }; - var obj_24 = { a: 0 }; - var obj_25 = { a: 0 }; - var obj_26 = { a: 0 }; - var obj_27 = { a: 0 }; - var obj_28 = { a: 0 }; - var obj_29 = { a: 0 }; - var obj_30 = { a: 0 }; - - // Doing a bailout after the return of the function call implies that we - // cannot resume before the function call. Thus, we would have to recover - // the 30 objects allocations during the bailout. - schedulegc(i % 100); - bailout(); - - obj_1.a = 1; - obj_2.a = 1; - obj_3.a = 1; - obj_4.a = 1; - obj_5.a = 1; - obj_6.a = 1; - obj_7.a = 1; - obj_8.a = 1; - obj_9.a = 1; - obj_10.a = 1; - obj_11.a = 1; - obj_12.a = 1; - obj_13.a = 1; - obj_14.a = 1; - obj_15.a = 1; - obj_16.a = 1; - obj_17.a = 1; - obj_18.a = 1; - obj_19.a = 1; - obj_20.a = 1; - obj_21.a = 1; - obj_22.a = 1; - obj_23.a = 1; - obj_24.a = 1; - obj_25.a = 1; - obj_26.a = 1; - obj_27.a = 1; - obj_28.a = 1; - obj_29.a = 1; - obj_30.a = 1; -} - -gcPreserveCode(); - -var o = {}; -for (var i = 0; i < 200; i++) { - // Do not inline 'f', to keep re-entering 'f' at every loop iteration. - with (o) { } - f(i); -} diff --git a/js/src/jit/Bailouts.cpp b/js/src/jit/Bailouts.cpp index 6476ce8b2728..00d35450c9a1 100644 --- a/js/src/jit/Bailouts.cpp +++ b/js/src/jit/Bailouts.cpp @@ -35,11 +35,11 @@ jit::Bailout(BailoutStack *sp, BaselineBailoutInfo **bailoutInfo) IsInRange(FAKE_JIT_TOP_FOR_BAILOUT + sizeof(IonCommonFrameLayout), 0, 0x1000), "Fake jitTop pointer should be within the first page."); cx->mainThread().jitTop = FAKE_JIT_TOP_FOR_BAILOUT; + gc::AutoSuppressGC suppress(cx); JitActivationIterator jitActivations(cx->runtime()); BailoutFrameInfo bailoutData(jitActivations, sp); JitFrameIterator iter(jitActivations); - MOZ_ASSERT(!iter.ionScript()->invalidated()); TraceLogger *logger = TraceLoggerForMainThread(cx->runtime()); TraceLogTimestamp(logger, TraceLogger::Bailout); @@ -78,17 +78,6 @@ jit::Bailout(BailoutStack *sp, BaselineBailoutInfo **bailoutInfo) EnsureExitFrame(iter.jsFrame()); } - // This condition was wrong when we entered this bailout function, but it - // might be true now. A GC might have reclaimed all the Jit code and - // invalidated all frames which are currently on the stack. As we are - // already in a bailout, we could not switch to an invalidation - // bailout. When the code of an IonScript which is on the stack is - // invalidated (see InvalidateActivation), we remove references to it and - // increment the reference counter for each activation that appear on the - // stack. As the bailed frame is one of them, we have to decrement it now. - if (iter.ionScript()->invalidated()) - iter.ionScript()->decref(cx->runtime()->defaultFreeOp()); - return retval; } @@ -102,6 +91,7 @@ jit::InvalidationBailout(InvalidationBailoutStack *sp, size_t *frameSizeOut, // We don't have an exit frame. cx->mainThread().jitTop = FAKE_JIT_TOP_FOR_BAILOUT; + gc::AutoSuppressGC suppress(cx); JitActivationIterator jitActivations(cx->runtime()); BailoutFrameInfo bailoutData(jitActivations, sp); diff --git a/js/src/jit/BaselineBailouts.cpp b/js/src/jit/BaselineBailouts.cpp index bbf6461f2ac8..c0ee98dbc2c6 100644 --- a/js/src/jit/BaselineBailouts.cpp +++ b/js/src/jit/BaselineBailouts.cpp @@ -385,33 +385,26 @@ struct BaselineStackBuilder // ahead of the bailout. class SnapshotIteratorForBailout : public SnapshotIterator { - JitActivation *activation_; - JitFrameIterator &iter_; - + RInstructionResults results_; public: - SnapshotIteratorForBailout(JitActivation *activation, JitFrameIterator &iter) + + SnapshotIteratorForBailout(const JitFrameIterator &iter) : SnapshotIterator(iter), - activation_(activation), - iter_(iter) + results_(iter.jsFrame()) { MOZ_ASSERT(iter.isBailoutJS()); } - ~SnapshotIteratorForBailout() { - // The bailout is complete, we no longer need the recover instruction - // results. - activation_->removeIonFrameRecovery(fp_); - } - // Take previously computed result out of the activation, or compute the // results of all recover instructions contained in the snapshot. - bool init(JSContext *cx) { + bool init(JSContext *cx, JitActivation *activation) { + activation->maybeTakeIonFrameRecovery(fp_, &results_); + if (!results_.isInitialized() && !computeInstructionResults(cx, &results_)) + return false; - // Under a bailout, there is no need to invalidate the frame after - // evaluating the recover instruction, as the invalidation is only - // needed to cause of the frame which has been introspected. - MaybeReadFallback recoverBailout(cx, activation_, &iter_, MaybeReadFallback::Fallback_DoNothing); - return initInstructionResults(recoverBailout); + MOZ_ASSERT(results_.isInitialized()); + instructionResults_ = &results_; + return true; } }; @@ -525,10 +518,6 @@ InitFromBailout(JSContext *cx, HandleScript caller, jsbytecode *callerPC, jsbytecode **callPC, const ExceptionBailoutInfo *excInfo, bool *poppedLastSPSFrameOut) { - // The Baseline frames we will reconstruct on the heap are not rooted, so GC - // must be suppressed here. - MOZ_ASSERT(cx->mainThread().suppressGC); - MOZ_ASSERT(script->hasBaselineScript()); MOZ_ASSERT(poppedLastSPSFrameOut); MOZ_ASSERT(!*poppedLastSPSFrameOut); @@ -1312,6 +1301,10 @@ jit::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, JitFrameIter bool invalidate, BaselineBailoutInfo **bailoutInfo, const ExceptionBailoutInfo *excInfo, bool *poppedLastSPSFrameOut) { + // The Baseline frames we will reconstruct on the heap are not rooted, so GC + // must be suppressed here. + MOZ_ASSERT(cx->mainThread().suppressGC); + MOZ_ASSERT(bailoutInfo != nullptr); MOZ_ASSERT(*bailoutInfo == nullptr); @@ -1390,8 +1383,8 @@ jit::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, JitFrameIter return BAILOUT_RETURN_FATAL_ERROR; JitSpew(JitSpew_BaselineBailouts, " Incoming frame ptr = %p", builder.startFrame()); - SnapshotIteratorForBailout snapIter(activation, iter); - if (!snapIter.init(cx)) + SnapshotIteratorForBailout snapIter(iter); + if (!snapIter.init(cx, activation)) return BAILOUT_RETURN_FATAL_ERROR; #ifdef TRACK_SNAPSHOTS @@ -1424,8 +1417,6 @@ jit::BailoutIonToBaseline(JSContext *cx, JitActivation *activation, JitFrameIter RootedScript topCaller(cx); jsbytecode *topCallerPC = nullptr; - gc::AutoSuppressGC suppress(cx); - while (true) { // Skip recover instructions as they are already recovered by |initInstructionResults|. snapIter.settleOnFrame(); diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index 42e1e68e22a9..acac206820f5 100644 --- a/js/src/jit/Ion.cpp +++ b/js/src/jit/Ion.cpp @@ -2685,7 +2685,7 @@ InvalidateActivation(FreeOp *fop, const JitActivationIterator &activations, bool size_t frameno = 1; for (JitFrameIterator it(activations); !it.done(); ++it, ++frameno) { - MOZ_ASSERT_IF(frameno == 1, it.type() == JitFrame_Exit || it.type() == JitFrame_Bailout); + MOZ_ASSERT_IF(frameno == 1, it.type() == JitFrame_Exit); #ifdef DEBUG switch (it.type()) { @@ -2727,7 +2727,7 @@ InvalidateActivation(FreeOp *fop, const JitActivationIterator &activations, bool } #endif - if (!it.isIonScripted()) + if (!it.isIonJS()) continue; bool calledFromLinkStub = false; @@ -2795,9 +2795,8 @@ InvalidateActivation(FreeOp *fop, const JitActivationIterator &activations, bool } ionCode->setInvalidated(); - // Don't adjust OSI points in the linkStub (which don't exist), or in a - // bailout path. - if (calledFromLinkStub || it.isBailoutJS()) + // Don't adjust OSI points in the linkStub (which don't exist). + if (calledFromLinkStub) continue; // Write the delta (from the return address offset to the diff --git a/js/src/jit/IonFrames.cpp b/js/src/jit/IonFrames.cpp index 274f10309d2e..3f2a9a53ebff 100644 --- a/js/src/jit/IonFrames.cpp +++ b/js/src/jit/IonFrames.cpp @@ -56,12 +56,6 @@ ReadFrameSlot(IonJSFrameLayout *fp, int32_t slot) return *(uintptr_t *)((char *)fp + OffsetOfFrameSlot(slot)); } -static inline void -WriteFrameSlot(IonJSFrameLayout *fp, int32_t slot, uintptr_t value) -{ - *(uintptr_t *)((char *)fp + OffsetOfFrameSlot(slot)) = value; -} - static inline double ReadFrameDoubleSlot(IonJSFrameLayout *fp, int32_t slot) { @@ -140,13 +134,8 @@ JitFrameIterator::checkInvalidation() const bool JitFrameIterator::checkInvalidation(IonScript **ionScriptOut) const { - JSScript *script = this->script(); - if (isBailoutJS()) { - *ionScriptOut = activation_->bailoutData()->ionScript(); - return !script->hasIonScript() || script->ionScript() != *ionScriptOut; - } - uint8_t *returnAddr = returnAddressToFp(); + JSScript *script = this->script(); // N.B. the current IonScript is not the same as the frame's // IonScript if the frame has since been invalidated. bool invalidated; @@ -983,42 +972,6 @@ MarkIonJSFrame(JSTracer *trc, const JitFrameIterator &frame) #endif } -static void -MarkBailoutFrame(JSTracer *trc, const JitFrameIterator &frame) -{ - IonJSFrameLayout *layout = (IonJSFrameLayout *)frame.fp(); - - layout->replaceCalleeToken(MarkCalleeToken(trc, layout->calleeToken())); - - // We have to mark the list of actual arguments, as only formal arguments - // are represented in the Snapshot. - MarkFrameAndActualArguments(trc, frame); - - // Under a bailout, do not have a Safepoint to only iterate over GC-things. - // Thus we use a SnapshotIterator to trace all the locations which would be - // used to reconstruct the Baseline frame. - // - // Note that at the time where this function is called, we have not yet - // started to reconstruct baseline frames. - - // The vector of recover instructions is already traced as part of the - // JitActivation. - SnapshotIterator snapIter(frame); - - // For each instruction, we read the allocations without evaluating the - // recover instruction, nor reconstructing the frame. We are only looking at - // tracing readable allocations. - while (true) { - while (snapIter.moreAllocations()) - snapIter.traceAllocation(trc); - - if (!snapIter.moreInstructions()) - break; - snapIter.nextInstruction(); - }; - -} - #ifdef JSGC_GENERATIONAL template void @@ -1353,9 +1306,6 @@ MarkJitActivation(JSTracer *trc, const JitActivationIterator &activations) case JitFrame_IonJS: MarkIonJSFrame(trc, frames); break; - case JitFrame_Bailout: - MarkBailoutFrame(trc, frames); - break; case JitFrame_Unwound_IonJS: MOZ_CRASH("invalid"); case JitFrame_Rectifier: @@ -1802,87 +1752,6 @@ SnapshotIterator::allocationValue(const RValueAllocation &alloc) } } -void -SnapshotIterator::writeAllocationValuePayload(const RValueAllocation &alloc, Value v) -{ - switch (alloc.mode()) { - case RValueAllocation::CONSTANT: - ionScript_->getConstant(alloc.index()) = v; - break; - - case RValueAllocation::CST_UNDEFINED: - case RValueAllocation::CST_NULL: - case RValueAllocation::DOUBLE_REG: - case RValueAllocation::FLOAT32_REG: - case RValueAllocation::FLOAT32_STACK: - MOZ_CRASH("Not a GC thing: Unexpected write"); - break; - - case RValueAllocation::TYPED_REG: - machine_.write(alloc.reg2(), *v.payloadUIntPtr()); - break; - - case RValueAllocation::TYPED_STACK: - switch (alloc.knownType()) { - default: - MOZ_CRASH("Not a GC thing: Unexpected write"); - break; - case JSVAL_TYPE_STRING: - case JSVAL_TYPE_SYMBOL: - case JSVAL_TYPE_OBJECT: - WriteFrameSlot(fp_, alloc.stackOffset2(), *v.payloadUIntPtr()); - break; - } - break; - -#if defined(JS_NUNBOX32) - case RValueAllocation::UNTYPED_REG_REG: - case RValueAllocation::UNTYPED_STACK_REG: - machine_.write(alloc.reg2(), *v.payloadUIntPtr()); - break; - - case RValueAllocation::UNTYPED_REG_STACK: - case RValueAllocation::UNTYPED_STACK_STACK: - WriteFrameSlot(fp_, alloc.stackOffset2(), *v.payloadUIntPtr()); - break; -#elif defined(JS_PUNBOX64) - case RValueAllocation::UNTYPED_REG: - machine_.write(alloc.reg(), *v.payloadUIntPtr()); - break; - - case RValueAllocation::UNTYPED_STACK: - WriteFrameSlot(fp_, alloc.stackOffset(), *v.payloadUIntPtr()); - break; -#endif - - case RValueAllocation::RECOVER_INSTRUCTION: - MOZ_CRASH("Recover instructions are handled by the JitActivation."); - break; - - default: - MOZ_CRASH("huh?"); - } -} - -void -SnapshotIterator::traceAllocation(JSTracer *trc) -{ - RValueAllocation alloc = readAllocation(); - if (!allocationReadable(alloc)) - return; - - Value v = allocationValue(alloc); - if (!v.isMarkable()) - return; - - Value copy = v; - gc::MarkValueRoot(trc, &v, "ion-typed-reg"); - if (v != copy) { - MOZ_ASSERT(SameType(v, copy)); - writeAllocationValuePayload(alloc, v); - } -} - const RResumePoint * SnapshotIterator::resumePoint() const { @@ -1930,11 +1799,8 @@ SnapshotIterator::initInstructionResults(MaybeReadFallback &fallback) // same reason, we need to recompile without optimizing away the // observable stack slots. The script would later be recompiled to have // support for Argument objects. - if (fallback.consequence == MaybeReadFallback::Fallback_Invalidate && - !ionScript_->invalidate(cx, /* resetUses = */ false, "Observe recovered instruction.")) - { + if (!ionScript_->invalidate(cx, /* resetUses = */ false, "Observe recovered instruction.")) return false; - } // Register the list of result on the activation. We need to do that // before we initialize the list such as if any recover instruction @@ -1954,7 +1820,7 @@ SnapshotIterator::initInstructionResults(MaybeReadFallback &fallback) // If the evaluation failed because of OOMs, then we discard the // current set of result that we collected so far. - fallback.activation->removeIonFrameRecovery(fp); + fallback.activation->maybeTakeIonFrameRecovery(fp, &tmp); return false; } } diff --git a/js/src/jit/JitFrameIterator.h b/js/src/jit/JitFrameIterator.h index 691af2d53191..8a8324828a20 100644 --- a/js/src/jit/JitFrameIterator.h +++ b/js/src/jit/JitFrameIterator.h @@ -299,33 +299,24 @@ struct MaybeReadFallback NoGC_MagicOptimizedOut }; - enum FallbackConsequence { - Fallback_Invalidate, - Fallback_DoNothing - }; - JSContext *maybeCx; JitActivation *activation; JitFrameIterator *frame; const NoGCValue unreadablePlaceholder_; - const FallbackConsequence consequence; MaybeReadFallback(const Value &placeholder = UndefinedValue()) : maybeCx(nullptr), activation(nullptr), frame(nullptr), - unreadablePlaceholder_(noGCPlaceholder(placeholder)), - consequence(Fallback_Invalidate) + unreadablePlaceholder_(noGCPlaceholder(placeholder)) { } - MaybeReadFallback(JSContext *cx, JitActivation *activation, JitFrameIterator *frame, - FallbackConsequence consequence = Fallback_Invalidate) + MaybeReadFallback(JSContext *cx, JitActivation *activation, JitFrameIterator *frame) : maybeCx(cx), activation(activation), frame(frame), - unreadablePlaceholder_(NoGC_UndefinedValue), - consequence(consequence) + unreadablePlaceholder_(NoGC_UndefinedValue) { } @@ -388,7 +379,6 @@ class SnapshotIterator Value allocationValue(const RValueAllocation &a); bool allocationReadable(const RValueAllocation &a); - void writeAllocationValuePayload(const RValueAllocation &a, Value v); void warnUnreadableAllocation(); public: @@ -503,8 +493,6 @@ class SnapshotIterator return fallback.unreadablePlaceholder(); } - void traceAllocation(JSTracer *trc); - void readCommonFrameSlots(Value *scopeChain, Value *rval) { if (scopeChain) *scopeChain = read(); diff --git a/js/src/vm/Stack.cpp b/js/src/vm/Stack.cpp index afd634c6760d..f4dc2733a027 100644 --- a/js/src/vm/Stack.cpp +++ b/js/src/vm/Stack.cpp @@ -1550,12 +1550,13 @@ jit::JitActivation::maybeIonFrameRecovery(IonJSFrameLayout *fp) } void -jit::JitActivation::removeIonFrameRecovery(IonJSFrameLayout *fp) +jit::JitActivation::maybeTakeIonFrameRecovery(IonJSFrameLayout *fp, RInstructionResults *results) { RInstructionResults *elem = maybeIonFrameRecovery(fp); if (!elem) return; + *results = mozilla::Move(*elem); ionRecovery_.erase(elem); } diff --git a/js/src/vm/Stack.h b/js/src/vm/Stack.h index 9ef20fea422c..2dcc08621784 100644 --- a/js/src/vm/Stack.h +++ b/js/src/vm/Stack.h @@ -1367,9 +1367,10 @@ class JitActivation : public Activation // Return the pointer to the Ion frame recovery, if it is already registered. RInstructionResults *maybeIonFrameRecovery(IonJSFrameLayout *fp); - // If an Ion frame recovery exists for the |fp| frame exists, then remove it - // from the activation. - void removeIonFrameRecovery(IonJSFrameLayout *fp); + // If an Ion frame recovery exists for the |fp| frame exists on the + // activation, then move its content to the |results| argument, and remove + // it from the activation. + void maybeTakeIonFrameRecovery(IonJSFrameLayout *fp, RInstructionResults *results); void markIonRecovery(JSTracer *trc);