From 98940f219ba0e3eb6d958af483b73dd9cc75c28c Mon Sep 17 00:00:00 2001 From: Mark Lam Date: Mon, 19 Dec 2022 17:32:15 -0800 Subject: [PATCH] Cherry-pick 252432.839@safari-7614-branch (71cdc1c09ef1). rdar://102531234 The provenType filtering in FTL's speculateRealNumber is incorrect. https://bugs.webkit.org/show_bug.cgi?id=248266 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/258113@main --- .../speculate-real-number-in-object-is.js | 22 +++++++++++++++++++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp | 8 +++---- 2 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 JSTests/stress/speculate-real-number-in-object-is.js diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp index 3ba2d21b8072..18d13f1941bb 100644 --- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp +++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp @@ -20574,18 +20574,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);