JS_EvaluateUCInStackFrame should recognize when it can't behave correctly

NEW
Unassigned

Status

()

Core
JavaScript Engine
7 years ago
7 years ago

People

(Reporter: jimb, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

7 years ago
This doesn't work, and we shouldn't bother supporting it:

js> var f;
js> var x = 42;
js> function g(t) { f = function () { return x; }; evalInFrame(0, t); }
js> g('')
js> f()
42
js> g('var x = "local";')
js> f()
42
js> 

(Treat the call to evalInFrame as a stand-in for having the debugger stop after the assignment to f and evaluate the new declaration of x in g's frame using JS_EvaluateUCInStackFrame.)

JS_EvaluateUCInStackFrame promises that the latter will return "local". Rather than failing silently, JS_EvaluateUCInStackFrame should raise an error when it cannot do its job correctly.

Implementing the ideal behavior in all cases requires retaining enough information to undo all the def/use analysis the bytecode compiler does, possibly re-instating scope objects that don't exist any more on closures' scope chains, and so on. This is impractical.

Tests like js/src/jit-test/tests/jaeger/bug563000/eif-call-newvar.js and js/src/jit-test/tests/jaeger/bug563000/eif-getter-newvar.js actually expect this to always work. I don't believe they cover the problem space very well; they should be removed.

Where JS_EvaluateUCInStackFrame can do its job --- where a new binding in the given frame would go on the global, say, or where it is a call object that could be extended by calls to eval (or function statements) and across which we must implement the general case variable look up already --- then it should do so. But it shouldn't try to extend call objects that were not known to the bytecode compiler to be extensible.
Duplicate of this bug: 662822
(Assignee)

Updated

7 years ago
Component: JavaScript Debugging/Profiling APIs → JavaScript Engine
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.