Changeset 260180 in webkit


Ignore:
Timestamp:
Apr 16, 2020 5:06:49 AM (4 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] Use ensureStillAliveHere in FTL when content of storage should be kept alive
​https://bugs.webkit.org/show_bug.cgi?id=210583
<rdar://problem/61831515>

Reviewed by Mark Lam.

The content of Butterfly / ArrayStorage is kept alive only when the owner JSCell is alive.
This means that we should keep the owner JSCell alive if we are loading content of storage
which includes JSCells. This patch inserts ensureStillAliveHere in FTL to ensure this invariant.

  • ftl/FTLJITCode.cpp:

(JSC::FTL::JITCode::~JITCode): Found that we get crash with dumpDisassembly if FTL::JITCode is destroyed while it fails to generate code while testing this.

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileArrayIndexOf):
(JSC::FTL::DFG::LowerDFGToB3::compileArrayPop):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCodePointAt):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByOffset):
(JSC::FTL::DFG::LowerDFGToB3::compileMultiGetByOffset):

Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r260166 r260180  
     12020-04-15  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] Use ensureStillAliveHere in FTL when content of storage should be kept alive
     4        https://bugs.webkit.org/show_bug.cgi?id=210583
     5        <rdar://problem/61831515>
     6
     7        Reviewed by Mark Lam.
     8
     9        The content of Butterfly / ArrayStorage is kept alive only when the owner JSCell is alive.
     10        This means that we should keep the owner JSCell alive if we are loading content of storage
     11        which includes JSCells. This patch inserts ensureStillAliveHere in FTL to ensure this invariant.
     12
     13        * ftl/FTLJITCode.cpp:
     14        (JSC::FTL::JITCode::~JITCode): Found that we get crash with `dumpDisassembly` if FTL::JITCode is destroyed while it fails to generate code while testing this.
     15        * ftl/FTLLowerDFGToB3.cpp:
     16        (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
     17        (JSC::FTL::DFG::LowerDFGToB3::compileArrayIndexOf):
     18        (JSC::FTL::DFG::LowerDFGToB3::compileArrayPop):
     19        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
     20        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharCodeAt):
     21        (JSC::FTL::DFG::LowerDFGToB3::compileStringCodePointAt):
     22        (JSC::FTL::DFG::LowerDFGToB3::compileGetByOffset):
     23        (JSC::FTL::DFG::LowerDFGToB3::compileMultiGetByOffset):
     24
    1252020-04-15  Keith Miller  <keith_miller@apple.com>
    226

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

    r255540 r260180  
    4444{
    4545    if (FTL::shouldDumpDisassembly()) {
    46         dataLog("Destroying FTL JIT code at ");
    47         CommaPrinter comma;
    48         dataLog(comma, m_b3Code);
    49         dataLog(comma, m_arityCheckEntrypoint);
    50         dataLog("\n");
     46        if (m_b3Code || m_arityCheckEntrypoint) {
     47            dataLog("Destroying FTL JIT code at ");
     48            CommaPrinter comma;
     49            if (m_b3Code)
     50                dataLog(comma, m_b3Code);
     51            if (m_arityCheckEntrypoint)
     52                dataLog(comma, m_arityCheckEntrypoint);
     53            dataLog("\n");
     54        }
    5155    }
    5256}

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

    r259786 r260180  
    43544354                } else
    43554355                    speculate(LoadFromHole, noValue(), 0, isHole);
     4356                // We have to keep base alive to keep content in storage alive.
     4357                if (m_node->arrayMode().type() == Array::Contiguous)
     4358                    ensureStillAliveHere(base);
    43564359                setJSValue(result);
    43574360                return;
    … …  
    43794382           
    43804383            m_out.appendTo(continuation, lastNext);
     4384            // We have to keep base alive to keep content in storage alive.
     4385            if (m_node->arrayMode().type() == Array::Contiguous)
     4386                ensureStillAliveHere(base);
    43814387            setJSValue(m_out.phi(Int64, fastResult, slowResult));
    43824388            return;
    … …  
    46574663                LValue result = m_out.load64(baseIndex(heap, storage, index, m_graph.varArgChild(m_node, 1)));
    46584664                speculate(LoadFromHole, noValue(), 0, m_out.isZero64(result));
     4665                // We have to keep base alive to keep content in storage alive.
     4666                ensureStillAliveHere(base);
    46594667                setJSValue(result);
    4660                 break;
     4668                return;
    46614669            }
    46624670
    … …  
    46824690
    46834691            m_out.appendTo(continuation, lastNext);
     4692            // We have to keep base alive to keep content in storage alive.
     4693            ensureStillAliveHere(base);
    46844694            setJSValue(m_out.phi(Int64, fastResult, slowResult));
    46854695            return;
    … …  
    56525662    {
    56535663        JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
     5664        LValue base = lowCell(m_graph.varArgChild(m_node, 0));
    56545665        LValue storage = lowStorage(m_node->numChildren() == 3 ? m_graph.varArgChild(m_node, 2) : m_graph.varArgChild(m_node, 3));
    56555666        LValue length = m_out.load32(storage, m_heaps.Butterfly_publicLength);
    … …  
    57545765
    57555766            m_out.appendTo(continuation, lastNext);
     5767            // We have to keep base alive since that keeps content of storage alive.
     5768            ensureStillAliveHere(base);
    57565769            setInt32(m_out.castToInt32(m_out.phi(pointerType(), notFoundResult, foundResult)));
    5757             break;
     5770            return;
    57585771        }
    57595772
    57605773        case StringUse:
    57615774            ASSERT(m_node->arrayMode().type() == Array::Contiguous);
     5775            // We have to keep base alive since that keeps storage alive.
     5776            ensureStillAliveHere(base);
    57625777            setInt32(m_out.castToInt32(vmCall(Int64, operationArrayIndexOfString, weakPointer(globalObject), storage, lowString(searchElementEdge), startIndex)));
    5763             break;
     5778            return;
    57645779
    57655780        case UntypedUse:
    … …  
    57675782            case Array::Double:
    57685783                setInt32(m_out.castToInt32(vmCall(Int64, operationArrayIndexOfValueDouble, weakPointer(globalObject), storage, lowJSValue(searchElementEdge), startIndex)));
    5769                 break;
     5784                return;
     5785            case Array::Contiguous:
     5786                // We have to keep base alive since that keeps content of storage alive.
     5787                ensureStillAliveHere(base);
     5788                FALLTHROUGH;
    57705789            case Array::Int32:
    5771             case Array::Contiguous:
    57725790                setInt32(m_out.castToInt32(vmCall(Int64, operationArrayIndexOfValueInt32OrContiguous, weakPointer(globalObject), storage, lowJSValue(searchElementEdge), startIndex)));
    5773                 break;
     5791                return;
    57745792            default:
    57755793                RELEASE_ASSERT_NOT_REACHED();
    5776                 break;
     5794                return;
    57775795            }
    5778             break;
     5796            return;
    57795797
    57805798        default:
    57815799            RELEASE_ASSERT_NOT_REACHED();
    5782             break;
     5800            return;
    57835801        }
    57845802    }
    … …  
    58145832            if (m_node->arrayMode().type() != Array::Double) {
    58155833                LValue result = m_out.load64(pointer);
     5834                // We have to keep base alive to keep content in storage alive.
     5835                if (m_node->arrayMode().type() == Array::Contiguous)
     5836                    ensureStillAliveHere(base);
    58165837                m_out.store64(m_out.int64Zero, pointer);
    58175838                results.append(m_out.anchor(result));
    … …  
    58595880            TypedPointer pointer = m_out.baseIndex(m_heaps.ArrayStorage_vector, storage, m_out.zeroExtPtr(newLength));
    58605881            LValue result = m_out.load64(pointer);
     5882            // We have to keep base alive to keep content in storage alive.
     5883            ensureStillAliveHere(base);
    58615884            m_out.branch(m_out.notZero64(result), usually(fastCase), rarely(slowCase));
    58625885
    … …  
    78177840           
    78187841        m_out.appendTo(continuation, lastNext);
     7842        // We have to keep base alive since that keeps storage alive.
     7843        ensureStillAliveHere(base);
    78197844        setJSValue(m_out.phi(Int64, results));
    78207845    }
    … …  
    78637888        m_out.appendTo(continuation, lastNext);
    78647889       
     7890        // We have to keep base alive since that keeps storage alive.
     7891        ensureStillAliveHere(base);
    78657892        setInt32(m_out.phi(Int32, char8Bit, char16Bit));
    78667893    }
    … …  
    79217948
    79227949        m_out.appendTo(continuation, lastNext);
     7950        // We have to keep base alive since that keeps storage alive.
     7951        ensureStillAliveHere(base);
    79237952        setInt32(m_out.phi(Int32, char8Bit, char16Bit, charSurrogatePair));
    79247953    }
    … …  
    79738002        StorageAccessData& data = m_node->storageAccessData();
    79748003       
    7975         setJSValue(loadProperty(
    7976             lowStorage(m_node->child1()), data.identifierNumber, data.offset));
     8004        LValue base = lowCell(m_node->child2());
     8005        LValue value = loadProperty(lowStorage(m_node->child1()), data.identifierNumber, data.offset);
     8006        // We have to keep base alive since that keeps content of storage alive.
     8007        ensureStillAliveHere(base);
     8008        setJSValue(value);
    79778009    }
    79788010   
    … …  
    80568088       
    80578089        m_out.appendTo(continuation, lastNext);
     8090        // We have to keep base alive since that keeps storage alive.
     8091        ensureStillAliveHere(base);
    80588092        setJSValue(m_out.phi(Int64, results));
    80598093    }
Note: See TracChangeset for help on using the changeset viewer.