← All reports

[JSC] Extract CheckInBounds from StringAt and StringCodePointAt

The bounds check that got deleted because it had already passed.

Component: JavaScriptCore DFG/FTL JIT | 5d1761d

Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp

- case StringCharCodeAt: {
+ case StringAt:
+ case StringCharCodeAt:
+ case StringCodePointAt: {
lowerStringBoundsCheck(m_graph.child(m_node, 0), m_graph.child(m_node, 1), m_graph.child(m_node, 2));
break;
}

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

- LValue index = m_node->op() == StringAt ? m_out.select(m_out.lessThan(originalIndex, m_out.int32Zero), m_out.add(stringLength, originalIndex), originalIndex) : originalIndex;
+ bool boundsCheckLowered = m_node->op() == StringAt && m_node->arrayMode().isInBounds();
+ LValue index = m_node->op() == StringAt && !boundsCheckLowered ? m_out.select(m_out.lessThan(originalIndex, m_out.int32Zero), m_out.add(stringLength, originalIndex), originalIndex) : originalIndex;
 
LBasicBlock fastPath = m_out.newBlock();
- LBasicBlock slowPath = m_out.newBlock();
+ LBasicBlock slowPath = boundsCheckLowered ? nullptr : m_out.newBlock();
LBasicBlock continuation = m_out.newBlock();

JSC's DFG and FTL tiers speculate that string index accesses are in bounds and encode that as an implicit check inside nodes like StringAt/StringCodePointAt; if the index is out of range the node exits — deoptimizes — back to a lower tier. The abstract interpreter uses the fact that the check must have passed to fold later comparisons such as === undefined to constants, which can leave the original node's value unused, at which point dead-code elimination deletes the node entirely. This commit extracts the bounds check into a standalone CheckInBounds node during SSA lowering, so StringAt, StringCharCodeAt, and StringCodePointAt all route through lowerStringBoundsCheck(), and teaches the FTL lowering to skip its own index clamping and slow path when the check has already been lowered.

Before:
  string.codePointAt(index) === undefined
    ├─ AI folds comparison to `false` (assumes the implicit bounds check fired)
    └─ DCE: StringCodePointAt has no users ──► entire node removed
  Runtime: OOB/negative index no longer exits — folded `false` used anyway

After (SSALoweringPhase):
  CheckInBounds(index, length)   ← separate node, survives DCE
  StringAt / StringCodePointAt   ← still removable if unused
  Runtime: OOB/negative index still triggers CheckInBounds exit

Before the fix, string.codePointAt(index) === undefined could return a stale folded result for out-of-bounds or negative indices instead of taking the safety exit, so JIT-compiled code computed on incorrect bounds-check assumptions.

The bug class worth hunting elsewhere is any DFG node that bundles a safety check — bounds, type, null — inside its own semantics rather than representing it as a standalone graph node, since the check is then silently deleted whenever abstract-interpreter folding makes the node's value dead. Narrow: audit GetByVal/PutByVal and the other TypedArray and array-access nodes in DFGSSALoweringPhase.cpp and FTLLowerDFGToB3.cpp for the same implicit-check-plus-DCE hazard, prioritising any speculation whose result is consumed only by a comparison the interpreter can fold. Wider: the same shape covers checks folded into non-array nodes that the interpreter can prove into constants — anything where a node's effect is a guard but its value is the only thing the graph tracks; the reachable set is every node kind lowerStringBoundsCheck-style helpers do not cover. Widest: in any optimizing compiler, a safety check that is not a first-class effect in the IR is one liveness analysis away from deletion — the invariant to carry is that guards must be nodes, not node properties. Code-review tell: a node case in a lowering phase that performs an index clamp or range comparison inline, with no corresponding Check* node emitted alongside it.