rizinorg / rizin

UNIX-like reverse engineering framework and command-line toolset.
https://rizin.re
GNU Lesser General Public License v3.0
2.66k stars 357 forks source link

MIPS -- add fp register; support for lwl, beqz and sltiu instructions #4513

Open matteius opened 4 months ago

matteius commented 4 months ago

Your checklist for this pull request

Detailed description

Also two quality of life improvements:

Test plan

I am using this code to insert instructions via cutter, and so I get console errors whenever I encounter something that cannot be assembled.

Closing issues

Maybe closes #4509 assuming the tests get run by default by existing in that directory.

matteius commented 4 months ago

Thanks for taking a look @wargio and @XVilka -- I didn't quite understand:

If you want to check both disassembly and assembly, you will need to use da in asm tests

XVilka commented 4 months ago

Lines should start not from d instruction but from da instruction, then it will check the both ways - disassembly and assembly.

wargio commented 4 months ago

For clarity, please check also https://github.com/rizinorg/rizin/tree/dev/test#writing-assembly-tests

matteius commented 4 months ago

I will look into the test failures more later on when I have more time, thanks!

wargio commented 4 months ago

looks like an endianness issue.

wargio commented 4 months ago

i'm ok for the assembly

XVilka commented 4 months ago

@matteius could you please convert "da" to just "a", so that only assembly is checked, and we will merge if tests pass?Note though, some assembly tests also produce errors, please take a look at these failure as well.

matteius commented 3 months ago

I expect the tests will fail again; I only changed to test just assembly, but I spent a bit of time trying to run the tests locally and couldn't. I was able to build rizin, and then I did:

(base) matteius@matteius-3-0:~/rizin/test$ ../build/binrz/rz-test/rz-test db/esil/mips_32 
...
exec: Permission denied

I tried also as sudo, but that gets that exec is not found.

wargio commented 1 week ago

since i'm updating the mips code, i can also integrate these changes into rizin.