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

[WIP]Add support for float basic (ieee754) #2005

Closed
wants to merge 6 commits into from
Closed

[WIP]Add support for float basic (ieee754) #2005

wants to merge 6 commits into from

Conversation

Heersin
Copy link
Member

@Heersin Heersin commented Nov 21, 2021 •
edited by XVilka
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
Working in progress. Create this PR for early review.

See http://binaryanalysisplatform.github.io/bap/api/master/bap-core-theory/Bap_core_theory/Theory/module-type-Float/index.html

Add support for ieee754 float operations

Test plan

  1. Test float format in test_il_definitions
  2. Test theory operations in a new test file

Partially address #1461

XVilka
XVilka reviewed Nov 22, 2021
View reviewed changes
librz/include/rz_il/definitions/float.h
RzILBitVector *s; ///< The bitvector of float
} RzILFloat;

RZ_API ut32 rzil_float_get_exp_len(RZ_NONNULL RzILFloat *f);
Copy link
Member

Choose a reason for hiding this comment

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

All these functions should be named rz_il_*() not rzil_*().

@wargio
Copy link
Member

wargio commented Nov 22, 2021

Please implement a to_string method and also add the statement output on rzil_export.c

wargio
wargio requested changes Nov 22, 2021
View reviewed changes
librz/il/definitions/float.c
}
}

ut32 rzil_float_get_exp_start_pos(RZ_NONNULL RzILFloat *f) {
Copy link
Member

Choose a reason for hiding this comment

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

this could be static inline

librz/il/definitions/float.c
Comment on lines +80 to +81
if (!result || !bv) {
return NULL;
Copy link
Member

Choose a reason for hiding this comment

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

remove !bv and add the rz_return_val_if_fail before checking r and bv.

@XVilka XVilka mentioned this pull request Dec 1, 2021
Open
@Heersin
Init float
0e9ff1b
@Heersin
Add float header
a9224a6
@Heersin
Add basic operations for flaot
224f7d5 
Add float type test and passes all

Add infinte and some helper functions

Add basic support of float

Add basic support of float
@Heersin
Add some arithmetic operations
731326f
@Heersin
Add some arithmetic operations
a2f902e
@Heersin
Remove from_ut32 in float test
4ed9098
@XVilka
Copy link
Member

XVilka commented Jan 10, 2022

@Heersin were there any updates since?
Please also rebase on top of the latest dev and the changed RzIL APIs.

@XVilka
Copy link
Member

XVilka commented Feb 14, 2022

@Heersin see these resources:

@thestr4ng3r if you have something to add, please do.

@thestr4ng3r
Copy link
Member

I think all of the code that exists at the moment here should go into RzUtil and have any IL reference removed. Having a float library independently of the IL can be useful also for other things, similar to bitvectors. Then the IL ops can be built on top.

@Heersin
Copy link
Member Author

Heersin commented Feb 14, 2022

I think all of the code that exists at the moment here should go into RzUtil and have any IL reference removed. Having a float library independently of the IL can be useful also for other things, similar to bitvectors. Then the IL ops can be built on top.

So it would be better to keep this in RzUtil just like the bitvector now.

@XVilka
Copy link
Member

XVilka commented May 19, 2022

@Heersin how is it going? Were there any updates since? Could you please also rebase on top of the latest dev and solve the conflicts?

@XVilka XVilka added waiting-for-author Used to mark PRs where more work is needed high-priority labels May 19, 2022
@Heersin
Copy link
Member Author

Heersin commented May 19, 2022

@Heersin how is it going? Were there any updates since? Could you please also rebase on top of the latest dev and solve the conflicts?

Recently working on float test basic arithmetic cases and debugging (https://github.com/Heersin/rizin/tree/rz_util_float). I will continue it next week

@Heersin Heersin mentioned this pull request Jul 5, 2022
Merged
5 tasks
@XVilka
Copy link
Member

XVilka commented Sep 14, 2022

@Heersin, do you plan to rebase and continue in this PR/branch, or should we close it?

@Heersin
Copy link
Member Author

Heersin commented Sep 14, 2022

I would like to close this one. and make a new for RzIL seperately

@Heersin Heersin closed this Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XVilka XVilka XVilka left review comments

@wargio wargio wargio requested changes

@kazarmy kazarmy Awaiting requested review from kazarmy kazarmy will be requested when the pull request is marked ready for review kazarmy is a code owner

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r will be requested when the pull request is marked ready for review thestr4ng3r is a code owner

@ret2libc ret2libc Awaiting requested review from ret2libc ret2libc will be requested when the pull request is marked ready for review ret2libc is a code owner

Assignees

@XVilka XVilka

@thestr4ng3r thestr4ng3r

Labels
high-priority rz-test RZIL waiting-for-author Used to mark PRs where more work is needed
Projects
RzIL
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Heersin @wargio @XVilka @thestr4ng3r