356402 - "Assertion failure: slot < fp->nvars" or [@ js_Interpret]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

js> (function() { new Script('for(var x in x) { }')(); })()

Debug:
  Assertion failure: slot < fp->nvars, at jsinterp.c:4629

Opt:
  Crash [@ js_Interpret] dereferencing an invalid address (e.g. 0x0320e238)

Comment 1

[Brendan Eich [:brendan]]

Attachment: I hate the Script object

Comment 2

[Brendan Eich [:brendan]]

Fixed on trunk:

Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.257; previous revision: 3.256
done

/be

Comment 3

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-356402.js

I couldn't reproduce the assert.

Comment 4

[Jesse Ruderman]

bc, can you reproduce in the regression test framework if you remove the "(function() {" and "})()" around the interesting part?  When I test in the shell, I can reproduce when there is exactly one anonymous function wrapped around it, but not when there are two, so maybe the fact that it's inside test() acts like having an extra function wrapped around it.

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-356402.js

Jesse: yes I can, thanks. I should have remembered to test it without the function wrapper as we have had other similar cases recently.

Brendan: Which is better for these tests? Do we need to have both situations where the test code is wrapped in a function and not wrapped? Would just having the non-wrapped version be sufficient? I wonder how many other "failures" are being hidden by the test function wrapping.

Comment 6

[Brendan Eich [:brendan]]

One (function(){ ... })() vs. zero is a difference that needs to be tested.  Two vs. one vs. zero may also matter.

/be

Comment 7

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20061014 windows/linux

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 242075 [details] [diff] [review]
I hate the Script object

a=mconnor on behalf of drivers for branch checkin for 1.8.0.9 and 1.8.1.1

Comment 9

[Brendan Eich [:brendan]]

Fixed on 1.8 branch:

Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.142.2.68; previous revision: 3.142.2.67
done

and on the 1.8.0 branch, with merging required:

Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.142.2.6.2.10; previous revision: 3.142.2.6.2.9
done

/be

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 20061122 1.8.0.9 windows/linux/mac*, 1.8.1.1 windows/linux/mac*, 1.9 windows/linux

Comment 11

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-356402.js,v  <--  regress-356402.js