Switch to capstone 5.0 #3653

wargio · 2023-07-11T04:54:13Z

DO NOT SQUASH!!!

Your checklist for this pull request

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the rizin book with the relevant information (if needed)

Detailed description

I have hand-verified all the instructions (encoding/decoding) using objdump

meson_options.txt

wargio · 2023-07-12T14:41:45Z

no, is likely missing some enums.

test/db/analysis/ppc

test/db/analysis/x86_32

test/db/analysis/x86_64

test/db/asm/x86_16

test/db/esil/arm_64

test/db/formats/mach0/imports

test/db/formats/mach0/objc

test/db/rzil/ppc32

test/db/tools/rz

Co-authored-by: wargio <wargio@libero.it>

wargio · 2023-07-13T12:05:40Z

not existed in v3 ?

i added some stuff, and i forgot to add the compiler fences.

Rot127

Only looked at the PPC part but looks good to me. ~~Although we should probably fix those bugs you mention in CS and make a v5.1.~~ Actually no. Lets just push for v6 as much as possible. Please add the comments about the bugs for easier comprehension nonetheless.

librz/analysis/arch/ppc/ppc_il.c

librz/analysis/arch/ppc/ppc_il_ops.c

Rot127 · 2023-07-14T05:07:32Z

I can not check it currently. But, if I recall correctly, the register was not added to the operand list of `cs_insn` if it was zero (only in v5. In v6 it is always added). But, because of the missing operand, the disponent is treated as register here and can give a valid result, right? Please add a test case, for an instruction with a base register set to 0 and a disponent of a range like `1-PPC_REG_ENDING`. 14 Jul 2023 05:12:17 Giovanni ***@***.***>:

…

