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

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

@@ -0,0 +1,101 @@
+//@ defaultRun; run("eager-catch-liveness", "--useLazyCatchLiveness=false")
+
+// A catch OSR entrypoint must not give the DFG an operand without a prediction. Later phases take SpecNone for code
+// that never ran: a double use of such a value became DoubleRep(NotCellNorBigIntUse), which is ToNumber, not a check.
+
+function thrower(shouldThrow) {
+  if (shouldThrow) throw new Error("thrown");
+}
+noInline(thrower);
+
+function doubleArray() {
+  const array = [0.5, 1.5];
+  array[0] = 2.5;
+  return array;
+}
+noInline(doubleArray);
+
+const oddValues = [null, true, undefined, false];
+
+function check(name, value, result) {
+  if (!Object.is(result, value) && result !== String(value))
+    throw new Error(name + ": saw " + result + " (" + typeof result + ") in place of " + value);
+}
+
+// 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 [t][0];
+    }
+    return t;
+  },
+
+  argumentInNewArrayInCatch(value, shouldThrow, t) {
+    try {
+      thrower(shouldThrow);
+    } catch {
+      return [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 oddValues) check(name, value, f({ p: value }, true, value));
+}
+
+// Whatever the option says: t is in its TDZ in each sample that reaches the prediction, and an empty bucket is no sample.
+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 array[0];
+        }
+      }
+  }
+}
+noInline(sampledWhileEmpty);
+
+for (let i = 0; i < 10; ++i) sampledWhileEmpty(0, { p: i + 0.5 }, true, doubleArray());
+for (let i = 0; i < testLoopCount; ++i) {
+  sampledWhileEmpty(0, { p: i + 0.5 }, false, doubleArray());
+  if (!(i % 20)) sampledWhileEmpty(1, { p: 0.5 }, true, doubleArray());
+}
+for (const value of oddValues)
+  check("sampledWhileEmpty", value, sampledWhileEmpty(0, { p: value }, true, doubleArray()));

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; it has no samples yet.
     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) {
+                // Options::useLazyCatchLiveness() creates the buffer apart from an execution of the catch. Later phases
+                // take SpecNone for code that never ran and convert such a value without a check.
+                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();