petrpavlu / valgrind-riscv64

Valgrind with support for the RISCV64/Linux platform.
GNU General Public License v2.0
55 stars 15 forks source link

riscv64: fix special instruction preambles #4

Closed laokz closed 2 years ago

laokz commented 2 years ago

In __SPECIAL_INSTRUCTION_PREAMBLE we modified a2 register, so have to inform compiler to avoid unexpected error.

Signed-off-by: Kai Zhang laokz@foxmail.com

petrpavlu commented 2 years ago

This is really a bug in the __SPECIAL_INSTRUCTION_PREAMBLE definition for PLAT_riscv64_linux. These preambles should be special sequences of instructions that are effectively a NOP. The current shift instructions in the riscv64 preamble should be normally replaced with rotations, same as on most other architectures supported by Valgrind. However, RV64GC doesn't have such rotate instructions so I think the correct approach here is to replace the a2 register in the preamble with zero, same as is done for MIPS.

This will require also changes in VEX/priv/guest_riscv64_toIR.c to recognize the updated sequence + the top comment in the same file should be appropriately amended.

laokz commented 2 years ago

@petrpavlu With your detailed explanation, I tried another one. Does it look right?

petrpavlu commented 2 years ago

LGTM, thanks for reporting this problem and the fix.