uxmal / reko

Reko is a binary decompiler.
https://uxmal.github.io/reko
GNU General Public License v2.0
2.09k stars 250 forks source link

RISC-V: immedate operands are incorrectly formatted #1304

Closed gregoral closed 5 months ago

gregoral commented 7 months ago

RISC-V disassembler formats immediate operands in hex.

This is not a problem in itself, but when a value is provided in hex it should be prefixed with 0x. The generated assembly cannot be compiled until the immediate operand is specified correctly.

Example:

csrrci x30,mtvec,0000000B

This should be written as either:

csrrci x30,mtvec,0x0000000B
# or
csrrci x30,mtvec,11

Also since most immediate operands are not "full-width" and are limited in size to a couple of bits, we don't need to write 8 digits. We could reduce the constant to the number of hex digits required.

For 5 bit immediate operand we need only two hex digits max. The previous example could be written as:

csrrci x30,mtvec,0x0B

or even:

csrrci x30,mtvec,0xB

if we omit the leading zero.

I'll take a look at what can be done about it.

uxmal commented 7 months ago

Look at the other, more mature architecture implementations, like ArmInstruction or X86Instruction and their RenderOperand methods. There may be other features to consider from the RiscV assembly manual: https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md

gregoral commented 7 months ago

Hi, thank you for pointing me to similar functionality on different architectures.

Do you have any specific features to consider in mind? I see I missed at least float immediate values, which are currently in draft ( RISC-V Zfa extension ).

So I think we should consider these features:

Some examples of instructions using immediate values:

# hex
auipc   a0, 0x0
lui a0, 0x40003

# decimal integers
auipc   a0, 0
addi    a0, a0, 128

# negative values
addi    a0,a0,-273

# offset
ld  a0, 0(a0)

# float ( `Zfa` draft )
fli.s   fa0, 0x1p-15
fli.s   fa1, 0.00390625
fli.s   fa2, 6.25e-02
gregoral commented 5 months ago

This has been fixed in #1317. Closing issue.