344959 - Functions may lose part or all of their scope chain after an exception

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Aiko]

(Build identifier: latest CVS head JS shell)

If an exception happens during the execution of a function then the function cannot access the variables in its scope chain any longer.

As an example, this statement works as expected and yields "with":
with ({x:"with"}) (function() { try {} catch(exc) {}; return x })()

But this (same as above, just a |throw 1| added) produces a reference error (or the value of x in the global scope, if it exists):
with ({x:"with"}) (function() { try { throw 1 } catch(exc) {}; return x })()

This also happens if you use the new |let| statement instead of |with|, but it's not a regression from JS 1.7, I also see this in Firefox 1.5.0.4.

Comment 1

[Aiko]

Attachment: testcase

Comment 2

[Brendan Eich [:brendan]]

Taking, patch next.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: fix

This is an old bug, which goes back to a fix that shaver and I did -- it would be good to get shaver's sr.  I didn't dress up the JS_SetPrivate calls with any kind of with-specific veneer, preferring to keep things concrete and avoid yet another layer.

/be

Comment 4

[Brendan Eich [:brendan]]

Fairly significant standards-conformance bug, more severe in light of "let".

/be

Comment 5

[Mike Shaver (:shaver emeritus)]

Comment on attachment 229551 [details] [diff] [review]
fix

sr=shaver.

Comment 6

[Brendan Eich [:brendan]]

Attachment: fix, v2

Comment 7

[Brendan Eich [:brendan]]

Nominating for 1.8.0.6/Firefox 1.5.0.6 since this regressed after Firefox 1.0, in February 2005 (rev 3.162 of jsinterp.c).

/be

Comment 8

[Brendan Eich [:brendan]]

Fixed on trunk.

/be

Comment 9

[Bob Clary [:bc] (inactive)]

Checking in regress-344959.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-344959.js,v  <--  regress-344959.js
initial revision: 1.1

Comment 10

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch.

/be

Comment 11

[Brendan Eich [:brendan]]

Attachment: 1.8.0 branch patch

Calling for code review just because 1.8.0 branch lacks let/block-scope.

/be

Comment 12

[Mike Shaver (:shaver emeritus)]

Comment on attachment 229723 [details] [diff] [review]
1.8.0 branch patch

sr=shaver.

Comment 13

[Brendan Eich [:brendan]]

*** Bug 345182 has been marked as a duplicate of this bug. ***

Comment 14

[Bob Clary [:bc] (inactive)]

verified fixed 1.8.1, trunk 20060723 win/linux/mac(ppc|tel)

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 229723 [details] [diff] [review]
1.8.0 branch patch

Not blocking, but we'll take the patch if you get it in by the deadline. approved for 1.8.0 branch, a=dveditz for drivers

Comment 16

[Brendan Eich [:brendan]]

Attachment: fix including patch for bug 347674

I'm checking this in -- it would be wrong not to fix bug 347674, since the patch for this bug contains the regression (by way of bug 343765).

/be

Comment 17

[Brendan Eich [:brendan]]

Attachment: fix including patch for bug 347674

Oops, copied wrong patch to staging directory.

/be

Comment 18

[Brendan Eich [:brendan]]

Fixed on the 1.8.0 branch.

/be

Comment 19

[Bob Clary [:bc] (inactive)]

2006081103 1.8.0.7 appears to assert in shell windows/mactel/linux and browser windows/mac(ppc|tel)/linux on js1_5/Regress/regress-344959.js,

 JSVAL_TO_OBJECT(v) == OBJ_GET_PARENT(cx, thisp), at jsinterp.c:506

Comment 20

[Bob Clary [:bc] (inactive)]

(In reply to comment #19)

See bug 337378

Comment 21

[Bob Clary [:bc] (inactive)]

verified fixed 1.8.0.7 20060818 windows/mac*/linux

Comment 22

[Daniel Veditz [:dveditz]]

I'm removing the whiteboard "not-fixed1.8.0.7" comment on the assumption it was due to the assertion covered by bug 337378, since Bob later added the verified1.8.0.7 keyword.