Changeset 242954 in webkit


Ignore:
Timestamp:
Mar 14, 2019 12:27:28 PM (5 years ago)
Author:
sbarati@apple.com
Message:

Fixup uses KnownInt32 incorrectly in some nodes
​https://bugs.webkit.org/show_bug.cgi?id=195279
<rdar://problem/47915654>

Reviewed by Yusuke Suzuki.

JSTests:

  • stress/known-int32-cant-be-used-across-bytecode-boundary.js: Added.

(foo):

Source/JavaScriptCore:

Fixup was sometimes using KnownInt32 edges when it knew some
incoming value is an Int32 based on what the bytecode would return.
However, because bytecode may result in Int32 for some node does
not mean we'll pick Int32 as the value format for that local. For example,
we may choose for a value to be represented as a double. This patch
corrects such uses of KnownInt32.

  • dfg/DFGArgumentsEliminationPhase.cpp:
  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileArrayPush):
(JSC::DFG::SpeculativeJIT::compileGetDirectPname):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileArrayPush):

Location:
trunk
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r242945 r242954  
     12019-03-14  Saam barati  <sbarati@apple.com>
     2
     3        Fixup uses KnownInt32 incorrectly in some nodes
     4        https://bugs.webkit.org/show_bug.cgi?id=195279
     5        <rdar://problem/47915654>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        * stress/known-int32-cant-be-used-across-bytecode-boundary.js: Added.
     10        (foo):
     11
    1122019-03-14  Keith Miller  <keith_miller@apple.com>
    213

  • trunk/Source/JavaScriptCore/ChangeLog

    r242945 r242954  
     12019-03-14  Saam barati  <sbarati@apple.com>
     2
     3        Fixup uses KnownInt32 incorrectly in some nodes
     4        https://bugs.webkit.org/show_bug.cgi?id=195279
     5        <rdar://problem/47915654>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        Fixup was sometimes using KnownInt32 edges when it knew some
     10        incoming value is an Int32 based on what the bytecode would return.
     11        However, because bytecode may result in Int32 for some node does
     12        not mean we'll pick Int32 as the value format for that local. For example,
     13        we may choose for a value to be represented as a double. This patch
     14        corrects such uses of KnownInt32.
     15
     16        * dfg/DFGArgumentsEliminationPhase.cpp:
     17        * dfg/DFGFixupPhase.cpp:
     18        (JSC::DFG::FixupPhase::fixupNode):
     19        * dfg/DFGSpeculativeJIT.cpp:
     20        (JSC::DFG::SpeculativeJIT::compileArrayPush):
     21        (JSC::DFG::SpeculativeJIT::compileGetDirectPname):
     22        * ftl/FTLLowerDFGToB3.cpp:
     23        (JSC::FTL::DFG::LowerDFGToB3::compileArrayPush):
     24
    1252019-03-14  Keith Miller  <keith_miller@apple.com>
    226

  • trunk/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp

    r241228 r242954  
    11/*
    2  * Copyright (C) 2015-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2015-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    662662                        break;
    663663
     664                    ASSERT(node->origin.exitOK);
     665                    ASSERT(node->child1().useKind() == Int32Use);
     666                    insertionSet.insertNode(
     667                        nodeIndex, SpecNone, Check, node->origin,
     668                        node->child1());
     669
    664670                    node->setOpAndDefaultFlags(PhantomCreateRest);
    665671                    // We don't need this parameter for OSR exit, we can find out all the information

  • trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

    r242715 r242954  
    11/*
    2  * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2012-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    11501150                switch (node->arrayMode().type()) {
    11511151                case Array::Int32:
    1152                     insertCheck<Int32Use>(element.node());
    1153                     fixEdge<KnownInt32Use>(element);
     1152                    fixEdge<Int32Use>(element);
    11541153                    break;
    11551154                case Array::Double:
    1156                     insertCheck<DoubleRepRealUse>(element.node());
    1157                     fixEdge<DoubleRepUse>(element);
     1155                    fixEdge<DoubleRepRealUse>(element);
    11581156                    break;
    11591157                case Array::Contiguous:
    … …  
    11641162                    break;
    11651163                }
    1166                 ASSERT(shouldNotHaveTypeCheck(element.useKind()));
    11671164            }
    11681165            break;
    … …  
    18691866            blessArrayOperation(m_graph.varArgChild(node, 0), m_graph.varArgChild(node, 1), m_graph.varArgChild(node, 2));
    18701867            fixEdge<CellUse>(m_graph.varArgChild(node, 0));
    1871             fixEdge<KnownInt32Use>(m_graph.varArgChild(node, 1));
     1868            fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
    18721869            break;
    18731870        }
    … …  
    18791876            fixEdge<CellUse>(base);
    18801877            fixEdge<KnownCellUse>(property);
    1881             fixEdge<KnownInt32Use>(index);
     1878            fixEdge<Int32Use>(index);
    18821879            fixEdge<KnownCellUse>(enumerator);
    18831880            break;
    … …  
    18901887        case GetEnumeratorStructurePname: {
    18911888            fixEdge<KnownCellUse>(node->child1());
    1892             fixEdge<KnownInt32Use>(node->child2());
     1889            fixEdge<Int32Use>(node->child2());
    18931890            break;
    18941891        }
    18951892        case GetEnumeratorGenericPname: {
    18961893            fixEdge<KnownCellUse>(node->child1());
    1897             fixEdge<KnownInt32Use>(node->child2());
     1894            fixEdge<Int32Use>(node->child2());
    18981895            break;
    18991896        }
    19001897        case ToIndexString: {
    1901             fixEdge<KnownInt32Use>(node->child1());
     1898            fixEdge<Int32Use>(node->child1());
    19021899            break;
    19031900        }
    … …  
    19911988        case CreateRest: {
    19921989            watchHavingABadTime(node);
    1993             fixEdge<KnownInt32Use>(node->child1());
     1990            fixEdge<Int32Use>(node->child1());
    19941991            break;
    19951992        }
    … …  
    21552152                fixEdge<UntypedUse>(propertyEdge);
    21562153            fixEdge<UntypedUse>(m_graph.varArgChild(node, 2));
    2157             fixEdge<KnownInt32Use>(m_graph.varArgChild(node, 3));
     2154            fixEdge<Int32Use>(m_graph.varArgChild(node, 3));
    21582155            break;
    21592156        }
    … …  
    22052202            fixEdge<CellUse>(m_graph.varArgChild(node, 2));
    22062203            fixEdge<CellUse>(m_graph.varArgChild(node, 3));
    2207             fixEdge<KnownInt32Use>(m_graph.varArgChild(node, 4));
     2204            fixEdge<Int32Use>(m_graph.varArgChild(node, 4));
    22082205            break;
    22092206        }

  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r242812 r242954  
    85718571        if (elementCount == 1) {
    85728572            Edge& element = m_jit.graph().varArgChild(node, elementOffset);
     8573            if (node->arrayMode().type() == Array::Int32) {
     8574                ASSERT(element.useKind() == Int32Use);
     8575                speculateInt32(element);
     8576            }
    85738577            JSValueOperand value(this, element, ManualOperandSpeculation);
    85748578            JSValueRegs valueRegs = value.jsValueRegs();
    8575 
    8576             if (node->arrayMode().type() == Array::Int32)
    8577                 DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecInt32Only));
    85788579
    85798580            m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), storageLengthGPR);
    … …  
    85918592        }
    85928593
     8594        if (node->arrayMode().type() == Array::Int32) {
     8595            for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
     8596                Edge element = m_jit.graph().varArgChild(node, elementIndex + elementOffset);
     8597                ASSERT(element.useKind() == Int32Use);
     8598                speculateInt32(element);
     8599            }
     8600        }
     8601
    85938602        GPRTemporary buffer(this);
    85948603        GPRReg bufferGPR = buffer.gpr();
    … …  
    86168625        for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
    86178626            Edge& element = m_jit.graph().varArgChild(node, elementIndex + elementOffset);
    8618             JSValueOperand value(this, element, ManualOperandSpeculation);
     8627            JSValueOperand value(this, element, ManualOperandSpeculation); // We did type checks above.
    86198628            JSValueRegs valueRegs = value.jsValueRegs();
    8620 
    8621             if (node->arrayMode().type() == Array::Int32)
    8622                 DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecInt32Only));
    86238629
    86248630            m_jit.storeValue(valueRegs, MacroAssembler::Address(bufferGPR, sizeof(EncodedJSValue) * elementIndex));
    … …  
    86448650        if (elementCount == 1) {
    86458651            Edge& element = m_jit.graph().varArgChild(node, elementOffset);
     8652            speculate(node, element);
    86468653            SpeculateDoubleOperand value(this, element);
    86478654            FPRReg valueFPR = value.fpr();
    8648 
    8649             DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecDoubleReal));
    86508655
    86518656            m_jit.load32(MacroAssembler::Address(storageGPR, Butterfly::offsetOfPublicLength()), storageLengthGPR);
    … …  
    86638668        }
    86648669
     8670        for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
     8671            Edge element = m_jit.graph().varArgChild(node, elementIndex + elementOffset);
     8672            ASSERT(element.useKind() == DoubleRepRealUse);
     8673            speculate(node, element);
     8674        }
     8675
    86658676        GPRTemporary buffer(this);
    86668677        GPRReg bufferGPR = buffer.gpr();
    … …  
    86908701            SpeculateDoubleOperand value(this, element);
    86918702            FPRReg valueFPR = value.fpr();
    8692 
    8693             DFG_ASSERT(m_jit.graph(), node, !needsTypeCheck(element, SpecDoubleReal));
    86948703
    86958704            m_jit.storeDouble(valueFPR, MacroAssembler::Address(bufferGPR, sizeof(double) * elementIndex));
    … …  
    1308713096    Edge& baseEdge = m_jit.graph().varArgChild(node, 0);
    1308813097    Edge& propertyEdge = m_jit.graph().varArgChild(node, 1);
     13098    Edge& indexEdge = m_jit.graph().varArgChild(node, 2);
    1308913099
    1309013100    SpeculateCellOperand base(this, baseEdge);
    … …  
    1309513105#if CPU(X86)
    1309613106    // Not enough registers on X86 for this code, so always use the slow path.
     13107    speculate(node, indexEdge);
    1309713108    flushRegisters();
    1309813109    JSValueRegsFlushedCallResult result(this);
    … …  
    1310213113    jsValueResult(resultRegs, node);
    1310313114#else
    13104     Edge& indexEdge = m_jit.graph().varArgChild(node, 2);
    1310513115    Edge& enumeratorEdge = m_jit.graph().varArgChild(node, 3);
    1310613116    SpeculateStrictInt32Operand index(this, indexEdge);

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

    r242715 r242954  
    46754675               
    46764676                Edge& element = m_graph.varArgChild(m_node, elementOffset);
     4677                speculate(element);
    46774678                if (m_node->arrayMode().type() != Array::Double) {
    46784679                    value = lowJSValue(element, ManualOperandSpeculation);
    4679                     if (m_node->arrayMode().type() == Array::Int32)
    4680                         DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecInt32Only));
    46814680                    storeType = Output::Store64;
    46824681                } else {
    46834682                    value = lowDouble(element);
    4684                     DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecDoubleReal));
    46854683                    storeType = Output::StoreDouble;
    46864684                }
    … …  
    47214719            }
    47224720
     4721            for (unsigned elementIndex = 0; elementIndex < elementCount; ++elementIndex) {
     4722                Edge element = m_graph.varArgChild(m_node, elementIndex + elementOffset);
     4723                speculate(element);
     4724            }
     4725
    47234726            LValue prevLength = m_out.load32(storage, m_heaps.Butterfly_publicLength);
    47244727            LValue newLength = m_out.add(prevLength, m_out.constInt32(elementCount));
    … …  
    47574760                if (m_node->arrayMode().type() != Array::Double) {
    47584761                    value = lowJSValue(element, ManualOperandSpeculation);
    4759                     if (m_node->arrayMode().type() == Array::Int32)
    4760                         DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecInt32Only));
    47614762                    storeType = Output::Store64;
    47624763                } else {
    47634764                    value = lowDouble(element);
    4764                     DFG_ASSERT(m_graph, m_node, !m_interpreter.needsTypeCheck(element, SpecDoubleReal));
    47654765                    storeType = Output::StoreDouble;
    47664766                }
Note: See TracChangeset for help on using the changeset viewer.