[JSC] A catch OSR entrypoint needs a sample of every operand

JSTests/stress/catch-osr-entry-is-made.js

@@ -0,0 +1,35 @@
+//@ runFTLNoCJIT; run("ftl-no-cjit-eager-catch-liveness", "--useFTLJIT=true", "--useConcurrentJIT=false", "--useLazyCatchLiveness=false")
+
+// This handler has to get a catch OSR entrypoint: at some point it runs in optimized code.
+
+function thrower(shouldThrow) {
+    if (shouldThrow)
+        throw new Error("thrown");
+}
+noInline(thrower);
+
+function shouldEnter(f) {
+    for (let i = 0; i < 10 * testLoopCount; ++i) {
+        if (f(!(i % 10)))
+            return;
+    }
+    if ($vm.useDFGJIT())
+        throw new Error(f.name + ": the handler never ran in optimized code");
+}
+
+// The completion value of the finally is live at the catch and always the empty value there.
+function catchInsideFinally(shouldThrow) {
+    let optimized = false;
+    try {
+        try {
+            thrower(shouldThrow);
+        } catch {
+            optimized = $vm.dfgTrue();
+        }
+    } finally {
+        thrower(false);
+    }
+    return optimized;
+}
+noInline(catchInsideFinally);
+shouldEnter(catchInsideFinally);

JSTests/stress/catch-osr-entry-operand-without-sample.js

@@ -0,0 +1,98 @@
+//@ defaultRun; run("eager-catch-liveness", "--useLazyCatchLiveness=false")
+
+// A catch OSR entrypoint must not give the DFG an operand without a prediction. The DFG takes SpecNone for code that
+// never ran: a double use of such a value is DoubleRep(NotCellNorBigIntUse), which is ToNumber, not a check.
+
+function shouldBe(actual, expected, name) {
+    if (actual !== expected)
+        throw new Error(name + ": expected " + expected + " but got " + actual);
+}
+
+function thrower(shouldThrow) {
+    if (shouldThrow)
+        throw new Error("thrown");
+}
+noInline(thrower);
+
+const notNumbers = [null, true, undefined, false];
+
+// With Options::useLazyCatchLiveness() these catches run before their value-profile buffer exists and not again
+// until the DFG has compiled the function, so the buffer has no sample at all.
+const neverSampled = {
+    localAfterCatch(value, shouldThrow) {
+        const t = value.p;
+        try {
+            thrower(shouldThrow);
+        } catch { }
+        return String(t);
+    },
+
+    localInFinally(value, shouldThrow) {
+        const t = value.p;
+        let result;
+        try {
+            try {
+                thrower(shouldThrow);
+            } finally {
+                result = String(t);
+            }
+        } catch { }
+        return result;
+    },
+
+    localInNewArrayInCatch(value, shouldThrow) {
+        const t = value.p;
+        try {
+            thrower(shouldThrow);
+        } catch {
+            return String([t][0]);
+        }
+        return t;
+    },
+
+    argumentInNewArrayInCatch(value, shouldThrow, t) {
+        try {
+            thrower(shouldThrow);
+        } catch {
+            return String([t][0]);
+        }
+        return t;
+    },
+};
+
+for (const name in neverSampled) {
+    const f = neverSampled[name];
+    noInline(f);
+    f({ p: 0.5 }, true, 0.5);
+    for (let i = 0; i < testLoopCount; ++i)
+        f({ p: i + 0.5 }, false, i + 0.5);
+    for (const value of notNumbers)
+        shouldBe(f({ p: value }, true, value), String(value), name);
+}
+
+// With or without the option: the last sample before each update of the prediction has t in its TDZ, the empty value.
+function sampledWhileEmpty(skipDeclaration, value, shouldThrow, array) {
+    switch (skipDeclaration) {
+    case 0:
+        let t = value.p;
+    case 1:
+        try {
+            thrower(shouldThrow);
+        } catch {
+            if (!skipDeclaration) {
+                array[0] = t;
+                return String(array[0]);
+            }
+        }
+    }
+}
+noInline(sampledWhileEmpty);
+
+sampledWhileEmpty(0, { p: 0.5 }, true, [0.5]);
+for (let i = 0; i < testLoopCount; ++i) {
+    sampledWhileEmpty(0, { p: i + 0.5 }, false, [0.5]);
+    if (!(i % 20))
+        sampledWhileEmpty(1, { p: 0.5 }, true, [0.5]);
+}
+for (const value of notNumbers)
+    shouldBe(sampledWhileEmpty(0, { p: value }, true, [0.5]), String(value), "sampledWhileEmpty");

Source/JavaScriptCore/bytecode/CodeBlock.h

