Changeset 34085 in webkit for trunk/JavaScriptCore


Ignore:
Timestamp:
May 23, 2008, 10:50:51 AM (17 years ago)
Author:
[email protected]
Message:

2008-05-23 Geoffrey Garen <​[email protected]>

Reviewed by Darin Adler, Kevin McCullough, and Oliver Hunt.


Fixed <rdar://problem/5959447> Crashes and incorrect reporting in the
JavaScript profiler

  • VM/Machine.cpp: (KJS::callEval): Made this profiler hooks slightly faster by passing in the eval function.


(KJS::Machine::unwindCallFrame): Fixed incorrect reporting / a crash
when unwinding from inside eval and/or program code: detect the
difference, and do the right thing. Also, be sure to notify the profiler
*before* deref'ing the scope chain, since the profiler uses the scope chain.

(KJS::Machine::execute): Fixed incorrect reporting / crash when calling
a JS function re-entrently: Machine::execute(FunctionBodyNode*...)
should not invoke the didExecute hook, because op_ret already does that.
Also, use the new function's ExecState when calling out to the profiler.
(Not important now, but could have become a subtle bug later.)

(KJS::Machine::privateExecute): Fixed a hard to reproduce crash when
profiling JS functions: notify the profiler *before* deref'ing the scope
chain, since the profiler uses the scope chain.

  • kjs/object.cpp: (KJS::JSObject::call): Removed these hooks, because they are now unnecessary.
  • profiler/Profile.cpp: Added a comment to explain a subtlety that only Kevin and I understood previously. (Now, the whole world can understand!)
  • profiler/Profiler.cpp: (KJS::shouldExcludeFunction): Don't exclude .call and .apply. That was a hack to fix bugs that no longer exist.

Finally, sped things up a little bit by changing the "Is the profiler
running?" check into an ASSERT, since we only call into the profiler
when it's running:

(KJS::Profiler::willExecute):
(KJS::Profiler::didExecute):