***@***.**** commented on this pull request. ---------------------------------------- In librz/analysis/arch/ppc/ppc_il_ops.c[#3653 (comment)]: > @@ -255,13 +256,13 @@ static RzILOpEffect *store_op(RZ_BORROW csh handle, RZ_BORROW cs_insn *insn, con case PPC_INS_STWCIX: case PPC_INS_STDCIX: #endif - base = VARG(rA); + base = rA ? VARG(rA) : NULL; tbh i don't know if it is a bug or else. From the disassembly prospective, i disassembled these using objdump and it reported the same instruction. but essentially instead of a register you can have *zero* as base (thus rA is NULL) — Reply to this email directly, view it on GitHub[#3653 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AK5ET6EYLLEOBERLM2FWCO3XQC2I7ANCNFSM6AAAAAA2FNVK4A]. You are receiving this because you were mentioned.[Tracking image][https://github.com/notifications/beacon/AK5ET6C5JAMYZCV36W5G5GLXQC2I7A5CNFSM6AAAAAA2FNVK4CWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTS3FLFQA.gif]

wargio · 2023-07-14T08:08:08Z

But, because of the missing operand, the disponent is treated as register here and can give a valid result, right?

According to the ISA, when 0 is actually 0, not R0

wargio · 2023-07-14T08:09:57Z

This is a test for one of those NULL cases.
e8000002 ppc32le lwa r0, 0(0)

Rot127 · 2023-07-14T15:48:19Z

Sorry, I would test it myself, but do not have access to my laptop currently. If the immediate is 0 it would return NULL (because `0 = REG_INVALID` so no reg name). Could you please add a test case like `

lwa r0, 0(4)`

. 14 Jul 2023 10:10:11 Giovanni ***@***.***>:

…

This is a test for one of those *NULL* cases. *e8000002* ppc32le *lwa r0, 0(0)* — Reply to this email directly, view it on GitHub[#3653 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AK5ET6EAYP2CSWGHC6K63GTXQD5GBANCNFSM6AAAAAA2FNVK4A]. You are receiving this because you were mentioned.[Tracking image][https://github.com/notifications/beacon/AK5ET6HSW6C7LLS7GQAERTLXQD5GBA5CNFSM6AAAAAA2FNVK4CWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTBPNKAG.gif]

wargio · 2023-07-14T16:11:41Z

lwa r0, 0(4) should be lwa r0, 0(r4) => e8040002
e9000012 => lwa r8, 0x10(0)

https://www.ibm.com/docs/en/aix/7.1?topic=set-lwa-load-word-algebraic-instruction

Rot127 · 2023-07-15T06:44:05Z

@wargio Checked it on my machine. I was confused, so ignore my comments above. But thanks for making the effort to go into it.

XVilka

Please squash some commits so that we have cleaner history and commit marks any test as BROKEN. See my other comments too. Except that is LGTM. Some of these numerous hacks will likely be gone once auto-sync PRs are merged.

librz/asm/p/asm_x86_cs.c

test/db/analysis/x86_64

Co-authored-by: wargio <wargio@libero.it>

XVilka

LGTM once Woodpecker is fixed - it doesn't look like a caching issue. MacOS zeroes buffers on initialization, maybe it is the reason?

XVilka · 2023-07-16T03:51:56Z

Opened an issue for this test here: #3658

wargio changed the title ~~Switch to capstone5~~ Switch to capstone 5.0 Jul 11, 2023

github-actions bot added documentation Improvements or additions to documentation infrastructure rz-test RzAnalysis API X86 RzAsm-Disassembler labels Jul 11, 2023

ret2libc reviewed Jul 11, 2023

View reviewed changes

meson_options.txt Outdated Show resolved Hide resolved

XVilka added this to the 0.6.0 milestone Jul 11, 2023

XVilka added the high-priority label Jul 11, 2023

github-actions bot added the PPC label Jul 12, 2023

This comment was marked as resolved.

Sign in to view

XVilka requested a review from Rot127 July 12, 2023 13:08

This comment was marked as resolved.

Sign in to view

wargio force-pushed the fuzz-capstone5 branch from 046d345 to c992d88 Compare July 13, 2023 03:17

github-actions bot added the ARM label Jul 13, 2023

wargio force-pushed the fuzz-capstone5 branch from ccd5485 to ded509b Compare July 13, 2023 03:27

wargio marked this pull request as ready for review July 13, 2023 03:27

wargio requested review from thestr4ng3r and kazarmy as code owners July 13, 2023 03:27

wargio requested review from XVilka and ret2libc July 13, 2023 03:28

wargio force-pushed the fuzz-capstone5 branch from ded509b to 0e32744 Compare July 13, 2023 03:34

This comment was marked as resolved.

Sign in to view

XVilka requested changes Jul 13, 2023

View reviewed changes

yossizap and others added 3 commits July 13, 2023 11:43

Set the default capstone version to v5 instead of v4

ac94d70

Co-authored-by: wargio <wargio@libero.it>

Update capstone versions in the docs

b4b6351

Update capstone instruction ids

d749c5d

wargio force-pushed the fuzz-capstone5 branch from 0e32744 to 7cc55d7 Compare July 13, 2023 03:45

This comment was marked as resolved.

Sign in to view

Rot127 reviewed Jul 13, 2023

View reviewed changes

librz/analysis/arch/ppc/ppc_il.c Show resolved Hide resolved

librz/analysis/arch/ppc/ppc_il_ops.c Show resolved Hide resolved

librz/analysis/arch/ppc/ppc_il_ops.c Show resolved Hide resolved

wargio force-pushed the fuzz-capstone5 branch from 8f30a13 to e82b656 Compare July 15, 2023 04:02

github-actions bot added the RzParse label Jul 15, 2023

wargio force-pushed the fuzz-capstone5 branch from e6dba8b to a7726d9 Compare July 15, 2023 09:18

wargio requested a review from XVilka July 15, 2023 09:19

XVilka requested changes Jul 15, 2023

View reviewed changes

librz/asm/p/asm_x86_cs.c Show resolved Hide resolved

test/db/analysis/x86_64 Show resolved Hide resolved

XVilka mentioned this pull request Jul 15, 2023

CAPSTONE-NG (AFTER 5.0) #3564

Closed

5 tasks

imbillow and others added 3 commits July 15, 2023 21:20

Fix tests for capstone v3/v4/v5

306090a

Co-authored-by: wargio <wargio@libero.it>

Improve sparc analysis and fix tests.

c7268dd

Fix v3 build due fall-thru

9b86fb7

wargio force-pushed the fuzz-capstone5 branch from a7726d9 to f538815 Compare July 15, 2023 13:20

XVilka approved these changes Jul 15, 2023

View reviewed changes

XVilka mentioned this pull request Jul 15, 2023

Capstone 5.0 void-linux/void-packages#44967

Merged

Mark as broken the ARM 16 test which fails to disassemble.

c3e309c

wargio force-pushed the fuzz-capstone5 branch from f538815 to c3e309c Compare July 16, 2023 03:43

XVilka added the merge-when-green label Jul 16, 2023

XVilka merged commit 99c5f83 into dev Jul 16, 2023
43 checks passed

XVilka deleted the fuzz-capstone5 branch July 16, 2023 05:22

XVilka mentioned this pull request Jul 17, 2023

Support building with capstone v5.0 release #3652

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to capstone 5.0 #3653

Switch to capstone 5.0 #3653

wargio commented Jul 11, 2023 •

edited by XVilka

Loading

This comment was marked as resolved.

This comment was marked as resolved.

wargio commented Jul 12, 2023

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

wargio commented Jul 13, 2023 •

edited

Loading

Rot127 left a comment •

edited

Loading

Rot127 commented Jul 14, 2023 via email

wargio commented Jul 14, 2023

wargio commented Jul 14, 2023

Rot127 commented Jul 14, 2023 via email

wargio commented Jul 14, 2023 •

edited

Loading

Rot127 commented Jul 15, 2023

XVilka left a comment •

edited

Loading

XVilka left a comment

XVilka commented Jul 16, 2023

Switch to capstone 5.0 #3653

Switch to capstone 5.0 #3653

Conversation

wargio commented Jul 11, 2023 • edited by XVilka Loading

DO NOT SQUASH!!!

This comment was marked as resolved.

This comment was marked as resolved.

wargio commented Jul 12, 2023

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

wargio commented Jul 13, 2023 • edited Loading

Rot127 left a comment • edited Loading

Choose a reason for hiding this comment

Rot127 commented Jul 14, 2023 via email

wargio commented Jul 14, 2023

wargio commented Jul 14, 2023

Rot127 commented Jul 14, 2023 via email

wargio commented Jul 14, 2023 • edited Loading

Rot127 commented Jul 15, 2023

XVilka left a comment • edited Loading

Choose a reason for hiding this comment

XVilka left a comment

Choose a reason for hiding this comment

XVilka commented Jul 16, 2023

wargio commented Jul 11, 2023 •

edited by XVilka

Loading

wargio commented Jul 13, 2023 •

edited

Loading

Rot127 left a comment •

edited

Loading

wargio commented Jul 14, 2023 •

edited

Loading

XVilka left a comment •

edited

Loading