slither icon indicating copy to clipboard operation
slither copied to clipboard

fix: preserve empty tuple components during declaration-to-assignment conversion

Open kevinclancy opened this issue 1 year ago • 2 comments

This fixes issue 1913.

Previously, when performing an assignment whose variables were originally declared in a tuple-based declaration, the projection components were reused:

    function test() external returns(int) {
        (int a, int b, int c) = threeRet(3);
        (a, c) = twoRet(b);
        return b;
    }
...
  Expression: (a,c) = twoRet(b)
  IRs:
    TUPLE_1(int256,int256) = INTERNAL_CALL, Test.twoRet(int256)(b)
    a(int256)= UNPACK TUPLE_1 index: 0 
    c(int256)= UNPACK TUPLE_1 index: 2 

This happened because of some code in the _post_assignment_operation function in expression_to_slithir.py:

                        # 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

We shouldn't be reusing the variable's original projection component, but there is a reason that code was written; namely, this code translates a multi-variable declaration into a multi-variable assignment.

It also removes padding None entries and records each component index in its corresponding variable instead. As we've seen, this approach doesn't work, because not all tuple assignments are generated from variable declarations. To solve this, this PR leaves the None components inside of the tuple in order to select the correct projection index for each variable.

kevinclancy avatar Jul 07 '23 00:07 kevinclancy

Can you move the tests to tests/unit/slithir and run pytest tests/e2e/detectors/test_detectors.py --insta update please?

0xalpharush avatar Sep 08 '23 02:09 0xalpharush

@0xalpharush When I run with --insta update-new, I get an error:

ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --insta update-new
  inifile: None
  rootdir: /Users/kevin.clancy/my-slither

kevinclancy avatar Sep 20 '23 23:09 kevinclancy