-
Notifications
You must be signed in to change notification settings - Fork 23
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
Show relocation diffs in function view when the data's content differs #153
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is to handle data symbols that contain multiple values in them at once, such as stringBase. If you compare the target symbol's bytes directly, then any part of the symbol having different bytes will cause *all* relocations to that symbol to show as a diff, even if the specific string being accessed is the same.
Fixed this by showing extern symbols correctly instead of skipping them.
Includes both a command line argument and a keyboard shortcut (S).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements #152. Relocations to data will be highlighted as a diff in the function view if the contents of the data are not the same on both sides:

I also added a new option called "Relax shifted data diffs". When this is checked, data relocations that have their contents matching but are at the wrong address will not show up as a diff. This can be helpful for float literals, as they all get shifted around when the number or order of any of the float literals in the TU are wrong.
In order to support diffing fake pool relocations (#140), I have added them to the end of the line visually, as a fake argument. Without this, objdiff would not check if the fake relocation matches on each side or not, as none of the real arguments reference this fake relocation.

This makes it easy to tell a glance when the value of a float literal is wrong, even when the offset into the pool is correct, without needing to hover over every single float load in the function to view the value in the tooltip.
This also works for stringBase pools:

In order to get this working, I had to switch it from comparing the bytes of the data to instead comparing the string representation of the data that gets shown on hover. This is because if the bytes are compared directly, then any single string in the entire stringBase being wrong would result in every reference to stringBase in the object being shown as wrong, because stringBase's bytes includes every string, even the ones not being referenced by the current instruction.
Note: When I initially was implementing this PR by comparing bytes directly, this resulted in all functions that use pools suddenly showing a false diff due to the dtk issue where the first symbol in a pool is shown instead of the pool symbol itself:

To get around this, I initially added a hack where a diff will be ignored if the right side's symbol name starts with the string "...", which wasn't ideal.
But later, after I switched it from comparing bytes to comparing display strings, I found a better way to fix this false positive. All I had to do was remove a separate hack I had implemented in #140, specifically the one where all
addi
opcodes are assumed to be referencing strings and it tries to show the data as a string, but creates many false positives. Once that was removed I could also remove the "..." hack, because then neither the left nor the right side were trying to display any visual representation of the pool symbol's data.I don't think removing the
addi
hack from the previous PR has much downside to it - the main reason it was originally added actually became obsolete by the time the previous PR was finished, but I forgot to remove that hack at the time.This PR still needs more testing to check for possible false positive diffs it could have introduced, I only tested it on 3-4 TWW TUs so far.