Bug 1242840 - Drop profiler invalidation event if we hit OOM and make invalidation infallible r=jandem

js/src/jit-test/tests/profiler/bug1242840.js

@@ -0,0 +1,16 @@
+if (!('oomTest' in this))
+    quit();
+
+enableSPSProfiling();
+oomTest(() => {
+    try {
+        for (quit of ArrayBuffer);
+    } catch (e) {
+        switch (1) {
+            case 0:
+                let x
+            case 1:
+                (function() x)()
+        }
+    }
+})

js/src/jit/Bailouts.cpp

@@ -272,7 +272,7 @@ jit::EnsureHasScopeObjects(JSContext* cx, AbstractFramePtr fp)
     return true;
 }
 
-bool
+void
 jit::CheckFrequentBailouts(JSContext* cx, JSScript* script, BailoutKind bailoutKind)
 {
     if (script->hasIonScript()) {
@@ -291,12 +291,9 @@ jit::CheckFrequentBailouts(JSContext* cx, JSScript* script, BailoutKind bailoutK
 
             JitSpew(JitSpew_IonInvalidate, "Invalidating due to too many bailouts");
 
-            if (!Invalidate(cx, script))
-                return false;
+            Invalidate(cx, script);
         }
     }
-
-    return true;
 }
 
 void

js/src/jit/Bailouts.h

@@ -211,7 +211,7 @@ uint32_t ExceptionHandlerBailout(JSContext* cx, const InlineFrameIterator& frame
 
 uint32_t FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfo);
 
-bool CheckFrequentBailouts(JSContext* cx, JSScript* script, BailoutKind bailoutKind);
+void CheckFrequentBailouts(JSContext* cx, JSScript* script, BailoutKind bailoutKind);
 
 } // namespace jit
 } // namespace js

js/src/jit/BaselineBailouts.cpp

@@ -1623,7 +1623,7 @@ jit::BailoutIonToBaseline(JSContext* cx, JitActivation* activation, JitFrameIter
     return BAILOUT_RETURN_OK;
 }
 
