search

petrpavlu / valgrind-riscv64

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

[WIP]riscv64: Fill in TODOs in sigframe-riscv64-linux.c #7

Closed laokz closed 2 years ago

laokz commented 2 years ago

Add save/restore float point registers.

There is one thing that I am not very clear. Following your template, I use VG_TRACK(copy_reg_to_mem/copy_mem_to_reg) to save/restore 'V bits' and 'otag' after updating the values. This looks correct to me that when we save/restore values, we also need to save/restore their mem-check status. But I also note that, all other arches don't use these functions. Instead, they just use VG_TRACK(post_mem_write) when do save operations, and they don't touch 'otag' at all. The VG_TRACK(post_mem_write) just simply write registers 'V bits' as DEFINED. Confused me, wish you can help. Thanks.

petrpavlu commented 2 years ago

Add save/restore float point registers.

LGTM, thanks. Note that I have been recently working on adding tests for saving/restoring integer and floating-point registers. I should push them soon, just mentioning it in case you planned to write similar tests so we don't duplicate work.

There is one thing that I am not very clear. Following your template, I use VG_TRACK(copy_reg_to_mem/copy_mem_to_reg) to save/restore 'V bits' and 'otag' after updating the values. This looks correct to me that when we save/restore values, we also need to save/restore their mem-check status. But I also note that, all other arches don't use these functions. Instead, they just use VG_TRACK(post_mem_write) when do save operations, and they don't touch 'otag' at all. The VG_TRACK(post_mem_write) just simply write registers 'V bits' as DEFINED. Confused me, wish you can help. Thanks.

Most architecture+OS pairs supported by Valgrind store the two shadow memory areas directly on the signal stack. This is a bit of a hack and described as such in the code, for example, coregrind/m_sigframe/sigframe-amd64-linux.c contains:

   /* XXX This is wrong.  Surely we should store the shadow values
      into the shadow memory behind the actual values? */
   VexGuestAMD64State vex_shadow1;
   VexGuestAMD64State vex_shadow2;

When the Solaris port was added to Valgrind, it implemented this correctly by introducing the mentioned copy_reg_to_mem and copy_mem_to_reg tool callbacks. Unfortunately, older ports have not been adjusted in the same way, but the RISC-V port follows the newer more precise implementation.

laokz commented 2 years ago

LGTM, thanks. Note that I have been recently working on adding tests for saving/restoring integer and floating-point registers. I should push them soon, just mentioning it in case you planned to write similar tests so we don't duplicate work.

Got it.

Most architecture+OS pairs supported by Valgrind store the two shadow memory areas directly on the signal stack. This is a bit of a hack and described as such in the code, for example, coregrind/m_sigframe/sigframe-amd64-linux.c contains:

   /* XXX This is wrong.  Surely we should store the shadow values
      into the shadow memory behind the actual values? */
   VexGuestAMD64State vex_shadow1;
   VexGuestAMD64State vex_shadow2;

When the Solaris port was added to Valgrind, it implemented this correctly by introducing the mentioned copy_reg_to_mem and copy_mem_to_reg tool callbacks. Unfortunately, older ports have not been adjusted in the same way, but the RISC-V port follows the newer more precise implementation.

Yes, I see. Thank you very much!