Skip to content

Lifting the base (integer) extension of RISC-V, some compressed variants of integer instructions, and some multiplication/division instructions#6287

Open
moste00 wants to merge 9 commits into
rizinorg:devfrom
moste00:feature/riscv_lifting_base
Open

Lifting the base (integer) extension of RISC-V, some compressed variants of integer instructions, and some multiplication/division instructions#6287
moste00 wants to merge 9 commits into
rizinorg:devfrom
moste00:feature/riscv_lifting_base

Conversation

@moste00

@moste00 moste00 commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

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 every RZ_API function and struct this PR changes.
  • I've added tests that prove my changes are effective (required for changes to RZ_API).
  • I've updated the Rizin book with the relevant information (if needed).
  • I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

...

Test plan

...

Closing issues

PR Stack Structure

This PR is the first of a 4-PR stack that lifts the IMAFD RISC-V archiecture plus some privileged instructions.

The following is a table for ease of navigation from any PR to any other, please filter by the commit(s) mentioned in the table when reviewing the diff in the "Files Changed" tab to avoid the noise of the base branches.

Extension PR Commits To Select in "Files Changed" State (Open, Merged, Draft, Closed)
I #6287 < (THIS PR) No filtering necessary O
M #6364 "implement the rest of the multiplication extension" D
A #6373 "implement the atomic extension trivially by ignoring concurrency guards" D
F/D #6374 "add floating and double instructions, and add privlieged ISA instructions like reading CSRs to implement fflags reading and writing" D

Because it's painful to maintain this stack manually with no Github or other tooling support, I won't grow it any further until one of those 4 PRs gets merged.

Depends-on: rizinorg/rz-tracetest#29
Depends-on: BinaryAnalysisPlatform/qemu#44

@moste00

moste00 commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

@notxvilka for visiblity, not ready for merge.

Next: Adapt the code to work for 64-bit arch as well as 32-bit, and test against machine traces from BAP QEMU.

@moste00
moste00 force-pushed the feature/riscv_lifting_base branch 2 times, most recently from 1e2a880 to 119d814 Compare May 1, 2026 00:23
@moste00
moste00 force-pushed the feature/riscv_lifting_base branch from 119d814 to 085c2ae Compare May 9, 2026 22:53
@moste00
moste00 marked this pull request as ready for review May 9, 2026 23:11
@moste00
moste00 force-pushed the feature/riscv_lifting_base branch from 085c2ae to 2e32ca1 Compare May 10, 2026 00:17
@moste00

moste00 commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

@notxvilka @Rot127 @wargio this is ready for a look btw, it's mostly complete but I'm fighting an issue with some compressed instruction decoding (capstone bug)

@moste00
moste00 force-pushed the feature/riscv_lifting_base branch from 33626ac to 36fa429 Compare May 14, 2026 21:46
Comment thread librz/arch/isa/riscv/riscv_il.c Outdated
Comment thread librz/arch/isa/riscv/riscv_il.h Outdated
Comment thread librz/arch/isa/riscv/riscv_il_base.h Outdated
Comment thread librz/arch/isa/riscv/riscv_il_base.h Outdated
Comment thread librz/arch/p/analysis/analysis_riscv_cs.c
@moste00

moste00 commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

NOTE-1: I initially intended this to only cover integer (rv32I and rv64I) subsets of the ISA, but in order to enable real-world testing on gcc-compiled binaries I had to support compressed subsets, so this PR is less orthogonal than I would like, there is even a couple of multiplication and division instructions in there.

Sorry. This is annoying but trying to link against libc from non-IMAFD binaries is even more annoying. I will try to keep future extensions as self-contained as possible.


NOTE-2:
The architecture of the lifter is that each unique instruction has exactly one lifter function named after it. This enables a single-level dispatch table in the entrypoint riscv_il.c where every instruction is listed by its capstone ID next to its lifter function. (As opposed to (A) two-level dispatch where a first-stage table maps a family of instructions, say all branches or all I-type, to a single lifting function then a switch inside that function decides the logic for each individual instruction. (B) Or a non-modular single-level giant switch over every RISC-V instruction in the same function, which would be bad for readability and correctness).

In order to modularize, each ISA extension is seperated into a different .h file. In order to avoid boilerplate, the common prelude file riscv_il_base.h defines several macros that allow you to define the lifter function concisely by specifying only 3 things:

1- The lifting function name
2- The function decoder, the decoder is the name of a macro that exctracts source and destination variables from the capstone operands
3- The logic of the instruction as a RzIL expression.

