Fix aglib reg_write to correctly handle writing to the "pc" register #2539
Conversation
| @@ -125,7 +125,8 @@ def __setattr__(self, attr: str, val: Any) -> None: | |||
| if attr in ("last", "previous"): | |||
| super().__setattr__(attr, val) | |||
| else: | |||
| pwndbg.dbg.selected_frame().reg_write(attr, int(val)) | |||
| if not pwndbg.dbg.selected_frame().reg_write(attr, int(val)): | |||
There was a problem hiding this comment.
Or inside here we should "remap" if attr == pc to correct real register?
Maybe reg_write should only accept real registers?
And not modify gdb.py / lldb/__init__.py
There was a problem hiding this comment.
And what about sp register? Like we have in fix() method:
def fix(self, expression: str) -> str:
for regname in set(self.all + ["sp", "pc"]):
expression = re.sub(rf"\$?\b{regname}\b", r"$" + regname, expression)
return expression
There was a problem hiding this comment.
Maybe reg_write should only accept real registers?
Yeah, maybe, and have __setattr__ handle the register aliases.
Or inside here we should "remap" if attr == pc to correct real register?
Personally, I think mapping pc to regs_sets[...].pc and still doing the sanity check against the registers listed in the reg sets is a cleaner solution, because that means we only ever have to deal with real register names in the Debugger API. Buuuut, both GDB and LLDB accept pc as a register name just fine, so it's not like this won't work, if you think it's better this way.
And what about sp register?
Yeap, this will probably need an exception for sp, too..
|
@mbrla0 Thoughts? Shall we merge this or should we change sth here>? |
Fixes #2537
@mbrla0 please look :)