Closed Bug 359062 Opened 14 years ago Closed 14 years ago

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

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: Seno.Aiko, Assigned: brendan)

References

Details

(Keywords: testcase, verified1.8.1.1)

Attachments

(2 files, 1 obsolete file)

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.
Attached file testcase
Attached patch fix (obsolete) — Splinter Review
Sorry, should have seen this a mile away.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #244334 - Flags: review?(igor.bukanov)
Attachment #244334 - Flags: approval1.8.1.1?
Flags: blocking1.8.1.1?
OS: Windows Server 2003 → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha
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.
Attached patch fix, v2Splinter Review
With assertion in JSOP_GENERATOR case.

/be
Attachment #244334 - Attachment is obsolete: true
Attachment #244369 - Flags: review?(igor.bukanov)
Attachment #244369 - Flags: approval1.8.1.1?
Attachment #244334 - Flags: review?(igor.bukanov)
Attachment #244334 - Flags: approval1.8.1.1?
Attachment #244369 - Flags: review?(igor.bukanov) → review+
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
Blocks: js1.7src
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
No problem. Thank you for fixing it so quickly.
Status: RESOLVED → VERIFIED
Checking in regress-359062.js;
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-359062.js,v  <--  regress-359062.js
initial revision: 1.1
done
Flags: in-testsuite+
Flags: blocking1.8.1.1? → blocking1.8.1.1-
Comment on attachment 244369 [details] [diff] [review]
fix, v2

Approved for 1.8.1 branch, a=jay for drivers.  Please land asap.  Thanks!
Attachment #244369 - Flags: approval1.8.1.1? → approval1.8.1.1+
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
Keywords: fixed1.8.1.1
verified fixed 20061122 1.8.1.1 windows/linux/mac*, 1.9 windows/linux
You need to log in before you can comment on or make changes to this bug.