Implement reg_{read,write}2 API #1835
Conversation
|
Look good to me in general though ideally your fixes and code format should be splitted into smaller commits. The work is pretty nice and complete! However, we also need to port this to new bindings like python, rust and your java bindings. But this is out of this PR's scope and I don't have further concerns. |
I think that we would not expose these bindings to Python or Java directly (maybe Rust though). Rather, we would just have Python/Java always call the new APIs for safety, since they will always know how big their input buffers are. |
Yes, that's my point. We should further update bindings to use the new API. |
|
Any news on this PR? |
Rushing on a conference deadline ;( Will have a look once getting it done. |
|
I'm done with my deadlines and will soon start to review your two PRs (and probably other stale ones XD). So far, for this one, I don't see any further problems. Thanks in advance! |
These APIs take size parameters, which can be used to properly bounds-check the inputs and outputs for various registers. Additionally, all backends now throw UC_ERR_ARG if the input register numbers are invalid. Completes unicorn-engine#1831.
This also comes with a performance bump due to inlining of reg_read/reg_write (as they're only called once now) and the unlikely() on CHECK_REG_TYPE.
Single reg_read/reg_write is now about 25% faster.
RISCV FP registers are 64-bit in size, even in 32-bit mode, because they can hold doubles. The test even uses the double-precision instruction fmv.d. Thus, the reads should be reading 64-bit registers.
|
Rebased to latest dev, and changed |
|
Merged, thanks for your big contributions! |
This code was commented out since 2021, but by default, the error code was initialized to `UC_REG_OK`, so there was no error returned until unicorn-engine#1835, where this was changed to be initialized to `UC_REG_ERR_ARG`. As a result, any write to `UC_ARM_REG_C1_C0_2` returned an error.
As suggested in #1831, this patch implements a "version 2" API for register read/write which takes a size parameter. Context and batch operations are all supported.
This patch also comes with some performance optimizations and some (slightly opinionated) refactoring. The performance improvement appears to be about 20-25% for a single read/write operation or about 10% for a batch operation of size 8, despite the addition of the size parameter.
As a side benefit, the register APIs now produce an appropriate error if an invalid (unknown) register is used.
Fixes #1831.