-static bool
+static void
 InvalidateAfterBailout(JSContext* cx, HandleScript outerScript, const char* reason)
 {
     // In some cases, the computation of recover instruction can invalidate the
@@ -1635,16 +1635,16 @@ InvalidateAfterBailout(JSContext* cx, HandleScript outerScript, const char* reas
     // objects no longer match the content of its properties (see Bug 1174547)
     if (!outerScript->hasIonScript()) {
         JitSpew(JitSpew_BaselineBailouts, "Ion script is already invalidated");
-        return true;
+        return;
     }
 
     MOZ_ASSERT(!outerScript->ionScript()->invalidated());
 
     JitSpew(JitSpew_BaselineBailouts, "Invalidating due to %s", reason);
-    return Invalidate(cx, outerScript);
+    Invalidate(cx, outerScript);
 }
 
-static bool
+static void
 HandleBoundsCheckFailure(JSContext* cx, HandleScript outerScript, HandleScript innerScript)
 {
     JitSpew(JitSpew_IonBailouts, "Bounds check failure %s:%d, inlined into %s:%d",
@@ -1654,16 +1654,12 @@ HandleBoundsCheckFailure(JSContext* cx, HandleScript outerScript, HandleScript i
     if (!innerScript->failedBoundsCheck())
         innerScript->setFailedBoundsCheck();
 
-    if (!InvalidateAfterBailout(cx, outerScript, "bounds check failure"))
-        return false;
-
-    if (innerScript->hasIonScript() && !Invalidate(cx, innerScript))
-        return false;
-
-    return true;
+    InvalidateAfterBailout(cx, outerScript, "bounds check failure");
+    if (innerScript->hasIonScript())
+        Invalidate(cx, innerScript);
 }
 
-static bool
+static void
 HandleShapeGuardFailure(JSContext* cx, HandleScript outerScript, HandleScript innerScript)
 {
     JitSpew(JitSpew_IonBailouts, "Shape guard failure %s:%d, inlined into %s:%d",
@@ -1675,20 +1671,20 @@ HandleShapeGuardFailure(JSContext* cx, HandleScript outerScript, HandleScript in
     // inner and outer scripts, instead of just the outer one.
     outerScript->setFailedShapeGuard();
 
-    return InvalidateAfterBailout(cx, outerScript, "shape guard failure");
+    InvalidateAfterBailout(cx, outerScript, "shape guard failure");
 }
 
-static bool
+static void
 HandleBaselineInfoBailout(JSContext* cx, HandleScript outerScript, HandleScript innerScript)
 {
     JitSpew(JitSpew_IonBailouts, "Baseline info failure %s:%d, inlined into %s:%d",
             innerScript->filename(), innerScript->lineno(),
             outerScript->filename(), outerScript->lineno());
 
-    return InvalidateAfterBailout(cx, outerScript, "invalid baseline info");
+    InvalidateAfterBailout(cx, outerScript, "invalid baseline info");
 }
 
-static bool
+static void
 HandleLexicalCheckFailure(JSContext* cx, HandleScript outerScript, HandleScript innerScript)
 {
     JitSpew(JitSpew_IonBailouts, "Lexical check failure %s:%d, inlined into %s:%d",
@@ -1698,13 +1694,9 @@ HandleLexicalCheckFailure(JSContext* cx, HandleScript outerScript, HandleScript
     if (!innerScript->failedLexicalCheck())
         innerScript->setFailedLexicalCheck();
 
-    if (!InvalidateAfterBailout(cx, outerScript, "lexical check failure"))
-        return false;
-
-    if (innerScript->hasIonScript() && !Invalidate(cx, innerScript))
-        return false;
-
-    return true;
+    InvalidateAfterBailout(cx, outerScript, "lexical check failure");
+    if (innerScript->hasIonScript())
+        Invalidate(cx, innerScript);
 }
 
 static bool
@@ -1919,8 +1911,7 @@ jit::FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfo)
       case Bailout_NonStringInputInvalidate:
       case Bailout_DoubleOutput:
       case Bailout_ObjectIdentityOrTypeGuard:
-        if (!HandleBaselineInfoBailout(cx, outerScript, innerScript))
-            return false;
+        HandleBaselineInfoBailout(cx, outerScript, innerScript);
         break;
 
       case Bailout_ArgumentCheck:
@@ -1928,16 +1919,13 @@ jit::FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfo)
         break;
       case Bailout_BoundsCheck:
       case Bailout_Detached:
-        if (!HandleBoundsCheckFailure(cx, outerScript, innerScript))
-            return false;
+        HandleBoundsCheckFailure(cx, outerScript, innerScript);
         break;
       case Bailout_ShapeGuard:
-        if (!HandleShapeGuardFailure(cx, outerScript, innerScript))
-            return false;
+        HandleShapeGuardFailure(cx, outerScript, innerScript);
         break;
       case Bailout_UninitializedLexical:
-        if (!HandleLexicalCheckFailure(cx, outerScript, innerScript))
-            return false;
+        HandleLexicalCheckFailure(cx, outerScript, innerScript);
         break;
       case Bailout_IonExceptionDebugMode:
         // Return false to resume in HandleException with reconstructed
@@ -1947,8 +1935,7 @@ jit::FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfo)
         MOZ_CRASH("Unknown bailout kind!");
     }
 
-    if (!CheckFrequentBailouts(cx, outerScript, bailoutKind))
-        return false;
+    CheckFrequentBailouts(cx, outerScript, bailoutKind);
 
     // We're returning to JIT code, so we should clear the override pc.
     topFrame->clearOverridePc();

js/src/jit/BaselineIC.cpp

@@ -7679,8 +7679,10 @@ ICTableSwitch::Compiler::getStub(ICStubSpace* space)
     pc += JUMP_OFFSET_LEN;
 
     void** table = (void**) space->alloc(sizeof(void*) * length);
-    if (!table)
+    if (!table) {
+        ReportOutOfMemory(cx);
         return nullptr;
+    }
 
     jsbytecode* defaultpc = pc_ + GET_JUMP_OFFSET(pc_);
 

js/src/jit/CodeGenerator.cpp

@@ -8259,8 +8259,7 @@ CodeGenerator::link(JSContext* cx, CompilerConstraintList* constraints)
         MOZ_ASSERT(script->ionScript()->isRecompiling());
         // Do a normal invalidate, except don't cancel offThread compilations,
         // since that will cancel this compilation too.
-        if (!Invalidate(cx, script, /* resetUses */ false, /* cancelOffThread*/ false))
-            return false;
+        Invalidate(cx, script, /* resetUses */ false, /* cancelOffThread*/ false);
     }
 
     if (scriptCounts_ && !script->hasScriptCounts() && !script->initScriptCounts(cx))

js/src/jit/Ion.cpp

@@ -3237,7 +3237,7 @@ jit::IonScript::invalidate(JSContext* cx, bool resetUses, const char* reason)
     Invalidate(cx, list, resetUses, true);
 }
 
-bool
+void
 jit::Invalidate(JSContext* cx, JSScript* script, bool resetUses, bool cancelOffThread)
 {
     MOZ_ASSERT(script->hasIonScript());
@@ -3252,15 +3252,14 @@ jit::Invalidate(JSContext* cx, JSScript* script, bool resetUses, bool cancelOffT
         if (filename == nullptr)
             filename = "<unknown>";
 
-        size_t len = strlen(filename) + 20;
-        char* buf = js_pod_malloc<char>(len);
-        if (!buf)
-            return false;
-
         // Construct the descriptive string.
-        JS_snprintf(buf, len, "Invalidate %s:%" PRIuSIZE, filename, script->lineno());
-        cx->runtime()->spsProfiler.markEvent(buf);
-        js_free(buf);
+        char* buf = JS_smprintf("Invalidate %s:%" PRIuSIZE, filename, script->lineno());
+
+        // Ignore the event on allocation failure.
+        if (buf) {
+            cx->runtime()->spsProfiler.markEvent(buf);
+            JS_smprintf_free(buf);
+        }
     }
 
     // RecompileInfoVector has inline space for at least one element.
@@ -3270,7 +3269,6 @@ jit::Invalidate(JSContext* cx, JSScript* script, bool resetUses, bool cancelOffT
     scripts.infallibleAppend(script->ionScript()->recompileInfo());
 
     Invalidate(cx, scripts, resetUses, cancelOffThread);
-    return true;
 }
 
 static void
@@ -3308,15 +3306,8 @@ jit::ForbidCompilation(JSContext* cx, JSScript* script)
 
     CancelOffThreadIonCompile(cx->compartment(), script);
 
-    if (script->hasIonScript()) {
-        // It is only safe to modify script->ion if the script is not currently
-        // running, because JitFrameIterator needs to tell what ionScript to
-        // use (either the one on the JSScript, or the one hidden in the
-        // breadcrumbs Invalidation() leaves). Therefore, if invalidation
-        // fails, we cannot disable the script.
-        if (!Invalidate(cx, script, false))
-            return;
-    }
+    if (script->hasIonScript())
+        Invalidate(cx, script, false);
 
     script->setIonScript(cx, ION_DISABLED_SCRIPT);
 }

js/src/jit/Ion.h

@@ -127,7 +127,7 @@ void Invalidate(TypeZone& types, FreeOp* fop,
                 bool cancelOffThread = true);
 void Invalidate(JSContext* cx, const RecompileInfoVector& invalid, bool resetUses = true,
                 bool cancelOffThread = true);
-bool Invalidate(JSContext* cx, JSScript* script, bool resetUses = true,
+void Invalidate(JSContext* cx, JSScript* script, bool resetUses = true,
                 bool cancelOffThread = true);
 
 void ToggleBarriers(JS::Zone* zone, bool needs);

js/src/jit/IonCaches.cpp

@@ -2253,10 +2253,10 @@ GetPropertyIC::update(JSContext* cx, HandleScript outerScript, size_t cacheIndex
         outerScript->setInvalidatedIdempotentCache();
 
         // Do not re-invalidate if the lookup already caused invalidation.
-        if (!outerScript->hasIonScript())
-            return true;
+        if (outerScript->hasIonScript())
+            Invalidate(cx, outerScript);
 
-        return Invalidate(cx, outerScript);
+        return true;
     }
 
     jsbytecode* pc = cache.idempotent() ? nullptr : cache.pc();