For example, DEFINE_LIFTER(addi, DECODE_RD_RS_IMM, ADD(rs, imm))

1- addi is the name of the lifter function, it's used later in the master dispatch table in riscv_il.c
2- DECODE_RD_RS_IMM is the name of the decoder macro which will extract rd, rs, and imm from the operand array in capstone
3- ADD(rs, imm) is the RzIL expression which will be inserted into an assignment to the destination register by the DEFINE_LIFTER macro (unless the destination register is x0, in which case the instruction is a NOP. This is handled by register-setter and the register-getter macros, RISCV_GET_REG and RISCV_SET_REG`)

This architecture allows arbitrary cross-sharing of decoding logic with any instruction instead of hardcoding it into a specific extension or instruction group. There is also DEFINE_ALIAS_LIFTER which will define a static const pointer that points to the lifter function of another instruction.

@moste00
moste00 force-pushed the feature/riscv_lifting_base branch 3 times, most recently from a357f47 to db4c13b Compare May 21, 2026 21:30

@Rot127 Rot127 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I checked it too late.

Comment thread librz/arch/isa/riscv/riscv_il_base.h
@moste00
moste00 force-pushed the feature/riscv_lifting_base branch 2 times, most recently from 1208ba9 to c6f4465 Compare June 16, 2026 22:03
@moste00

moste00 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

@Rot127 @wargio Ping

@moste00
moste00 force-pushed the feature/riscv_lifting_base branch from 20ea324 to f50a6d6 Compare June 20, 2026 19:51
@moste00 moste00 changed the title lifting the base (integer) extension of RISC-V and some compressed variants of the instructions in it for 32-bit arch variant only, and add some tests lifting the base (integer) extension of RISC-V, some compressed variants of integer instructions, and some multiplication/division instructions Jun 20, 2026
@moste00 moste00 changed the title lifting the base (integer) extension of RISC-V, some compressed variants of integer instructions, and some multiplication/division instructions Lifting the base (integer) extension of RISC-V, some compressed variants of integer instructions, and some multiplication/division instructions Jun 20, 2026

@Rot127 Rot127 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please finish those ones before continuing to work on the others.

There are still a few failing and some leaks.

Comment thread librz/arch/isa/riscv/riscv_il.c Outdated
Comment thread librz/arch/isa/riscv/riscv_il_integer.h
Comment thread librz/arch/isa/riscv/riscv_il_base.h Outdated
Comment thread test/db/asm/riscv_32
Comment thread librz/arch/isa/riscv/riscv_il_compressed.h
Comment thread librz/arch/isa/riscv/riscv_il_base.h
Comment thread librz/arch/isa/riscv/riscv_il_integer_reg_names.h Outdated
Comment on lines +76 to +90
RZ_LOG_ERROR("op_str: %s\n", insn->op_str);
RZ_LOG_ERROR("need_effective_addr: %d\n", insn->detail->riscv.need_effective_addr);
RZ_LOG_ERROR("op_count: %u\n", insn->detail->riscv.op_count);
for (int i = 0; i < insn->detail->riscv.op_count; i++) {
RZ_LOG_ERROR("operands[%d].type: %d\n", i, insn->detail->riscv.operands[i].type);
if (insn->detail->riscv.operands[i].type == RISCV_OP_REG) {
RZ_LOG_ERROR(" REG = %d\n", insn->detail->riscv.operands[i].reg);
} else if (insn->detail->riscv.operands[i].type == RISCV_OP_IMM) {
RZ_LOG_ERROR(" IMM = 0x%" PFMT64x "\n", (ut64)insn->detail->riscv.operands[i].imm);
} else if (insn->detail->riscv.operands[i].type == RISCV_OP_MEM) {
RZ_LOG_ERROR(" MEM base = %d, disp = 0x%" PFMT64x "\n", insn->detail->riscv.operands[i].mem.base, (ut64)insn->detail->riscv.operands[i].mem.disp);
}
}
}

@Rot127 Rot127 Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please remove this logging function before merging.

@Rot127

Rot127 commented Jun 21, 2026

Copy link
Copy Markdown
Member

Ouh, and please add a test which prints instructions with aoi and check no errors are thrown.
For 32 and 64 bit binaries.

Because there are already a few:

ERROR: op_str: s6, 0x10
ERROR: need_effective_addr: 0
ERROR: op_count: 2
ERROR: operands[0].type: 1
ERROR:   REG = 64
ERROR: operands[1].type: 2
ERROR:   IMM = 0x10
Invalid instruction of lifting not implemented.

@moste00
moste00 force-pushed the feature/riscv_lifting_base branch from 16bdfd8 to 3275ee2 Compare July 17, 2026 20:42

@Rot127 Rot127 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The branch instructions leak:

=================================================================
==34077==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x75d3c0ae68a3 in calloc (/lib64/libasan.so.8+0xe68a3) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
    #1 0x75d3bb244749 in rz_il_op_new_bitv_from_ut64 ../librz/il/il_opcodes.c:180
    #2 0x75d3b39571d3 in riscv_il_get_reg ../librz/arch/isa/riscv/riscv_il_base.h:17
    #3 0x75d3b3959a58 in rz_riscv_lift_c_beqz ../librz/arch/isa/riscv/riscv_il_compressed.c:69
    #4 0x75d3b393d8ad in rz_riscv_lift_instr ../librz/arch/isa/riscv/riscv_il.c:152
    #5 0x75d3b3f3d9c9 in analyze_op ../librz/arch/p/analysis/analysis_riscv_cs.c:1929
    #6 0x75d3b408cad1 in rz_analysis_op ../librz/arch/op.c:126
    #7 0x75d3c067d960 in print_disassembly_output ../librz/main/rz-asm.c:331
    #8 0x75d3c068393e in rz_main_rz_asm ../librz/main/rz-asm.c:896
    #9 0x0000004007ad in main ../binrz/rz-asm/rz-asm.c:8
    #10 0x75d3bcc105b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4) (BuildId: 88c03969b4319fc0140734fe863825ec4a1cdb0d)
    #11 0x75d3bcc10667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667) (BuildId: 88c03969b4319fc0140734fe863825ec4a1cdb0d)
    #12 0x0000004006c4 in _start (/home/user/.local/bin/rz-asm+0x4006c4) (BuildId: 7e9c8cb46628eb0719490f7b5b48ea6d6bb3e8b9)

Indirect leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x75d3c0ae68a3 in calloc (/lib64/libasan.so.8+0xe68a3) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
    #1 0x75d3bd61a391 in rz_bv_new ../librz/util/bitvector.c:101
    #2 0x75d3bd627c56 in rz_bv_new_from_ut64 ../librz/util/bitvector.c:1747
    #3 0x75d3bb244725 in rz_il_op_new_bitv_from_ut64 ../librz/il/il_opcodes.c:176
    #4 0x75d3b39571d3 in riscv_il_get_reg ../librz/arch/isa/riscv/riscv_il_base.h:17
    #5 0x75d3b3959a58 in rz_riscv_lift_c_beqz ../librz/arch/isa/riscv/riscv_il_compressed.c:69
    #6 0x75d3b393d8ad in rz_riscv_lift_instr ../librz/arch/isa/riscv/riscv_il.c:152
    #7 0x75d3b3f3d9c9 in analyze_op ../librz/arch/p/analysis/analysis_riscv_cs.c:1929
    #8 0x75d3b408cad1 in rz_analysis_op ../librz/arch/op.c:126
    #9 0x75d3c067d960 in print_disassembly_output ../librz/main/rz-asm.c:331
    #10 0x75d3c068393e in rz_main_rz_asm ../librz/main/rz-asm.c:896
    #11 0x0000004007ad in main ../binrz/rz-asm/rz-asm.c:8
    #12 0x75d3bcc105b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4) (BuildId: 88c03969b4319fc0140734fe863825ec4a1cdb0d)
    #13 0x75d3bcc10667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667) (BuildId: 88c03969b4319fc0140734fe863825ec4a1cdb0d)
    #14 0x0000004006c4 in _start (/home/user/.local/bin/rz-asm+0x4006c4) (BuildId: 7e9c8cb46628eb0719490f7b5b48ea6d6bb3e8b9)

SUMMARY: AddressSanitizer: 72 byte(s) leaked in 2 allocation(s).

Also many others, please run the asm tests with LSAN

@Rot127

Rot127 commented Jul 18, 2026

Copy link
Copy Markdown
Member

Also:

rz-test test/db/asm/riscv_64
Running rz-test on linux-x64
Running from /home/user/repos/rizin_wt_I/test
Loaded 308 tests.
[XX] 00h00m03s817ms /home/user/repos/rizin_wt_I/test/db/asm/riscv_64 <asm> jal 2
-- <asm> jal 2 <--- 0920 ---> <IL>
-- disassembly
--- expected
+++ actual
@@ -1,1 +1,1 @@
-jal 2
+invalid

-- IL
--- expected
+++ actual
@@ -1,1 +1,0 @@
-(seq (set ra (bv 32 0x2)) (jmp (bv 32 0x2)))


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants