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.

Sign up for GitHub

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 reorder argument edge case #2614

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Fix reorder argument edge case #2614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions slither/slithir/convert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,11 @@ def reorder_arguments(
assert len(call_names) == len(names)

args_ret = []
index_seen = []
index_seen = [] # Will have the correct index order

# We search for declaration of parameters that is fully compatible with the call arguments
# that is why if a arg name is not present in the call name we break and clear index_seen
# Known issue if the overridden function reuse the same parameters' name but in different positions
for names in decl_names:
if len(index_seen) == len(args):
break
Expand All @@ -445,9 +448,12 @@ def reorder_arguments(
if ind in index_seen:
continue
except ValueError:
continue
index_seen.clear()
break
index_seen.append(ind)
args_ret.append(args[ind])

for ind in index_seen:
args_ret.append(args[ind])

return args_ret

Expand Down
16 changes: 12 additions & 4 deletions tests/unit/slithir/test_argument_reorder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,15 @@ def test_overridden_function_reorder(solc_binary_path) -> None:

assert (
isinstance(internal_calls[0].arguments[0], Constant)
and internal_calls[0].arguments[0].value == 34
and internal_calls[0].arguments[0].value == 23
)
assert (
isinstance(internal_calls[0].arguments[1], Constant)
and internal_calls[0].arguments[1].value == 23
and internal_calls[0].arguments[1].value == 36
)
assert (
isinstance(internal_calls[0].arguments[2], Constant)
and internal_calls[0].arguments[2].value == 34
)

operations = slither.contracts[1].functions[1].slithir_operations
Expand All @@ -90,9 +94,13 @@ def test_overridden_function_reorder(solc_binary_path) -> None:

assert (
isinstance(internal_calls[0].arguments[0], Constant)
and internal_calls[0].arguments[0].value == 34
and internal_calls[0].arguments[0].value == 23
)
assert (
isinstance(internal_calls[0].arguments[1], Constant)
and internal_calls[0].arguments[1].value == 23
and internal_calls[0].arguments[1].value == 36
)
assert (
isinstance(internal_calls[0].arguments[2], Constant)
and internal_calls[0].arguments[2].value == 34
)
6 changes: 3 additions & 3 deletions tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
contract A {
function a(uint256 q, uint256 e) internal virtual {}
function b() public { a({e: 23, q: 34}); }
function a(uint256 e, uint256 q, uint256 w) internal virtual {}
function b() public { a({e: 23, w: 34, q: 36}); }
}

contract B is A {
function a(uint256 w, uint256 q) internal override {}
function a(uint256 q, uint256 ww, uint256 e) internal override {}
}
Loading