Closed
Bug 470388
Opened 16 years ago
Closed 16 years ago
TM: "Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp"
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9.1b3
People
(Reporter: gkw, Assigned: graydon)
Details
(5 keywords, Whiteboard: fixed-in-tracemonkey)
Attachments
(2 files)
(deleted),
patch
|
brendan
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
for each (let x in [function(){}, new Boolean(false), function(){}]) {}
asserts TM at Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp:3688
This seems to be a fairly recent regression from a few hours ago and jsfunfuzz _always_ hits it when dealing with dbg js shells due to its simplicity. Seems harmless in opt though.
Approximate regression range on TM:
works (no assert) - http://hg.mozilla.org/tracemonkey/rev/db553f94394c
asserts - http://hg.mozilla.org/tracemonkey/rev/b8d8494faf18
Flags: blocking1.9.1?
Comment 1•16 years ago
|
||
Bogus assertion due to patch for bug 470300. Will try to fix without neutering.
/be
Assignee: general → brendan
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
Target Milestone: --- → mozilla1.9.1b3
Updated•16 years ago
|
Status: NEW → ASSIGNED
Comment 2•16 years ago
|
||
Harmless in opt shell. The test forces js_GetScopeChain to reify block objects for the closure in the 0th element of the array being for-each'ed. But the trace ends before the frame pops, so the JSFRAME_POP_BLOCKS flag works as it does in the interpreter. Nesting let in the loop body block shouldn't mess things up because the closures are not in scope of the body. If a closure were in the body we'd abort.
/be
Comment 3•16 years ago
|
||
I think this bug can also trigger:
Assertion failure: blockDepth <= StackDepth(script), at ../jsinterp.cpp:6732
Comment 4•16 years ago
|
||
note, i was running into this during the topsite testrun:
Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at /work/mozilla/builds/1.9.1/mozilla/js/src/jstracer.cpp:3672
http://www.evite.com: EXIT STATUS: CRASHED signal 5 SIGTRAP (84.565852 seconds)
Comment 5•16 years ago
|
||
I "crash" three to four times a day due to this.
Biggest site I've seen it on is IMDb, e.g. http://www.imdb.com/find?s=all&q=dark+knight
Updated•16 years ago
|
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Reporter | ||
Comment 6•16 years ago
|
||
I've discovered variants of this bug that have the above assert in debug and crash at the following locations in opt: (TM-only)
JS_GetGlobalForObject at 0x000000000000000c
for each (let x in [function(){}, {}, {}, function(){}, function(){}, function(){}]) { ([<y/> for (y in x)]); }
js_PutBlockObject at null
for each (let x in [function(){}, {}, {}, function(){}, function(){}]) { ([<y/> for (y in x)]); }
See bug 471660 for a possibly-related crash at js_UnwindScope that has a similar assert.
(In reply to comment #2)
> Harmless in opt shell.
So far these additional discoveries are crashes at locations that are unlikely to be exploitable.
Keywords: crash
Summary: TM: "Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp" → TM: "Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp" [@ JS_GetGlobalForObjec]
Summary: TM: "Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp" [@ JS_GetGlobalForObjec] → TM: "Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp" [@ JS_GetGlobalForObject]
Assignee | ||
Comment 7•16 years ago
|
||
Reproducible crash on NY times article:
http://www.nytimes.com/2009/01/06/technology/companies/06apple.html
I can look into this. Feels like a blocker to me? Easy to trigger, crashes the browser. Why marked -?
Assignee: brendan → graydon
Comment 8•16 years ago
|
||
Yeah, I second graydon's view. Lets work on this.
Flags: blocking1.9.1- → blocking1.9.1?
Comment 9•16 years ago
|
||
It was minused per comment 2: harmless in an opt build. Is it just an assert botch, or is this a crasher in normal builds too?
Assignee | ||
Comment 10•16 years ago
|
||
Sorry if this is "working on a non-blocker"; it was however preventing general use of a debug build, which I'm finding essential for all other bugs that are diagnosable and fixable through use of JS_ASSERT.
Turns out it's a mostly-bogus failure: the assert is only firing when the POP_BLOCKS flag is set on the incoming-and-outgoing frame (calldepth 0), not a reconstructed frame. Sharpening up the assert to exclude this particular combination fixes matters.
Attachment #355679 -
Flags: review?(brendan)
Comment 11•16 years ago
|
||
Comment on attachment 355679 [details] [diff] [review]
Fix the bug
>diff -r adbe8e4b21dc -r e24333561e6b js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp Mon Jan 05 22:09:23 2009 +0100
>+++ b/js/src/jstracer.cpp Tue Jan 06 16:43:34 2009 -0800
>@@ -3527,6 +3527,7 @@
> JS_ASSERT(tm->recoveryDoublePoolPtr >= tm->recoveryDoublePool + MAX_NATIVE_STACK_SLOTS);
>
> #ifdef DEBUG
>+ bool jsframe_pop_blocks_set_on_entry = bool(cx->fp->flags & JSFRAME_POP_BLOCKS);
> memset(stack_buffer, 0xCD, sizeof(stack_buffer));
> memset(global, 0xCD, (globalFrameSize+1)*sizeof(double));
> #endif
>@@ -3678,7 +3679,8 @@
> the side exit struct. */
> JSStackFrame* fp = cx->fp;
>
>- JS_ASSERT(!(fp->flags & JSFRAME_POP_BLOCKS));
>+ JS_ASSERT((!(fp->flags & JSFRAME_POP_BLOCKS)) ||
Over-parenthesized on the outside of the !(...), no need. Even better, use JS_ASSERT_IF:
JS_ASSERT_IF(fp->flags & JSFRAME_POP_BLOCKS,
calldepth == 0 && jsframe_pop_blocks_set_on_entry)
r=me with that.
/be
Attachment #355679 -
Flags: review?(brendan) → review+
Updated•16 years ago
|
Attachment #355679 -
Flags: approval1.9.1?
Assignee | ||
Comment 12•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 13•16 years ago
|
||
Requesting in‑testsuite? for testcases in comment #0 and comment #6.
Flags: in-testsuite?
Comment 14•16 years ago
|
||
Looks like debug only so removing request for blocking.
Flags: blocking1.9.1?
Reporter | ||
Comment 15•16 years ago
|
||
The testcase in comment #0 got fixed but testcases in comment #6 morphed into something else, spun them off as bug 472450.
Reporter | ||
Updated•16 years ago
|
Summary: TM: "Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp" [@ JS_GetGlobalForObject] → TM: "Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp"
Comment 17•16 years ago
|
||
Comment on attachment 355679 [details] [diff] [review]
Fix the bug
a191=beltzner
Attachment #355679 -
Flags: approval1.9.1? → approval1.9.1+
Updated•16 years ago
|
Whiteboard: fixed-in-tracemonkey
Comment 18•16 years ago
|
||
Keywords: fixed1.9.1
Comment 19•16 years ago
|
||
browser only crash @ js_PutBlockObject js/src/jsobj.cpp:2225 in yesterdays 1.9.2 build with -02 at least
bp-01ef7bf6-14ab-45b2-b647-5dc3a2090123
bp-31506d7e-03b6-4723-8853-36f252090123
I'll be checking these tests in later today.
Updated•16 years ago
|
Attachment #358392 -
Attachment is patch: true
Comment 20•16 years ago
|
||
Checking in js1_7/regress/regress-470388-01.js
Checking in js1_7/regress/regress-470388-02.js
Checking in js1_7/regress/regress-470388-03.js
http://hg.mozilla.org/mozilla-central/rev/3ce50075697b
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-litmus-
Comment 21•16 years ago
|
||
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•