Fix finish command for PNaCl.

Fix finish command for PNaCl.

Since PNaCl hides base address, frame stack addresss may contain
base address or not depending on whether it comes from register
or stack frame.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=3600
TEST= none
R=eaeltsin@google.com

Committed: https://git.chromium.org/gitweb?p=native_client/nacl-gdb.git;a=commit;h=6344469

[eaeltsin, 2013-08-06 18:14:42]

https://codereview.chromium.org/22399003/diff/1/gdb/frame.c
File gdb/frame.c (right):

https://codereview.chromium.org/22399003/diff/1/gdb/frame.c#newcode451
gdb/frame.c:451: uint32_t max_addr = (uint32_t) -1;
const or #define?

if you use this for &, then either provide a synonym like 'addr_mask', or use %
instead.

https://codereview.chromium.org/22399003/diff/1/gdb/frame.c#newcode467
gdb/frame.c:467: || (l.stack_addr & max_addr) != (r.stack_addr & max_addr))
This definitely needs a comment, explaining what this does and also marking this
as a temporary, non-upstreamable thing.

What is the origin of l.stack_addr and r.stack_addr. If these are both extracted
from stack, then both should probably be 32-bit or 64-bit simultaneously? How
rbp value gets into these variables?

BTW, seems the following works OK in both modes:
  (l.stack_addr & ADDR_MASK == r.stack_addr & ADDR_MASK)

[halyavin, 2013-08-07 08:04:25]

https://codereview.chromium.org/22399003/diff/1/gdb/frame.c
File gdb/frame.c (right):

https://codereview.chromium.org/22399003/diff/1/gdb/frame.c#newcode451
gdb/frame.c:451: uint32_t max_addr = (uint32_t) -1;
On 2013/08/06 18:14:42, eaeltsin wrote:
> const or #define?
> 
> if you use this for &, then either provide a synonym like 'addr_mask', or use
%
> instead.

I used conversion instead of &.

https://codereview.chromium.org/22399003/diff/1/gdb/frame.c#newcode467
gdb/frame.c:467: || (l.stack_addr & max_addr) != (r.stack_addr & max_addr))
On 2013/08/06 18:14:42, eaeltsin wrote:
> This definitely needs a comment, explaining what this does and also marking
this
> as a temporary, non-upstreamable thing.
> 
> What is the origin of l.stack_addr and r.stack_addr. If these are both
extracted
> from stack, then both should probably be 32-bit or 64-bit simultaneously? How
> rbp value gets into these variables?
One value is read from the stack, the other is read from the register.
> 
> BTW, seems the following works OK in both modes:
>   (l.stack_addr & ADDR_MASK == r.stack_addr & ADDR_MASK)
Yes, but it increase false positives for normal 64-bit mode.

[eaeltsin, 2013-08-07 18:03:38]

lgtm

Still have questions on the check, but don't think they are blocking.

https://codereview.chromium.org/22399003/diff/6001/gdb/frame.c
File gdb/frame.c (right):

https://codereview.chromium.org/22399003/diff/6001/gdb/frame.c#newcode470
gdb/frame.c:470: || (uint32_t)l.stack_addr != (uint32_t)r.stack_addr)
Do we have any way to figure out we are in NaCl vs. non-NaCl?
If yes, remove max_addr and just replace the check with

  else if (l.stack_addr != r.stack_addr ||
           (is_nacl_mode && (
               (uint32_t)l.stack_addr != (uint32)r.stack_addr)))


BTW, as we screw up non-NaCl 64-bit mode anyway, how bad would be to do just

  else if ((uint32_t)l.stack_addr != (uint32)r.stack_addr)

?

[halyavin, 2013-08-08 06:15:46]

Committed patchset #2 manually as r6344469 (presubmit successful).