Track prologue offset of Code and use for more precise return addresses

Track prologue offset of Code and use for more precise return addresses

- Remember the prologue offset in the Code object.
- Size of Code object does not change.
- Use the prologue offset to assist in locating the return address of the current frame.
- Prologue offset + instruction pattern matching will be accurate for all Dart code, most intrinsic prefixed Dart code, and some stub code.
- By default only collect Dart frames (normal Dart developers only care about Dart frames)

R=srdjan@google.com

Committed: https://github.com/dart-lang/sdk/commit/baa34257fc4fd554c64ffc44eb13febf15be3abf

[Cutch, 2015-05-20 22:56:10]

johnmccutchan@google.com changed reviewers:
+ srdjan@google.com

[srdjan, 2015-05-21 16:06:34]

LGTM

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
File runtime/vm/instructions_ia32.cc (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
runtime/vm/instructions_ia32.cc:58: const int* ProloguePattern::pattern() const
{
maybe describe somewhere what kind of prolog you are targeting here.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
File runtime/vm/instructions_ia32.h (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
runtime/vm/instructions_ia32.h:109: 
Please add tests for these two new patterns, best use Assembler::EnterFrame
since the pattern is tied to this. Also for X64. 

Maybe instead of tests add an assert in EnterFrame, that the pattern matches
what EnterFrame is emitting.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
runtime/vm/instructions_ia32.h:114: virtual const int* pattern() const;
I wonder why we are using int instead of intptr_t here. It is not the objective
of this CL to do that cleanup.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:12283: if (object.IsNull() || !object.IsSmi()) {
How can it be non-null and non-Smi? (Add comment if test correct)

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:12332: ASSERT(value.IsOld());
ONe too many check for value.IsOld()

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:326: // push ebp      <--- here
Or corresponding code for X64

[Cutch, 2015-05-22 01:11:15]

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
File runtime/vm/instructions_ia32.cc (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
runtime/vm/instructions_ia32.cc:58: const int* ProloguePattern::pattern() const
{
On 2015/05/21 16:06:33, srdjan wrote:
> maybe describe somewhere what kind of prolog you are targeting here.

Done.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
File runtime/vm/instructions_ia32.h (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
runtime/vm/instructions_ia32.h:109: 
On 2015/05/21 16:06:33, srdjan wrote:
> Please add tests for these two new patterns, best use Assembler::EnterFrame
> since the pattern is tied to this. Also for X64. 
> 
> Maybe instead of tests add an assert in EnterFrame, that the pattern matches
> what EnterFrame is emitting.

Done.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/instructions...
runtime/vm/instructions_ia32.h:114: virtual const int* pattern() const;
On 2015/05/21 16:06:33, srdjan wrote:
> I wonder why we are using int instead of intptr_t here. It is not the
objective
> of this CL to do that cleanup.

Acknowledged.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:12283: if (object.IsNull() || !object.IsSmi()) {
On 2015/05/21 16:06:33, srdjan wrote:
> How can it be non-null and non-Smi? (Add comment if test correct)

I've added a comment. In the future we may overload return_address_metadata_
with something other than a Smi for precise return addresses in stubs and other
non-standard Code objects.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:12332: ASSERT(value.IsOld());
On 2015/05/21 16:06:34, srdjan wrote:
> ONe too many check for value.IsOld()

Done.

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/1150633002/diff/20001/runtime/vm/profiler.cc#...
runtime/vm/profiler.cc:326: // push ebp      <--- here
On 2015/05/21 16:06:34, srdjan wrote:
> Or corresponding code for X64

Added a comment about register names on X64.

[Cutch, 2015-05-22 01:35:28]

Committed patchset #4 (id:60001) manually as
baa34257fc4fd554c64ffc44eb13febf15be3abf.