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

Missing lift for if-then (it, itt, ittt, itttt) instructions #1720

Closed
nshp opened this issue May 27, 2020 · 3 comments
Closed

Missing lift for if-then (it, itt, ittt, itttt) instructions #1720

nshp opened this issue May 27, 2020 · 3 comments
Assignees
Labels
Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin Component: Architecture Issue needs changes to an architecture plugin Effort: Low Issue should take < 1 week Impact: Medium Issue is impactful with a bad, or no, workaround
Milestone

Comments

@nshp
Copy link
Contributor

nshp commented May 27, 2020

I frequently see HLIL like this:

arg2 - 2
bool cond:0 = arg2 == 2
bool cond:1 = arg2 == 2
bool cond:2 = arg2 == 2
if (arg2 == 2)
    ...
...
if (cond:0)
    ...
if (cond:1)
    ...
if (not(cond:2))
    ...

I don't think this is architecture-specific, but this particular example is from Thumb code:

cmp r1, #2 
it eq
...; ...
itt eq
...; ...
beq true
false: ...
true: ...

There are several suboptimal things there:

  • The arg2 - 2 should have been eliminated, not sure why it wasn't
  • All the cond vars are the same and could be merged
  • The first if is also the same, maybe it should also use the condvar in cases like this?
  • The second and third if are also the same, and could be merged
  • The final if (not(...)) could be an else
  • Finally, this is a pattern like if (x == 1) { ... } else { if (x == 2) { ... } else { if (x == 3) } } } which could be turned into a switch/case, or at least flattened out to if (x == 1) { ...} else if (x == 2) { ... } ...
@plafosse plafosse added Core: HLIL Issue involves High Level IL IL Optimization Issue involving optimization of representation (not correctness) Core: LLIL Issue involves Low Level IL labels May 27, 2020
@plafosse plafosse added Effort: Low Issue should take < 1 week Impact: Medium Issue is impactful with a bad, or no, workaround labels May 5, 2021
@fuzyll fuzyll changed the title Eliminate duplicate condition variables Missing lift for it, itt, ittt, itttt instructions May 22, 2023
@fuzyll fuzyll changed the title Missing lift for it, itt, ittt, itttt instructions Missing lift for if-then (it, itt, ittt, itttt) instructions May 22, 2023
@fuzyll fuzyll added Component: Architecture Issue needs changes to an architecture plugin Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin and removed Core: LLIL Issue involves Low Level IL Core: HLIL Issue involves High Level IL IL Optimization Issue involving optimization of representation (not correctness) labels May 22, 2023
@fuzyll
Copy link
Contributor

fuzyll commented May 22, 2023

I'm using this now as the issue tracking our inability to lift the if-then (it and related instructions) in ARM, simply because it was the oldest one I found.

We discussed this today and it's either easy (because all we have to do is apply the condition to the following instructions) or hard (because we may not be able to look forward/backward far enough to see things). Either way, this appears to have been an issue for awhile and is pretty common in Thumb code. See linked tickets above for other examples where this causes issues for us.

@lwerdna
Copy link
Contributor

lwerdna commented May 24, 2023

Alright, so this issue is chosen as the representative of issue #3976 and issue #4305.

Here's a 5k binary with every variation of the if-then instruction for testing:

hello.zip

@plafosse
Copy link
Member

We fixed the underlying lifting issues in 3.5.4298

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: ARM/Thumb Issues with the ARM/Thumb architecture plugin Component: Architecture Issue needs changes to an architecture plugin Effort: Low Issue should take < 1 week Impact: Medium Issue is impactful with a bad, or no, workaround
Projects
None yet
Development

No branches or pull requests

4 participants