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

Try Catch Improvement #4305

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

Try Catch Improvement #4305

wants to merge 5 commits into from

Conversation

j-barnak
Copy link

Note: This is a WIP

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

Bin

  • Load exception information by default

Analysis

  • Make analyzing exception scopes the default
  • Analyze all exception sources as functions
  • Do not hack basic blocks flow to add try/catch information

Graph

  • Add an edge pointing to the catch block for each basic block inside the try scope
  • Add configuration graph.trycatch to control if graphing the exception blocks is enabled

Problems

https://github.com/rizinorg/rizin/commit/8ff06e9232d7be31d930233e2b59ec86b78cb6d6 Is basically copy-pasted logic from block.c
This can make graphs a lot noisier
Cutter will have to implement its own version of the logic if it wants to copy rizin in showing the connection between the blocks and its catch blocks
I didn't add tests yet

Test plan

Open bins\pe\microsoft_seh_tests\x64\xcpt4.exe for example
Optional: idp to load debug info for binary
aaa
Optional: .iw to show try catch scopes as flags
s main
Enter visual graph mode, make sure all edges, lines are painted correctly, and that there area now new edges pointing each basic block to its catch block if it is inside a try scope

Open file from #2247
aaa
s main
Enter visual graph mode, make sure issue is fixed

Closing issues

Closes #2247

j-barnak and others added 3 commits February 23, 2024 04:56
Your checklist for this pull request

- [x] I've read the guidelines for contributing to this repository
- [x] 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

Bin

- Load exception information by default

Analysis

-  Make analyzing exception scopes the default
-  Analyze all exception sources as functions
-  Do not hack basic blocks flow to add try/catch information

Graph

- Add an edge pointing to the catch block for each basic block inside the try scope
-  Add configuration graph.trycatch to control if graphing the exception blocks is enabled

Problems

    8ff06e9 Is basically copy-pasted logic from block.c
    This can make graphs a lot noisier
    Cutter will have to implement its own version of the logic if it wants to copy rizin in showing the connection between the blocks and its catch blocks
    I didn't add tests yet

Test plan

Open bins\pe\microsoft_seh_tests\x64\xcpt4.exe for example
Optional: idp to load debug info for binary
aaa
Optional: .iw to show try catch scopes as flags
s main
Enter visual graph mode, make sure all edges, lines are painted correctly, and that there area now new edges pointing each basic block to its catch block if it is inside a try scope

Open file from rizinorg#2247
aaa
s main
Enter visual graph mode, make sure issue is fixed

Closing issues

Closes rizinorg#2247
}
title = get_title(catch_bb->addr);
v = rz_agraph_get_node(g, title);
free(title);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
free(title);
RZ_FREE(title);

Use RZ_FREE when a pointer is reused.
RZ_FREE will invoke free and also set the freed pointer to NULL.

Comment on lines +4030 to 4031
rz_core_analysis_calls(core, false); // "aac"
(void)rz_core_analysis_calls(core, false); // "aac"
Copy link
Member

Choose a reason for hiding this comment

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

?

@github-actions github-actions bot removed the RzUtil label Mar 3, 2024
@XVilka
Copy link
Member

XVilka commented Mar 21, 2024

@j-barnak there are a lot of conflicts, please rebase.

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.

Not analyzing an x64 SEH handler after RaiseException
3 participants