Support debugger inspection of locals in optimized frames

Support debugger inspection of locals in optimized frames

Optimized frames are now handled by the debugger. When discovering optimized frames during stack inspection in the debugger they are "deoptimized" using the normal deoptimization code and the deoptimizer output information is used to provide frame information to the debugger.

Before this change the debugger reported each optimized frame as one frame no matter the number of inlined functuions that might have been called inside of it. Also all locals where reported as undefined. Locals can still be reposted as undefined when their value is not "known" by the optimized frame.

As the structures used to calculate the output frames when deoptimizing are not GC safe the information for the debugger is copied to another structure (DeoptimizedFrameInfo) which is registered with the global deoptimizer data and processed during GC.

R=fschneider@chromium.org

BUG=v8:1140
TEST=test/mjsunit/debug-evaluate-locals-optimized*

Committed: http://code.google.com/p/v8/source/detail?r=8464

[Søren Thygesen Gjesse, 2011-06-29 08:53:30]

Uploaded additional fix for setting the scope info for lazily compiled inlined
functions. Activated the test which failed due to this.

[fschneider, 2011-06-29 10:47:35]

LGTM with comments.

http://codereview.chromium.org/7230045/diff/8002/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

http://codereview.chromium.org/7230045/diff/8002/src/arm/deoptimizer-arm.cc#n...
src/arm/deoptimizer-arm.cc:549: input_->SetDoubleRegister(i, -1.0 * i);
Why not just set it to 0.0 or some other constant default instead?

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc
File src/deoptimizer.cc (right):

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode169
src/deoptimizer.cc:169: 
Maybe put the GC-unsafe part inside an AssertNoAllocation-scope?

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode173
src/deoptimizer.cc:173: 
Don't you have to de-allocate the deoptimizer? Or can it be stack-allocated
instead of new-allocated?

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode551
src/deoptimizer.cc:551: // Check of the heap number to materialize actually
belongd to the frame
Check that the heap number to materialize actually belongs...

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode564
src/deoptimizer.cc:564: PrintF("Materializing a new heap number %p [%e] in index
%d\n",
Remove duplicate PrintF, maybe add the expression index to the PrintF above?

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.h
File src/deoptimizer.h (right):

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.h#newcode195
src/deoptimizer.h:195: void MaterializeHeapNumbersForDebuggerInspectableFrame(
Maybe add 

#ifdef ENABLE_DEBUGGER_SUPPORT

here too.

http://codereview.chromium.org/7230045/diff/8002/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/7230045/diff/8002/src/ia32/deoptimizer-ia32.cc...
src/ia32/deoptimizer-ia32.cc:615: input_->SetRegister(i, i * 4);
Maybe use some constant as default value?

http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-...
File test/mjsunit/debug-evaluate-locals-optimized-double.js (right):

http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-...
test/mjsunit/debug-evaluate-locals-optimized-double.js:48: assertEquals(i * 2 +
1 + (i * 2 + 1) / 100, frame.localValue(0).value());
Long line.

http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-...
test/mjsunit/debug-evaluate-locals-optimized-double.js:49: assertEquals(i * 2 +
2 + (i * 2 + 2) / 100, frame.localValue(1).value());
Long line.

[Søren Thygesen Gjesse, 2011-06-29 12:42:10]

http://codereview.chromium.org/7230045/diff/8002/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

http://codereview.chromium.org/7230045/diff/8002/src/arm/deoptimizer-arm.cc#n...
src/arm/deoptimizer-arm.cc:549: input_->SetDoubleRegister(i, -1.0 * i);
On 2011/06/29 10:47:35, fschneider wrote:
> Why not just set it to 0.0 or some other constant default instead?

It was for debugging to see if these values turned up somewhere. Changed to 0.0.

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc
File src/deoptimizer.cc (right):

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode169
src/deoptimizer.cc:169: 
On 2011/06/29 10:47:35, fschneider wrote:
> Maybe put the GC-unsafe part inside an AssertNoAllocation-scope?

The already happens due to the Deoptimizer constructor turns off allocation and
Deoptimizer::DeleteFrameDescriptions() turns it back on (deoptimizer.cc:358).

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode173
src/deoptimizer.cc:173: 
On 2011/06/29 10:47:35, fschneider wrote:
> Don't you have to de-allocate the deoptimizer? Or can it be stack-allocated
> instead of new-allocated?

Good catch! Of cause.

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode551
src/deoptimizer.cc:551: // Check of the heap number to materialize actually
belongd to the frame
On 2011/06/29 10:47:35, fschneider wrote:
> Check that the heap number to materialize actually belongs...

Done.

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode564
src/deoptimizer.cc:564: PrintF("Materializing a new heap number %p [%e] in index
%d\n",
On 2011/06/29 10:47:35, fschneider wrote:
> Remove duplicate PrintF, maybe add the expression index to the PrintF above?

Done.

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.h
File src/deoptimizer.h (right):

http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.h#newcode195
src/deoptimizer.h:195: void MaterializeHeapNumbersForDebuggerInspectableFrame(
On 2011/06/29 10:47:35, fschneider wrote:
> Maybe add 
> 
> #ifdef ENABLE_DEBUGGER_SUPPORT
> 
> here too.

Done.

http://codereview.chromium.org/7230045/diff/8002/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/7230045/diff/8002/src/ia32/deoptimizer-ia32.cc...
src/ia32/deoptimizer-ia32.cc:615: input_->SetRegister(i, i * 4);
On 2011/06/29 10:47:35, fschneider wrote:
> Maybe use some constant as default value?

Done.

http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-...
File test/mjsunit/debug-evaluate-locals-optimized-double.js (right):

http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-...
test/mjsunit/debug-evaluate-locals-optimized-double.js:48: assertEquals(i * 2 +
1 + (i * 2 + 1) / 100, frame.localValue(0).value());
On 2011/06/29 10:47:35, fschneider wrote:
> Long line.

Done.

http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-...
test/mjsunit/debug-evaluate-locals-optimized-double.js:49: assertEquals(i * 2 +
2 + (i * 2 + 2) / 100, frame.localValue(1).value());
On 2011/06/29 10:47:35, fschneider wrote:
> Long line.

Done.