590006 - escaping closures on trace don't get block objects in their scope chain

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

If we create a closure on trace that needs a scope chain, that scope chain doesn't contain block objects.  Thus, in the following code, the reference to 'local' is undefined when called after f() returns.

function f() {
    var ret;
    for (var i = 0; i < 10; ++i) {
        let (local = 3) {
            ret = function() { print(local++); }
        }
    }
    return ret;
}
f()();  // test.js:5: ReferenceError: local is not defined

I think the fix is to abort trace when creating a function object where the scope chain would contain block objects.

Comment 1

[Luke Wagner [:luke]]

Also, it seems that, with this fixed, the test "if (cx->fp()->getBlockChain() == lexicalBlock) return ARECORD_STOP" in record_JSOP_LEAVEBLOCK could be removed.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch to fix this

This patch aborts tracing if we record a LAMBDA instruction when the block chain is non-null. I also removed the useless lexicalBlock check. The patch fixes the test case above and passes trace-tests.

I added a similar check to the LAMBDA_FC instruction. I can't figure out a way to make a flat closure that actually accesses its scope chain, but it still seems prudent to have the check there.

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 471354 [details] [diff] [review]
patch to fix this

Cool, thanks.  Need a checkin?

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to comment #3)
> Comment on attachment 471354 [details] [diff] [review]
> patch to fix this
> 
> Cool, thanks.  Need a checkin?

Yes, thanks.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 471354 [details] [diff] [review]
patch to fix this

RETURN_STOP_A("reason"); is nicer than bare return ARECORD_STOP; for future JIT spew readers debugging "why isn't my let-based code tracing" situations.

/be

Comment 6

[Luke Wagner [:luke]]

(In reply to comment #5)
Good catch

http://hg.mozilla.org/tracemonkey/rev/391c5c261186