Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RzIL MIPS Uplifting #3508

Draft
wants to merge 19 commits into
base: dev
Choose a base branch
from
Draft

Conversation

brightprogrammer
Copy link
Contributor

@brightprogrammer brightprogrammer commented May 11, 2023

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description
This PR is related to my GSoC 2023 summer project on MIPS uplifting. To view table of uplifted instructions, one can go here

Corresponding BAP's QEMU PR (for rz-tracetest) is BinaryAnalysisPlatform/qemu#26

TODO

  • JUMP instruction family behaves differently for MIPS and microMIPS. First complete MIPS32 then add support for some "else" cases to handle microMIPS cases too.
  • Update all operations that start/end an atomic read/modify/write (RMW) operation.

Test plan

  • CI is green
  • rz-tracetest passes with a good threshold

Closing issues

Partially addresses #2080

@wargio wargio marked this pull request as draft May 11, 2023 12:54
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend also to start cleaning up and extracting ESIL uplifting in librz/analysis/p/analysis_mips_cs.c in separate functions, so that when time comes it can be removed quickly and painlessly.

Also, there are some tests in this file, that check mips.gnu, we should make sure they pass with mips.cs as well:

test/db/analysis/mips
99:e asm.arch=mips.gnu
121:e asm.arch=mips.gnu
527:e asm.arch=mips.gnu
748:NAME=mips.gnu regs
751:e asm.arch=mips.gnu

Or modified/removed

@XVilka
Copy link
Member

XVilka commented May 19, 2023

@brightprogrammer please maintain the current status/TODO in the PR description

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please start adding tests. Otherwise you have a risk of writing a lot of wrong code.

@XVilka
Copy link
Member

XVilka commented Jun 6, 2023

Please maintain the list of instructions that are not uplifted yet.

@XVilka
Copy link
Member

XVilka commented Jul 27, 2023

You can remove status_update.py now, I think, and have it locally only. It will make the CI green as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants