Add ActivationFrame_GetFramePointer method and test.

Add ActivationFrame_GetFramePointer method and test.

BUG=
R=asiva@google.com, hausner@google.com

Committed: https://code.google.com/p/dart/source/detail?r=40171

[Jacob, 2014-09-05 16:39:32]

Patchset #1 (id:1) has been deleted

[Jacob, 2014-09-05 17:45:14]

Patchset #1 (id:20001) has been deleted

[Jacob, 2014-09-05 17:47:16]

jacobr@google.com changed reviewers:
+ asiva@google.com, hausner@google.com

[hausner, 2014-09-05 19:16:59]

LGTM but the masking of bits in V8 is not playing well with this change.

If you know the address where the stack begins (on both platforms) you could use
a stack-relative FP value.

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
File runtime/vm/debugger_api_impl.cc (right):

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
runtime/vm/debugger_api_impl.cc:319: *frame_pointer =
static_cast<uintptr_t>(frame->fp());
This is fine. How does it play with the V8 side of your change where you mask
the top bits so the FP fits into 53 bits? if the stack is in the top of the
address space, this would not work. (Yes, only 48 bit are used on x64, but the
only requirement is that bits 48..63 are either all 0 or all 1. So,
0xffffffff'ffff0000 for example is a valid address.

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
File runtime/vm/debugger_api_impl_test.cc (right):

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
runtime/vm/debugger_api_impl_test.cc:284: EXPECT_GT(frame_pointer,
last_frame_pointer);
Maybe a one-line comment reminding us that we expect the stack to grow from high
to low addresses.

[Jacob, 2014-09-05 20:12:25]

ptal

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
File runtime/vm/debugger_api_impl.cc (right):

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
runtime/vm/debugger_api_impl.cc:319: *frame_pointer =
static_cast<uintptr_t>(frame->fp());
On 2014/09/05 19:16:59, hausner wrote:
> This is fine. How does it play with the V8 side of your change where you mask
> the top bits so the FP fits into 53 bits? if the stack is in the top of the
> address space, this would not work. (Yes, only 48 bit are used on x64, but the
> only requirement is that bits 48..63 are either all 0 or all 1. So,
> 0xffffffff'ffff0000 for example is a valid address.
In blink I truncate Dart stack frame values before storing them as doubles to
make it consistent with V8. I thought it was cleaner that way as there is no
need for Dart to know about a hack that is there just to work around the lack of
a 64bit int type in JS.

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
File runtime/vm/debugger_api_impl_test.cc (right):

https://codereview.chromium.org/545263002/diff/40001/runtime/vm/debugger_api_...
runtime/vm/debugger_api_impl_test.cc:284: EXPECT_GT(frame_pointer,
last_frame_pointer);
On 2014/09/05 19:16:59, hausner wrote:
> Maybe a one-line comment reminding us that we expect the stack to grow from
high
> to low addresses.

done

[siva, 2014-09-09 00:50:49]

lgtm

https://codereview.chromium.org/545263002/diff/50001/runtime/include/dart_deb...
File runtime/include/dart_debugger_api.h (right):

https://codereview.chromium.org/545263002/diff/50001/runtime/include/dart_deb...
runtime/include/dart_debugger_api.h:470: * Any of the out parameters above may
be NULL.
Why do you want to allow for the frame_pointer being NULL?

[Jacob, 2014-09-10 18:02:56]

https://codereview.chromium.org/545263002/diff/50001/runtime/include/dart_deb...
File runtime/include/dart_debugger_api.h (right):

https://codereview.chromium.org/545263002/diff/50001/runtime/include/dart_deb...
runtime/include/dart_debugger_api.h:470: * Any of the out parameters above may
be NULL.
On 2014/09/09 00:50:49, siva wrote:
> Why do you want to allow for the frame_pointer being NULL?

No particularly good reason. I'm just matching the signatures of other API
methods in this file to keep the API consistent. I can't think of any additional
out parameters we would want to add to this method in the future so it is
overkill.

[Jacob, 2014-09-11 18:17:08]

Committed patchset #3 (id:70001) manually as 40171 (presubmit successful).