Changeset 260512 in webkit


Ignore:
Timestamp:
Apr 22, 2020 8:40:06 AM (4 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] AI results of BigInt32 Bitwise shift operation does not match to runtime results
​https://bugs.webkit.org/show_bug.cgi?id=210839

Reviewed by Saam Barati.

JSTests:

  • stress/v8-bigint32-add.js: Added.

(main):

  • stress/v8-bigint32-and.js: Added.

(main):

  • stress/v8-bigint32-dec.js: Added.

(main):

  • stress/v8-bigint32-div.js: Added.

(main):

  • stress/v8-bigint32-inc.js: Added.

(main):

  • stress/v8-bigint32-mod.js: Added.

(main):

  • stress/v8-bigint32-mul.js: Added.

(main):

  • stress/v8-bigint32-neg.js: Added.

(main):

  • stress/v8-bigint32-not.js: Added.

(main):

  • stress/v8-bigint32-or.js: Added.

(main):

  • stress/v8-bigint32-sar.js: Added.

(main):

  • stress/v8-bigint32-shl.js: Added.

(main):

  • stress/v8-bigint32-sub.js: Added.

(main):

  • stress/v8-bigint32-xor.js: Added.

(main):

Source/JavaScriptCore:

While runtime function of bitwise ops with BigInt32 sometimes returns HeapBigInt, DFG AI is setting SpecBigInt32
as a result value. This leads to miscompilation particularly in FTL since FTL uses this information to remove
a lot of branches.

And we found that FTL BigInt32 predicate is not correctly checking state. This patch fixes it too.

Added test case found this (v8-bigint32-sar.js).

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileValueBitRShift):
(JSC::FTL::DFG::LowerDFGToB3::compileToNumeric):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
(JSC::FTL::DFG::LowerDFGToB3::compileIsBigInt):
(JSC::FTL::DFG::LowerDFGToB3::boolify):
(JSC::FTL::DFG::LowerDFGToB3::buildTypeOf):
(JSC::FTL::DFG::LowerDFGToB3::lowBigInt32):
(JSC::FTL::DFG::LowerDFGToB3::isBigInt32KnownNotCell):
(JSC::FTL::DFG::LowerDFGToB3::isBigInt32KnownNotNumber):
(JSC::FTL::DFG::LowerDFGToB3::isNotBigInt32KnownNotNumber):
(JSC::FTL::DFG::LowerDFGToB3::isNotAnyBigIntKnownNotNumber):
(JSC::FTL::DFG::LowerDFGToB3::isNotHeapBigIntUnknownWhetherCell):
(JSC::FTL::DFG::LowerDFGToB3::speculateBigInt32):
(JSC::FTL::DFG::LowerDFGToB3::speculateAnyBigInt):
(JSC::FTL::DFG::LowerDFGToB3::isBigInt32): Deleted.
(JSC::FTL::DFG::LowerDFGToB3::isNotBigInt32): Deleted.
(JSC::FTL::DFG::LowerDFGToB3::isNotAnyBigInt): Deleted.

