Closed
Bug 533705
Opened 15 years ago
Closed 15 years ago
Possible Stack Corruption starting at ntdll!DbgBreakPoint +0x0000000000000000 called from mozjs!js_AddProperty+0x000000000000006b
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
Tracking | Status | |
---|---|---|
blocking2.0 | --- | alpha1+ |
status1.9.2 | --- | unaffected |
status1.9.1 | --- | unaffected |
People
(Reporter: cbook, Assigned: jorendorff)
References
()
Details
(Keywords: crash, testcase, Whiteboard: [crashkill-automation][fixed-in-tracemonkey][sg:critical])
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
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:
Reporter | ||
Comment 1•15 years ago
|
||
(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
Comment 2•15 years ago
|
||
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.
Comment 3•15 years ago
|
||
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.
Flags: blocking1.9.2?
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
Comment 7•15 years ago
|
||
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
Comment 8•15 years ago
|
||
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)
Assignee | ||
Comment 9•15 years ago
|
||
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+
Updated•15 years ago
|
Flags: blocking1.9.2? → blocking1.9.2+
Assignee | ||
Comment 10•15 years ago
|
||
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]
Comment 11•15 years ago
|
||
(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
Comment 12•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 13•15 years ago
|
||
(In reply to comment #12)
> http://hg.mozilla.org/mozilla-central/rev/74ad683e3ae2
What about comment 11?
/be
Comment 14•15 years ago
|
||
(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?
Comment 15•15 years ago
|
||
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
Comment 16•15 years ago
|
||
This doesn't bite on the branch, because the no-middle-deletes patch didn't land there.
Flags: wanted1.9.2?
Flags: blocking1.9.2-
Flags: blocking1.9.2+
Assignee | ||
Comment 17•15 years ago
|
||
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.
Comment 18•15 years ago
|
||
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
Comment 19•15 years ago
|
||
If I'm interpreting comment 16 right, the branches are unaffected so we can now unhide this bug?
status1.9.1:
--- → unaffected
status1.9.2:
--- → unaffected
Updated•15 years ago
|
blocking2.0: ? → alpha1
Updated•14 years ago
|
Group: core-security
Keywords: testcase
Whiteboard: [crashkill-automation][fixed-in-tracemonkey] → [crashkill-automation][fixed-in-tracemonkey][sg:critical]
Updated•13 years ago
|
Flags: wanted1.9.2?
Comment 21•12 years ago
|
||
Automatically extracted testcase for this bug was committed:
https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•