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 "return" key in disassembler widget (#3090) #3140

Closed
wants to merge 3 commits into from

Conversation

whol-hoopa
Copy link

Your checklist for this pull request

Detailed description

the "return" key used to not be able to jump to the addresses with conditional jumps other than "jmp"
the "return" key can do it now

Test plan (required)

  1. go to disassembly widget
  2. put the disassembly cursor onto a disassembly line with a jump
  3. press "return" key

Closing issues

closes #3090

@XVilka XVilka added this to the 2.3 milestone Mar 1, 2023
@karliss
Copy link
Member

karliss commented Mar 2, 2023

I don't think this a proper fix. jumpToOffsetUnderCursor is also used in other places and those will probably have the same problems as enter key.

Other issue is that the offsetUnderCursor handled not only jump arrows. It also included reference to absolute memory locations like for mov and lea instructions.

@whol-hoopa
Copy link
Author

I'll look at it again.

@whol-hoopa whol-hoopa closed this Mar 5, 2023
@whol-hoopa
Copy link
Author

I'll open a new one because I messed up with git commands

@whol-hoopa whol-hoopa deleted the whola-jump_to_address branch March 5, 2023 07:23
@wargio
Copy link
Member

wargio commented Mar 5, 2023

no worries

@whol-hoopa
Copy link
Author

whol-hoopa commented Mar 10, 2023

I reopened the pull request. It's pull request #3146 now.
@karliss I changed it and it should be working now. I made changes to the graph and disassembly widget, because they use jumpToOffsetUnderCursor.

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.

Can not follow conditional branching targets (JNE/JNZ)
4 participants