@@ -916,7 +916,7 @@ class CodeBlock : public JSCell {
 
     // Null while Options::useLazyCatchLiveness() defers creating the buffer; op_catch does not profile until then.
     ValueProfileAndVirtualRegisterBuffer* ensureCatchLivenessIsComputedForBytecodeIndex(BytecodeIndex);
-    // Mutator only, on the baseline block, before a DFG / FTL plan parses it: creates the buffers useLazyCatchLiveness deferred. True if it created one.
+    // Mutator only, on the baseline block: creates the buffers useLazyCatchLiveness deferred. True if it created one, which has no samples.
     bool ensureCatchLivenessIsComputedForExecutedCatches()
     {
         if (m_hasCatchThatExecutedWithoutBuffer) [[unlikely]]

Source/JavaScriptCore/bytecode/ValueProfile.h

@@ -157,6 +157,15 @@ struct ArgumentValueProfile : public ValueProfileBase<1, 1> {
 };
 
 struct ValueProfileAndVirtualRegister : public ValueProfile {
+    // An empty bucket means no sample, so the empty value goes to the prediction: a sampled operand never predicts SpecNone.
+    void sample(JSValue value)
+    {
+        if (value)
+            m_buckets[0] = JSValue::encode(value);
+        else
+            mergeSpeculation(m_prediction, SpecEmpty);
+    }
+
     VirtualRegister m_operand;
 };
 

Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -10207,18 +10207,16 @@ void ByteCodeParser::parseBlock(unsigned limit)
                 NEXT_OPCODE(op_catch); // This catch has yet to execute. Note: this load can be racy with the main thread.
             }
 
-            // We're now committed to compiling this as an entrypoint.
-            m_currentBlock->isCatchEntrypoint = true;
-            m_graph.m_roots.append(m_currentBlock);
-
             Vector<SpeculatedType> argumentPredictions(m_numArguments);
             Vector<SpeculatedType> localPredictions;
             UncheckedKeyHashSet<unsigned, WTF::IntHash<unsigned>, WTF::UnsignedWithZeroKeyHashTraits<unsigned>> seenArguments;
+            bool everyOperandIsSampled = true;
 
             {
                 buffer->forEach([&](ValueProfileAndVirtualRegister& profile) {
                     VirtualRegister operand(profile.m_operand);
                     SpeculatedType prediction = profile.computeUpdatedPrediction();
+                    everyOperandIsSampled &= prediction != SpecNone;
                     if (operand.isLocal())
                         localPredictions.append(prediction);
                     else {
@@ -10236,6 +10234,16 @@ void ByteCodeParser::parseBlock(unsigned limit)
                 }
             }
 
+            if (!everyOperandIsSampled) {
+                // A buffer exists before its first sample: Options::useLazyCatchLiveness() creates it at a tier-up check, and the
+                // mutator may still be filling it. The DFG takes SpecNone for code that never ran and converts the value unchecked.
+                NEXT_OPCODE(op_catch);
+            }
+
+            // We're now committed to compiling this as an entrypoint.
+            m_currentBlock->isCatchEntrypoint = true;
+            m_graph.m_roots.append(m_currentBlock);
+
             // We're not allowed to exit here since we would not properly recover values.
             // We first need to bootstrap the catch entrypoint state.
             m_exitOK = false; 

Source/JavaScriptCore/dfg/DFGDriver.cpp

@@ -147,8 +147,6 @@ static CompilationResult compileImpl(
         vm.typeProfilerLog()->processLogEntries(vm, "Preparing for DFG compilation."_s);
 
     prepareLazyStateOfInlineCandidates(vm, codeBlock, profiledDFGCodeBlock, mode);
-    if (mode != JITCompilationMode::FTLForOSREntry)
-        codeBlock->baselineAlternative()->ensureCatchLivenessIsComputedForExecutedCatches(); // the parser makes a catch OSR entrypoint only where the buffer exists
 
     Ref<Plan> plan = adoptRef(*new Plan(codeBlock, profiledDFGCodeBlock, mode, osrEntryBytecodeIndex, WTF::move(mustHandleValues)));
 

Source/JavaScriptCore/jit/JITOperations.cpp

@@ -3254,7 +3254,7 @@ JSC_DEFINE_JIT_OPERATION(operationTryOSREnterAtCatchAndValueProfile, UGPRPair, (
     if (!buffer)
         OPERATION_RETURN(scope, encodeResult(nullptr, nullptr));
     buffer->forEach([&] (ValueProfileAndVirtualRegister& profile) {
-        profile.m_buckets[0] = JSValue::encode(callFrame->uncheckedR(profile.m_operand).jsValue());
+        profile.sample(callFrame->uncheckedR(profile.m_operand).jsValue());
     });
 
     OPERATION_RETURN(scope, encodeResult(nullptr, nullptr));

Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

@@ -2605,7 +2605,7 @@ LLINT_SLOW_PATH_DECL(slow_path_profile_catch)
     if (!buffer)
         LLINT_END();
     buffer->forEach([&] (ValueProfileAndVirtualRegister& profile) {
-        profile.m_buckets[0] = JSValue::encode(callFrame->uncheckedR(profile.m_operand).jsValue());
+        profile.sample(callFrame->uncheckedR(profile.m_operand).jsValue());
     });
 
     LLINT_END();