diff --git a/slither/utils/expression_manipulations.py b/slither/utils/expression_manipulations.py index 75d97042c2..32b88e9b32 100644 --- a/slither/utils/expression_manipulations.py +++ b/slither/utils/expression_manipulations.py @@ -147,7 +147,7 @@ def convert_expressions( for next_expr in expression.expressions: # TODO: can we get rid of `NoneType` expressions in `TupleExpression`? # montyly: this might happen with unnamed tuple (ex: (,,,) = f()), but it needs to be checked - if next_expr: + if next_expr is not None: if self.conditional_not_ahead( next_expr, true_expression, false_expression, f_expressions @@ -158,6 +158,9 @@ def convert_expressions( true_expression.expressions[-1], false_expression.expressions[-1], ) + else: + true_expression.expressions.append(None) + false_expression.expressions.append(None) def convert_index_access( self, next_expr: IndexAccess, true_expression: Expression, false_expression: Expression diff --git a/tests/unit/slithir/test_data/ternary_expressions.sol b/tests/unit/slithir/test_data/ternary_expressions.sol index ebfb96e801..1ccd51d34d 100644 --- a/tests/unit/slithir/test_data/ternary_expressions.sol +++ b/tests/unit/slithir/test_data/ternary_expressions.sol @@ -1,6 +1,6 @@ interface Test { function test() external payable returns (uint); - function testTuple() external payable returns (uint, uint); + function testTuple(uint) external payable returns (uint, uint); } contract C { // TODO @@ -36,21 +36,23 @@ contract C { } // Unused tuple variable - function g(address one) public { - (, uint x) = Test(one).testTuple(); - } - uint[] myIntegers; - function _h(uint c) internal returns(uint) { - return c; - } - function h(bool cond, uint a, uint b) public { - uint d = _h( - myIntegers[cond ? a : b] - ); + function g(address one, bool cond, uint a, uint b) public { + (, uint x) = Test(one).testTuple(myIntegers[cond ? a : b]); } - - function i(bool cond) public { + + function h(bool cond) public { bytes memory a = new bytes(cond ? 1 : 2); } } + +contract D { + function values(uint n) internal returns (uint, uint) { + return (0, 1); + } + + function a(uint n) external { + uint a; + (a,) = values(n > 0 ? 1 : 0); + } +} diff --git a/tests/unit/slithir/test_ternary_expressions.py b/tests/unit/slithir/test_ternary_expressions.py index 712c9582b0..bf8556f85d 100644 --- a/tests/unit/slithir/test_ternary_expressions.py +++ b/tests/unit/slithir/test_ternary_expressions.py @@ -16,30 +16,53 @@ def test_ternary_conversions(solc_binary_path) -> None: """This tests that true and false sons define the same number of variables that the father node declares""" solc_path = solc_binary_path("0.8.0") slither = Slither(Path(TEST_DATA_DIR, "ternary_expressions.sol").as_posix(), solc=solc_path) - for contract in slither.contracts: - if not contract.is_signature_only: - for function in contract.functions: - vars_declared = 0 - vars_assigned = 0 - for node in function.nodes: - if node.type in [NodeType.IF, NodeType.IFLOOP]: - - # Iterate over true and false son - for inner_node in node.sons: - # Count all variables declared - expression = inner_node.expression - if isinstance( - expression, (AssignmentOperation, NewElementaryType, CallExpression) - ): - var_expr = expression.expression_left - # Only tuples declare more than one var - if isinstance(var_expr, TupleExpression): - vars_declared += len(var_expr.expressions) - else: - vars_declared += 1 - - for ir in inner_node.irs: - # Count all variables defined - if isinstance(ir, (Assignment, Unpack)): - vars_assigned += 1 - assert vars_declared == vars_assigned and vars_assigned != 0 + contract = next(c for c in slither.contracts if c.name == "C") + for function in contract.functions: + vars_declared = 0 + vars_assigned = 0 + for node in function.nodes: + if node.type in [NodeType.IF, NodeType.IFLOOP]: + + # Iterate over true and false son + for inner_node in node.sons: + # Count all variables declared + expression = inner_node.expression + if isinstance( + expression, (AssignmentOperation, NewElementaryType, CallExpression) + ): + var_expr = expression.expression_left + # Only tuples declare more than one var + if isinstance(var_expr, TupleExpression): + vars_declared += len(var_expr.expressions) + else: + vars_declared += 1 + + for ir in inner_node.irs: + # Count all variables defined + if isinstance(ir, (Assignment, Unpack)): + vars_assigned += 1 + assert vars_declared == vars_assigned and vars_assigned != 0 + + +def test_ternary_tuple(solc_binary_path) -> None: + """ + Test that in the ternary liftings of an assignment of the form `(z, ) = ...`, + we obtain `z` from an unpack operation in both lifitings + """ + solc_path = solc_binary_path("0.8.0") + slither = Slither(Path(TEST_DATA_DIR, "ternary_expressions.sol").as_posix(), solc=solc_path) + contract = next(c for c in slither.contracts if c.name == "D") + fn = next(f for f in contract.functions if f.name == "a") + + if_nodes = [n for n in fn.nodes if n.type == NodeType.IF] + assert len(if_nodes) == 1 + + if_node = if_nodes[0] + assert isinstance(if_node.son_true.expression, AssignmentOperation) + assert ( + len([ir for ir in if_node.son_true.all_slithir_operations() if isinstance(ir, Unpack)]) == 1 + ) + assert ( + len([ir for ir in if_node.son_false.all_slithir_operations() if isinstance(ir, Unpack)]) + == 1 + )