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

fix: if-statement CFG construction #23

Merged
merged 1 commit into from
Jan 27, 2023
Merged

fix: if-statement CFG construction #23

merged 1 commit into from
Jan 27, 2023

Conversation

gfeng-certik
Copy link

Notes

When constructing if-statement cfgs, the true- and false-branch nodes were constructed and connected to the condition node before connecting anything to the end-if node. The issue with this is that when the true-branch is an empty block, the true-branch node is set to still be the condition node. Thus, when the false-branch node was constructed and connected, it became the first child of the condition node, leading it to be interpreted as being the true-branch.

To fix this, the connection from the true-branch node to the end-if node now happens before constructing and connecting the false-branch node. Now, in the case of an empty block true-branch, the condition node is connected to the end-if node first.

Testing

  • Run the slither cfg printer with the following test file:
    pragma solidity ^0.8.0;
    
    contract IfElseBug {
        function test_used_in_one_path_buggy(uint a, uint b) public view returns(uint) {
          uint v = a + b;
          uint w;
    
          // empty true-branch; previously buggy
          if( a > b ) {}
          else {
              w = v;
          }
         // both branches empty
          if (a > b) {}
          else {}
          // empty false-branch
          if (a <= b) {
              w = v;
          }
          else {}
          // no false-branch
          if (v <= w) {
              w = v;
          }
          // both branches nonempty
          if (a == b) {
              v = w;
          } else {
              v = 2 * w;
          }
          return w;
      }
    }
  • Ensure the cfg generated is correct for each type of if-statement.

Related Issue

https://github.com/CertiKProject/slither-task/issues/241

…ecting condition node to false-statementnode
@gfeng-certik gfeng-certik requested a review from duckki January 27, 2023 19:13
Copy link

@duckki duckki left a comment

Choose a reason for hiding this comment

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

Nice catch! The fix looks good. I've confirmed the example code's CFG is fixed.

@duckki duckki merged commit dda1275 into certik Jan 27, 2023
@duckki duckki deleted the if-stmt-cfg-fix branch January 27, 2023 22:15
whonore pushed a commit that referenced this pull request May 24, 2024
…ecting condition node to false-statement node (#23)

- fixes the issue with wrong if-statement CFG when the then-clause is empty.

https://github.com/CertiKProject/slither-task/issues/241
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants