Support scope information and evaluation in optimized frames

Support scope information and evaluation in optimized frames

R=svenpanne@chromium.org

BUG=v8:1140
TEST=test/mjsunit/debug-evaluate-locals-optimized.js,test/mjsunit/debug-evaluate-locals-optimized-double.js

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

[Sven Panne, 2011-07-12 13:40:32]

http://codereview.chromium.org/7343005/diff/1/src/deoptimizer.h
File src/deoptimizer.h (right):

http://codereview.chromium.org/7343005/diff/1/src/deoptimizer.h#newcode723
src/deoptimizer.h:723: class ScopedDeoptimizedFrameInfo {
This class looks a bits strange: If the frame passed to the constructor is not
optimized, every single method call would segfault, meaning that the constructed
object is illegal from a semantical point of view. Therefore, it shouldn't be
possible to construct such an instance in the first place. The test should
probably be moved to the caller or, alternatively, a null object pattern should
be used.

[Søren Thygesen Gjesse, 2011-07-12 14:20:36]

http://codereview.chromium.org/7343005/diff/1/src/deoptimizer.h
File src/deoptimizer.h (right):

http://codereview.chromium.org/7343005/diff/1/src/deoptimizer.h#newcode723
src/deoptimizer.h:723: class ScopedDeoptimizedFrameInfo {
On 2011/07/12 13:40:32, Sven wrote:
> This class looks a bits strange: If the frame passed to the constructor is not
> optimized, every single method call would segfault, meaning that the
constructed
> object is illegal from a semantical point of view. Therefore, it shouldn't be
> possible to construct such an instance in the first place. The test should
> probably be moved to the caller or, alternatively, a null object pattern
should
> be used.

That is true, but it was the simplest way to have a scoped class that ensured
that Deoptimizer::DeleteDebuggerInspectableFrame was called. Let me consider to
make it hold the JavaScriptFrame pointer as well.

[Søren Thygesen Gjesse, 2011-07-13 10:45:13]

Sven, please take a look now.

[Sven Panne, 2011-07-13 10:53:46]

LGTM apart from a minor problem understanding the code (see comment)

http://codereview.chromium.org/7343005/diff/7001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7343005/diff/7001/src/runtime.cc#newcode10024
src/runtime.cc:10024: void set_frame(JavaScriptFrame* frame) {
Is it OK (or even intended for correct execution of the code below) that frame_
and deoptimized_frame_ are "out of sync" after this call? I don't understand
enough details to decide this. If it *is* OK, perhaps a comment about this might
be helpful.

[Søren Thygesen Gjesse, 2011-07-13 11:12:37]

http://codereview.chromium.org/7343005/diff/7001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7343005/diff/7001/src/runtime.cc#newcode10024
src/runtime.cc:10024: void set_frame(JavaScriptFrame* frame) {
On 2011/07/13 10:53:46, Sven wrote:
> Is it OK (or even intended for correct execution of the code below) that
frame_
> and deoptimized_frame_ are "out of sync" after this call? I don't understand
> enough details to decide this. If it *is* OK, perhaps a comment about this
might
> be helpful.

Well, this is somewhat odd. The issue is that when inspecting a JavaScript frame
(not inlined) it always have the correct number of arguments. If the provided
parameters are different the frame below holds the provided arguments. And as a
JavaScriptFrameIterator can only expose one JavaScriptFrame it is replaced. I
will elaborate the comment and narrow down the usage of this.

An even better solution might be to have the FrameInspector control the
JavaScriptFrameIterator, but that will be a separate change.