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

Some work to consummate esil-rs #9

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ZhangZhuoSJTU
Copy link

@ZhangZhuoSJTU ZhangZhuoSJTU commented Dec 25, 2017

Todo List

  • Correct default size for '[]' and variants opcodes in lexer.rs
  • Make sure esil-rs handles internal values in the right way
  • Add support for unimplemented opcodes, seeing this issue
  • Add support for new opcodes, seeing Sushant's issue and Anton's link
  • Never panic
  • Add support for unimplemented internal variables: jt, js, ds
  • Add support for bit-level change for internal variables.
  • If possible, complete vm.rs
  • Bug fix in ESP arch (Emmm, which bug? I may need some tests to remind me ;P)
  • Support '=[*]' and variants oparators in the right way (which is not an easy work)

@ZhangZhuoSJTU
Copy link
Author

ZhangZhuoSJTU commented Dec 25, 2017

Btw, I found esil-rs built a basic assumption about handling internal variables, which is Internal varibables will never be used before EEq/EPoke and after these basic arithmetic operands in ESIL.

For example, following ESIL string will never appear in practice.

1, rax, +, $c63, cf, =

To make sure the validity, @sushant94 would you mind checking this assumption?

@sushant94
Copy link
Collaborator

@ZhangZhuoSJTU Imo, the above esil does not make sense. $c is not set by this operation as there is no assignment to rax. If you look at the code for esil emulation (in radare), you'll see the esil_old and esil_cur are only set on assignment to a register.

@ZhangZhuoSJTU
Copy link
Author

@sushant94 That's it! I just want to make sure it does work in the correct way. Thanks ;P

@ZhangZhuoSJTU
Copy link
Author

This PR is suspended for it is not so urgent and I am working on VSA right now.

@XVilka
Copy link
Contributor

XVilka commented Sep 30, 2018

@ZhangZhuoSJTU ping? @kriw @chinmaydd @HMPerson1 please take a look.

@kriw
Copy link
Contributor

kriw commented Sep 30, 2018

I think it would be better to split TODOs into different PRs.

@@ -236,14 +246,16 @@ impl Tokenize for Tokenizer {
"%=" => vec![Token::PCopy(1), Token::EMod, Token::PPop(1),
Token::EEq],

"=[]" => vec![Token::EPoke(64)],
"=[]" => vec![Token::EPoke(USE_DEFAULT_SIZE)],
Copy link
Contributor

@kriw kriw Sep 30, 2018

Choose a reason for hiding this comment

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

Why not using(define) new enum value (e.g., Token::EPokeDefaultSize) instead of Token::EPoke(USE_DEFAULT_SIZE)

@chinmaydd
Copy link

chinmaydd commented Oct 1, 2018

Agree with @kriw here, we can break down issues addressed in this PR into multiple ones.

Maybe we can have Rust enthusiasts contribute for Hacktoberfest? IMO following can be good-first-issue:

  • Never panic
  • Add support for unimplemented internal variables: jt, js, ds
  • Add support for bit-level change for internal variables.

Implementing vm.rs would require design discussions especially if we plan to support features like snapshotting, timeless vm evaluation, etc.

EDIT: Also, I think @ZhangZhuoSJTU is almost done with checkpoint 3 in this PR. If we can clean things up and merge existing work it would be great.

@XVilka
Copy link
Contributor

XVilka commented Oct 29, 2018

Ping?

@XVilka
Copy link
Contributor

XVilka commented Jun 17, 2019

TODO: Migrate to the https://github.com/radareorg/radeco-lib repository.

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

Successfully merging this pull request may close these issues.

5 participants