AArch64: keep the Rm operand of the register-form shifts - #3063
Open
gaul wants to merge 1 commit into
Open
Conversation
The register-form variable shifts -- LSLV/LSRV/ASRV/RORV, printed
lsl/lsr/asr/ror with a register amount -- were generated with only two
detail operands. add_non_alias_details() folds the shift amount Rm into
the preceding operand's shift.value (with an AARCH64_SFT_*_REG type) to
describe the shift, and then called AArch64_dec_op_count(), dropping Rm
from the operand list entirely:
ror w0, w1, w2
op_count: 2
operands[0].type: REG = w0 access: WRITE
operands[1].type: REG = w1 access: READ
shift: ROR_REG w2
A consumer walking the operand list therefore never saw w2, the shift
amount and a genuine read. cs_regs_access() still reported it -- the
shift-value harvest at the bottom of AArch64_reg_access() reads
shift.value back as a register -- so, exactly as with the STLUR Rt bug
(capstone-engine#3046), only the operand view was broken.
Drop the dec_op_count() so Rm stays in the list as the third operand it
prints as. The shift annotation on operand 1 is kept, so no information
is lost; AArch64_reg_access() de-duplicates the restored operand against
the shift-value read via arr_exist(), so the reported register lists do
not change. The printed text is rendered from the MCInst and was never
affected.
Extends the four existing per-mnemonic regression cases (and their three
duplicates) with the restored operand, and adds the 64-bit forms as new
coverage.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Your checklist for this pull request
Detailed description
The register-form variable shifts -- LSLV/LSRV/ASRV/RORV, printed lsl/lsr/asr/ror with a register amount -- were generated with only two detail operands. add_non_alias_details() folds the shift amount Rm into the preceding operand's shift.value (with an AARCH64_SFT_*_REG type) to describe the shift, and then called AArch64_dec_op_count(), dropping Rm from the operand list entirely:
A consumer walking the operand list therefore never saw w2, the shift amount and a genuine read. cs_regs_access() still reported it -- the shift-value harvest at the bottom of AArch64_reg_access() reads shift.value back as a register -- so, exactly as with the STLUR Rt bug (#3046), only the operand view was broken.
Drop the dec_op_count() so Rm stays in the list as the third operand it prints as. The shift annotation on operand 1 is kept, so no information is lost; AArch64_reg_access() de-duplicates the restored operand against the shift-value read via arr_exist(), so the reported register lists do not change. The printed text is rendered from the MCInst and was never affected.
Test plan
Extends the four existing per-mnemonic regression cases (and their three duplicates) with the restored operand, and adds the 64-bit forms as new coverage.
Closing issues