Location:
trunk/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r34080 r34085  
     12008-05-23  Geoffrey Garen  <[email protected]>
     2
     3        Reviewed by Darin Adler, Kevin McCullough, and Oliver Hunt.
     4       
     5        Fixed <rdar://problem/5959447> Crashes and incorrect reporting in the
     6        JavaScript profiler
     7
     8        * VM/Machine.cpp:
     9        (KJS::callEval): Made this profiler hooks slightly faster by passing in
     10        the eval function.
     11       
     12        (KJS::Machine::unwindCallFrame): Fixed incorrect reporting / a crash
     13        when unwinding from inside eval and/or program code: detect the
     14        difference, and do the right thing. Also, be sure to notify the profiler
     15        *before* deref'ing the scope chain, since the profiler uses the scope chain.
     16
     17        (KJS::Machine::execute): Fixed incorrect reporting / crash when calling
     18        a JS function re-entrently: Machine::execute(FunctionBodyNode*...)
     19        should not invoke the didExecute hook, because op_ret already does that.
     20        Also, use the new function's ExecState when calling out to the profiler.
     21        (Not important now, but could have become a subtle bug later.)
     22
     23        (KJS::Machine::privateExecute): Fixed a hard to reproduce crash when
     24        profiling JS functions: notify the profiler *before* deref'ing the scope
     25        chain, since the profiler uses the scope chain.
     26
     27        * kjs/object.cpp:
     28        (KJS::JSObject::call): Removed these hooks, because they are now unnecessary.
     29
     30        * profiler/Profile.cpp: Added a comment to explain a subtlety that only
     31        Kevin and I understood previously. (Now, the whole world can understand!)
     32
     33        * profiler/Profiler.cpp:
     34        (KJS::shouldExcludeFunction): Don't exclude .call and .apply. That was
     35        a hack to fix bugs that no longer exist.
     36
     37        Finally, sped things up a little bit by changing the "Is the profiler
     38        running?" check into an ASSERT, since we only call into the profiler
     39        when it's running:
     40
     41        (KJS::Profiler::willExecute):
     42        (KJS::Profiler::didExecute):
     43
    1442008-05-23  Adam Roben  <[email protected]>
    245

  • trunk/JavaScriptCore/VM/Machine.cpp

    r34075 r34085  
    438438}
    439439
    440 static NEVER_INLINE JSValue* callEval(ExecState* exec, JSObject* thisObj, ScopeChainNode* scopeChain, RegisterFile* registerFile, Register* r, int argv, int argc, JSValue*& exceptionValue)
     440static NEVER_INLINE JSValue* callEval(ExecState* exec, JSObject* thisObj, JSObject* evalFunction, ScopeChainNode* scopeChain, RegisterFile* registerFile, Register* r, int argv, int argc, JSValue*& exceptionValue)
    441441{
    442442    Profiler** profiler = Profiler::enabledProfilerReference();
    443     JSObject* evalFunction = scopeChain->globalObject()->evalFunction();
    444443    if (*profiler)
    445444        (*profiler)->willExecute(exec, evalFunction);
    … …  
    577576    }
    578577
     578    Register* callFrame = r - oldCodeBlock->numLocals - CallFrameHeaderSize;
     579   
     580    if (Profiler* profiler = *Profiler::enabledProfilerReference()) {
     581        if (!isGlobalCallFrame(registerBase, r) && callFrame[Callee].u.jsObject) // Check for global and eval code
     582            profiler->didExecute(exec, callFrame[Callee].u.jsObject);
     583        else
     584            profiler->didExecute(exec, codeBlock->ownerNode->sourceURL(), codeBlock->ownerNode->lineNo());
     585    }
     586
    579587    if (oldCodeBlock->needsFullScopeChain)
    580588        scopeChain->deref();
    … …  
    583591        return false;
    584592
    585     Register* callFrame = r - oldCodeBlock->numLocals - CallFrameHeaderSize;
    586    
    587593    codeBlock = callFrame[CallerCodeBlock].u.codeBlock;
    588594    if (!codeBlock)
    … …  
    602608    vPC = callFrame[ReturnVPC].u.vPC;
    603609
    604     if (Profiler* profiler = *Profiler::enabledProfilerReference())
    605         profiler->didExecute(exec, callFrame[Callee].u.jsObject);
    606610    return true;
    607611}
    … …  
    669673        scopeChain = scopeChain->copy();
    670674
     675    ExecState newExec(exec, this, registerFile, scopeChain, -1);
     676
    671677    Profiler** profiler = Profiler::enabledProfilerReference();
    672678    if (*profiler)
    673679        (*profiler)->willExecute(exec, programNode->sourceURL(), programNode->lineNo());
    674 
    675     ExecState newExec(exec, this, registerFile, scopeChain, -1);
    676680
    677681    m_reentryDepth++;
    … …  
    739743    JSValue* result = privateExecute(Normal, &newExec, registerFile, r, scopeChain, newCodeBlock, exception);
    740744    m_reentryDepth--;
    741 
    742     if (*profiler)
    743         (*profiler)->didExecute(exec, function);
    744745
    745746    registerFile->shrink(oldSize);
    … …  
    18651866            registerFile->setSafeForReentry(true);
    18661867
    1867             JSValue* result = callEval(exec, thisObject, scopeChain, registerFile, r, argv, argc, exceptionValue);
     1868            JSValue* result = callEval(exec, thisObject, static_cast<JSObject*>(funcVal), scopeChain, registerFile, r, argv, argc, exceptionValue);
    18681869
    18691870            registerFile->setSafeForReentry(false);
    … …  
    19751976            activation->copyRegisters();
    19761977        }
     1978
     1979        if (*enabledProfilerReference)
     1980            (*enabledProfilerReference)->didExecute(exec, callFrame[Callee].u.jsObject);
    19771981
    19781982        if (codeBlock->needsFullScopeChain)
    … …  
    19962000        int r0 = callFrame[ReturnValueRegister].u.i;
    19972001        r[r0].u.jsValue = returnValue;
    1998 
    1999         if (*enabledProfilerReference)
    2000             (*enabledProfilerReference)->didExecute(exec, callFrame[Callee].u.jsObject);
    20012002
    20022003        NEXT_OPCODE;

  • trunk/JavaScriptCore/kjs/object.cpp

    r34074 r34085  
    9292  }
    9393#endif
    94     CallData data;
    95     JSValue *ret;
    96     if (this->getCallData(data) == CallTypeJS) {
    97         // A native function will enter the VM, which will allow the profiler
    98         // to detect entry
    99         ret = callAsFunction(exec,thisObj,args);
    100     } else {
    101         Profiler** profiler = Profiler::enabledProfilerReference();
    102         if (*profiler)
    103             (*profiler)->willExecute(exec, this);
    104        
    105         ret = callAsFunction(exec,thisObj,args);
    106        
    107         if (*profiler)
    108             (*profiler)->didExecute(exec, this);
    109     }
     94  JSValue* ret = callAsFunction(exec,thisObj,args);
     95
    11096#if KJS_MAX_STACK > 0
    11197  --depth;

  • trunk/JavaScriptCore/profiler/Profile.cpp

    r34060 r34085  
    5757{
    5858    ASSERT(m_currentNode);
    59 
    6059    m_currentNode = m_currentNode->willExecute(callIdentifier);
    6160}
    … …  
    6362void Profile::didExecute(const CallIdentifier& callIdentifier)
    6463{
     64    ASSERT(m_currentNode);
     65
     66    // In case the profiler started recording calls in the middle of a stack frame,
     67    // when returning up the stack it needs to insert the calls it missed on the
     68    // way down.
    6569    if (m_currentNode == m_headNode) {
    6670        m_currentNode = ProfileNode::create(callIdentifier, m_headNode.get(), m_headNode.get());

  • trunk/JavaScriptCore/profiler/Profiler.cpp

    r34065 r34085  
    9999}
    100100
    101 static inline bool shouldExcludeFunction(ExecState* exec, JSObject* calledFunction)
    102 {
    103     if (!calledFunction->inherits(&InternalFunctionImp::info))
    104         return false;
    105     // Don't record a call for function.call and function.apply.
    106     if (static_cast<InternalFunctionImp*>(calledFunction)->functionName() == exec->propertyNames().call)
    107         return true;
    108     if (static_cast<InternalFunctionImp*>(calledFunction)->functionName() == exec->propertyNames().apply)
    109         return true;
    110     return false;
    111 }
    112 
    113101static inline void dispatchFunctionToProfiles(const Vector<RefPtr<Profile> >& profiles, Profile::ProfileFunction function, const CallIdentifier& callIdentifier, unsigned currentPageGroupIdentifier)
    114102{
    … …  
    120108void Profiler::willExecute(ExecState* exec, JSObject* calledFunction)
    121109{
    122     if (m_currentProfiles.isEmpty())
    123         return;
    124 
    125     if (shouldExcludeFunction(exec, calledFunction))
    126         return;
     110    ASSERT(!m_currentProfiles.isEmpty());
    127111
    128112    dispatchFunctionToProfiles(m_currentProfiles, &Profile::willExecute, createCallIdentifier(calledFunction), exec->lexicalGlobalObject()->pageGroupIdentifier());
    … …  
    131115void Profiler::willExecute(ExecState* exec, const UString& sourceURL, int startingLineNumber)
    132116{
    133     if (m_currentProfiles.isEmpty())
    134         return;
     117    ASSERT(!m_currentProfiles.isEmpty());
    135118
    136119    CallIdentifier callIdentifier = createCallIdentifier(sourceURL, startingLineNumber);
    … …  
    141124void Profiler::didExecute(ExecState* exec, JSObject* calledFunction)
    142125{
    143     if (m_currentProfiles.isEmpty())
    144         return;
    145 
    146     if (shouldExcludeFunction(exec, calledFunction))
    147         return;
     126    ASSERT(!m_currentProfiles.isEmpty());
    148127
    149128    dispatchFunctionToProfiles(m_currentProfiles, &Profile::didExecute, createCallIdentifier(calledFunction), exec->lexicalGlobalObject()->pageGroupIdentifier());
    … …  
    152131void Profiler::didExecute(ExecState* exec, const UString& sourceURL, int startingLineNumber)
    153132{
    154     if (m_currentProfiles.isEmpty())
    155         return;
     133    ASSERT(!m_currentProfiles.isEmpty());
    156134
    157135    dispatchFunctionToProfiles(m_currentProfiles, &Profile::didExecute, createCallIdentifier(sourceURL, startingLineNumber), exec->lexicalGlobalObject()->pageGroupIdentifier());
Note: See TracChangeset for help on using the changeset viewer.