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

Fix iS. paddr vaddr confusion ##core #22674

Merged
merged 4 commits into from
Mar 15, 2024
Merged

Conversation

mrmacete
Copy link
Collaborator

@mrmacete mrmacete commented Mar 8, 2024

  • Mark this if you consider it ready to merge
  • I've added tests (optional)
  • I wrote some lines in the book (optional)

Description

This change removes the part of the printHere address check where it explicitly checked for paddr even if the paddr / vaddr dichotomy is already handled via the addr variable, which is the only one we have to check here.

That caused wrong behaviour in binaries where the vaddr and paddr space numerically overlap (like old DOS/4GW LE binaries) where iS. was in fact picking the paddr instead of vaddr to filter if such an entry came first in the list.

@mrmacete mrmacete marked this pull request as draft March 8, 2024 09:00
@mrmacete
Copy link
Collaborator Author

mrmacete commented Mar 8, 2024

draft, i'm going to check the tests soon

This change removes the part of the `printHere` address check where it
explicitly checked for `paddr` even if the paddr / vaddr dichotomy is
already handled via the `addr` variable, which is the only one we have
to check here.

That caused wrong behaviour in binaries where the vaddr and paddr
space numerically overlap (like old DOS/4GW LE binaries) where `iS.`
was in fact picking the `paddr` instead of `vaddr` to filter if such
an entry came first in the list.
@mrmacete mrmacete force-pushed the fix/sections-paddr-vaddr-here branch from cf94579 to 57be3c9 Compare March 15, 2024 17:18
@mrmacete mrmacete marked this pull request as ready for review March 15, 2024 17:18
@mrmacete
Copy link
Collaborator Author

new test needs radareorg/radare2-testbins#94

@trufae trufae merged commit 5abb47d into master Mar 15, 2024
38 checks passed
@trufae trufae deleted the fix/sections-paddr-vaddr-here branch March 17, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants