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

Move constant to the right-hand side in comparison operations #4339

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RootCubed
Copy link
Contributor

@RootCubed RootCubed commented Jun 12, 2022

Comparing a variable to a constant is usually done by writing the variable on the left-hand and the constant on the right-hand side of the expression. Ghidra's decompiler is sometimes able to conform its C output to this rule for expressions in if blocks by re-ordering the branches and negating the condition, so that e.g. 5 < a gets negated to !(5 < a) and becomes a < 6.

This PR extends this logic to work anywhere in the decompilation output by checking if the left-hand side is a constant, and if it is, swapping the left-hand and right-hand sides and converting the operation (e.g. from < to >).

Side note: this PR might make the logic that re-orders if blocks and negates the expression obsolete, but I have kept it in as to leave the PR as clean as possible.

@RootCubed
Copy link
Contributor Author

It seems I've misunderstood what the code in PrintC::opCbranch does; it has nothing to do with reordering the operands of the comparison. The reason why Ghidra sometimes will display the constant on the right is simply because the P-code translation and simplifications result in the constant being on the right most of the time, by chance. A counterexample can be seen by compiling the following cpp function:

bool test(int a) {
    bool gt5 = a > 5;
    if (gt5) {
        printf(">5");
    }
    return gt5;
}

The decompilation of this function is

bool __Z4testi(int param_1)

{
  if (5 < param_1) {
    _printf(">5");
  }
  return 5 < param_1;
}

@RootCubed
Copy link
Contributor Author

I had initially forgotten to add the swap tokens for equal and not_equal, this has been fixed now.

Also, any chance this PR can get looked into at some point? (It seems to have been inactive for almost a year now)
A simple demonstration of the functionality of this PR can be seen in the following program (Download compiled binary):

int test(int a) {
    bool test = a > 5;
    if (test) {
        return -1;
    }
    return test;
}

Without the commit, the decompiler produces:

int test(int param_1)

{
  int test;
  
  if (5 < param_1) {
    test = -1;
  }
  else {
    test = (int)(5 < param_1);
  }
  return test;
}

With the commit applied, the decompiler produces:

int test(int param_1)

{
  int test;
  
  if (param_1 > 5) {
    test = -1;
  }
  else {
    test = (int)(param_1 > 5);
  }
  return test;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Decompiler Status: Triage Information is being gathered
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants