642136 - Debugger access to closure environments

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[John J. Barton]

For memory analysis, program understanding, and hot-code reloading tools we need to be able to access all referents to an object. We can traverse the global object, all the event handlers bound by the windows, and all of the scopes on a breakpoint, but we can't access the closure environments.  

Just to give a concrete example to see if I understand what I am talking about:

window.aGlobal = {a: 'a'};
function outer() {
   var closeOverMe = aGlobal;
   window.addEventListener('unload', function inner() { 
         console.log("Closed over ", closeOverMe);
   }, false);
   window.removeEventListener('load', outer, false);
}
window.addEventListener('load', outer, false);

We can find one ref to aGlobal and outer (with two refs before load event) but we can't express the second ref to aGlobal.

Comment 1

[Jim Blandy :jimb]

In the new Debug object, the functionScope and parent functions on Debug.Object.prototype should let you walk the scope chain a function has closed over. A Debug.Frame instance's environment property will do the same if you're starting with a frame.

Comment 2

[Jim Blandy :jimb]

Dave, does your heap profiler walk parent links in scope chain elements and stack frames? I guess it must, because it just uses the GC's tracer.

Comment 3

[John J. Barton]

(In reply to comment #1)
> In the new Debug object, the functionScope and parent functions on
> Debug.Object.prototype should let you walk the scope chain a function has
> closed over. A Debug.Frame instance's environment property will do the same if
> you're starting with a frame.

https://wiki.mozilla.org/Debug_Object#Properties_of_the_Debug.Object_prototype

Ok, so I would start with the list of event handlers, find |inner|, then apply
getFunctionScope().getOwnPropertyNames(). In that array I would find |closeOverMe| referencing aGlobal. 

No that's close but not quite right: I need some way to walk the scope chain, but it's not clear from the doc page.

Comment 4

[Jim Blandy :jimb]

(In reply to comment #3)
> Ok, so I would start with the list of event handlers, find |inner|, then apply
> getFunctionScope().getOwnPropertyNames(). In that array I would find
> |closeOverMe| referencing aGlobal. 

Right, almost.

1) You find |inner| --- that is, you get a Debug.Object instance whose referent is a closure produced by evaluating the function expression named |inner| that has been set as the window's 'unload' handler. Call this I.

2) I.getFunctionScope() returns a Debug.Object referring to a Call object for the call to 'outer'. Call this C.

3) C.getOwnPropertyNames() returns ["closeOverMe"], and C.getOwnPropertyDescriptor("closeOverMe").value is a Debug.Object referring to the {a:'a'} object, because this is the value of the |closeOverMe| variable. It doesn't refer to aGlobal; variables don't refer to variables.

4) C.parent() returns a Debug.Object referring to the global object; call this G.

5) G.getOwnPropertyNames() returns an array including the string "aGlobal"; G.getOwnPropertyDescriptor("aGlobal").value is a Debug.Object referring to the {a:'a'} object --- in fact, exactly the same Debug.Object instance we got in step 3, since Debug.Object instances are one-to-one with the debuggee objects they represent.

Comment 5

[Jim Blandy :jimb]

That's the sort of scope chain walking you're looking for, right?

Comment 6

[Jim Blandy :jimb]

In the first implementation of the Debug object, the flat closure and null closure optimizations will mean that parent chains may not contain all the information that a developer would expect to find, based on the source code.

When we get to that point, we'll file a bug and work with the JS engine hackers to recover as much as possible, and present it in an authentic-looking way. But I don't think we'll be able to completely reverse the optimizations; their whole goal is to throw away information the program won't use.

Comment 7

[Dave Herman [:dherman]]

If you're throwing away variables because they don't appear in the source, I'd argue it's useful for the programmer to see the optimized closure anyway. It'll help people understand the performance characteristics, and only a spec zealot would care that it's *actually* a scope chain exactly the way it's described in the spec. (The spec didn't really have to be written that way anyway.)

Dave

Comment 8

[John J. Barton]

