Merge pull request #406 from crytic/dev-modifier-calls

Change how modifier/constructor calls statements are handled

slither/core/cfg/node.py

@@ -63,7 +63,7 @@ class NodeType:
     PLACEHOLDER = 0x40
 
     # Node not related to the CFG
-    # Use for state variable declaration, or modifier calls
+    # Use for state variable declaration
     OTHER_ENTRYPOINT = 0x50
 
 
@@ -111,6 +111,15 @@ def link_nodes(n1, n2):
     n1.add_son(n2)
     n2.add_father(n1)
 
+def insert_node(origin, node_inserted):
+    sons = origin.sons
+    link_nodes(origin, node_inserted)
+    for son in sons:
+        son.remove_father(origin)
+        origin.remove_son(son)
+
+        link_nodes(node_inserted, son)
+
 def recheable(node):
     '''
     Return the set of nodes reacheable from the node

slither/core/declarations/function.py

@@ -1357,13 +1357,6 @@ class Function(ChildContract, ChildInheritance, SourceMapping):
         for node in self.nodes:
             node.slithir_generation()
 
-        for modifier_statement in self.modifiers_statements:
-            for node in modifier_statement.nodes:
-                node.slithir_generation()
-
-        for modifier_statement in self.explicit_base_constructor_calls_statements:
-            for node in modifier_statement.nodes:
-                node.slithir_generation()
 
         self._analyze_read_write()
         self._analyze_calls()

slither/formatters/naming_convention/naming_convention.py

@@ -392,16 +392,6 @@ def _explore_variables_declaration(slither, variables, result, target, convert,
 
 
 
-def _explore_modifiers_calls(slither, function, result, target, convert):
-    for modifier in function.modifiers_statements:
-        for node in modifier.nodes:
-            if node.irs:
-                _explore_irs(slither, node.irs, result, target, convert)
-    for modifier in function.explicit_base_constructor_calls_statements:
-        for node in modifier.nodes:
-            if node.irs:
-                _explore_irs(slither, node.irs, result, target, convert)
-
 def _explore_structures_declaration(slither, structures, result, target, convert):
     for st in structures:
         # Explore the variable declared within the structure (VariableStructure)
@@ -514,7 +504,6 @@ def _explore_irs(slither, irs, result, target, convert):
 def _explore_functions(slither, functions, result, target, convert):
     for function in functions:
         _explore_variables_declaration(slither, function.variables, result, target, convert, True)
-        _explore_modifiers_calls(slither, function, result, target, convert)
         _explore_irs(slither, function.all_slithir_operations(), result, target, convert)
 
         if isinstance(target, Function) and function.canonical_name == target.canonical_name:

slither/printers/summary/slithir.py

@@ -33,10 +33,6 @@ class PrinterSlithIR(AbstractPrinter):
                         txt += '\t\tIRs:\n'
                         for ir in node.irs:
                             txt += '\t\t\t{}\n'.format(ir)
-                for modifier_statement in function.modifiers_statements:
-                    txt += f'\t\tModifier Call {modifier_statement.entry_point.expression}\n'
-                for modifier_statement in function.explicit_base_constructor_calls_statements:
-                    txt += f'\t\tConstructor Call {modifier_statement.entry_point.expression}\n'
             for modifier in contract.modifiers:
                 txt += '\tModifier {}\n'.format(modifier.canonical_name)
                 for node in modifier.nodes:

slither/slithir/operations/internal_call.py

@@ -1,3 +1,4 @@
+from slither.core.declarations import Modifier
 from slither.core.declarations.function import Function
 from slither.slithir.operations.call import Call
 from slither.slithir.operations.lvalue import OperationWithLValue
@@ -47,6 +48,14 @@ class InternalCall(Call, OperationWithLValue):
     def type_call(self):
         return self._type_call
 
+    @property
+    def is_modifier_call(self):
+        """
+        Check if the destination is a modifier
+        :return: bool
+        """
+        return isinstance(self.function, Modifier)
+
     def __str__(self):
         args = [str(a) for a in self.arguments]
         if not self.lvalue:
@@ -55,7 +64,10 @@ class InternalCall(Call, OperationWithLValue):
             lvalue = '{}({}) = '.format(self.lvalue, ','.join(str(x) for x in self.lvalue.type))
         else:
             lvalue = '{}({}) = '.format(self.lvalue, self.lvalue.type)
-        txt = '{}INTERNAL_CALL, {}({})'
+        if self.is_modifier_call:
+            txt = '{}MODIFIER_CALL, {}({})'
+        else:
+            txt = '{}INTERNAL_CALL, {}({})'
         return txt.format(lvalue,
                           self.function.canonical_name,
                           ','.join(args))

slither/slithir/utils/ssa.py

@@ -17,7 +17,7 @@ from slither.slithir.operations import (Assignment, Balance, Binary, Condition,
                                         OperationWithLValue, Phi, PhiCallback,
                                         Push, Return, Send, SolidityCall,
                                         Transfer, TypeConversion, Unary,
-                                        Unpack)
+                                        Unpack, Nop)
 from slither.slithir.variables import (Constant, LocalIRVariable,
                                        ReferenceVariable, ReferenceVariableSSA,
                                        StateIRVariable, TemporaryVariable,
@@ -620,6 +620,8 @@ def copy_ir(ir, *instances):
         new_ir = NewStructure(structure, lvalue)
         new_ir.arguments = get_arguments(ir, *instances)
         return new_ir
+    elif isinstance(ir, Nop):
+        return Nop()
     elif isinstance(ir, Push):
         array = get_variable(ir, lambda x: x.array, *instances)
         lvalue = get_variable(ir, lambda x: x.lvalue, *instances)

slither/solc_parsing/declarations/function.py

@@ -2,7 +2,7 @@
 """
 import logging
 
-from slither.core.cfg.node import NodeType, link_nodes, recheable
+from slither.core.cfg.node import NodeType, link_nodes, insert_node
 from slither.core.declarations.contract import Contract
 from slither.core.declarations.function import Function, ModifierStatements, FunctionType
 
@@ -220,12 +220,7 @@ class FunctionSolc(Function):
         for node in self.nodes:
             node.analyze_expressions(self)
 
-        if self._filter_ternary():
-            for modifier_statement in self.modifiers_statements:
-                modifier_statement.nodes = recheable(modifier_statement.entry_point)
-
-            for modifier_statement in self.explicit_base_constructor_calls_statements:
-                modifier_statement.nodes = recheable(modifier_statement.entry_point)
+        self._filter_ternary()
 
         self._remove_alone_endif()
 
@@ -910,23 +905,37 @@ class FunctionSolc(Function):
     def _parse_modifier(self, modifier):
         m = parse_expression(modifier, self)
         self._expression_modifiers.append(m)
+
+        # Do not parse modifier nodes for interfaces
+        if not self._is_implemented:
+            return
+
         for m in ExportValues(m).result():
             if isinstance(m, Function):
-                entry_point = self._new_node(NodeType.OTHER_ENTRYPOINT, modifier['src'])
                 node = self._new_node(NodeType.EXPRESSION, modifier['src'])
                 node.add_unparsed_expression(modifier)
-                link_nodes(entry_point, node)
+                # The latest entry point is the entry point, or the latest modifier call
+                if self._modifiers:
+                    latest_entry_point = self._modifiers[-1].nodes[-1]
+                else:
+                    latest_entry_point = self.entry_point
+                insert_node(latest_entry_point, node)
                 self._modifiers.append(ModifierStatements(modifier=m,
-                                                          entry_point=entry_point,
-                                                          nodes=[entry_point, node]))
+                                                          entry_point=latest_entry_point,
+                                                          nodes=[latest_entry_point, node]))
+
             elif isinstance(m, Contract):
-                entry_point = self._new_node(NodeType.OTHER_ENTRYPOINT, modifier['src'])
                 node = self._new_node(NodeType.EXPRESSION, modifier['src'])
                 node.add_unparsed_expression(modifier)
-                link_nodes(entry_point, node)
+                # The latest entry point is the entry point, or the latest constructor call
+                if self._explicit_base_constructor_calls:
+                    latest_entry_point = self._explicit_base_constructor_calls[-1].nodes[-1]
+                else:
+                    latest_entry_point = self.entry_point
+                insert_node(latest_entry_point, node)
                 self._explicit_base_constructor_calls.append(ModifierStatements(modifier=m,
-                                                                                entry_point=entry_point,
-                                                                                nodes=[entry_point, node]))
+                                                                                entry_point=latest_entry_point,
+                                                                                nodes=[latest_entry_point, node]))
 
     # endregion
     ###################################################################################