-
-
Notifications
You must be signed in to change notification settings - Fork 791
Add backwards memory examination for the dereference command #942
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
Conversation
gef.py
Outdated
| to_insnum = self.repeat_count * nb - 1 | ||
| insnum_step = -1 | ||
| else: | ||
| from_insnum = 0 + self.repeat_count * nb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 0 +?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous implementation had them, so I simply left them in place. I can't tell why they were placed, so I just left them there in case there's some obsure edge-case that I'm not aware of.
gef.py
Outdated
| if nb > 0: | ||
| from_insnum = nb * (self.repeat_count + 1) - 1 | ||
| to_insnum = self.repeat_count * nb - 1 | ||
| insnum_step = -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have to repeat this line in both cases
gef.py
Outdated
| if nb > 0: | ||
| from_insnum = 0 + self.repeat_count * nb | ||
| to_insnum = nb * (self.repeat_count + 1) | ||
| insnum_step = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have to repeat this in both cases
gef.py
Outdated
| to_insnum = nb * (self.repeat_count + 1) | ||
| insnum_step = 1 | ||
| if nb > 0: | ||
| from_insnum = 0 + self.repeat_count * nb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 0+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests in tests/commands/dereference.py, other than that and graz remarks, it looks good to me
tests/commands/dereference.py
Outdated
| ] | ||
| res = gdb_start_silent_cmd(cmd=setup, after=cmd) | ||
| self.assertNoException(res) | ||
| print(res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
tests/commands/dereference.py
Outdated
| ] | ||
| res = gdb_start_silent_cmd(cmd=setup, after=cmd) | ||
| self.assertNoException(res) | ||
| print(res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
gef.py
Outdated
| to_insnum = nb * (self.repeat_count + 1) | ||
| else: | ||
| from_insnum = nb * (self.repeat_count + 1) + 1 | ||
| to_insnum = (0 + self.repeat_count * nb) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why 0+
tests/commands/dereference.py
Outdated
| """ | ||
| res = res.splitlines()[-2] | ||
| self.assertTrue("AAAAAAAA" not in res) | ||
| self.assertTrue("BBBBBBBB" in res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertIn
tests/commands/dereference.py
Outdated
| Hence, we want to look at the second last line of the output | ||
| """ | ||
| res = res.splitlines()[-1] | ||
| self.assertTrue("AAAAAAAA" in res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertIn
tests/commands/dereference.py
Outdated
| """ | ||
| res = res.splitlines()[-1] | ||
| self.assertTrue("AAAAAAAA" in res) | ||
| self.assertTrue("BBBBBBBB" not in res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertNotIn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Description/Motivation/Screenshots
This commit extends the
dereferencecommand, allowing to examine memory backwards via a negative--lengthvalue, analogous to the behaviour ofx.Against which architecture was this tested ?
"Tested" indicates that the PR works and the unit test (see
docs/testing.md) run passes without issue.Checklist
devbranch, notmain.