[ARMv7] Decode VFP vmov (register) instead of vmov (immediate) - #8596
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
In the VFP data-processing space (opc1 = 1x11, opc2 = 0000), opc3 = 01 is VMOV (register) and opc3 = 11 is VABS. The decoder treated opc3 = 01 as a floating-point immediate move, so `vmov.f32 s1, s2` (0xeef00a41) disassembled as `vmov.f32 d1, Vector35#2.125000`, and the f64 form reported a Q register. The lifter then wrote a constant into the wrong register. Decode opc3 = 01 with the same S/D register operands as VABS, add disassembler tests for the f32, f64 and conditional forms, and correct the lifting test that encoded the old output.
|
|
This branch has not been deployed
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
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 PR proposes decoding the ARMv7 VFP
vmov(register) form as a register move instead of a floating-point immediate move, which fixes #3986. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/746. You can sign in with your GitHub ID to claim ownership of the project.In the VFP data-processing space (opc1 = 1x11, opc2 = 0000), opc3 = 01 is VMOV (register) and opc3 = 11 is VABS; VMOV (immediate) is only the opc3 = x0 slot.
armv7_floating_point_data_processingsent opc3 = 01 down an immediate path, readingVn:Vmas an 8-bit float immediate and producing a D register for f32 (a Q register for f64). The fix gives opc3 = 01 the same S/D register operands the VABS branch already uses and only switches the operation. Because the lifter's genericREG, REGcase ofARMV7_VMOVnow receives real registers, the instruction also lifts to a register copy instead of writing a constant into an unrelated register.Reproduction on current
dev(84d2a2d), using the harness inarch/armv7/armv7_disasm/test.c(gcc -g test.c armv7.c -o test):With the change these print
vmov.f32 s1, s2,vmov.f64 d0, d1andvmoveq.f32 s0, s2.Verification: I added five cases to
armv7_disasm/test.py(f32, f64, high D registers, and a conditional form). Sincetest.pystops at its first mismatch, and 137 of its existing cases already fail ondev(branch-target formatting,dmb/dsboptions,ldc/stcoffsets and similar), I ran every case and compared the failure sets: the five new cases fail ondevand pass with the fix, and the other 137 failures are identical before and after. A sweep of all 262,144 encodings in the1110 1D11 xxxx Vd 101 sz xxxx xxxxblock showed output changing only for the 2,048 VMOV (register) encodings (opc2 = 0000, opc3 = 01, bit 4 clear), and all 2,048 now match Capstone 5 exactly. VABS and VMOV (immediate) output is unchanged.test_lift.pyhad one case built on the old decoding (ee b0 0a 60, commentedvmov.f32 d0, #2.000000, expectingLLIL_SET_REG.q(d0,LLIL_CONST.q(0x40000000))). That encoding isvmov.f32 s0, s1, so I changed its expectation toLLIL_SET_REG.d(s0,LLIL_REG.d(s1)), in the same form as the existingvsellift cases. That script needs a licensed core, so I could not run it here and worked out the expected string from the lifter source; it is worth a run on your side. I also saw the issue is assigned, so feel free to close this if a fix is already underway.How this was managed
This work was tracked as a story on a board imported from this repository's issues and pull requests (8,064 stories): https://eastagiletracker.com/projects/746/stories/773507 on the board at https://eastagiletracker.com/projects/746.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com