(In reply to comment #5)
> That's the sort of scope chain walking you're looking for, right?

Yes. Only a small API question: is |parent()| clearly "enclosing scope"? A Debug.Object is very generic and thus I imagine it could be part of more than one tree. The function name gives us no hints that it is related to scope.

Comment 9

[John J. Barton]

(In reply to comment #6)
> In the first implementation of the Debug object, the flat closure and null
> closure optimizations will mean that parent chains may not contain all the
> information that a developer would expect to find, based on the source code.
> 
> When we get to that point, we'll file a bug and work with the JS engine hackers
> to recover as much as possible, and present it in an authentic-looking way. But
> I don't think we'll be able to completely reverse the optimizations; their
> whole goal is to throw away information the program won't use.

The ideal case for the developer would be to know what happened.  So rather than recovering we'd like to know "optimized away" or whatever. If the values are just missing, then devs will 'know' why: the debugger is buggy :-(

Comment 10

[Jim Blandy :jimb]

(In reply to comment #8)
> Only a small API question: is |parent()| clearly "enclosing scope"? A
> Debug.Object is very generic and thus I imagine it could be part of more than
> one tree. The function name gives us no hints that it is related to scope.

True --- 'parent' is a very generic-seeming name, and there are certainly non-scope Debug.Object instances that would have other sorts of 'parents'. However, it is named after the JS_GetParent JSAPI function, which is named after the actual JSObject field (formerly a special slot).

For now, I've renamed it to 'outerEnvironment', and changed the description of functionScope, to better align their terminology with the ES5 spec. (ES5 does say that function objects have a [[Scope]] property, so I think functionScope is a good name.)

In the long term, I think SpiderMonkey will move away from using ordinary JSObjects to represent the scope chain, since most scope chain elements --- blocks, calls, declarative environments, with blocks, strict eval environments --- have ended up being special object classes that mustn't be allowed to escape to ordinary JS code, and the new harmony modules will change the global end of the chain, too.  So I think our long term goal will be to switch towards having Debug.Environment instances.

Comment 11

[Jim Blandy :jimb]

(In reply to comment #7)
> If you're throwing away variables because they don't appear in the source, I'd
> argue it's useful for the programmer to see the optimized closure anyway. It'll
> help people understand the performance characteristics, and only a spec zealot
> would care that it's *actually* a scope chain exactly the way it's described in
> the spec. (The spec didn't really have to be written that way anyway.)

That's true, but to some extent we need to protect less-sophisticated debugger code from our implementation details. We don't want to break API every time someone comes up with some new hack.

(In reply to comment #9)
> The ideal case for the developer would be to know what happened.  So rather
> than recovering we'd like to know "optimized away" or whatever. If the values
> are just missing, then devs will 'know' why: the debugger is buggy :-(

I think the ideal would be something that makes a best effort to reconstruct the scope chain, so that naive API clients can stumble along, but provides annotations where things are not as expected. For example, getOwnPropertyDescriptor could return "interesting" property descriptors for removed variables:

{ omitted:true }

for variables whose values aren't used, or

{ value:V, writable:false, flatClosureCopy:true }

for variables that are not assigned to, and have thus been copied into some closures.

Comment 12

[Jim Blandy :jimb]

(In reply to comment #9)
> The ideal case for the developer would be to know what happened.  So rather
> than recovering we'd like to know "optimized away" or whatever. If the values
> are just missing, then devs will 'know' why: the debugger is buggy :-(

Yeah, GDB is always blamed when GCC writes bad DWARF. :(

Comment 13

[Jim Blandy :jimb]

(In reply to comment #11)
> That's true, but to some extent we need to protect less-sophisticated debugger
> code from our implementation details. We don't want to break API every time
> someone comes up with some new hack.

More fundamentally, it's the debugger's job to present a model of the debuggee as it executes. That model should have some kind of coherence beyond "here's how SM implements JS today". We can show optimizations, but ideally they shouldn't completely distort the model, because that's hard to work with.

Comment 14

[Jim Blandy :jimb]

We've implemented Debugger.Environment, so I think this bug is closed.