improve intel instruction & operand formatting #1463
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 fixes three issues when formatting an x86_64 instruction or its operands: 1) Fix the order of operands when formatting the instruction to be the AT&T syntax order. If the instruction had three or more operands. Conversion from the internal (Intel) order rotated right by 1 instead of reversing the operands. 2) Eliminate special treatment of register %kN as the first operand as mask registers in Instruction::format as Operand::format already formats these as masks (enclosed in braces) and not all uses of mask registers are as a mask. 3) Fix Operand::format to produce the correct disassembly operand string. The formatting of the internal Expression needs to be done for some indirect values. This was done in the Instruction::format instead of directly in Operand::format * x86Formatter::getInstructionString - fixes 1, 2, 3 * Operand::format - fixes 3 * x86Formatter::formatRegister - cleanup, remove malloc that could leak
kupsch
added
bug
code cleanup
hainest
reviewed
Jul 25, 2023
| } else { | ||
| source_ops += "," + op; | ||
| } | ||
| std::string s; |
There was a problem hiding this comment.
Could you add a quick comment with an example? On first pass, I missed the 'r' in crbegin.
Since we're in here fixing things, we might as well change the param to a const&. But I'll let you decide if you want to do that.
There was a problem hiding this comment.
I added a comment. What type of example are looking for?
I was going to update the ArchSpecificFormatter interface to use const refs for vectors and strings for all the methods in a separate PR.
There was a problem hiding this comment.
I guess an example wouldn't really be helpful here.
hainest
approved these changes
Jul 25, 2023
| } else { | ||
| source_ops += "," + op; | ||
| } | ||
| std::string s; |
There was a problem hiding this comment.
I guess an example wouldn't really be helpful here.
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 fixes three issues when formatting an x86_64 instruction or its operands:
Fix the order of operands when formatting the instruction to be the
AT&T syntax order. If the instruction had three or more operands.
Conversion from the internal (Intel) order rotated right by 1 instead
of reversing the operands.
Fixes Fix x86_64 instruction formatting for >2 operands #1459
Eliminate special treatment of register %kN as the first operand as
mask registers in Instruction::format as Operand::format already
formats these as masks (enclosed in braces) and not all uses of mask
registers are as a mask.
Fixes formatting x86_64 instruction incorrectly makes kN registers masks in some cases #1461
Fix Operand::format to produce the correct disassembly operand
string. The formatting of the internal Expression needs to be done
for some indirect values. This was done in the Instruction::format
instead of directly in Operand::format
Fixes Intel X86_64 operand formatting is incorrect #1460
Methods changed: