Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context displays wrong registers values for frames other than the newest one #636

Closed
disconnect3d opened this issue May 8, 2019 · 1 comment
Assignees
Labels

Comments

@disconnect3d
Copy link
Member

There are two bugs regarding displaying of registers:

  1. Wrong registers read
  2. Improper caching

Wrong registers read

We read registers from newest_frame instead of selected_frame - as a result, we display registers not from the current frame. This happens only for GDB>=7.9 as can be seen below.

To see this - break somewhere in a deep call, then do up 2, then display context regs and compare IP/PC with info reg pc.

Code responsible for the issue:

pwndbg/pwndbg/regs.py

Lines 246 to 252 in a729a82

@pwndbg.proc.OnlyWhenRunning
def gdb77_get_register(name):
return gdb.parse_and_eval('$' + name)
@pwndbg.proc.OnlyWhenRunning
def gdb79_get_register(name):
return gdb.newest_frame().read_register(name)

Some example:
image

Improper caching

I have fixed the 1st issue locally but apparently the context display wasn't right. When I did pi pwndbg.memoize.memoize.caching = False and then context regs it has been fixed.

So I investigated this and found out that we have two (or more?) methods to fetch registers:

  • __getattr__
  • __getitem__

This can be seen here:

pwndbg/pwndbg/regs.py

Lines 269 to 306 in a729a82

@pwndbg.memoize.reset_on_stop
@pwndbg.memoize.reset_on_prompt
def __getattr__(self, attr):
attr = attr.lstrip('$')
try:
# Seriously, gdb? Only accepts uint32.
if 'eflags' in attr:
value = gdb77_get_register(attr)
value = value.cast(pwndbg.typeinfo.uint32)
else:
value = get_register(attr)
value = value.cast(pwndbg.typeinfo.ptrdiff)
value = int(value)
return value & pwndbg.arch.ptrmask
except (ValueError, gdb.error):
return None
@pwndbg.memoize.reset_on_stop
def __getitem__(self, item):
if isinstance(item, six.integer_types):
return arch_to_regs[pwndbg.arch.current][item]
if not isinstance(item, six.string_types):
print("Unknown register type: %r" % (item))
import pdb, traceback
traceback.print_stack()
pdb.set_trace()
return None
# e.g. if we're looking for register "$rax", turn it into "rax"
item = item.lstrip('$')
item = getattr(self, item.lower())
if isinstance(item, six.integer_types):
return int(item) & pwndbg.arch.ptrmask
return item

Not that far ago, we had some problems with caching and we added the @pwndbg.memoize.reset_on_prompt cache invalidation for __getattr__.

It turns out, that some registers are fetched via __getitem__ and cached and this bites us here: it seems the context register values are fetched, cached and not invalidated this way.

So to fix that, we need to add @pwndbg.memoize.reset_on_prompt to __getitem__.

One could ask if we should have both methods at the first place - or - why do we have both of them? I don't know. We should probably remove one of them and refactor other places to use just the other one. But this can be done in the future. If someone is willing to refactor this, please do so :).

@disconnect3d disconnect3d self-assigned this May 8, 2019
disconnect3d added a commit to disconnect3d/pwndbg that referenced this issue May 8, 2019
TLDR:
1. We read registers from `newest_frame` instead of `selected_frame` for GDB>=7.9.
2. We have two ways to fetch registers - `regs.__getitem__` and
`regs.__getattr__` - one of them didn't invalidate cache and so after
fixing 1st, we still shown the old register after switching frames.
@stnevans
Copy link
Contributor

stnevans commented May 9, 2019

It looks like __getitem__ functions as a wrapper for __getattr__ when you ask for a string (e.g.) "rax". Otherwise it uses arch_to_regs. Do you know why getitem uses arch_to_regs whereas getattr uses get_register()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants