Improper Instructions Crashing Core
Closed this issue · 2 comments
Hello, thanks for sharing this core. I was working on a class project with it, and I noticed the following...
Problem:
Invalid instructions can crash the core if they are in a format similar to the DIV and REM instructions but with improper func3 and func7 values. For example, when I fed 0x42437733 or 0x3384F333 as inputs, the program fell into the ALU section and executed the REMU instruction. At this point, I get a floating point error in my program from calculating Reg % 0 at line 598 in rv.c when one or both of the registers is zero.
Solution:
This one is up to you. It could be fixed with a bit extra decoding to check proper func3 and func7 values. Just having the % operation be the part of the else statement was the only real danger.
Thanks
Hi, apologies for taking so long to respond. I don't have a good handle on GitHub notifications and only saw this about an hour ago.
This bug was actually kind of tricky to fix. The RISC-V M Extension defines the rem and remu instructions as evaluating to the dividend if the divisor is 0. Coincidentally, the ARM64 instruction set, which I develop this project on, defines its udiv instruction to do the same thing. ARM64 has no modulus instruction, so it defines unsigned modulus in terms of unsigned division, so in rv, the remu instruction was actually working as the RISC-V spec intended.
On x86-64, which I assume you are on because you mentioned a floating point exception, dividing by zero (using the IDIV instruction) causes an exception (and eventually a SIGFPE). Here's a Godbolt link demonstrating this.
Notably, this would actually work improperly with all modulus by zero, not just ones with invalid encodings.
Your suggested fix was spot on. The extra decoding correctly handles those cases.
Thank you for reaching out, and for the well-written issue! I am curious about the class project you are doing, especially how you are using the core, and would like to hear more if you're willing to share.
Hey, thanks for the response. That's interesting about the ARM vs x86 differences, that never crossed my mind. As for the class project, I was using a fuzzer on your simulator to see if I could find any crashes. I was just using AFL++ with the built-in random mutation. My research is in hardware verification / fuzzing, so a lot of the fuzzing techniques from that field can be applied to verifying ISA simulators as well. If you want a super robust simulator, you can always set up a differential fuzzer to compare you simulator's outputs (registers, PC, memory) to those from Spike or one of the other RISC-V ISA simulators.