509639 - TM: Crash [@ TraceRecorder::scopeChainProp]

Status: RESOLVED DUPLICATE of bug 505591

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

(function(){ var c; eval("for(c in [1,2,3,4]) {}") })();

Crash [@ TraceRecorder::scopeChainProp]

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to bug 503694 :

The first bad revision is:
changeset:   31059:f5fd44e7cfdb
user:        Graydon Hoare
date:        Thu Aug 06 12:40:51 2009 -0700
summary:     Bug 503694 - TM: Code run off an event handler is never traced, r=brendan.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This occurs on Win32 as well. It occurs sufficiently frequently enough to interfere with jsfunfuzz. :(

Comment 3

[Jason Orendorff [:jorendorff]]

OK, a few funny things worth explaining here:

1. At the point where we crash, there is no JSStackFrame for the JSNative obj_eval on the JS stack. This is because eval *removes* its own JSStackFrame by passing the caller's frame to js_Execute.

2. At the point where we crash, cx->fp->fun and cx->fp->argv are not NULL as I expected, because js_Execute copies most of the caller's stack frame into the frame in which eval code executes.

I don't know why we do it that way.

Anyway, on to the crash.

            // Compute number of scope chain links to result.
            jsint scopeIndex = 0;
            JSObject* tmp = JSVAL_TO_OBJECT(cx->fp->argv[-2]);
            while (tmp != obj) {
                tmp = OBJ_GET_PARENT(cx, tmp);
                scopeIndex++;
            }

This assumes that the callee's scope chain contains the target variable. Not true in this case, since "the callee" as calculated by cx->fp->argv[-2] turns out to be the anonymous function in the test.

So obviously if we're in eval code we don't want to start with cx->fp->argv[-2]. I'm not sure what object we do want to start with though. The hoops obj_eval jumps through to select a scope object are nontrivial.

N.B.: Not taking this bug.

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #3)
> OK, a few funny things worth explaining here:
> 
> 1. At the point where we crash, there is no JSStackFrame for the JSNative
> obj_eval on the JS stack. This is because eval *removes* its own JSStackFrame
> by passing the caller's frame to js_Execute.

True. Old code, not sure if it would be better to let the eval call's native frame intervene, but it would need work somewhere, I'm sure. eval runs in its dynamic scope, so intuitively it's not too strange (or ha-ha) funny that we do this.

The way js_Execute works, you pass it the caller (down) frame and it copies members from that frame into the new frame. Mostly this saves a bunch of parameters, and it tends to be the right thing. Maybe the JIT just needs to cope?

> 2. At the point where we crash, cx->fp->fun and cx->fp->argv are not NULL as I
> expected, because js_Execute copies most of the caller's stack frame into the
> frame in which eval code executes.

For eval and similar debugger eval-in-frame stuff that really wants dynamic scope. Such calls need their own frame but most of its state should come from the calling frame.

> Anyway, on to the crash.
> 
>             // Compute number of scope chain links to result.
>             jsint scopeIndex = 0;
>             JSObject* tmp = JSVAL_TO_OBJECT(cx->fp->argv[-2]);
>             while (tmp != obj) {
>                 tmp = OBJ_GET_PARENT(cx, tmp);
>                 scopeIndex++;
>             }
> 
> This assumes that the callee's scope chain contains the target variable. Not
> true in this case, since "the callee" as calculated by cx->fp->argv[-2] turns
> out to be the anonymous function in the test.

Eval code != Function code, to use ECMA-262 chapter 10 terms.

> So obviously if we're in eval code we don't want to start with
> cx->fp->argv[-2]. I'm not sure what object we do want to start with though.

cx->fp->scopeChain.

> The hoops obj_eval jumps through to select a scope object are nontrivial.

Indeed.

Need an owner -- dmandelin, you game?

/be

Comment 5

[Graydon Hoare :graydon]

I assumed this would be mine given that I "broke" it by re-enabling tracing of this code. Is it not just a matter of finding a new way of inhibiting the JIT?

Comment 6

[David Mandelin [:dmandelin]]

My patch for bug 505591 fixes this.

Comment 8

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug509639.js.