202678 - JS nested function environment capture broken

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

Test case for the JS shell coming up.  The problem is that call_enumerate does
not refresh the values of call object properties that already were resolved. 
Instead it assumes that all properties have yet to be resolved.  Patch coming
right after the test.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: js shell test case

Variation showing two environment captures coming up next.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: variation on last test case

Comment 3

[Brendan Eich [:brendan]]

Attachment: proposed fix

If call_enumerate looks up a property not yet resolved, it'll be defined with
the current value from the appropriate stack frame vector (argv or vars).

But if, as in the test cases, call_enumerate is called when capturing an
environment (from js_PutCallObject on return from a heavyweight function),
*and* that heavyweight called an inner function that referenced the outer arg
or var, then the outer frame has a call object that's already resolved the call
property reflecting the stack slot.  We must copy from the stack slot in this
case, and we can't distinguish it from the first case within call_enumerate, so
we just copy each enumerated property's value from the appropriate slot.

The extra load, store, and address computations are tiny in the first case, I
think.	I'm not worried about any optimizations until we fix this bad old bug,
in any event!

/be

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 121095 [details] [diff] [review]
proposed fix

I'm _stunned_ that we didn't see this before. r=shaver.

Comment 5

[Brendan Eich [:brendan]]

Fixed -- thanks!

/be

Comment 6

[Phil Schwartau]

Verified FIXED.

Testcases added to JS testsuite:

      mozilla/js/tests/js1_5/Scope/regress-202678-001.js
      mozilla/js/tests/js1_5/Scope/regress-202678-002.js

Both pass in SpiderMonkey with the above patch applied -