Closed Bug 533705 Opened 11 years ago Closed 11 years ago
Possible Stack Corruption starting at ntdll!Dbg
Break Point +0x0000000000000000 called from mozjs!js _Add Property+0x000000000000006b
Reproduced with a recent Firefox 3.7 Trunk Debug build STR: Load : -> http://www.mapquest.com/maps?1c=Kannapolis&1s=NC&1a=153+Fairmont+Cir&1z=28083-7819&1y=US&1l=35.486246&1g=-80.602619&1v=ADDRESS&2c=Concord&2s=NC&2a=77+Union+St+S&2z=28025-5013&2y=US&2l=35.40922&2g=-80.579819&2v=ADDRESS -> Crash on load after a few seconds Exploitability Classification: UNKNOWN Recommended Bug Title: Possible Stack Corruption starting at ntdll!DbgBreakPoint +0x0000000000000000 called from mozjs!js_AddProperty+0x000000000000006b (Hash=0x 23154549.0x2e043a68) The stack trace contains one or more locations for which no symbol or module cou ld be found. This may be a sign of stack corruption. ChildEBP RetAddr WARNING: Stack unwind information not available. Following frames may be wrong. 0012e3e8 00668d0b ntdll!DbgBreakPoint 0012e414 0639ecaa mozjs!js_AddProperty+0x6b 0012e484 005ffb6c 0x639ecaa 0012e494 005ff57c mozjs!ExecuteTrace+0x3c 0012e540 00602483 mozjs!ExecuteTree+0x20c 0012e598 00530e83 mozjs!js_MonitorLoopEdge+0x2f3 0012ebcc 0052dce4 mozjs!js_Interpret+0x1b53 0012ec5c 004d5099 mozjs!js_Execute+0x424 0012ec84 0284afe8 mozjs!JS_EvaluateUCScriptForPrincipals+0xe9 0012ed40 026a92c7 gklayout!nsJSContext::EvaluateString+0x328 0012ee38 026a8c9f gklayout!nsScriptLoader::EvaluateScript+0x377 0012eefc 026a806a gklayout!nsScriptLoader::ProcessRequest+0x10f 0012f3fc 02bb82b9 gklayout!nsScriptLoader::ProcessScriptElement+0xc3a 0012f430 02bd0374 gklayout!nsScriptElement::MaybeProcessScript+0x149 0012f4e8 02bd008f gklayout!nsHTMLScriptElement::MaybeProcessScript+0x24 0012f4f4 02767cff gklayout!nsHTMLScriptElement::DoneAddingChildren+0x1f 0012f518 02762bbd gklayout!HTMLContentSink::ProcessSCRIPTEndTag+0xcf 0012f54c 02765ff0 gklayout!SinkContext::CloseContainer+0x31d 0012f564 01e5fb7a gklayout!HTMLContentSink::CloseContainer+0xa0 0012f594 01e5cc00 htmlpars!CNavDTD::CloseContainer+0x18a quit:
(5b0.7bc): Break instruction exception - code 80000003 (!!! second chance !!!) eax=0000006e ebx=0ad3e700 ecx=461e4ead edx=10313d38 esi=0ad3f1c0 edi=051783e0 eip=7c90120e esp=0012e3e4 ebp=0012e3e8 iopl=0 nv up ei pl nz na pe nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00000206
Here is a reduced test case. Brendan spotted the cause of the bug over my shoulder: we have two functions that have the same shape, but different numbers of reserved slots. Thus, the freeslot in the object is different. I tried to write a patch to fix this by adding the reserved slot count to JSEmptyScope and sharing scopes only if the reserved slot count is the same. It didn't work because I was calling obj->getClass()->reserveSlots in createEmptyScope for that check, but some reserveSlots functions (e.g., args_reserveSlots) require a fully initialized object, which we don't have yet at that point.
Assigning to jorendorff because he's the expert on this stuff.
Assignee: general → jorendorff
I just hit this in regular browsing. In bug 533945 Tomcat hit it on Facebook.
Actually I hit the assertion "slot == scope->freeslot" failure, which Brendan duped to this.
FWIW this is my stack: #3 0x003b6790 in JS_Assert (s=0x4530f0 "slot == scope->freeslot", file=0x46636c "/Users/roc/mozilla-checkin/js/src/jsbuiltins.cpp", ln=238) at /Users/roc/mozilla-checkin/js/src/jsutil.cpp:70 #4 0x0044542e in js_AddProperty (cx=0x22536e00, obj=0xe53d2a0, sprop=0x222ab410) at /Users/roc/mozilla-checkin/js/src/jsbuiltins.cpp:238 #5 0x0bb68953 in ?? () #6 0x003d6f25 in ExecuteTrace (cx=0x22536e00, f=0x32377504, state=@0xbfffc7f0) at /Users/roc/mozilla-checkin/js/src/jstracer.cpp:6469 #7 0x003e74a3 in ExecuteTree (cx=0x22536e00, f=0x32377504, inlineCallCount=@0xbfffcc20, innermostNestedGuardp=0xbfffc8c8) at /Users/roc/mozilla-checkin/js/src/jstracer.cpp:6567 #8 0x0040561a in js_MonitorLoopEdge (cx=0x22536e00, inlineCallCount=@0xbfffcc20, reason=Record_Branch) at /Users/roc/mozilla-checkin/js/src/jstracer.cpp:7057 #9 0x0030e655 in js_Interpret (cx=0x22536e00) at jsops.cpp:360 #10 0x00335b1c in js_Execute (cx=0x22536e00, chain=0xe568580, script=0x266a5400, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1619 #11 0x002aa1ed in JS_EvaluateUCScriptForPrincipals (cx=0x22536e00, obj=0xe568580, principals=0x3358c7b4, chars=0xa9ff008, length=52323, filename=0x2dce10c8 "http://static.thebigchair.com.au/mycareerjobbox/script/fd.mt.scroller.js", lineno=1, rval=0x0) at /Users/roc/mozilla-checkin/js/src/jsapi.cpp:5057 #12 0x02ceeb07 in nsJSContext::EvaluateString (this=0xff0bf40, aScript=@0x272a2ec4, aScopeObject=0xe568580, aPrincipal=0x3358c7b0, aURL=0x2dce10c8 "http://static.thebigchair.com.au/mycareerjobbox/script/fd.mt.scroller.js", aLineNo=1, aVersion=0, aRetValue=0x0, aIsUndefined=0xbfffd1e4) at /Users/roc/mozilla-checkin/dom/base/nsJSEnvironment.cpp:1713
This seems like a bad bug. Should it block 1.9.2 or did other factors keep it from looming large there? Patch coming up, my hg blame is all over this -- Jason, I hope it's ok to put up a patch. I'm on vacation and will be afk a lot, so if you could review and test and land, I would be grateful. /be
OS: Windows XP → All
Priority: -- → P1
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.3a1
The slotchanges property cache event was completely bogus, it papered over this bug. I got rid of it. There's a minimal fix to OOM_EXIT from js_AddProperty on a slot change but I think the interpreter needs fixing too, instead of trying to slam in a new entry vword (sprop2). Separate fix: must lock proto-scope across canProvideEmptyScope/getEmptyScope. /be
Attachment #417876 - Flags: review?(jorendorff)
Comment on attachment 417876 [details] [diff] [review] proposed fix r=me with some comments, which I'll provide. Thanks for the patch.
Attachment #417876 - Flags: review?(jorendorff) → review+
Actually I checked in less delta to jsops.cpp than the patch has. http://hg.mozilla.org/tracemonkey/rev/74ad683e3ae2
Whiteboard: [crashkill-automation] → [crashkill-automation][fixed-in-tracemonkey]
(In reply to comment #10) > Actually I checked in less delta to jsops.cpp than the patch has. > > http://hg.mozilla.org/tracemonkey/rev/74ad683e3ae2 This leaves entry->vword and sprop differing, which does not break anything (yet) AFAICS in TR::record_SetPropHit, TR::setProp, etc. -- but it is a bit scary, or at least worrying. Comment-worthy, at least? /be
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to comment #12) > http://hg.mozilla.org/mozilla-central/rev/74ad683e3ae2 What about comment 11? /be
(In reply to comment #13) > (In reply to comment #12) > > http://hg.mozilla.org/mozilla-central/rev/74ad683e3ae2 > > What about comment 11? > > /be I think jorendorff took off for vacation right before you left comment 11. Is it ok to just file a follow up on him to write a comment?
It's not a comment but agreement between the data structures maintained by the interpreter and used by the recorder and on trace. IIRC Jason agreed on IRC, the effect would be the original patch landing. But I'll let Jason check me on that, followup bug is fine. /be
This doesn't bite on the branch, because the no-middle-deletes patch didn't land there.
Was there a followup bug for this? I agree landing the original patch is the right thing but I don't know if it ever happened.
The cset that hit tm is now in m-c: http://hg.mozilla.org/mozilla-central/rev/74ad683e3ae2 Need a followup per comment 15. Jason, will you file and patch? Thanks, /be
If I'm interpreting comment 16 right, the branches are unaffected so we can now unhide this bug?
Whiteboard: [crashkill-automation][fixed-in-tracemonkey] → [crashkill-automation][fixed-in-tracemonkey][sg:critical]
Automatically extracted testcase for this bug was committed: https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
You need to log in before you can comment on or make changes to this bug.