radareorg
Fix `USE_DS` for MIPS ##emu
- [ ] Mark this if you consider it ready to merge
- [ ] I've added tests (optional)
- [ ] I wrote some lines in the book (optional)
Description
This PR is mostly a question on how one should handle branch delay slots when emulating ESIL.
For context, I've been working on adding MIPS support to radius2, a symbolic execution engine for ESIL. One problem was handling of branch delay slots, at least until I noticed that everything I needed from r2 was already there. It's just hidden behind the USE_DS #define. With this define active the delay-slot behavior is explicit in the ESIL of an instruction via the (SETD and SETJT directives). Given this information the emulation is pretty straight forward.
The included patches fix the compilation with USE_DS=1 and correct a few errors where instructions without a delay slot (compact instructions) were treated incorrectly as having a delay slot. There shouldn't be any change for USE_DS=0 builds. The patch is incomplete in that "likely" instructions, where the delay slot is only executed if the branch is taken, are still incorrect. Fixing this by encoding the behavior in the ESIL string would be pretty straight-forward.
However, if I understand it correctly the USE_DS way of embedding delay slot behavior in the ESIL string is legacy (code doesn't even compile + all the errors in instruction semantics).
This brings me to my question: What is the intended way for external tools that want to work with ESIL to learn about and handle delay slots? With the information that I currently get via pdj after analyzing a function I cannot handle them properly. I am at least missing information about: how many delay slot has this instruction?, are the delay slots always executed or is there some condition (like the branch being taken for "likely" instructions)?.
[If there is no "intended" way to get this information, it would be OK for me to rebuild r2 with the define enabled as part of my build process, but then it would be nice to have a way to enable USE_DS via the build system.]
Cheers
I made the discussed changes (only to the Capstone backend for now). In short:
- There is now only one compile time version of the ESIL for each instruction. Its the one that you would have gotten with
USE_DS = 1before. I removedUSE_DS. - We no longer use
$$and then replace, but rather construct the full string in one go usinginsn->address. - I fixed the semantics for the "likely" versions of instructions, e.g.,
MIPS_INS_BNEL. - I fixed the semantics for some conditional branch instructions, e.g.,
MIPS_INS_BLEZC.
Before I apply the same changes to the GNU backend I think it would make sense if we check whether they work out nicely with the rest of the code. Will start by looking at the tests I guess.
Sorry I was afk busy in a conference and didnt had time to do a proper review.
Yes, take a look at the failing tests. all the changes you did looks good to me, but im unsure about the behaviour change, so better check in detail to not break any test before merging.
ping
Yea, I got side-tracked with other stuff, so nothing happened here ... but still got this one on my list of things to do :)
Good! We are in abi breaking season now and i want @condret to be aware of this pr so your work dont overlap