Location:
trunk
Files:
14 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r260490 r260512  
     12020-04-22  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] AI results of BigInt32 Bitwise shift operation does not match to runtime results
     4        https://bugs.webkit.org/show_bug.cgi?id=210839
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/v8-bigint32-add.js: Added.
     9        (main):
     10        * stress/v8-bigint32-and.js: Added.
     11        (main):
     12        * stress/v8-bigint32-dec.js: Added.
     13        (main):
     14        * stress/v8-bigint32-div.js: Added.
     15        (main):
     16        * stress/v8-bigint32-inc.js: Added.
     17        (main):
     18        * stress/v8-bigint32-mod.js: Added.
     19        (main):
     20        * stress/v8-bigint32-mul.js: Added.
     21        (main):
     22        * stress/v8-bigint32-neg.js: Added.
     23        (main):
     24        * stress/v8-bigint32-not.js: Added.
     25        (main):
     26        * stress/v8-bigint32-or.js: Added.
     27        (main):
     28        * stress/v8-bigint32-sar.js: Added.
     29        (main):
     30        * stress/v8-bigint32-shl.js: Added.
     31        (main):
     32        * stress/v8-bigint32-sub.js: Added.
     33        (main):
     34        * stress/v8-bigint32-xor.js: Added.
     35        (main):
     36
    1372020-04-21  Yusuke Suzuki  <ysuzuki@apple.com>
    238

  • trunk/Source/JavaScriptCore/ChangeLog

    r260501 r260512  
     12020-04-22  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] AI results of BigInt32 Bitwise shift operation does not match to runtime results
     4        https://bugs.webkit.org/show_bug.cgi?id=210839
     5
     6        Reviewed by Saam Barati.
     7
     8        While runtime function of bitwise ops with BigInt32 sometimes returns HeapBigInt, DFG AI is setting SpecBigInt32
     9        as a result value. This leads to miscompilation particularly in FTL since FTL uses this information to remove
     10        a lot of branches.
     11
     12        And we found that FTL BigInt32 predicate is not correctly checking state. This patch fixes it too.
     13
     14        Added test case found this (v8-bigint32-sar.js).
     15
     16        * dfg/DFGAbstractInterpreterInlines.h:
     17        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
     18        * ftl/FTLLowerDFGToB3.cpp:
     19        (JSC::FTL::DFG::LowerDFGToB3::compileValueBitRShift):
     20        (JSC::FTL::DFG::LowerDFGToB3::compileToNumeric):
     21        (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
     22        (JSC::FTL::DFG::LowerDFGToB3::compileIsBigInt):
     23        (JSC::FTL::DFG::LowerDFGToB3::boolify):
     24        (JSC::FTL::DFG::LowerDFGToB3::buildTypeOf):
     25        (JSC::FTL::DFG::LowerDFGToB3::lowBigInt32):
     26        (JSC::FTL::DFG::LowerDFGToB3::isBigInt32KnownNotCell):
     27        (JSC::FTL::DFG::LowerDFGToB3::isBigInt32KnownNotNumber):
     28        (JSC::FTL::DFG::LowerDFGToB3::isNotBigInt32KnownNotNumber):
     29        (JSC::FTL::DFG::LowerDFGToB3::isNotAnyBigIntKnownNotNumber):
     30        (JSC::FTL::DFG::LowerDFGToB3::isNotHeapBigIntUnknownWhetherCell):
     31        (JSC::FTL::DFG::LowerDFGToB3::speculateBigInt32):
     32        (JSC::FTL::DFG::LowerDFGToB3::speculateAnyBigInt):
     33        (JSC::FTL::DFG::LowerDFGToB3::isBigInt32): Deleted.
     34        (JSC::FTL::DFG::LowerDFGToB3::isNotBigInt32): Deleted.
     35        (JSC::FTL::DFG::LowerDFGToB3::isNotAnyBigInt): Deleted.
     36
    1372020-04-21  Yusuke Suzuki  <ysuzuki@apple.com>
    238

  • trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

    r260377 r260512  
    544544        if (node->binaryUseKind() == BigInt32Use) {
    545545#if USE(BIGINT32)
    546             setTypeForNode(node, SpecBigInt32);
     546            // FIXME: We should have inlined implementation that always returns BigInt32.
     547            // https://bugs.webkit.org/show_bug.cgi?id=210847
     548            setTypeForNode(node, SpecBigInt);
    547549#else
    548550            RELEASE_ASSERT_NOT_REACHED();

  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r260397 r260512  
    34173417            // Things are a bit tricky because a right-shift by a negative number is a left-shift for BigInts.
    34183418            // So even a right shift can overflow.
     3419            // https://bugs.webkit.org/show_bug.cgi?id=210847
    34193420
    34203421            LValue left = lowJSValue(m_node->child1(), ManualOperandSpeculation);
    … …  
    75527553            LBasicBlock lastNext = m_out.appendTo(notNumber, continuation);
    75537554#if USE(BIGINT32)
    7554             m_out.branch(isBigInt32(value, provenType(m_node->child1())), unsure(continuation), unsure(notBigInt32));
     7555            m_out.branch(isBigInt32KnownNotNumber(value, provenType(m_node->child1())), unsure(continuation), unsure(notBigInt32));
    75557556            m_out.appendTo(notBigInt32);
    75567557#endif
    … …  
    87178718            LBasicBlock continuation = m_out.newBlock();
    87188719
     8720            FTL_TYPE_CHECK(jsValueValue(left), m_node->child1(), ~SpecFullNumber, isNumber(left));
     8721            FTL_TYPE_CHECK(jsValueValue(right), m_node->child2(), ~SpecFullNumber, isNumber(right));
     8722
    87198723            // Inserts a check that a value is a HeapBigInt, assuming only that we know it is not a BigInt32
    87208724            auto checkIsHeapBigInt = [&](LValue lowValue, Edge highValue) {
    87218725                if (m_interpreter.needsTypeCheck(highValue, SpecHeapBigInt)) {
    87228726                    ASSERT(mayHaveTypeCheck(highValue.useKind()));
    8723                     LValue checkFailed = isNotHeapBigIntUnknownWhetherCell(lowValue, ~SpecBigInt32);
     8727                    LValue checkFailed = isNotHeapBigIntUnknownWhetherCell(lowValue, provenType(highValue) & ~SpecBigInt32);
    87248728                    appendOSRExit(BadType, jsValueValue(lowValue), highValue.node(), checkFailed, m_origin);
    87258729                }
    87268730            };
    87278731
    8728             m_out.branch(isBigInt32(left, provenType(m_node->child1())), unsure(leftIsBigInt32), unsure(leftIsNotBigInt32));
     8732            m_out.branch(isBigInt32KnownNotNumber(left, provenType(m_node->child1())), unsure(leftIsBigInt32), unsure(leftIsNotBigInt32));
    87298733
    87308734            LBasicBlock lastNext = m_out.appendTo(leftIsBigInt32, bothAreBigInt32);
    8731             m_out.branch(isBigInt32(right, provenType(m_node->child2())), unsure(bothAreBigInt32), unsure(onlyLeftIsBigInt32));
     8735            m_out.branch(isBigInt32KnownNotNumber(right, provenType(m_node->child2())), unsure(bothAreBigInt32), unsure(onlyLeftIsBigInt32));
    87328736
    87338737            m_out.appendTo(bothAreBigInt32, onlyLeftIsBigInt32);
    … …  
    87508754
    87518755            m_out.appendTo(leftIsHeapBigInt, rightIsBigInt32);
    8752             m_out.branch(isBigInt32(right, provenType(m_node->child2())), unsure(rightIsBigInt32), unsure(rightIsNotBigInt32));
     8756            m_out.branch(isBigInt32KnownNotNumber(right, provenType(m_node->child2())), unsure(rightIsBigInt32), unsure(rightIsNotBigInt32));
    87538757
    87548758            m_out.appendTo(rightIsBigInt32, rightIsNotBigInt32);
    … …  
    1092310927        LBasicBlock lastNext = m_out.appendTo(isNotCellCase, isCellCase);
    1092410928        // FIXME: we should filter the provenType to include the fact that we know we are not dealing with a cell
    10925         ValueFromBlock notCellResult = m_out.anchor(isBigInt32(value, provenType(m_node->child1())));
     10929        ValueFromBlock notCellResult = m_out.anchor(isBigInt32KnownNotCell(value, provenType(m_node->child1())));
    1092610930        m_out.jump(continuation);
    1092710931
    … …  
    1546615470            m_out.appendTo(notDoubleCase, bigInt32Case);
    1546715471            m_out.branch(
    15468                 isBigInt32(value, provenType(edge) & ~SpecCell),
     15472                isBigInt32KnownNotNumber(value, provenType(edge) & ~SpecCell),
    1546915473                unsure(bigInt32Case), unsure(notBigInt32Case));
    1547015474
    … …  
    1619216196#if USE(BIGINT32)
    1619316197        m_out.appendTo(notNumberCase, notBigInt32Case);
    16194         m_out.branch(isBigInt32(value, provenType(child) & ~SpecCell), unsure(bigIntCase), unsure(notBigInt32Case));
     16198        m_out.branch(isBigInt32KnownNotNumber(value, provenType(child) & ~SpecCell), unsure(bigIntCase), unsure(notBigInt32Case));
    1619516199
    1619616200        m_out.appendTo(notBigInt32Case, notNullCase);
    … …  
    1691216916        if (isValid(value)) {
    1691316917            LValue result = value.value();
    16914             FTL_TYPE_CHECK(jsValueValue(result), edge, SpecBigInt32, isNotBigInt32(result));
     16918            FTL_TYPE_CHECK(jsValueValue(result), edge, ~SpecFullNumber, isNumber(result));
     16919            FTL_TYPE_CHECK(jsValueValue(result), edge, SpecBigInt32, isNotBigInt32KnownNotNumber(result));
    1691516920            return result;
    1691616921        }
    … …  
    1710917114
    1711017115#if USE(BIGINT32)
    17111     LValue isBigInt32(LValue jsValue, SpeculatedType type = SpecFullTop)
     17116    LValue isBigInt32KnownNotCell(LValue jsValue, SpeculatedType type = SpecFullTop)
    1711217117    {
    1711317118        if (LValue proven = isProvenValue(type, SpecBigInt32))
    1711417119            return proven;
    17115         return m_out.equal(
    17116             m_out.bitAnd(jsValue, m_out.constInt64(JSValue::BigInt32Mask)),
    17117             m_out.constInt64(JSValue::BigInt32Tag));
    17118     }
    17119     LValue isNotBigInt32(LValue jsValue, SpeculatedType type = SpecFullTop)
     17120        return m_out.equal(m_out.bitAnd(jsValue, m_out.constInt64(JSValue::BigInt32Mask)), m_out.constInt64(JSValue::BigInt32Tag));
     17121    }
     17122    LValue isBigInt32KnownNotNumber(LValue jsValue, SpeculatedType type = SpecFullTop)
     17123    {
     17124        if (LValue proven = isProvenValue(type, SpecBigInt32))
     17125            return proven;
     17126        return m_out.equal(m_out.bitAnd(jsValue, m_out.constInt64(JSValue::BigInt32Tag)), m_out.constInt64(JSValue::BigInt32Tag));
     17127    }
     17128    LValue isNotBigInt32KnownNotNumber(LValue jsValue, SpeculatedType type = SpecFullTop)
    1712017129    {
    1712117130        if (LValue proven = isProvenValue(type, ~SpecBigInt32))
    1712217131            return proven;
    17123         return m_out.notEqual(
    17124             m_out.bitAnd(jsValue, m_out.constInt64(JSValue::BigInt32Mask)),
    17125             m_out.constInt64(JSValue::BigInt32Tag));
     17132        return m_out.notEqual(m_out.bitAnd(jsValue, m_out.constInt64(JSValue::BigInt32Tag)), m_out.constInt64(JSValue::BigInt32Tag));
    1712617133    }
    1712717134    LValue unboxBigInt32(LValue jsValue)
    … …  
    1713517142            m_out.constInt64(JSValue::BigInt32Tag));
    1713617143    }
    17137     LValue isNotAnyBigInt(LValue jsValue, SpeculatedType type = SpecFullTop)
     17144    LValue isNotAnyBigIntKnownNotNumber(LValue jsValue, SpeculatedType type = SpecFullTop)
    1713817145    {
    1713917146        if (LValue proven = isProvenValue(type, ~SpecBigInt))
    … …  
    1715117158        LBasicBlock continuation = m_out.newBlock();
    1715217159
    17153         m_out.branch(isBigInt32(jsValue, type), unsure(isBigInt32Case), unsure(isNotBigInt32Case));
     17160        m_out.branch(isBigInt32KnownNotNumber(jsValue, type), unsure(isBigInt32Case), unsure(isNotBigInt32Case));
    1715417161
    1715517162        LBasicBlock lastNext = m_out.appendTo(isBigInt32Case, isNotBigInt32Case);
    … …  
    1768817695        LBasicBlock continuation = m_out.newBlock();
    1768917696
    17690         ValueFromBlock defaultToFalse = m_out.anchor(m_out.booleanFalse);
     17697        ValueFromBlock defaultToTrue = m_out.anchor(m_out.booleanTrue);
    1769117698        m_out.branch(isCell(value, type), unsure(isCellCase), unsure(continuation));
    1769217699
    1769317700        LBasicBlock lastNext = m_out.appendTo(isCellCase, continuation);
    17694         ValueFromBlock returnForCell = m_out.anchor(isHeapBigInt(value, type));
     17701        ValueFromBlock returnForCell = m_out.anchor(isNotHeapBigInt(value, type));
    1769517702        m_out.jump(continuation);
    1769617703
    1769717704        m_out.appendTo(continuation, lastNext);
    17698         LValue result = m_out.phi(Int32, defaultToFalse, returnForCell);
     17705        LValue result = m_out.phi(Int32, defaultToTrue, returnForCell);
    1769917706        return result;
    1770017707    }
    … …  
    1818218189    {
    1818318190        LValue value = lowJSValue(edge, ManualOperandSpeculation);
    18184         FTL_TYPE_CHECK(jsValueValue(value), edge, SpecBigInt32, isNotBigInt32(value));
     18191        FTL_TYPE_CHECK(jsValueValue(value), edge, ~SpecFullNumber, isNumber(value));
     18192        FTL_TYPE_CHECK(jsValueValue(value), edge, SpecBigInt32, isNotBigInt32KnownNotNumber(value));
    1818518193    }
    1818618194
    … …  
    1818818196    {
    1818918197        LValue value = lowJSValue(edge, ManualOperandSpeculation);
    18190         FTL_TYPE_CHECK(jsValueValue(value), edge, SpecBigInt, isNotAnyBigInt(value));
     18198        FTL_TYPE_CHECK(jsValueValue(value), edge, ~SpecFullNumber, isNumber(value));
     18199        FTL_TYPE_CHECK(jsValueValue(value), edge, SpecBigInt, isNotAnyBigIntKnownNotNumber(value));
    1819118200    }
    1819218201#endif
Note: See TracChangeset for help on using the changeset viewer.