[Bug]: If-statement CFG is produced incorrectly when the true-branch is an empty block
- Dominant language
- Python
- Stars
- 6.4k
- Forks
- 1.1k
- PR merge metrics
- No merged PRs in 30d
Description
### Describe the issue:
When the true-branch of an if-statement is an empty block and the false-branch is non-empty, the branches of the condition node are swapped. This can be seen when running the cfg printer.
This is due to a bug in the `_parse_if` method in `slither/solc_parsing/declarations/function.py`. There are a few key details relating to why this happens:
1. The `_parse_statement` method calls for setting `trueStatement` and `falseStatement` will link the produced node as the child of the given condition_node .
2. In the case of an empty block, `_parse_statement` does not produce a new node, rather, it returns the same node given unchanged. This is because it ends up calling `_parse_block` with the given node, which iteratively builds up links of statement nodes in that block; when the block is empty, the node is unchanged since the statements list is empty:
```python
def _parse_block(self, block: Dict, node: NodeSolc, check_arithmetic: bool = False):
"""
Return:
Node
"""
assert block[self.get_key()] == "Block"
if self.is_compact_ast:
statements = block["statements"]
else:
statements = block[self.get_children("children")]
check_arithmetic = check_arithmetic | node.underlying_node.scope.is_checked
new_scope = Scope(check_arithmetic, False, node.underlying_node.scope)
for statement in statements: ######### does nothing if `statements` is empty; `node` is unchanged
node = self._parse_statement(statement, node, new_scope)
return node
```
Due to these behaviors, when the true-branch is empty, `trueStatement` is the same node as `condition_node`. In particular, no child was added to the `condition_node`.
What comes next is building the `falseStatement`, which if non-empty, produces a new node and thus adds a child to `condition_node`. With this, it actually becomes the first child of `condition_node`.
At the end, the `trueStatement` and `falseStatement` nodes are linked to the end-if node, and in the empty true-branch case, it was only at this point that `condition_node`'s other child is linked to the end-if node. This makes the end-if node the second child, even though it should be the first.
### Code example to reproduce the issue:
Reproducer:
```solidity
function test(uint a, uint b) public view returns(uint) {
uint v = a + b;
uint w;
if( a > b ) {}
else
w = v;
return w;
}
```
If-statement CFG:

### Version:
0.9.2
### Relevant log output:
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.