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

[Bug]: Ternary_expressions test does not test what it claims to #2032

Closed
kevinclancy opened this issue Jul 6, 2023 · 0 comments
Closed

[Bug]: Ternary_expressions test does not test what it claims to #2032

kevinclancy opened this issue Jul 6, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@kevinclancy
Copy link
Contributor

Describe the issue:

The test slither/tests/unit/slithir/test_ternary_expressions.py assumes that all nodes contain declarations. It tries to verify that the number of variables assigned in the IR of each declaration node is equal to the number of variables declared in the node's expression.

The problem is that it identifies assignments by matching the Assignment operation class. There are other forms of assignments. For example, here is the IR for g:

        Function C.g(address) (*)
                Expression: (x) = Test(one).testTuple()
                IRs:
                        TMP_36 = CONVERT one to Test
                        TUPLE_0(uint256,uint256) = HIGH_LEVEL_CALL, dest:TMP_36(Test), function:testTuple, arguments:[]  
                        x(uint256)= UNPACK TUPLE_0 index: 1 

The variable x is assigned using an UNPACK operation.

Code example to reproduce the issue:

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:
        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):
                            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):
                                vars_assigned += 1
            print(f"\nfunction: {function} declared: {vars_declared} \n assigned: {vars_assigned}")
            assert vars_declared == vars_assigned

Version:

0.9.6

Relevant log output:

function: test
declared: 0
assigned: 0

function: testTuple
declared: 0
assigned: 0

function: a
declared: 4
assigned: 4

function: b
declared: 4
assigned: 4

function: c
declared: 2
assigned: 2

function: d
declared: 2
assigned: 2

function: e
declared: 4
assigned: 4

function: f
declared: 6
assigned: 6

function: g
declared: 0
assigned: 0

function: _h
declared: 0
assigned: 0

function: h
declared: 2
assigned: 2

function: i
declared: 2
assigned: 2
@kevinclancy kevinclancy added the bug-candidate Bugs reports that are not yet confirmed label Jul 6, 2023
@kevinclancy kevinclancy changed the title [Bug-Candidate]: Ternary_expressions test does not test what it claims [Bug-Candidate]: Ternary_expressions test does not test what it claims to Jul 6, 2023
@0xalpharush 0xalpharush added bug Something isn't working and removed bug-candidate Bugs reports that are not yet confirmed labels Jul 25, 2023
@0xalpharush 0xalpharush changed the title [Bug-Candidate]: Ternary_expressions test does not test what it claims to [Bug]: Ternary_expressions test does not test what it claims to Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants