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.

Sign up for GitHub

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

Jump to bottom

Add RzIL for floating-point x86 instructions #3865

Merged
merged 55 commits into from
Feb 18, 2024
Merged

Add RzIL for floating-point x86 instructions #3865

merged 55 commits into from
Feb 18, 2024

Conversation

DMaroo
Copy link
Member

@DMaroo DMaroo commented Sep 15, 2023 •
edited
Loading

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

Add RzIL implementation for x86 floating point instructions. This is a WIP currently.

All the unrelated commits will go away once #3864 gets merged. Done.

Test plan

Added tests in db/asm/x86_64. Pretty much all the instructions are covered. May add more tests depending on the code coverage.

Tracetesting has been put on hold for now (see this comment), but will add the pending tracetesting in the tracker issue.

Closing issues

Closes #3861.

@DMaroo DMaroo mentioned this pull request Sep 15, 2023
Merged
5 tasks
@DMaroo DMaroo force-pushed the x86-il-floating-point branch 2 times, most recently from df8b835 to 487a2ea Compare September 16, 2023 19:14
@XVilka XVilka added the RZIL label Sep 18, 2023
@XVilka XVilka mentioned this pull request Sep 18, 2023
Open
8 tasks
@github-actions github-actions bot removed the RZIL label Oct 23, 2023
@DMaroo DMaroo self-assigned this Oct 25, 2023
@DMaroo DMaroo force-pushed the x86-il-floating-point branch from 1f596ea to 444bf1f Compare October 25, 2023 19:55
@DMaroo DMaroo force-pushed the x86-il-floating-point branch 2 times, most recently from 4d7b434 to 80679fa Compare December 16, 2023 22:06
XVilka
XVilka requested changes Dec 17, 2023
View reviewed changes
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 also recommend taking some binary and targeting FP instructions located in it. Either one already in rizin-testbins, or take/compile a new one. Then, it can be tested as early as a subset of the instructions used is implemented.

@DMaroo
Copy link
Member Author

DMaroo commented Dec 17, 2023

I'm planning to write a testing 2D matrix multiplication code for floating point numbers to test the IL. I am not done with the arithmetic FP operations, so I need to lift them first before adding such a test. Maybe some programs from here can also be used: https://benchmarksgame-team.pages.debian.net/benchmarksgame/index.html.

@DMaroo DMaroo mentioned this pull request Dec 23, 2023
Open
@DMaroo DMaroo force-pushed the x86-il-floating-point branch from 4c3f2e5 to c08110f Compare December 23, 2023 21:33
@DMaroo DMaroo force-pushed the x86-il-floating-point branch from b560deb to bc55191 Compare December 26, 2023 05:50
@DMaroo
Copy link
Member Author

DMaroo commented Jan 1, 2024

50+ x86 floating point instructions have been lifted to RzIL! This is a significant majority of the x86 floating point instructions (discount SSE, MMX, AVX and other such ISA extensions). Most of the remaining instructions are either too complicated and rare to bother implementing or use transcendental operations unsupported by RzIL or are lower-level instructions which would not have any equivalent reasonable representation in RzIL. I believe the lifted instructions are more than enough to allow for a very satisfactory usage experience.

I need to now add asm tests for all these instructions and then perform tracetesting. I am not adding any more instruction liftings until the testing of the existing liftings is over.

@github-actions github-actions bot added the X86 label Jan 2, 2024
@DMaroo DMaroo force-pushed the x86-il-floating-point branch 2 times, most recently from 1911073 to f285206 Compare January 2, 2024 18:52
@DMaroo
Copy link
Member Author

DMaroo commented Jan 2, 2024 •
edited
Loading

ASM tests pass with the existing x86 FPU instructions in db/asm. Will probably add some more instructions in the test file before moving forward with tracetesting.

@DMaroo DMaroo mentioned this pull request Jan 3, 2024
Merged
5 tasks
@DMaroo DMaroo force-pushed the x86-il-floating-point branch from 68aa3c5 to c4621c8 Compare January 3, 2024 18:45
@DMaroo DMaroo force-pushed the x86-il-floating-point branch from 43c3228 to a34b196 Compare January 5, 2024 07:51
@DMaroo DMaroo marked this pull request as ready for review January 5, 2024 07:51
DMaroo added 17 commits February 17, 2024 13:25
@DMaroo
Remove global variable use_rmode and use pass around a context instead
7f7d9a9 
    * Define a new `X86ILContext` struct to pass around information
      whether we need to intitialize the rouding mode or not
    * Remove the global variable which was previously responsible for
      doing so
