Bad `while (true)` loop recognition leads to excessive indentation level
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 25/100
- Issue type
- Feature
- Clarity
- Mostly clear
- Activity status
- Stale
- Tech stack
- cpp
- Domain
- reverse-engineering
Research direction
Start by reproducing the reported behavior with the attached vmquacks_combinator.zip sample, especially its main function, and inspect the Binary Ninja debug report for the identified loop region. Compare the generated while (true) structure with the requested do-while-style alternatives; done means the relevant control-flow cases are selected without adding unnecessary indentation.
Written by the indexing model from the issue text.
Description
What is the feature you'd like to have?
When identifying the loop region, consider reverse the condition to choose anorther node when the node is too large or the max indentation level inside the node is too high; When the node is small or the max indentation level inside the node is low, embed the loop region. This helps to convert certain while (true) loop into do-while loop, reduce indentation levels, and improve readability.
Is your feature request related to a problem?
It seems that BN is too inclined to generate while loops. I found that in the following situation, BN(Personal 4.1.5747-Stable) generates complex while (true) loops, while IDA generates do-while loops, significantly reducing complexity:
In BN debug report, the above area will be identified as the following loop region:
Eventually, a frustrating while (true) loop was generated, causing the majority of the code following the function to have an extra indentation level😢:
In my personal opinion, when identifying the loop body, if the conditional node(as shown in the blue border, the default branch is true) points to a node that is too large, the condition should be reversed or the branch should be switched into false to select other smaller node. If the node pointed to by the branch is small, it can be considered to include it in the loop body instead of using the default break statement. At the same time, adding more diverse loop structure pattern matching should be able to correctly transform this situation.
Are any alternative solutions acceptable?
Does BN seem to not support user intervention in its control flow structuring process?🥲
Additional Information:
Here is the sample(also a CTF chall lol), which main funcstion contains this situation.
vmquacks_combinator.zip
- Dominant language
- C++
- Stars
- 1.3k
- Forks
- 298
- Avg merge
- 5d 5h
- Merged PRs (30d)
- 19
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from Vector35/binaryninja-api
-
Difficulty 1/5 1-3 hours Newbie friendliness 88/100
Vector35/binaryninja-api#8540 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
Vector35/binaryninja-api#8516 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 92/100
Vector35/binaryninja-api#8503 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100
Vector35/binaryninja-api#8446 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100
Vector35/binaryninja-api#8444 ·
All issues in Vector35/binaryninja-api
Similar issues
-
Website Doc Typo Open
Difficulty 1/5 Under an hour Newbie friendliness 92/100
-
Difficulty 1/5 1-3 hours Newbie friendliness 92/100
autowarefoundation/autoware_universe#13413 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
automated-analysis bug memory-safety
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100