-
-
Notifications
You must be signed in to change notification settings - Fork 361
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
PPC uplifting #2823
PPC uplifting #2823
Conversation
20d7d01
to
589ed42
Compare
* Mainly adds vector and float registers. As well as some control and system registers.
Uplifts: * Branch instructions * Fixed point instructions * Memory load/stores
* Write cache needs to get flushed so load and store tests don't share the same memory.
@Rot127 @wargio I think if such simplification is planned, it's better to do it in a separate PR; also easier to review. If no major objections to this - I suggest merging this as is and continuing to improve separately. Moreover, it will benefit from stuff like CodeQL, Coverity, fuzzing tests, etc that happen on the |
60d693e
to
1ac86d5
Compare
efac3af
to
0b8c56c
Compare
i will optimize those checks later, in another PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few valid instructions are still missing tests:
https://app.codecov.io/gh/rizinorg/rizin/blob/ppc-rzil-uplifting-cov/librz/analysis/arch/ppc/ppc_il_ops.c
https://app.codecov.io/gh/rizinorg/rizin/blob/ppc-rzil-uplifting-cov/librz/analysis/arch/ppc/ppc_il.c
Please add asm tests for those, and perhaps also some for ppc_32.
`la` is a mnemonic for `addi`. I could not find a single occurrence in any of the test binaries of it. The assembler always resolves the `la` mnemonic into `addi`. I couldn't find a documented bit difference between `la` and `addi`. Because the instruction can not be tested it is removed.
… yet. For most set/read SPR instructions QEMU segfaults. Those which don't cause segfaults are not traced in QEMU currently. Because they are barely used anyways I removed them (instead if imeplenting the tracing in QEMU which would have cost too much time for me currently). In case of SPR 1 (xer), 8 (lr) and 9 (ctr) the assembler resolves them to their mnemonics (mtxer, mtlr etc.). This means the code here is never reached. To test the get_xer code MFXER was added again. The rz-tracetests will fail for this instructions (due to missing ca32, ov32). But this case is covert in an issue.
This should cover the uncovered branch in `get_cr_bit`
@thestr4ng3r Regarding |
Since @Rot127 addressed your comments @thestr4ng3r, I am merging. If you have any additional feedback, please open a new issue, ok? |
capstone include paths were adjusted in 2b8104b, and this wasn't picked up in the "PPC uplifting" commit, leading to a build failure with system capstone. Also fix the path of the ppc.h include, which seems to mistakenly use capstone's ppc.h when building with bundled capstone. Fixes: b46e7bd ("PPC uplifting to RzIL (rizinorg#2823)") Signed-off-by: John Helmert III <ajak@gentoo.org>
capstone include paths were adjusted in 2b8104b, and this wasn't picked up in the "PPC uplifting" commit, leading to a build failure with system capstone. Also fix the path of the ppc.h include, which seems to mistakenly use capstone's ppc.h when building with bundled capstone. Fixes: b46e7bd ("PPC uplifting to RzIL (rizinorg#2823)") Signed-off-by: John Helmert III <ajak@gentoo.org>
capstone include paths were adjusted in 2b8104b, and this wasn't picked up in the "PPC uplifting" commit, leading to a build failure with system capstone. Also fix the path of the ppc.h include, which seems to mistakenly use capstone's ppc.h when building with bundled capstone. Fixes: b46e7bd ("PPC uplifting to RzIL (#2823)") Signed-off-by: John Helmert III <ajak@gentoo.org>
capstone include paths were adjusted in 2b8104b, and this wasn't picked up in the "PPC uplifting" commit, leading to a build failure with system capstone. Also fix the path of the ppc.h include, which seems to mistakenly use capstone's ppc.h when building with bundled capstone. Fixes: b46e7bd ("PPC uplifting to RzIL (#2823)") Signed-off-by: John Helmert III <ajak@gentoo.org>
capstone include paths were adjusted in 2b8104b, and this wasn't picked up in the "PPC uplifting" commit, leading to a build failure with system capstone. Also fix the path of the ppc.h include, which seems to mistakenly use capstone's ppc.h when building with bundled capstone. Fixes: b46e7bd ("PPC uplifting to RzIL (#2823)") Signed-off-by: John Helmert III <ajak@gentoo.org>
Supersedes #2497
Overview
This PR uplifts the most common instructions of PPC32 and PPC64. Most common means almost all instructions with a occurrence of 0.001% in ~17,5 million tested (libraries for this statistic were:
libc
,libglib
,libcairo
and two or three more.)The semantics of the instructions was tested against
ppc64le
andppc32be
binaries (see test bins PR).Missing instructions
A handful instructions are not uplifted.
Due to missing operand information in Capstone v4 instructions (fixed in v5):
PPC_INS_SLDI
-> The immediate operand is missing in the instruction.PPC_INS_ISEL
-> TheBC
operand is set to a GPR. Although it should be CR register.Due to missing operand information in Capstone v5 instructions
PPC_INS_MTOCRF
,PPC_INS_MFOCRF
-> [PPC]mtocrf
,mfocrf
missesfxm
immediate capstone-engine/capstone#1903Because they were too complex to implement
PPC_INS_LWARX
: Needs some kind of "reserve address" mechanism. Also: Is disassembled toinvalid
in v4PPC_INS_LMW
(0.07163% occurrence): Iterates of GPR X to 31, where x is determined during runtime -> needs a new RZIL effect.Because register
ca32
,ov32
are not implemented (see TODO list below):PPC_INS_MFXER
-> Copies a value fromxer
to agpr
register. QEMU and Rizin have a mismatch in register content here. The adapter can only change the value of thexer
register. Not thegpr
register. Hence this instruction fails in rz-tracetest.Others
Not implemented 0.001% instructions
After merge
After this PR is merged issues about the missing features should be opened:
dcbz
instruction zeros a cache line/block. The size of this line depends on the PPC implementation (ESIL hardcodes it to 128. RZIL to 32). It should be replaced with a plugin config option once RzArch is done.Also, if this PR is merged before RzArch, an issue should be opened.
cpu=ISA300
option and implement flagsca32
andov32
again. Mind to update the rz-tracetest adapter.libvle
disassembles VLE instructions. Since many 32bit VLE instructions match the normal instructions by operands and exeution semantics, maybe an adapter function betweenvle_t
<->cs_insn
could be added. This would allow to use the current code to be used bylibvle
as well. For the 16bit instructions additional code should be written.Relevant other PRs this relies on:
Test binaries: rizinorg/rizin-testbins#75
rz-tracetest adapter: rizinorg/rz-tracetest#6
QEMU trace: BinaryAnalysisPlatform/qemu#20