diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 35ca51aebe..07e09398a6 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -815,19 +815,26 @@ def _parse_variable_definition(self, statement: Dict, node: NodeSolc, scope: Sco new_node = self._parse_variable_definition_init_tuple( new_statement, i, new_node, scope ) + else: + variables.append(None) i = i + 1 var_identifiers = [] # craft of the expression doing the assignement for v in variables: - identifier = { - "nodeType": "Identifier", - "src": v["src"], - "name": v["name"], - "referencedDeclaration": v["id"], - "typeDescriptions": {"typeString": v["typeDescriptions"]["typeString"]}, - } - var_identifiers.append(identifier) + if v is not None: + identifier = { + "nodeType": "Identifier", + "src": v["src"], + "name": v["name"], + "referencedDeclaration": v["id"], + "typeDescriptions": { + "typeString": v["typeDescriptions"]["typeString"] + }, + } + var_identifiers.append(identifier) + else: + var_identifiers.append(None) tuple_expression = { "nodeType": "TupleExpression", diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index 005ad81a44..f5c0f7adde 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -197,12 +197,6 @@ def _post_assignement_operation(self, expression: AssignmentOperation) -> None: for idx, _ in enumerate(left): if not left[idx] is None: index = idx - # The following test is probably always true? - if ( - isinstance(left[idx], LocalVariableInitFromTuple) - and left[idx].tuple_index is not None - ): - index = left[idx].tuple_index operation = Unpack(left[idx], right, index) operation.set_expression(expression) self._result.append(operation) diff --git a/tests/unit/core/test_data/tuple_assign/test_tuple_assign.sol b/tests/unit/core/test_data/tuple_assign/test_tuple_assign.sol new file mode 100644 index 0000000000..54abf2822e --- /dev/null +++ b/tests/unit/core/test_data/tuple_assign/test_tuple_assign.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.8.13; + +contract Other { + struct S { + uint z; + uint b; + uint c; + uint d; + } + + mapping(uint => S) public M; +} + +contract TestEmptyComponent { + Other other; + + function test() external { + (uint z, , uint c, uint d) = other.M(3); + } +} + +contract TestTupleReassign { + function threeRet(int z) internal returns(int, int, int) { + return (1,2,3); + } + function twoRet(int z) internal returns(int, int) { + return (3,4); + } + + function test() external returns(int) { + (int a, int b, int c) = threeRet(3); + (a, c) = twoRet(b); + return b; + } +} diff --git a/tests/unit/core/test_tuple_assign.py b/tests/unit/core/test_tuple_assign.py new file mode 100644 index 0000000000..09217c9d11 --- /dev/null +++ b/tests/unit/core/test_tuple_assign.py @@ -0,0 +1,32 @@ +from pathlib import Path + +from slither import Slither +from slither.slithir.operations.unpack import Unpack + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "tuple_assign" + + +def test_tuple_order(solc_binary_path) -> None: + """Test that the correct tuple components are projected when some components are left empty.""" + solc_path = solc_binary_path("0.8.15") + slither = Slither(Path(TEST_DATA_DIR, "test_tuple_assign.sol").as_posix(), solc=solc_path) + fn = slither.get_contract_from_name("TestEmptyComponent")[0].get_function_from_canonical_name( + "TestEmptyComponent.test()" + ) + unpacks = [ir for ir in fn.slithir_operations if isinstance(ir, Unpack)] + assert unpacks[0].index == 0 + assert unpacks[1].index == 2 + assert unpacks[2].index == 3 + + +def test_tuple_reassign(solc_binary_path) -> None: + """Test that tuple component indexes are not reused across assignments""" + solc_path = solc_binary_path("0.8.15") + slither = Slither(Path(TEST_DATA_DIR, "test_tuple_assign.sol").as_posix(), solc=solc_path) + fn = slither.get_contract_from_name("TestTupleReassign")[0].get_function_from_canonical_name( + "TestTupleReassign.test()" + ) + unpacks = [ir for ir in fn.slithir_operations if isinstance(ir, Unpack)] + assert len(unpacks) == 5 + assert unpacks[3].index == 0 + assert unpacks[4].index == 1