Skip to content

Commit

Permalink
Cherry-pick 71cdc1c. rdar://problem/102700508
Browse files Browse the repository at this point in the history
    The provenType filtering in FTL's speculateRealNumber is incorrect.
    https://bugs.webkit.org/show_bug.cgi?id=248266
    <rdar://problem/102531234>

    Reviewed by Justin Michaud.

    speculateRealNumber does a doubleEqual compare, which filters out double values which
    are not NaN.  NaN values will fall through to the `intCase` block.  In the `intCase` block,
    the isNotInt32() check there was given a proven type that wrongly filters out ~SpecFullDouble.

    Consider a scenario where the edge was proven to be { SpecInt32Only, SpecDoubleReal,
    SpecDoublePureNaN }.  SpecFullDouble is defined as SpecDoubleReal | SpecDoubleNaN, and
    SpecDoubleNaN is defined as SpecDoublePureNaN | SpecDoubleImpureNaN.  Hence, the filtering
    of the proven type with ~SpecFullDouble means that isNotInt32() will effectively be given
    a proven type of

        { SpecInt32Only, SpecDoubleReal, SpecDoublePureNaN } - { SpecDoubleReal, SpecDoublePureNaN }

    which yields

        { SpecInt32Only }.

    As a result, the compiler will think that that isNotIn32() check will always fail.  This
    is not correct if the actual incoming value for that edge is actually a PureNaN.  In this
    case, speculateRealNumber should have OSR exited, but it doesn't because it thinks that
    the isNotInt32() check will always fail and elide the check altogether.

    In this patch, we fix this by replacing the ~SpecFullDouble with ~SpecDoubleReal.  We also
    rename the `intCase` block to `intOrNaNCase` to document what it actually handles.

    * JSTests/stress/speculate-real-number-in-object-is.js: Added.
    (test.object_is_opt):
    (test):
    * Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
    (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):

    Canonical link: https://commits.webkit.org/252432.839@safari-7614-branch

Canonical link: https://commits.webkit.org/252432.819@safari-7614.3.7.3-branch
  • Loading branch information
Mark Lam authored and rjepstein committed Nov 28, 2022
1 parent 8907635 commit 4b3a155
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
22 changes: 22 additions & 0 deletions JSTests/stress/speculate-real-number-in-object-is.js
@@ -0,0 +1,22 @@
function test() {
function object_is_opt(value) {
const tmp = {p0: value};

if (Object.is(value, NaN))
return 0;

return value;
}

object_is_opt(NaN);

for (let i = 0; i < 0x20000; i++)
object_is_opt(1.1);

return isNaN(object_is_opt(NaN));
}

resultIsNaN = test();
if (resultIsNaN)
throw "FAILED";

8 changes: 4 additions & 4 deletions Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Expand Up @@ -20285,18 +20285,18 @@ IGNORE_CLANG_WARNINGS_END
LValue value = lowJSValue(edge, ManualOperandSpeculation);
LValue doubleValue = unboxDouble(value);

LBasicBlock intCase = m_out.newBlock();
LBasicBlock intOrNaNCase = m_out.newBlock();
LBasicBlock continuation = m_out.newBlock();

m_out.branch(
m_out.doubleEqual(doubleValue, doubleValue),
usually(continuation), rarely(intCase));
usually(continuation), rarely(intOrNaNCase));

LBasicBlock lastNext = m_out.appendTo(intCase, continuation);
LBasicBlock lastNext = m_out.appendTo(intOrNaNCase, continuation);

typeCheck(
jsValueValue(value), m_node->child1(), SpecBytecodeRealNumber,
isNotInt32(value, provenType(m_node->child1()) & ~SpecFullDouble));
isNotInt32(value, provenType(m_node->child1()) & ~SpecDoubleReal));
m_out.jump(continuation);

m_out.appendTo(continuation, lastNext);
Expand Down

0 comments on commit 4b3a155

Please sign in to comment.