Changeset 44844 in webkit


Ignore:
Timestamp:
Jun 19, 2009 12:10:49 AM (15 years ago)
Author:
oliver@apple.com
Message:

Bug 26532: Native functions do not correctly unlink from optimised callsites when they're collected
<https://bugs.webkit.org/show_bug.cgi?id=26532> <rdar://problem/6625385>

Reviewed by Gavin "Viceroy of Venezuela" Barraclough.

We need to make sure that each native function instance correctly unlinks any references to it
when it is collected. Allowing this to happen required a few changes:

  • Every native function needs a codeblock to track the link information
  • To have this codeblock, every function now also needs its own functionbodynode so we no longer get to have a single shared instance.
  • Identifying a host function is now done by looking for CodeBlock::codeType() == NativeCode
Location:
trunk/JavaScriptCore
Files:
11 edited

Legend:

Unmodified
Added
Removed
  • trunk/JavaScriptCore/ChangeLog

    r44843 r44844  
     12009-06-18  Oliver Hunt  <oliver@apple.com>
     2
     3        Reviewed by Gavin "Viceroy of Venezuela" Barraclough.
     4
     5        Bug 26532: Native functions do not correctly unlink from optimised callsites when they're collected
     6        <https://bugs.webkit.org/show_bug.cgi?id=26532> <rdar://problem/6625385>
     7
     8        We need to make sure that each native function instance correctly unlinks any references to it
     9        when it is collected.  Allowing this to happen required a few changes:
     10            * Every native function needs a codeblock to track the link information
     11            * To have this codeblock, every function now also needs its own functionbodynode
     12              so we no longer get to have a single shared instance.
     13            * Identifying a host function is now done by looking for CodeBlock::codeType() == NativeCode
     14
     15        * JavaScriptCore.exp:
     16        * bytecode/CodeBlock.cpp:
     17        (JSC::CodeBlock::CodeBlock):
     18           Constructor for NativeCode CodeBlock
     19        (JSC::CodeBlock::derefStructures):
     20        (JSC::CodeBlock::refStructures):
     21        (JSC::CodeBlock::reparseForExceptionInfoIfNecessary):
     22        (JSC::CodeBlock::handlerForBytecodeOffset):
     23        (JSC::CodeBlock::lineNumberForBytecodeOffset):
     24        (JSC::CodeBlock::expressionRangeForBytecodeOffset):
     25        (JSC::CodeBlock::getByIdExceptionInfoForBytecodeOffset):
     26        (JSC::CodeBlock::functionRegisterForBytecodeOffset):
     27        (JSC::CodeBlock::hasGlobalResolveInstructionAtBytecodeOffset):
     28        (JSC::CodeBlock::hasGlobalResolveInfoAtBytecodeOffset):
     29        (JSC::CodeBlock::setJITCode):
     30           Add assertions to ensure we don't try and use NativeCode CodeBlocks as
     31           a normal codeblock.
     32
     33        * bytecode/CodeBlock.h:
     34        (JSC::):
     35        (JSC::CodeBlock::source):
     36        (JSC::CodeBlock::sourceOffset):
     37        (JSC::CodeBlock::evalCodeCache):
     38        (JSC::CodeBlock::createRareDataIfNecessary):
     39          More assertions.
     40
     41        * jit/JIT.cpp:
     42        (JSC::JIT::privateCompileCTIMachineTrampolines):
     43        (JSC::JIT::linkCall):
     44          Update logic to allow native function caching
     45
     46        * jit/JITStubs.cpp:
     47        * parser/Nodes.cpp:
     48        (JSC::FunctionBodyNode::createNativeThunk):
     49        (JSC::FunctionBodyNode::isHostFunction):
     50        * parser/Nodes.h:
     51        * runtime/JSFunction.cpp:
     52        (JSC::JSFunction::JSFunction):
     53        (JSC::JSFunction::~JSFunction):
     54        (JSC::JSFunction::mark):
     55        * runtime/JSGlobalData.cpp:
     56        (JSC::JSGlobalData::~JSGlobalData):
     57        * runtime/JSGlobalData.h:
     58
    1592009-06-18  Gavin Barraclough  <barraclough@apple.com>
    260
  • trunk/JavaScriptCore/JavaScriptCore.exp

    r44831 r44844  
    332332__ZNK3JSC12StringObject8toStringEPNS_9ExecStateE
    333333__ZNK3JSC14JSGlobalObject14isDynamicScopeEv
     334__ZNK3JSC16FunctionBodyNode14isHostFunctionEv
    334335__ZNK3JSC16InternalFunction9classInfoEv
    335336__ZNK3JSC16JSVariableObject16isVariableObjectEv
  • trunk/JavaScriptCore/bytecode/CodeBlock.cpp

    r44076 r44844  
    12661266}
    12671267
     1268CodeBlock::CodeBlock(ScopeNode* ownerNode)
     1269    : m_numCalleeRegisters(0)
     1270    , m_numConstants(0)
     1271    , m_numVars(0)
     1272    , m_numParameters(0)
     1273    , m_ownerNode(ownerNode)
     1274    , m_globalData(0)
     1275#ifndef NDEBUG
     1276    , m_instructionCount(0)
     1277#endif
     1278    , m_needsFullScopeChain(false)
     1279    , m_usesEval(false)
     1280    , m_isNumericCompareFunction(false)
     1281    , m_codeType(NativeCode)
     1282    , m_source(0)
     1283    , m_sourceOffset(0)
     1284    , m_exceptionInfo(0)
     1285{
     1286#if DUMP_CODE_BLOCK_STATISTICS
     1287    liveCodeBlockSet.add(this);
     1288#endif
     1289}
    12681290
    12691291CodeBlock::CodeBlock(ScopeNode* ownerNode, CodeType codeType, PassRefPtr<SourceProvider> sourceProvider, unsigned sourceOffset)
     
    13431365void CodeBlock::derefStructures(Instruction* vPC) const
    13441366{
     1367    ASSERT(m_codeType != NativeCode);
    13451368    Interpreter* interpreter = m_globalData->interpreter;
    13461369
     
    13881411void CodeBlock::refStructures(Instruction* vPC) const
    13891412{
     1413    ASSERT(m_codeType != NativeCode);
    13901414    Interpreter* interpreter = m_globalData->interpreter;
    13911415
     
    14421466void CodeBlock::reparseForExceptionInfoIfNecessary(CallFrame* callFrame)
    14431467{
     1468    ASSERT(m_codeType != NativeCode);
    14441469    if (m_exceptionInfo)
    14451470        return;
     
    15121537HandlerInfo* CodeBlock::handlerForBytecodeOffset(unsigned bytecodeOffset)
    15131538{
     1539    ASSERT(m_codeType != NativeCode);
    15141540    ASSERT(bytecodeOffset < m_instructionCount);
    15151541
     
    15301556int CodeBlock::lineNumberForBytecodeOffset(CallFrame* callFrame, unsigned bytecodeOffset)
    15311557{
     1558    ASSERT(m_codeType != NativeCode);
    15321559    ASSERT(bytecodeOffset < m_instructionCount);
    15331560
     
    15551582int CodeBlock::expressionRangeForBytecodeOffset(CallFrame* callFrame, unsigned bytecodeOffset, int& divot, int& startOffset, int& endOffset)
    15561583{
     1584    ASSERT(m_codeType != NativeCode);
    15571585    ASSERT(bytecodeOffset < m_instructionCount);
    15581586
     
    15941622bool CodeBlock::getByIdExceptionInfoForBytecodeOffset(CallFrame* callFrame, unsigned bytecodeOffset, OpcodeID& opcodeID)
    15951623{
     1624    ASSERT(m_codeType != NativeCode);
    15961625    ASSERT(bytecodeOffset < m_instructionCount);
    15971626
     
    16221651bool CodeBlock::functionRegisterForBytecodeOffset(unsigned bytecodeOffset, int& functionRegisterIndex)
    16231652{
     1653    ASSERT(m_codeType != NativeCode);
    16241654    ASSERT(bytecodeOffset < m_instructionCount);
    16251655
     
    16481678bool CodeBlock::hasGlobalResolveInstructionAtBytecodeOffset(unsigned bytecodeOffset)
    16491679{
     1680    ASSERT(m_codeType != NativeCode);
    16501681    if (m_globalResolveInstructions.isEmpty())
    16511682        return false;
     
    16681699bool CodeBlock::hasGlobalResolveInfoAtBytecodeOffset(unsigned bytecodeOffset)
    16691700{
     1701    ASSERT(m_codeType != NativeCode);
    16701702    if (m_globalResolveInfos.isEmpty())
    16711703        return false;
     
    16901722void CodeBlock::setJITCode(JITCode jitCode)
    16911723{
     1724    ASSERT(m_codeType != NativeCode);
    16921725    ownerNode()->setJITCode(jitCode);
    16931726#if !ENABLE(OPCODE_SAMPLING)
  • trunk/JavaScriptCore/bytecode/CodeBlock.h

    r44711 r44844  
    5050    class ExecState;
    5151
    52     enum CodeType { GlobalCode, EvalCode, FunctionCode };
     52    enum CodeType { GlobalCode, EvalCode, FunctionCode, NativeCode };
    5353
    5454    static ALWAYS_INLINE int missingThisObjectMarker() { return std::numeric_limits<int>::max(); }
     
    218218        friend class JIT;
    219219    public:
     220        CodeBlock(ScopeNode* ownerNode);
    220221        CodeBlock(ScopeNode* ownerNode, CodeType, PassRefPtr<SourceProvider>, unsigned sourceOffset);
    221222        ~CodeBlock();
     
    340341        CodeType codeType() const { return m_codeType; }
    341342
    342         SourceProvider* source() const { return m_source.get(); }
    343         unsigned sourceOffset() const { return m_sourceOffset; }
     343        SourceProvider* source() const { ASSERT(m_codeType != NativeCode); return m_source.get(); }
     344        unsigned sourceOffset() const { ASSERT(m_codeType != NativeCode); return m_sourceOffset; }
    344345
    345346        size_t numberOfJumpTargets() const { return m_jumpTargets.size(); }
     
    433434        SymbolTable& symbolTable() { return m_symbolTable; }
    434435
    435         EvalCodeCache& evalCodeCache() { createRareDataIfNecessary(); return m_rareData->m_evalCodeCache; }
     436        EvalCodeCache& evalCodeCache() { ASSERT(m_codeType != NativeCode); createRareDataIfNecessary(); return m_rareData->m_evalCodeCache; }
    436437
    437438        void shrinkToFit();
     
    457458        void createRareDataIfNecessary()
    458459        {
     460            ASSERT(m_codeType != NativeCode);
    459461            if (!m_rareData)
    460462                m_rareData.set(new RareData);
  • trunk/JavaScriptCore/jit/JIT.cpp

    r44745 r44844  
    547547
    548548    // (3) Trampolines for the slow cases of op_call / op_call_eval / op_construct.
    549    
     549    COMPILE_ASSERT(sizeof(CodeType) == 4, CodeTypeEnumMustBe32Bit);
     550
    550551    Label virtualCallPreLinkBegin = align();
    551552
     
    554555    loadPtr(Address(regT3, FIELD_OFFSET(FunctionBodyNode, m_code)), regT0);
    555556    Jump hasCodeBlock1 = branchTestPtr(NonZero, regT0);
    556     // If m_code is null and m_jitCode is not, then we have a native function, so arity is irrelevant
    557     loadPtr(Address(regT3, FIELD_OFFSET(FunctionBodyNode, m_jitCode)), regT0);
    558     Jump isNativeFunc1 = branchTestPtr(NonZero, regT0);
    559557    preverveReturnAddressAfterCall(regT3);
    560558    restoreArgumentReference();
     
    564562    restoreReturnAddressBeforeReturn(regT3);
    565563    hasCodeBlock1.link(this);
     564
     565    Jump isNativeFunc1 = branch32(Equal, Address(regT0, FIELD_OFFSET(CodeBlock, m_codeType)), Imm32(NativeCode));
    566566
    567567    // Check argCount matches callee arity.
     
    596596    loadPtr(Address(regT3, FIELD_OFFSET(FunctionBodyNode, m_code)), regT0);
    597597    Jump hasCodeBlock2 = branchTestPtr(NonZero, regT0);
    598     // If m_code is null and m_jitCode is not, then we have a native function, so arity is irrelevant
    599     loadPtr(Address(regT3, FIELD_OFFSET(FunctionBodyNode, m_jitCode)), regT0);
    600     Jump isNativeFunc2 = branchTestPtr(NonZero, regT0);
    601598    preverveReturnAddressAfterCall(regT3);
    602599    restoreArgumentReference();
     
    606603    restoreReturnAddressBeforeReturn(regT3);
    607604    hasCodeBlock2.link(this);
     605
     606    Jump isNativeFunc2 = branch32(Equal, Address(regT0, FIELD_OFFSET(CodeBlock, m_codeType)), Imm32(NativeCode));
    608607
    609608    // Check argCount matches callee arity.
     
    637636    loadPtr(Address(regT3, FIELD_OFFSET(FunctionBodyNode, m_code)), regT0);
    638637    Jump hasCodeBlock3 = branchTestPtr(NonZero, regT0);
    639     // If m_code is null and m_jitCode is not, then we have a native function, so arity is irrelevant
    640     loadPtr(Address(regT3, FIELD_OFFSET(FunctionBodyNode, m_jitCode)), regT0);
    641     Jump isNativeFunc3 = branchTestPtr(NonZero, regT0);
    642638    preverveReturnAddressAfterCall(regT3);
    643639    restoreArgumentReference();
     
    648644    loadPtr(Address(regT2, FIELD_OFFSET(JSFunction, m_body)), regT3); // reload the function body nody, so we can reload the code pointer.
    649645    hasCodeBlock3.link(this);
     646   
     647    Jump isNativeFunc3 = branch32(Equal, Address(regT0, FIELD_OFFSET(CodeBlock, m_codeType)), Imm32(NativeCode));
    650648
    651649    // Check argCount matches callee arity.
     
    662660    loadPtr(Address(regT2, FIELD_OFFSET(JSFunction, m_body)), regT3); // reload the function body nody, so we can reload the code pointer.
    663661    arityCheckOkay3.link(this);
     662    isNativeFunc3.link(this);
     663
    664664    // load ctiCode from the new codeBlock.
    665665    loadPtr(Address(regT3, FIELD_OFFSET(FunctionBodyNode, m_jitCode)), regT0);
    666     isNativeFunc3.link(this);
    667 
     666   
    668667    compileOpCallInitializeCallFrame();
    669668    jump(regT0);
     
    912911void JIT::linkCall(JSFunction* callee, CodeBlock* calleeCodeBlock, JITCode& code, CallLinkInfo* callLinkInfo, int callerArgCount, JSGlobalData* globalData)
    913912{
     913    ASSERT(calleeCodeBlock);
    914914    RepatchBuffer repatchBuffer;
    915915
    916916    // Currently we only link calls with the exact number of arguments.
    917917    // If this is a native call calleeCodeBlock is null so the number of parameters is unimportant
    918     if (!calleeCodeBlock || callerArgCount == calleeCodeBlock->m_numParameters) {
     918    if (callerArgCount == calleeCodeBlock->m_numParameters || calleeCodeBlock->codeType() == NativeCode) {
    919919        ASSERT(!callLinkInfo->isLinked());
    920920   
  • trunk/JavaScriptCore/jit/JITStubs.cpp

    r44838 r44844  
    11921192
    11931193    JSFunction* function = asFunction(stackFrame.args[0].jsValue());
     1194    ASSERT(!function->isHostFunction());
    11941195    FunctionBodyNode* body = function->body();
    11951196    ScopeChainNode* callDataScopeChain = function->scope().node();
     
    12051206    CallFrame* callFrame = stackFrame.callFrame;
    12061207    CodeBlock* newCodeBlock = stackFrame.args[3].codeBlock();
     1208    ASSERT(newCodeBlock->codeType() != NativeCode);
    12071209    int argCount = stackFrame.args[2].int32();
    12081210
     
    12661268    if (!callee->isHostFunction())
    12671269        codeBlock = &callee->body()->bytecode(callee->scope().node());
     1270    else
     1271        codeBlock = &callee->body()->generatedBytecode();
    12681272
    12691273    CallLinkInfo* callLinkInfo = &stackFrame.callFrame->callerFrame()->codeBlock()->getCallLinkInfo(stackFrame.args[1].returnAddress());
  • trunk/JavaScriptCore/parser/Nodes.cpp

    r44224 r44844  
    20382038    RefPtr<FunctionBodyNode> body = new FunctionBodyNode(globalData);
    20392039    globalData->parser->arena().reset();
     2040    body->m_code.set(new CodeBlock(body.get()));
    20402041    body->m_jitCode = JITCode(JITCode::HostFunction(globalData->jitStubs.ctiNativeCallThunk()));
    20412042    return body.release();
    20422043}
    20432044#endif
     2045
     2046bool FunctionBodyNode::isHostFunction() const
     2047{
     2048    return m_code && m_code->codeType() == NativeCode;
     2049}
    20442050
    20452051FunctionBodyNode* FunctionBodyNode::create(JSGlobalData* globalData)
  • trunk/JavaScriptCore/parser/Nodes.h

    r44224 r44844  
    15551555        }
    15561556
    1557         bool isHostFunction() const
    1558         {
    1559 #if ENABLE(JIT)
    1560             return !!m_jitCode && !m_code;
    1561 #else
    1562             return true;
    1563 #endif
    1564         }
     1557        bool isHostFunction() const;
    15651558
    15661559        virtual void mark();
  • trunk/JavaScriptCore/runtime/JSFunction.cpp

    r43661 r44844  
    4949    : Base(&exec->globalData(), structure, name)
    5050#if ENABLE(JIT)
    51     , m_body(exec->globalData().nativeFunctionThunk())
     51    , m_body(FunctionBodyNode::createNativeThunk(&exec->globalData()))
    5252#else
    5353    , m_body(0)
     
    7777    // are based on a check for the this pointer value for this JSFunction - which will no longer be valid once
    7878    // this memory is freed and may be reused (potentially for another, different JSFunction).
    79     if (!isHostFunction()) {
    80         if (m_body && m_body->isGenerated())
    81             m_body->generatedBytecode().unlinkCallers();
     79    if (m_body && m_body->isGenerated())
     80        m_body->generatedBytecode().unlinkCallers();
     81    if (!isHostFunction())
    8282        scopeChain().~ScopeChain();
    83     }
    84    
    8583#endif
    8684}
     
    8987{
    9088    Base::mark();
    91     if (!isHostFunction()) {
    92         m_body->mark();
     89    m_body->mark();
     90    if (!isHostFunction())
    9391        scopeChain().mark();
    94     }
    9592}
    9693
  • trunk/JavaScriptCore/runtime/JSGlobalData.cpp

    r44813 r44844  
    165165    regExpConstructorTable->deleteTable();
    166166    stringTable->deleteTable();
    167 #if ENABLE(JIT)
    168     lazyNativeFunctionThunk.clear();
    169 #endif
    170167
    171168    fastDelete(const_cast<HashTable*>(arrayTable));
     
    228225}
    229226
    230 #if ENABLE(JIT)
    231 
    232 void JSGlobalData::createNativeThunk()
    233 {
    234     lazyNativeFunctionThunk = FunctionBodyNode::createNativeThunk(this);
    235 }
    236 
    237 #endif
    238 
    239227// FIXME: We can also detect forms like v1 < v2 ? -1 : 0, reverse comparison, etc.
    240228const Vector<Instruction>& JSGlobalData::numericCompareFunction(ExecState* exec)
  • trunk/JavaScriptCore/runtime/JSGlobalData.h

    r44813 r44844  
    121121#if ENABLE(JIT)
    122122        JITThunks jitStubs;
    123         FunctionBodyNode* nativeFunctionThunk()
    124         {
    125             if (!lazyNativeFunctionThunk)
    126                 createNativeThunk();
    127             return lazyNativeFunctionThunk.get();
    128         }
    129         RefPtr<FunctionBodyNode> lazyNativeFunctionThunk;
    130123#endif
    131124        TimeoutChecker timeoutChecker;
Note: See TracChangeset for help on using the changeset viewer.