Fix short jump address calculation in x64 disassembler

Fix short jump address calculation in x64 disassembler

- Make Observatory robust to weird jump addresses.
- Use signed address offset when computing short jump target address.

R=fschneider@google.com, srdjan@google.com

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

[Cutch, 2015-08-10 23:49:03]

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

[srdjan, 2015-08-11 06:17:45]

lgtm

https://codereview.chromium.org/1273223004/diff/1/runtime/observatory/lib/src...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/1273223004/diff/1/runtime/observatory/lib/src...
runtime/observatory/lib/src/service/object.dart:3158: }
You may want to report it to console that there is an issue with
relativeAddress.

[Florian Schneider, 2015-08-11 08:13:42]

fschneider@google.com changed reviewers:
+ fschneider@google.com

[Florian Schneider, 2015-08-11 08:13:42]

lgtm

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

https://codereview.chromium.org/1273223004/diff/1/runtime/vm/disassembler_x64...
runtime/vm/disassembler_x64.cc:880: uint8_t b = *(data + 1);
Why not cast here?

int8_t b = *reinterpret_cast<int8_t*>(data + 1);

or

int8_t b = static_cast<int8_t>(*(data + 1));

should work too.

It seems more natural at the definition, rather than cast at all uses of b (even
though there is only one here, it is easy to forget).

[Cutch, 2015-08-11 13:17:05]

https://codereview.chromium.org/1273223004/diff/1/runtime/observatory/lib/src...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/1273223004/diff/1/runtime/observatory/lib/src...
runtime/observatory/lib/src/service/object.dart:3158: }
On 2015/08/11 06:17:45, srdjan wrote:
> You may want to report it to console that there is an issue with
> relativeAddress.

Done.

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

https://codereview.chromium.org/1273223004/diff/1/runtime/vm/disassembler_x64...
runtime/vm/disassembler_x64.cc:880: uint8_t b = *(data + 1);
On 2015/08/11 08:13:42, Florian Schneider wrote:
> Why not cast here?
> 
> int8_t b = *reinterpret_cast<int8_t*>(data + 1);
> 
> or
> 
> int8_t b = static_cast<int8_t>(*(data + 1));
> 
> should work too.
> 
> It seems more natural at the definition, rather than cast at all uses of b
(even
> though there is only one here, it is easy to forget).

The pattern of casting it at use is repeated multiple times in this file. I'm
going to go with consistency for this CL.

[Cutch, 2015-08-11 13:18:02]

Committed patchset #3 (id:40001) manually as
a8e2180b781ad82d81206696da220a920310b5db (presubmit successful).

[Florian Schneider, 2015-08-11 13:20:07]

On 2015/08/11 13:17:05, Cutch wrote:
>
https://codereview.chromium.org/1273223004/diff/1/runtime/observatory/lib/src...
> File runtime/observatory/lib/src/service/object.dart (right):
> 
>
https://codereview.chromium.org/1273223004/diff/1/runtime/observatory/lib/src...
> runtime/observatory/lib/src/service/object.dart:3158: }
> On 2015/08/11 06:17:45, srdjan wrote:
> > You may want to report it to console that there is an issue with
> > relativeAddress.
> 
> Done.
> 
>
https://codereview.chromium.org/1273223004/diff/1/runtime/vm/disassembler_x64.cc
> File runtime/vm/disassembler_x64.cc (right):
> 
>
https://codereview.chromium.org/1273223004/diff/1/runtime/vm/disassembler_x64...
> runtime/vm/disassembler_x64.cc:880: uint8_t b = *(data + 1);
> On 2015/08/11 08:13:42, Florian Schneider wrote:
> > Why not cast here?
> > 
> > int8_t b = *reinterpret_cast<int8_t*>(data + 1);
> > 
> > or
> > 
> > int8_t b = static_cast<int8_t>(*(data + 1));
> > 
> > should work too.
> > 
> > It seems more natural at the definition, rather than cast at all uses of b
> (even
> > though there is only one here, it is easy to forget).
> 
> The pattern of casting it at use is repeated multiple times in this file. I'm
> going to go with consistency for this CL.

Why not fix the other handful of occurrences? I counted one other place...