Changeset 34085 in webkit for trunk/JavaScriptCore/profiler/Profiler.cpp

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):

File:

• trunk/JavaScriptCore/profiler/Profiler.cpp (modified) (5 diffs)

trunk/JavaScriptCore/profiler/Profiler.cpp

@@ -99 +99 @@
 }
 
-static inline bool shouldExcludeFunction(ExecState* exec, JSObject* calledFunction)
-{
-    if (!calledFunction->inherits(&InternalFunctionImp::info))
-        return false;
-    // Don't record a call for function.call and function.apply.
-    if (static_cast<InternalFunctionImp*>(calledFunction)->functionName() == exec->propertyNames().call)
-        return true;
-    if (static_cast<InternalFunctionImp*>(calledFunction)->functionName() == exec->propertyNames().apply)
-        return true;
-    return false;
-}
-
 static inline void dispatchFunctionToProfiles(const Vector<RefPtr<Profile> >& profiles, Profile::ProfileFunction function, const CallIdentifier& callIdentifier, unsigned currentPageGroupIdentifier)
 {
@@ -120 +108 @@
 void Profiler::willExecute(ExecState* exec, JSObject* calledFunction)
 {
-    if (m_currentProfiles.isEmpty())
-        return;
-
-    if (shouldExcludeFunction(exec, calledFunction))
-        return;
+    ASSERT(!m_currentProfiles.isEmpty());
 
     dispatchFunctionToProfiles(m_currentProfiles, &Profile::willExecute, createCallIdentifier(calledFunction), exec->lexicalGlobalObject()->pageGroupIdentifier());
@@ -131 +115 @@
 void Profiler::willExecute(ExecState* exec, const UString& sourceURL, int startingLineNumber)
 {
-    if (m_currentProfiles.isEmpty())
-        return;
+    ASSERT(!m_currentProfiles.isEmpty());
 
     CallIdentifier callIdentifier = createCallIdentifier(sourceURL, startingLineNumber);
@@ -141 +124 @@
 void Profiler::didExecute(ExecState* exec, JSObject* calledFunction)
 {
-    if (m_currentProfiles.isEmpty())
-        return;
-
-    if (shouldExcludeFunction(exec, calledFunction))
-        return;
+    ASSERT(!m_currentProfiles.isEmpty());
 
     dispatchFunctionToProfiles(m_currentProfiles, &Profile::didExecute, createCallIdentifier(calledFunction), exec->lexicalGlobalObject()->pageGroupIdentifier());
@@ -152 +131 @@
 void Profiler::didExecute(ExecState* exec, const UString& sourceURL, int startingLineNumber)
 {
-    if (m_currentProfiles.isEmpty())
-        return;
+    ASSERT(!m_currentProfiles.isEmpty());
 
     dispatchFunctionToProfiles(m_currentProfiles, &Profile::didExecute, createCallIdentifier(sourceURL, startingLineNumber), exec->lexicalGlobalObject()->pageGroupIdentifier());