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.

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

Slither crashes when two for-loops use the same variable name #151

Closed
mrice32 opened this issue Jan 24, 2019 · 2 comments
Closed

Slither crashes when two for-loops use the same variable name #151

mrice32 opened this issue Jan 24, 2019 · 2 comments
Assignees
Labels
bug Something isn't working High Priority
Milestone

Comments

@mrice32
Copy link
Contributor

mrice32 commented Jan 24, 2019

This is the crash we're seeing:

ERROR:root:Error in test.sol
ERROR:root:Traceback (most recent call last):
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/__main__.py", line 250, in main_impl
    (results, number_contracts) = process(filename, args, detector_classes, printer_classes)
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/__main__.py", line 35, in process
    slither = Slither(filename, args.solc, args.disable_solc_warnings, args.solc_args, ast)
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/slither.py", line 41, in __init__
    self._analyze_contracts()
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/solc_parsing/slitherSolc.py", line 201, in _analyze_contracts
    self._convert_to_slithir()
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/solc_parsing/slitherSolc.py", line 327, in _convert_to_slithir
    contract.convert_expression_to_slithir()
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/solc_parsing/declarations/contract.py", line 374, in convert_expression_to_slithir
    func.generate_slithir_ssa(all_ssa_state_variables_instances)
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/solc_parsing/declarations/function.py", line 935, in generate_slithir_ssa
    add_ssa_ir(self, all_ssa_state_variables_instances)
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/slithir/utils/ssa.py", line 69, in add_ssa_ir
    add_phi_origins(function.entry_point, init_definition, dict())
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/slithir/utils/ssa.py", line 363, in add_phi_origins
    add_phi_origins(succ, local_variables_definition, state_variables_definition)
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/slithir/utils/ssa.py", line 363, in add_phi_origins
    add_phi_origins(succ, local_variables_definition, state_variables_definition)
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/slithir/utils/ssa.py", line 363, in add_phi_origins
    add_phi_origins(succ, local_variables_definition, state_variables_definition)
  [Previous line repeated 2 more times]
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/slithir/utils/ssa.py", line 356, in add_phi_origins
    phi_node.add_phi_origin_local_variable(variable, n)
  File "/Users/matt/anaconda3/lib/python3.7/site-packages/slither_analyzer-0.5.0-py3.7.egg/slither/core/cfg/node.py", line 215, in add_phi_origin_local_variable
    assert v == variable
AssertionError

This is our sample case:

pragma solidity ^0.5.0;

contract Test {
    function twoForLoops() pure external returns (uint sum) {
        for (uint i = 0; i < 5; i++) {
            sum += 1;
        }

        for (uint i = 0; i < 6; i++) {
            sum += 2;
        }
    }
}

If one changes i in the second loop to j, slither no longer crashes.

cc @ptare

@montyly
Copy link
Member

montyly commented Jan 24, 2019

Good catch thanks!

This is due to the variable scope change in 0.5.0, which allows re-definition of variables, and breaks the SSA

Another example:

pragma solidity ^0.5.0;

contract Test {
    function variable_redefinition() pure external returns (uint sum) {
        { uint i;
        }
        { uint i;
          if(true){
            i = i +1;
          }
        }
    }
}

We are going to investigate how to fix it

@montyly montyly self-assigned this Jan 24, 2019
@montyly montyly added the bug Something isn't working label Jan 24, 2019
@montyly montyly added this to the 0.6.0 milestone Jan 28, 2019
@montyly montyly closed this as completed in f3992c3 Feb 6, 2019
@montyly
Copy link
Member

montyly commented Feb 6, 2019

We fixed the bug in master.
In the case of variable redefinition, Slither will change the name of the second definition (and adds a suffix). It's a short term fix; for a long term fix we need to find how to handle at the same time the scope variable definition of solidity 0.4 and 0.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working High Priority
Projects
None yet
Development

No branches or pull requests

2 participants