Last Comment Bug 653631 - Crash [@ JSStackFrame::isDummyFrame] with mozRequestAnimationFrame
: Crash [@ JSStackFrame::isDummyFrame] with mozRequestAnimationFrame
Status: RESOLVED FIXED
fixed-in-tracemonkey
: crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: ---
Assigned To: Luke Wagner [:luke]
:
:
Mentors:
Depends on:
Blocks: 326633 594645 602994
  Show dependency treegraph
 
Reported: 2011-04-28 18:33 PDT by Jesse Ruderman
Modified: 2011-06-13 10:01 PDT (History)
8 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
-


Attachments
testcase (crashes Firefox when loaded) (342 bytes, text/html)
2011-04-28 18:33 PDT, Jesse Ruderman
no flags Details
stack trace (7.60 KB, text/plain)
2011-04-28 18:34 PDT, Jesse Ruderman
no flags Details
fix (832 bytes, patch)
2011-04-29 10:13 PDT, Luke Wagner [:luke]
mrbkap: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-04-28 18:33:37 PDT
Created attachment 529010 [details]
testcase (crashes Firefox when loaded)

Related to bug 652401?
Comment 1 Jesse Ruderman 2011-04-28 18:34:13 PDT
Created attachment 529012 [details]
stack trace
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-04-28 22:17:31 PDT
> Related to bug 652401?

Sorta.

So we crash because cx->hasfp() is true, cx->fp()->isDummyFrame() is true, and cx->fp->prev() is null.  So js::CurrentScriptFileAndLineSlow called from js::CurrentScriptFileAndLine called from Function() ends up crashing with a null-deref.

We push dummy frames on AutoCompartment::enter.  In this case that happens with this stack:

#0  js::AutoCompartment::enter (this=0x1da0cfe0) at /Users/bzbarsky/mozilla/vanilla/mozilla/js/src/jswrapper.cpp:379
#1  0x04649d19 in JS_EnterCrossCompartmentCall (cx=0x5131620, target=0x217f0410) at /Users/bzbarsky/mozilla/vanilla/mozilla/js/src/jsapi.cpp:1210
#2  0x04649dc3 in JSAutoEnterCompartment::enter (this=0xbfffc324, cx=0x5131620, target=0x217f0410) at /Users/bzbarsky/mozilla/vanilla/mozilla/js/src/jsapi.cpp:1260
#3  0x00fa2d2c in GetContextFromObject (obj=0x217f0410) at /Users/bzbarsky/mozilla/vanilla/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:569
#4  0x00fa39c7 in nsXPCWrappedJSClass::CallMethod (this=0x1da3a090, wrapper=0x1da2b8e0, methodIndex=3, info=0x58b97c8, nativeParams=0xbfffc7e0) at /Users/bzbarsky/mozilla/vanilla/mozil a/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1294
#5  0x00f9ba59 in nsXPCWrappedJS::CallMethod (this=0x1da2b8e0, methodIndex=3, info=0x58b97c8, params=0xbfffc7e0) at /Users/bzbarsky/mozilla/vanilla/mozilla/js/src/xpconnect/src/xpcwrap edjs.cpp:586

GetContextFromObject only starts entering compartments and the like if the JS context stack is empty.  So that's where bug 652401 comes in.

However this seems like a general bug in Function.  It seems to assume that if hasfp() then there is a non-dummy frame on the stack... but that's just not true.  Luke added this code a few days ago in bug 602994.
Comment 3 Luke Wagner [:luke] 2011-04-29 10:13:08 PDT
Created attachment 529130 [details] [diff] [review]
fix

Argh, this is a failed specialization of js_GetScriptedCaller (missing "fp &&" conjunct).  I think I initially did this since sometimes eval is on the hot path and js_GetScriptedCaller is a call + LeaveTrace on every eval.  I don't think it really matters, though, so I'll just put js_GetScriptedCaller back.
Comment 4 Luke Wagner [:luke] 2011-05-03 03:41:15 PDT
http://hg.mozilla.org/tracemonkey/rev/e4861593d134
Comment 5 Chris Leary [:cdleary] (not checking bugmail) 2011-05-10 15:12:53 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/e4861593d134
Comment 6 Johnathan Nightingale [:johnath] 2011-05-24 14:39:33 PDT
Done, made the merge, not likely to back out, don't need to track for ff6

Note You need to log in before you can comment on or make changes to this bug.