fix: reorder named params to match declaration order before generatin…

…g IR (#47)

https://github.com/CertiKProject/slither-task/issues/502

slither/core/expressions/call_expression.py

@@ -4,19 +4,37 @@
 
 
 class CallExpression(Expression):  # pylint: disable=too-many-instance-attributes
-    def __init__(self, called: Expression, arguments: List[Any], type_call: str) -> None:
+    def __init__(self, called: Expression, arguments: List[Any], type_call: str, names: Optional[List[str]] = None) -> None:
+        """
+        #### Parameters
+        called -
+            The expression denoting the function to be called
+        arguments -
+            List of argument expressions
+        type_call -
+            A string formatting of the called function's return type
+        names -
+            For calls with named fields, a list of the fields in the order listed in the call.
+            For calls without named fields, None.
+        """
         assert isinstance(called, Expression)
+        assert (names == None) or isinstance(names, list)
         super().__init__()
         self._called: Expression = called
         self._arguments: List[Expression] = arguments
         self._type_call: str = type_call
+        self._names: Optional[List[str]] = names
         # gas and value are only available if the syntax is {gas: , value: }
         # For the .gas().value(), the member are considered as function call
         # And converted later to the correct info (convert.py)
         self._gas: Optional[Expression] = None
         self._value: Optional[Expression] = None
         self._salt: Optional[Expression] = None
 
+    @property
+    def names(self) -> Optional[List[str]]:
+        return self._names
+
     @property
     def call_value(self) -> Optional[Expression]:
         return self._value
@@ -62,4 +80,9 @@ def __str__(self) -> str:
             if gas or value or salt:
                 options = [gas, value, salt]
                 txt += "{" + ",".join([o for o in options if o != ""]) + "}"
-        return txt + "(" + ",".join([str(a) for a in self._arguments]) + ")"
+        args = (
+            "{" + ",".join([f"{n}:{str(a)}" for (a, n) in zip(self._arguments, self._names)]) + "}"
+            if self._names != None
+            else ",".join([str(a) for a in self._arguments])
+        )
+        return txt + "(" + args + ")"

slither/slithir/convert.py

@@ -400,6 +400,71 @@ def integrate_value_gas(result: List[Operation]) -> List[Operation]:
 ###################################################################################
 ###################################################################################
 
+def get_declared_param_names(ins: Call) -> Optional[List[str]]:
+    """
+    Given a call operation, return the list of parameter names, in the order
+    listed in the function declaration.
+
+    #### Parameters
+    ins -
+        The call instruction
+
+    #### Possible Returns
+    List[str] -
+        A list of the parameters in declaration order
+    None -
+        Workaround: Unable to obtain list of parameters in declaration order
+    """
+    if isinstance(ins, NewStructure):
+        return [x.name for x in ins.structure.elems_ordered if not isinstance(x.type, MappingType)]
+    elif isinstance(ins, NewContract):
+        return [p.name for p in ins.contract.constructor.parameters]
+    elif isinstance(ins, (LowLevelCall, NewElementaryType, NewArray)):
+        # named arguments are incompatible with these call forms
+        assert False
+    elif isinstance(ins, HighLevelCall) and isinstance(ins.function, str):
+        return None
+    elif isinstance(ins, (InternalCall, LibraryCall, HighLevelCall)):
+        if isinstance(ins.function, Function):
+            return [p.name for p in ins.function.parameters]
+        else:
+            return None
+    elif isinstance(ins, InternalDynamicCall):
+        return [p.name for p in ins.function_type.params]
+    elif isinstance(ins, EventCall):
+        return None
+    else:
+        assert False
+
+def reorder_arguments(args: List[Variable], call_names: List[str], decl_names: List[str]) -> List[Variable]:
+    """
+    Reorder named struct constructor arguments so that they match struct declaration ordering rather
+    than call ordering
+
+    E.g. for `struct S { int x; int y; }` we reorder `S({y : 2, x : 3})` to `S(3, 2)`
+    #### Parameters
+    args -
+        Arguments to constructor call, in call order
+    names -
+        Parameter names in call order
+    decl_names -
+        Parameter names in declaration order
+
+    #### Returns
+    Reordered arguments to constructor call, now in declaration order
+    """
+    assert isinstance(args, list)
+    assert isinstance(call_names, list)
+    assert isinstance(decl_names, list)
+    assert len(args) == len(call_names)
+    assert len(call_names) == len(decl_names)
+
+    args_ret = []
+    for n in decl_names:
+        ind = call_names.index(n)
+        args_ret.append(args[ind])
+
+    return args_ret
 
 def propagate_type_and_convert_call(result: List[Operation], node: "Node") -> List[Operation]:
     """
@@ -450,6 +515,11 @@ def propagate_type_and_convert_call(result: List[Operation], node: "Node") -> Li
             if ins.call_id in calls_gas:
                 ins.call_gas = calls_gas[ins.call_id]
 
+        if isinstance(ins, Call) and (ins.names != None):
+            decl_param_names = get_declared_param_names(ins)
+            if decl_param_names != None:
+                call_data = reorder_arguments(call_data, ins.names, decl_param_names)
+
         if isinstance(ins, (Call, NewContract, NewStructure)):
             # We might have stored some arguments for libraries
             if ins.arguments:
@@ -883,7 +953,7 @@ def extract_tmp_call(
         if isinstance(ins.ori.variable_left, Contract):
             st = ins.ori.variable_left.get_structure_from_name(ins.ori.variable_right)
             if st:
-                op = NewStructure(st, ins.lvalue)
+                op = NewStructure(st, ins.lvalue, names=ins.names)
                 op.set_expression(ins.expression)
                 op.call_id = ins.call_id
                 return op
@@ -921,6 +991,7 @@ def extract_tmp_call(
                 ins.nbr_arguments,
                 ins.lvalue,
                 ins.type_call,
+                names=ins.names
             )
             custom_error_sym = resolve_error(str(ins.ori.variable_right), ins.ori.variable_left)
             if custom_error_sym is not None:
@@ -983,6 +1054,7 @@ def extract_tmp_call(
                     ins.lvalue,
                     "d",
                     has_receiver_arg=True,
+                    names=ins.names
                 )
                 lib_call.set_expression(ins.expression)
                 lib_call.set_node(ins.node)
@@ -1064,6 +1136,7 @@ def extract_tmp_call(
             ins.nbr_arguments,
             ins.lvalue,
             ins.type_call,
+            names = ins.names
         )
         msgcall.call_id = ins.call_id
 
@@ -1115,7 +1188,7 @@ def extract_tmp_call(
         return n
 
     if isinstance(ins.called, Structure):
-        op = NewStructure(ins.called, ins.lvalue)
+        op = NewStructure(ins.called, ins.lvalue, names=ins.names)
         op.set_expression(ins.expression)
         op.call_id = ins.call_id
         op.set_expression(ins.expression)
@@ -1139,7 +1212,7 @@ def extract_tmp_call(
         if len(ins.called.constructor.parameters) != ins.nbr_arguments:
             return Nop()
         internalcall = InternalCall(
-            ins.called.constructor, ins.nbr_arguments, ins.lvalue, ins.type_call
+            ins.called.constructor, ins.nbr_arguments, ins.lvalue, ins.type_call, ins.names
         )
         internalcall.call_id = ins.call_id
         internalcall.set_expression(ins.expression)
@@ -1465,7 +1538,7 @@ def look_for_library_or_top_level(
                 len(destination.parameters) == len(arguments)
                 and _find_function_from_parameter(arguments, [destination], True) is not None
             ):
-                internalcall = InternalCall(destination, ir.nbr_arguments, ir.lvalue, ir.type_call)
+                internalcall = InternalCall(destination, ir.nbr_arguments, ir.lvalue, ir.type_call, names=ir.names)
                 internalcall.set_expression(ir.expression)
                 internalcall.set_node(ir.node)
                 internalcall.arguments = [ir.destination] + ir.arguments
@@ -1491,6 +1564,7 @@ def look_for_library_or_top_level(
                 ir.lvalue,
                 ir.type_call,
                 has_receiver_arg=True,
+                names=ir.names
             )
             lib_call.set_expression(ir.expression)
             lib_call.set_node(ir.node)

slither/slithir/operations/call.py

@@ -4,9 +4,15 @@
 
 
 class Call(Operation):
-    def __init__(self) -> None:
+    def __init__(self, names : Optional[List[str]] = None) -> None:
         super().__init__()
+        assert (names == None) or isinstance(names, list)
         self._arguments = []
+        self._names = names
+
+    @property
+    def names(self) -> Optional[List[str]]:
+        return self._names
 
     @property
     def arguments(self):

slither/slithir/operations/high_level_call.py

@@ -28,14 +28,16 @@ def __init__(
         result: Optional[Union[TemporaryVariable, TupleVariable, TemporaryVariableSSA]],
         type_call: str,
         has_receiver_arg: bool = False,
+        names: Optional[List[str]] = None
     ) -> None:
         """
         - has_receiver_arg: True if the receiver expression is used as the first argument
+        - names: For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order
         """
         assert isinstance(function_name, Constant)
         assert is_valid_lvalue(result) or result is None
         self._check_destination(destination)
-        super().__init__()
+        super().__init__(names=names)
         self._destination = destination
         self._function_name = function_name
         self._nbr_arguments = nbr_arguments

slither/slithir/operations/internal_call.py

@@ -20,8 +20,9 @@ def __init__(
             Union[TupleVariableSSA, TemporaryVariableSSA, TupleVariable, TemporaryVariable]
         ],
         type_call: str,
+        names: Optional[List[str]] = None
     ) -> None:
-        super().__init__()
+        super().__init__(names=names)
         self._contract_name = ""
         if isinstance(function, Function):
             self._function = function

slither/slithir/operations/new_contract.py

@@ -10,11 +10,14 @@
 
 class NewContract(Call, OperationWithLValue):  # pylint: disable=too-many-instance-attributes
     def __init__(
-        self, contract_name: Constant, lvalue: Union[TemporaryVariableSSA, TemporaryVariable]
+        self,
+        contract_name: Constant,
+        lvalue: Union[TemporaryVariableSSA, TemporaryVariable],
+        names: Optional[List[str]] = None
     ) -> None:
         assert isinstance(contract_name, Constant)
         assert is_valid_lvalue(lvalue)
-        super().__init__()
+        super().__init__(names=names)
         self._contract_name = contract_name
         # todo create analyze to add the contract instance
         self._lvalue = lvalue

slither/slithir/operations/new_structure.py

@@ -1,4 +1,4 @@
-from typing import List, Union
+from typing import List, Optional, Union
 
 from slither.slithir.operations.call import Call
 from slither.slithir.operations.lvalue import OperationWithLValue
@@ -14,14 +14,15 @@
 
 class NewStructure(Call, OperationWithLValue):
     def __init__(
-        self, structure: StructureContract, lvalue: Union[TemporaryVariableSSA, TemporaryVariable]
+        self, structure: StructureContract, lvalue: Union[TemporaryVariableSSA, TemporaryVariable], names: Optional[List[str]]
     ) -> None:
-        super().__init__()
+        super().__init__(names = names)
         assert isinstance(structure, Structure)
         assert is_valid_lvalue(lvalue)
         self._structure = structure
         # todo create analyze to add the contract instance
         self._lvalue = lvalue
+        self._names = names
 
     @property
     def read(self) -> List[Union[TemporaryVariableSSA, TemporaryVariable, Constant]]:

slither/slithir/tmp_operations/tmp_call.py

@@ -1,4 +1,4 @@
-from typing import Optional, Union
+from typing import List, Optional, Union
 
 from slither.core.declarations import (
     Event,
@@ -25,6 +25,7 @@ def __init__(
         nbr_arguments: int,
         result: Union[TupleVariable, TemporaryVariable],
         type_call: str,
+        names: Optional[List[str]]
     ) -> None:
         assert isinstance(
             called,
@@ -42,13 +43,18 @@ def __init__(
         self._called = called
         self._nbr_arguments = nbr_arguments
         self._type_call = type_call
+        self._names = names
         self._lvalue = result
         self._ori = None  #
         self._callid = None
         self._gas = None
         self._value = None
         self._salt = None
 
+    @property
+    def names(self) -> Optional[List[str]]:
+        return self._names
+
     @property
     def call_value(self):
         return self._value

slither/slithir/utils/ssa.py

@@ -738,12 +738,13 @@ def copy_ir(ir: Operation, *instances) -> Operation:
         destination = get_variable(ir, lambda x: x.destination, *instances)
         function_name = ir.function_name
         nbr_arguments = ir.nbr_arguments
+        names = ir.names
         lvalue = get_variable(ir, lambda x: x.lvalue, *instances)
         type_call = ir.type_call
         if isinstance(ir, LibraryCall):
-            new_ir = LibraryCall(destination, function_name, nbr_arguments, lvalue, type_call)
+            new_ir = LibraryCall(destination, function_name, nbr_arguments, lvalue, type_call, names=names)
         else:
-            new_ir = HighLevelCall(destination, function_name, nbr_arguments, lvalue, type_call)
+            new_ir = HighLevelCall(destination, function_name, nbr_arguments, lvalue, type_call, names=names)
         new_ir.call_id = ir.call_id
         new_ir.call_value = get_variable(ir, lambda x: x.call_value, *instances)
         new_ir.call_gas = get_variable(ir, lambda x: x.call_gas, *instances)
@@ -763,9 +764,10 @@ def copy_ir(ir: Operation, *instances) -> Operation:
     if isinstance(ir, InternalCall):
         function = ir.function
         nbr_arguments = ir.nbr_arguments
+        names = ir.names
         lvalue = get_variable(ir, lambda x: x.lvalue, *instances)
         type_call = ir.type_call
-        new_ir = InternalCall(function, nbr_arguments, lvalue, type_call)
+        new_ir = InternalCall(function, nbr_arguments, lvalue, type_call, names=names)
         new_ir.arguments = get_arguments(ir, *instances)
         return new_ir
     if isinstance(ir, InternalDynamicCall):
@@ -815,8 +817,9 @@ def copy_ir(ir: Operation, *instances) -> Operation:
         return new_ir
     if isinstance(ir, NewStructure):
         structure = ir.structure
+        names = ir.names
         lvalue = get_variable(ir, lambda x: x.lvalue, *instances)
-        new_ir = NewStructure(structure, lvalue)
+        new_ir = NewStructure(structure, lvalue, names=names)
         new_ir.arguments = get_arguments(ir, *instances)
         return new_ir
     if isinstance(ir, Nop):

slither/solc_parsing/declarations/function.py

@@ -794,6 +794,7 @@ def _get_unknown_call(
                     "isConstant": False,
                     "typeDescriptions" : { "typeIdentifier": f"t_{typename}_memory_ptr", "typeString": f"{typename} memory" },
                     "arguments": [],
+                    "names": [],
                     "expression": {
                         "nodeType": "Identifier",
                         "name": f"certik_unknown_{typename}",
@@ -813,6 +814,7 @@ def _get_unknown_call(
                 "isConstant": False,
                 "typeDescriptions" : { "typeIdentifier": "t_uint_ptr", "typeString": "uint" },
                 "arguments": [],
+                "names": [],
                 "expression": {
                     "nodeType": "Identifier",
                     "name": "certik_unknown_uint",

slither/solc_parsing/expressions/expression_parsing.py

@@ -170,16 +170,18 @@ def parse_call(
         arguments = []
         if expression["arguments"]:
             arguments = [parse_expression(a, caller_context) for a in expression["arguments"]]
+        names = expression["names"] if len(expression["names"]) > 0 else None
     else:
         children = expression["children"]
         called = parse_expression(children[0], caller_context)
         arguments = [parse_expression(a, caller_context) for a in children[1::]]
+        names = expression["names"] if len(expression["names"]) > 0 else None
 
     if isinstance(called, SuperCallExpression):
         sp = SuperCallExpression(called, arguments, type_return)
         sp.set_offset(expression["src"], caller_context.compilation_unit)
         return sp
-    call_expression = CallExpression(called, arguments, type_return)
+    call_expression = CallExpression(called, arguments, type_return, names=names)
     call_expression.set_offset(src, caller_context.compilation_unit)
 
     # Only available if the syntax {gas:, value:} was used