Fix debugger stub patching on x64 architecture

Fix debugger stub patching on x64 architecture

Instead of patching the entry in the object pool, patch the offset
into the pool at the call site.

SIMARM and SIMMIPS will have to be fixed the same way in a later change.

R=iposva@google.com

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

[Ivan Posva, 2014-01-14 22:12:38]

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_ia32.cc
File runtime/vm/code_patcher_ia32.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_ia32...
runtime/vm/code_patcher_ia32.cc:224: UNIMPLEMENTED();
This should be UNREACHABLE() as the ia32 assembler does not use pool pointers.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_ia32...
runtime/vm/code_patcher_ia32.cc:230: UNIMPLEMENTED();
ditto

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64.cc
File runtime/vm/code_patcher_x64.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:166: uint32_t pp_offset() const {
Aren't offsets signed on x64?

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:170: void set_pp_offset(uint32_t offset) const {
ditto

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:240: uint32_t CodePatcher::GetPoolOffsetAt(uword
return_address) {
ditto

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:246: void CodePatcher::SetPoolOffsetAt(uword
return_address, uint32_t offset) {
ditto

https://codereview.chromium.org/131853006/diff/1/runtime/vm/debugger_x64.cc
File runtime/vm/debugger_x64.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/debugger_x64.cc#n...
runtime/vm/debugger_x64.cc:45: return
reinterpret_cast<uword>(object_pool.At(index));
There are some assertions that you can make about the saved_value_ and the
address we are loading here. Where should we make sure our assumptions are
correct?

[hausner, 2014-01-14 22:49:40]

PTAL. Comments addressed and debugging printfs removed.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_ia32.cc
File runtime/vm/code_patcher_ia32.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_ia32...
runtime/vm/code_patcher_ia32.cc:224: UNIMPLEMENTED();
On 2014/01/14 22:12:39, Ivan Posva wrote:
> This should be UNREACHABLE() as the ia32 assembler does not use pool pointers.

Done.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_ia32...
runtime/vm/code_patcher_ia32.cc:230: UNIMPLEMENTED();
On 2014/01/14 22:12:39, Ivan Posva wrote:
> ditto

Done.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64.cc
File runtime/vm/code_patcher_x64.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:166: uint32_t pp_offset() const {
On 2014/01/14 22:12:39, Ivan Posva wrote:
> Aren't offsets signed on x64?

Done.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:170: void set_pp_offset(uint32_t offset) const {
On 2014/01/14 22:12:39, Ivan Posva wrote:
> ditto

Done.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:240: uint32_t CodePatcher::GetPoolOffsetAt(uword
return_address) {
On 2014/01/14 22:12:39, Ivan Posva wrote:
> ditto

Done.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:246: void CodePatcher::SetPoolOffsetAt(uword
return_address, uint32_t offset) {
On 2014/01/14 22:12:39, Ivan Posva wrote:
> ditto

Done.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/debugger_x64.cc
File runtime/vm/debugger_x64.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/debugger_x64.cc#n...
runtime/vm/debugger_x64.cc:45: return
reinterpret_cast<uword>(object_pool.At(index));
On 2014/01/14 22:12:39, Ivan Posva wrote:
> There are some assertions that you can make about the saved_value_ and the
> address we are loading here. Where should we make sure our assumptions are
> correct?

Done.

[Ivan Posva, 2014-01-14 23:09:14]

LGTMwC

-Ivan

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64.cc
File runtime/vm/code_patcher_x64.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:166: uint32_t pp_offset() const {
On 2014/01/14 22:49:41, hausner wrote:
> On 2014/01/14 22:12:39, Ivan Posva wrote:
> > Aren't offsets signed on x64?
> 
> Done.

.

[hausner, 2014-01-14 23:26:19]

Comment addressed and build issues fixed.

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64.cc
File runtime/vm/code_patcher_x64.cc (right):

https://codereview.chromium.org/131853006/diff/1/runtime/vm/code_patcher_x64....
runtime/vm/code_patcher_x64.cc:166: uint32_t pp_offset() const {
On 2014/01/14 23:09:15, Ivan Posva wrote:
> On 2014/01/14 22:49:41, hausner wrote:
> > On 2014/01/14 22:12:39, Ivan Posva wrote:
> > > Aren't offsets signed on x64?
> > 
> > Done.
> 
> .

Really done now.

[hausner, 2014-01-14 23:27:22]

Committed patchset #5 manually as r31812 (presubmit successful).

[Florian Schneider, 2014-01-16 10:47:58]

On 2014/01/14 23:27:22, hausner wrote:
> Committed patchset #5 manually as r31812 (presubmit successful).

Is there any regression test for this bug fix?