X64: Implement debugger hooks.

X64: Implement debugger hooks.
Debugger is now fully functional.
Fix difference in emitting statement positions to match ia32.

[Lasse Reichstein, 2009-08-18 12:27:18]

Large review. 
Søren, if you could just check that the debugger details doesn't look odd in
some way.

[Søren Thygesen Gjesse, 2009-08-18 13:10:43]

It is great th have all this working now!

http://codereview.chromium.org/171107/diff/1/3
File src/debug.cc (right):

http://codereview.chromium.org/171107/diff/1/3#newcode1481
Line 1481: // Move one byte back to where the call instruction was placed.
Comment out of sync with code.

http://codereview.chromium.org/171107/diff/1/7
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/171107/diff/1/7#newcode233
Line 233: return pc_[11] != 0xCC;
Please explain why non int3 instruction at pc + 11 is a call instruction :-).

http://codereview.chromium.org/171107/diff/1/13
File src/x64/debug-x64.cc (right):

http://codereview.chromium.org/171107/diff/1/13#newcode165
Line 165: // generated by X64CodeGenerator::ExitJSFrame.
Please remove any reference to X64CodeGenerator::ExitJSFrame, as this function
does not exist. Maybe refer to CodeGenerator::GenerateReturnSequence instead. It
is the same issue with the ia32 version.

http://codereview.chromium.org/171107/diff/1/15
File src/x64/virtual-frame-x64.cc (left):

http://codereview.chromium.org/171107/diff/1/15#oldcode934
Line 934: 
Accidental edit?

[Lasse Reichstein, 2009-08-19 07:07:55]

http://codereview.chromium.org/171107/diff/1/3
File src/debug.cc (right):

http://codereview.chromium.org/171107/diff/1/3#newcode1481
Line 1481: // Move one byte back to where the call instruction was placed.
Fixed

http://codereview.chromium.org/171107/diff/1/7
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/171107/diff/1/7#newcode233
Line 233: return pc_[11] != 0xCC;
Has been elaborated.
We only need to distinguish a call sequence from a JS return sequence, and only
the latter has int3 at that position.

http://codereview.chromium.org/171107/diff/1/13
File src/x64/debug-x64.cc (right):

http://codereview.chromium.org/171107/diff/1/13#newcode165
Line 165: // generated by X64CodeGenerator::ExitJSFrame.
Changed to refer to GenerateReturnSequence, and same in ia32.

http://codereview.chromium.org/171107/diff/1/15
File src/x64/virtual-frame-x64.cc (left):

http://codereview.chromium.org/171107/diff/1/15#oldcode934
Line 934: 
Indeed. Fixed.

[William Hesse, 2009-08-19 08:44:00]

LGTM.

http://codereview.chromium.org/171107/diff/1/7
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/171107/diff/1/7#newcode246
Line 246: Assembler::set_target_address_at(pc_ +
Assembler::kReturnAddrPatchPrefixSize,
I really think this name is bad.  Maybe there should be a friend Patcher class,
and this could be Patcher::ReturnSequenceAddressOffset.
Patcher::ReturnSequenceSize,
and so on.

http://codereview.chromium.org/171107/diff/1/13
File src/x64/debug-x64.cc (right):

http://codereview.chromium.org/171107/diff/1/13#newcode95
Line 95: // overwritten by the address of DebugBreakXXX.
Is there really a function called DebugBreakXXX?

http://codereview.chromium.org/171107/diff/1/14
File src/x64/macro-assembler-x64.cc (left):

http://codereview.chromium.org/171107/diff/1/14#oldcode498
Line 498: WriteRecordedPositions();
How can it be correct to remove this?

[Lasse Reichstein, 2009-08-19 10:12:32]

http://codereview.chromium.org/171107/diff/1/7
File src/x64/assembler-x64-inl.h (right):

http://codereview.chromium.org/171107/diff/1/7#newcode246
Line 246: Assembler::set_target_address_at(pc_ +
Assembler::kReturnAddrPatchPrefixSize,
I can't see a good and quick way to separate it. I have renamed the constants to
kPatchReturnSequenceAddressOffset and kPatchReturnSequenceLength. We should
think about a prettier solution.

http://codereview.chromium.org/171107/diff/1/13
File src/x64/debug-x64.cc (right):

http://codereview.chromium.org/171107/diff/1/13#newcode95
Line 95: // overwritten by the address of DebugBreakXXX.
No,  it was meant to refer to the DebugBreak<something> functions. Clearly it
wasn't successful in doing that.

http://codereview.chromium.org/171107/diff/1/14
File src/x64/macro-assembler-x64.cc (left):

http://codereview.chromium.org/171107/diff/1/14#oldcode498
Line 498: WriteRecordedPositions();
It's correct in the sense that it matches what we do in ia32.
We only record positions where we can set breakpoints, and we can only set
breakpoints at stub calls and JSReturn sequences. In ia32 that means
call(Handle<...>) and at function return.