476655 - TM: "Assertion failure: count <= (size_t) (fp->regs->sp - StackBase(fp) - depth), at ../jsobj.cpp" or "Assertion failure: depth <= (size_t) (fp->regs->sp - StackBase(fp)), at ../jsobj.cpp"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(function (){ for (var y in this) {} })();
[''.watch("", function(){}) for each (x in ['', '', eval, '', '']) if (x)].map(Function)

asserts debug TM but seems to work as expected in opt TM, as well as in non-TM shells.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I tried bisecting this but only got as far as:

TM changeset 22805 - does not show this assertion (seems to work as expected). (http://hg.mozilla.org/tracemonkey/rev/8353e26475a8) - just before the period of time when the JSFRAME_POP_BLOCKS assertion appeared.
TM changeset 23270 - shows this assertion. (http://hg.mozilla.org/tracemonkey/rev/e06f46f4f9d9)

Bisecting further takes me back to changeset 23106 (tried testing 23105 too) but was inconclusive with me hitting Assertion failure: !(fp->flags & JSFRAME_POP_BLOCKS), at ../jstracer.cpp:3672 continuously with the comment #0 testcase.

23270 was the changeset when the fix for the JSFRAME_POP_BLOCKS assertion landed. I'm suspecting that the testcase in comment #0 may not have been fixed when 23270 landed, though I'm not sure.

I think this might be a regression, just that I'm unable to fully narrow down the regression window.

cc'ing Graydon who fixed the JSFRAME_POP_BLOCKS assertion.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #1)
> cc'ing Graydon who fixed the JSFRAME_POP_BLOCKS assertion.

Oops, forgot to link to bug 470388 which is the bug for the mentioned JSFRAME_POP_BLOCKS assertion.

Comment 3

[Jesse Ruderman]

Still fails for me on tip of TM branch, despite Gal's "watch" fix today.

Assertion failure: count <= (size_t) (fp->regs->sp - StackBase(fp) - depth), at ../jsobj.cpp:2394

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

for(let y in ['', '']) try {for(let y in ['', '']) ( /x/g ); } finally { with({}){} } this.zzz.zzz


Assertion failure: depth <= (size_t) (fp->regs->sp - StackBase(fp)), at ../jsobj.cpp:2408

Almost identical symptoms as the testcase in comment #0, regression window included.

Comment 5

[Jim Blandy :jimb]

What am I doing wrong here?

js> (function (){ for (var y in this) {} })();
js> [''.watch("", function(){}) for each (x in ['', '', eval, '', '']) if
(x)].map(Function)
typein:3: SyntaxError: malformed formal parameter
js> for(let y in ['', '']) try {for(let y in ['', '']) ( /x/g ); } finally {
with({}){} } this.zzz.zzz
typein:5: TypeError: this.zzz is undefined
js>

Comment 6

[Brendan Eich [:brendan]]

(In reply to comment #5)
> What am I doing wrong here?
> 
> js> (function (){ for (var y in this) {} })();
> js> [''.watch("", function(){}) for each (x in ['', '', eval, '', '']) if
> (x)].map(Function)
> typein:3: SyntaxError: malformed formal parameter

Nothing, it's a crazy fuzz test that no longer botches an assertion, but correctly results in an error when map calls Function (the function object constructor) with arguments undefined, 0, and the array comprehension result, [undefined].

> js> for(let y in ['', '']) try {for(let y in ['', '']) ( /x/g ); } finally {
> with({}){} } this.zzz.zzz
> typein:5: TypeError: this.zzz is undefined

Another case that no longer botches. Wonder what fixed these?

/be

Comment 7

[Andreas Gal :gal]

I bet my abort on setter patch fixed this (still on the road, so no bug #)

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows that the patch for bug 480132 might have fixed this:

The first bad revision is:
changeset:   26112:acd36c659a72
user:        Jim Blandy
date:        Sat Mar 14 02:09:28 2009 -0400
summary:     Bug 480132. SpiderMonkey clones too many blocks into the heap. r=igor

I have manually verified that changeset 26111 asserts and changeset 26112 does not.

Due to oranges and backouts, changeset 26116 (the checkin of the same patch) does not assert, but changeset 26117 (the backout due to orange) does assert.

Once bug 480132 is resolved fixed, this bug can also be resolved fixed. Setting depends on.

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #8)

Let me rephrase..

I have manually verified that changeset 26111 ( http://hg.mozilla.org/tracemonkey/rev/95f3b4659cb2 ) asserts and changeset 26112 ( http://hg.mozilla.org/tracemonkey/rev/acd36c659a72 ) does not.

Due to oranges and backouts, changeset 26116 ( http://hg.mozilla.org/tracemonkey/rev/e2eb801f5b61 ) (the checkin of the same patch) does not assert, but changeset 26117 ( http://hg.mozilla.org/tracemonkey/rev/1234f1b7442f ) (the backout due to orange) does assert.

Comment 10

[Jim Blandy :jimb]

480132 has landed again, so I've marked this fixed-in-tracemonkey.

Comment 11

[Robert Sayre]

fixed by:
http://hg.mozilla.org/mozilla-central/rev/74416c7004c5

Comment 12

[Bob Clary [:bc] (inactive)]

http://hg.mozilla.org/tracemonkey/rev/cdbf312efd3b

/cvsroot/mozilla/js/tests/js1_8/regress/regress-476655.js,v  <--  regress-476655.js
initial revision: 1.1

/cvsroot/mozilla/js/tests/js1_6/Regress/regress-476655.js,v  <--  regress-476655.js
initial revision: 1.1

Comment 13

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/921352e9b2ce

Comment 14

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2