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

Fold switch cases pointing to the same instruction #10497

Merged
merged 4 commits into from Jun 26, 2018

Conversation

@cyanpencil
Copy link
Contributor

@cyanpencil cyanpencil commented Jun 25, 2018

Closes #10474

It confronts the address of the switch instruction, so it should work even if multiple switches point to the same instruction.

Before:
photo_2018-06-25_22-05-05

After:
2018-06-25-215835_796x145_scrot

@cyanpencil
Copy link
Contributor Author

@cyanpencil cyanpencil commented Jun 25, 2018

Wait, changing the address from the pointed instruction to the address of the switch broke some tests like this:

-0x0040bf80 1 case.0.0x40bf80
-0x0040bf88 1 case.1.0x40bf88
-0x0040bf98 1 case.2.0x40bf98
-0x0040bfac 1 case.3.0x40bfac
+0x0040bf80 1 case.0.0x40bf68
+0x0040bf88 1 case.1.0x40bf68
+0x0040bf98 1 case.2.0x40bf68
+0x0040bfac 1 case.3.0x40bf68

0x40bf80 is the pointed instruction, while 0x40bf68 is the address of the switch.

Do I change them? @radare

@radare
Copy link
Collaborator

@radare radare commented Jun 25, 2018

yes, fix the tests, the new flag name makes more sense.. but maybe we should use case.${addr}.${value} instead

@radare
Copy link
Collaborator

@radare radare commented Jun 25, 2018

maybe this needs changes in r2dec /cc @wargio

@wargio
Copy link
Contributor

@wargio wargio commented Jun 26, 2018

Yup, I'm ok with that.
I'll add a fix for r2dec when this will be merged.

@cyanpencil cyanpencil force-pushed the cyanpencil:switch-cases-fix branch from 60d92b0 to d48c62d Jun 26, 2018
@cyanpencil cyanpencil force-pushed the cyanpencil:switch-cases-fix branch from d48c62d to 7928f72 Jun 26, 2018
@cyanpencil
Copy link
Contributor Author

@cyanpencil cyanpencil commented Jun 26, 2018

@radare radare merged commit 2745486 into radareorg:master Jun 26, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants