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: add aezsue to support Step until PC equals given address and output VM changes #4466

Merged
merged 3 commits into from
May 4, 2024

Conversation

imbillow
Copy link
Contributor

@imbillow imbillow commented Apr 30, 2024

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

...

Test plan

...

Closing issues

...

librz/il/il_vm_eval.c Show resolved Hide resolved
librz/il/il_vm_eval.c Outdated Show resolved Hide resolved
librz/arch/il/analysis_il.c Outdated Show resolved Hide resolved
librz/arch/il/analysis_il.c Outdated Show resolved Hide resolved
Comment on lines 319 to 321
RZ_API RzAnalysisILStepResult rz_analysis_il_vm_step_while(
RZ_NONNULL RzAnalysis *analysis, RZ_NONNULL RzAnalysisILVM *vm, RZ_NULLABLE RzReg *reg,
RZ_NONNULL cond_callback cond, RZ_NULLABLE void *user) {
Copy link
Member

Choose a reason for hiding this comment

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

If cond_callback is used in the RZ_API then is worth moving it with a proper name into the header.

RzAnalysisILStepResult res = RZ_ANALYSIS_IL_STEP_RESULT_SUCCESS;
while (cond(vm, user)) {
ut64 addr = rz_bv_to_ut64(vm->vm->pc);
ut8 code[32] = { 0 };
analysis->read_at(analysis, addr, code, sizeof(code));
RzAnalysisOp op = { 0 };
rz_analysis_op_init(&op);
Copy link
Contributor

Choose a reason for hiding this comment

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

Move it outside

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get it. It seems to me that rz_analysis_op_init needs to be called before every call to rz_analysis_op and rz_analysis_op_fini needs to be called after every call to rz_analysis_op.

So ‘Move it outside’ doesn't work? @pelijah

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird, rz_analysis_op_fini() doesn't work as expected so leave it as is.

Copy link
Contributor

@pelijah pelijah May 3, 2024

Choose a reason for hiding this comment

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

@XVilka We definetely should standardize *_fini() APIs behaviour.

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.

LGTM once @pelijah feedback is addressed.

@XVilka XVilka merged commit a420733 into dev May 4, 2024
46 of 47 checks passed
@XVilka XVilka deleted the rzil-aezsue branch May 4, 2024 01:31
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.

4 participants