When an app is using WebKit and it also implements WebScriptDebuggerDelegate::exceptionWasRaised to sniff exceptions thrown from JS engine, and then uses [WebScriptCallFrame functionName] to get the function which throws the exception. If the call frame is the top global call frame, [WebScriptCallFrame functionName] crashes. The reason is JSC::DebuggerCallFrame::functionName() doesn't check if the call frame is a Program or or Function before calling m_callFrame->callee(). For global call frame, m_callFrame->callee() return 0 and it crashes later on in asFunction().
Created attachment 55032 [details] Bailout if callframe's callee is null.
<rdar://problem/7928746>
Comment on attachment 55032 [details] Bailout if callframe's callee is null. Looks OK. r=me
Comment on attachment 55032 [details] Bailout if callframe's callee is null. Clearing flags on attachment: 55032 Committed r58779: <http://trac.webkit.org/changeset/58779>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/58779 might have broken Qt Linux Release The following changes are on the blame list: http://trac.webkit.org/changeset/58779 http://trac.webkit.org/changeset/58780
Reopen this since we still see crashes on the same spot when global top frame is called with functionName.
It crashes only in the case when the exception is throw from the second most top frame, aka. its caller frame is the top most frame, AND the top most frame is an anonymous function (like an eventHandler). In this case, m_callFrame->callee() in JSC::debuggerCallFrame::functionName() returns 0x2. We need to check if this callFrame is top callFrame in [WebScriptCallFrame functionName].
Created attachment 77301 [details] Check if this CallFrame is top call frame in [WebScriptCallFrame functionName].
Comment on attachment 77301 [details] Check if this CallFrame is top call frame in [WebScriptCallFrame functionName]. View in context: https://bugs.webkit.org/attachment.cgi?id=77301&action=review > WebKit/mac/WebView/WebScriptDebugDelegate.mm:207 > + if (!_private->caller) > + return nil; Checking this seems OK, but random; not clearly connected to the rest of this function. Could you give more detail on what fails when caller is 0? Why is adding this check the best fix? Should we be making a fix inside the JavaScriptcore functionName function?
There must be a way to fix this in JavaScriptCore. I’m not satisfied with the current patch. Geoff, Ollie, ideas?
(In reply to comment #10) > (From update of attachment 77301 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=77301&action=review > > > WebKit/mac/WebView/WebScriptDebugDelegate.mm:207 > > + if (!_private->caller) > > + return nil; > > Checking this seems OK, but random; not clearly connected to the rest of this function. Could you give more detail on what fails when caller is 0? Why is adding this check the best fix? Should we be making a fix inside the JavaScriptcore functionName function? (In reply to comment #11) > There must be a way to fix this in JavaScriptCore. I’m not satisfied with the current patch. Geoff, Ollie, ideas? I agree we should fix it in JavaScriptCore. Here is what I saw. For top call frames, the caller is 0 and functionName of a top callframe is nil. As far as I understand, top call frame's callee (in JSC::DebuggerCallFrame::functionName()) should be 0, that way, we can bail out in DebuggerCallFrame::functionName() quickly (which is http://trac.webkit.org/changeset/58779 did). The odd thing is for this particular top call frame, the callee in DebuggerCallFrame is 0x2, seems like it was read from a wrong offset in the RegisterFile, and it crashes later on in asFunction(m_callFrame->callee()). Geoff, Ollie, any idea why the callee is 0x2 in this case?
It turns out WebScriptDebugger in WebKit has empty implementations for willExecuteProgram and didExecuteProgram. As a result, if the top call frame is from a program, WebKitScriptDebugger doesn't record that callframe as the top frame, and WebScriptDebugger's callframe stack is wrong from this point. That could cause crash if we trying to access the top call from from this stack when an exception throws because the saved top frame could be invalid.
Created attachment 77362 [details] Maintain top callframe in willExecuteProgram and didExecuteProgram to keep WebScriptDebugger's callframe stack in sync with JavaScriptCore's stack.
Created attachment 77363 [details] Maintain top callframe in willExecuteProgram and didExecuteProgram to keep WebScriptDebugger's callframe stack in sync with JavaScriptCore's stack.
Comment on attachment 77363 [details] Maintain top callframe in willExecuteProgram and didExecuteProgram to keep WebScriptDebugger's callframe stack in sync with JavaScriptCore's stack. Looks good. How did you test?
(In reply to comment #16) > (From update of attachment 77363 [details]) > Looks good. How did you test? thanks. I tested with the app in <rdar://problem/7928746>, and the crash doesn't happen after the fix. It would be nice to have an unit test for this, but it involves a WebKit based app which using WebScriptDebugger API directly.
Comment on attachment 77363 [details] Maintain top callframe in willExecuteProgram and didExecuteProgram to keep WebScriptDebugger's callframe stack in sync with JavaScriptCore's stack. Clearing flags on attachment: 77363 Committed r74586: <http://trac.webkit.org/changeset/74586>