359062 - Accessing a generator's local variables from nested functions is broken

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Aiko]

A function that is nested in a generator can't get the value of the generator's local variables. Vars in the global scope with the same name are not found either but trying to access these vars doesn't produce a Reference Error.

See the attached testcase which should print "Generator string" but prints "undefined undefined". I've tested this with the latest CVS shell and Firefox 2.

Comment 1

[Aiko]

Attachment: testcase

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

Sorry, should have seen this a mile away.

/be

Comment 3

[Igor Bukanov]

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

>+/*
>+ * Called from the JSOP_GENERATOR case in the interpreter, with fp referring
>+ * to the frame by which the generator function was activated.  Create a new
>+ * JSGenerator object, which contains its own JSStackFrame that we populate
>+ * from *fp.  We know that upon return, the JSOP_GENERATOR opcode will return
>+ * from the activation in fp, so we can steal away fp->callobj and fp->argsobj
>+ * if they are non-null.
>+ */

r=+ with an extra assert in JSOP_GENERATOR case of the interpreter that fp->callobj and fp->argsobj are NULL after succesful JS_NewGenerator.

Comment 4

[Brendan Eich [:brendan]]

Attachment: fix, v2

With assertion in JSOP_GENERATOR case.

/be

Comment 5

[Brendan Eich [:brendan]]

Fixed on trunk:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.303; previous revision: 3.302
done
Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v  <--  jsiter.c
new revision: 3.55; previous revision: 3.54
done

Thanks for finding this, Seno!

/be

Comment 6

[Aiko]

No problem. Thank you for fixing it so quickly.

Comment 7

[Bob Clary [:bc] (inactive)]

Checking in regress-359062.js;
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-359062.js,v  <--  regress-359062.js
initial revision: 1.1
done

Comment 8

[Jay Patel [:jay]]

Comment on attachment 244369 [details] [diff] [review]
fix, v2

Approved for 1.8.1 branch, a=jay for drivers.  Please land asap.  Thanks!

Comment 9

[Brendan Eich [:brendan]]

Fixed on the 1.8 branch:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.74; previous revision: 3.181.2.73
done
Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v  <--  jsiter.c
new revision: 3.17.2.26; previous revision: 3.17.2.25
done

/be

Comment 10

[Bob Clary [:bc] (inactive)]

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