@DMaroo
Add RzIL for FICOM and FICOMP, add register bindings and fix bugs
2a6d566
@DMaroo
Make rz-test more robust against IL outputs with newlines in them
b87c768
@DMaroo
Add RzIL tests in db/asm for x86 FPU instructions
bcead9f
@DMaroo
Fix the bug when using FADDP with Capstone version > 4
1f8e4ff
@DMaroo
Fix remaining db tests
e84d490
@DMaroo
Remove tests for FSTSW or FSTCW instructions
9061b13 
    * We already test `WAIT` (in `db/asm/x86_32`) and `FSTSW` (or
      `FSTCW`) (in `db/asm/x86_64`)
    * Remove the newline replacement code in `rz-test` as well, since we
      wouldn't need that functionality anymore
@DMaroo
Add asm tests for math constant push instructions
d0a53da
@DMaroo
Add RZ_IPI annotation for all the functions exposed through the header
2904923
@DMaroo
Add RZ_OWN, RZ_BORROW and RZ_NONNULL annotations
cb4f92d
@DMaroo
Add Doxygen doc for ctx argument
75e2c3c
@DMaroo
Add description for the EXEC_WITH_RMODE macro
ae94780
@DMaroo
Move the non-null check for ctx inside the valid branch
5108da2 
    * When we pop the FPU stack we pass in a `NULL` context since no
      resizing would be needed, which causes a failure if the non-null
      check for the `ctx` is outside the branch
    * Fixes failing tests in `db/asm`
@DMaroo
Minor bug fixes + make the annotations less strict than they need to be
436077f
@DMaroo
Minor bug fixes
921618d 
    * Incorrect implementation for `RET` and `CALL`
    * Swapped arguments for shift right
@DMaroo
Fix asm tests for JMP and RET instructions
8e5cf1d
@DMaroo
Fix indexing error for FXCH
0a4aa7b
@DMaroo DMaroo force-pushed the x86-il-floating-point branch from 0a3b638 to 0a4aa7b Compare February 17, 2024 07:58
@DMaroo DMaroo marked this pull request as ready for review February 17, 2024 07:58
@DMaroo DMaroo requested review from Rot127 and XVilka February 17, 2024 07:58
XVilka
XVilka requested changes Feb 17, 2024
View reviewed changes
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.

Even though there is no emulateme test, I propose merging this before the release, just mention that this support is experimental in the release notes, and will be improved in the next one. What do you think?

librz/analysis/arch/x86/common.c Outdated Show resolved Hide resolved
@XVilka XVilka requested a review from Heersin February 17, 2024 08:20
@DMaroo
Review changes
9318e02 
    * Move all unimplemented instructions to the end
    * Remove support for Capstone version less than 4
@DMaroo DMaroo requested a review from XVilka February 17, 2024 12:34
@DMaroo
Copy link
Member Author

DMaroo commented Feb 17, 2024

I agree about getting this merged. It is incorrect for some floating-point operations (due to the 80-bit floating-point shenanigans), but I'm confident that the rest of the instructions function as expected.

@XVilka XVilka merged commit de4a129 into dev Feb 18, 2024
46 checks passed
@XVilka XVilka deleted the x86-il-floating-point branch February 18, 2024 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XVilka XVilka XVilka approved these changes

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

@wargio wargio Awaiting requested review from wargio wargio is a code owner

@kazarmy kazarmy Awaiting requested review from kazarmy kazarmy is a code owner

@ret2libc ret2libc Awaiting requested review from ret2libc ret2libc is a code owner

@imbillow imbillow Awaiting requested review from imbillow

@Rot127 Rot127 Awaiting requested review from Rot127

@Heersin Heersin Awaiting requested review from Heersin

Assignees

@DMaroo DMaroo

Labels
high-priority rz-test RzAnalysis X86
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.

x86 RzIL uplifting - floating point instructions
3 participants
@DMaroo @XVilka @Rot127