Merge pull request #2376 from crytic/feat/virtual-override-with-refs

This PR improves the reference API to account for virtual inheritance and abstract contracts. It treats each virtual override defintion and contract definition that inherits from an interface or abstract contract as an "implementation". This lays the groundwork for LSP features and unused definition/ import detector.

setup.py

@@ -15,8 +15,8 @@
         "packaging",
         "prettytable>=3.3.0",
         "pycryptodome>=3.4.6",
-        "crytic-compile>=0.3.5,<0.4.0",
-        # "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile",
+        # "crytic-compile>=0.3.5,<0.4.0",
+        "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile",
         "web3>=6.0.0",
         "eth-abi>=4.0.0",
         "eth-typing>=3.0.0",

slither/core/declarations/contract.py

@@ -93,6 +93,7 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit", scope: "FileScope
         self._is_interface: bool = False
         self._is_library: bool = False
         self._is_fully_implemented: bool = False
+        self._is_abstract: bool = False
 
         self._signatures: Optional[List[str]] = None
         self._signatures_declared: Optional[List[str]] = None
@@ -203,12 +204,34 @@ def comments(self, comments: str):
 
     @property
     def is_fully_implemented(self) -> bool:
+        """
+        bool: True if the contract defines all functions.
+        In modern Solidity, virtual functions can lack an implementation.
+        Prior to Solidity 0.6.0, functions like the following would be not fully implemented:
+        ```solidity
+        contract ImplicitAbstract{
+            function f() public;
+        }
+        ```
+        """
         return self._is_fully_implemented
 
     @is_fully_implemented.setter
     def is_fully_implemented(self, is_fully_implemented: bool):
         self._is_fully_implemented = is_fully_implemented
 
+    @property
+    def is_abstract(self) -> bool:
+        """
+        Note for Solidity < 0.6.0 it will always be false
+        bool: True if the contract is abstract.
+        """
+        return self._is_abstract
+
+    @is_abstract.setter
+    def is_abstract(self, is_abstract: bool):
+        self._is_abstract = is_abstract
+
     # endregion
     ###################################################################################
     ###################################################################################
@@ -996,16 +1019,14 @@ def get_enum_from_canonical_name(self, enum_name: str) -> Optional["Enum"]:
 
     def get_functions_overridden_by(self, function: "Function") -> List["Function"]:
         """
-            Return the list of functions overriden by the function
+            Return the list of functions overridden by the function
         Args:
             (core.Function)
         Returns:
             list(core.Function)
 
         """
-        candidatess = [c.functions_declared for c in self.inheritance]
-        candidates = [candidate for sublist in candidatess for candidate in sublist]
-        return [f for f in candidates if f.full_name == function.full_name]
+        return function.overrides
 
     # endregion
     ###################################################################################

slither/core/declarations/function.py

@@ -37,7 +37,7 @@
         HighLevelCallType,
         LibraryCallType,
     )
-    from slither.core.declarations import Contract
+    from slither.core.declarations import Contract, FunctionContract
     from slither.core.cfg.node import Node, NodeType
     from slither.core.variables.variable import Variable
     from slither.slithir.variables.variable import SlithIRVariable
@@ -46,7 +46,6 @@
     from slither.slithir.operations import Operation
     from slither.core.compilation_unit import SlitherCompilationUnit
     from slither.core.scope.scope import FileScope
-    from slither.slithir.variables.state_variable import StateIRVariable
 
 LOGGER = logging.getLogger("Function")
 ReacheableNode = namedtuple("ReacheableNode", ["node", "ir"])
@@ -126,6 +125,9 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit") -> None:
         self._pure: bool = False
         self._payable: bool = False
         self._visibility: Optional[str] = None
+        self._virtual: bool = False
+        self._overrides: List["FunctionContract"] = []
+        self._overridden_by: List["FunctionContract"] = []
 
         self._is_implemented: Optional[bool] = None
         self._is_empty: Optional[bool] = None
@@ -441,6 +443,49 @@ def payable(self) -> bool:
     def payable(self, p: bool):
         self._payable = p
 
+    # endregion
+    ###################################################################################
+    ###################################################################################
+    # region Virtual
+    ###################################################################################
+    ###################################################################################
+
+    @property
+    def is_virtual(self) -> bool:
+        """
+        Note for Solidity < 0.6.0 it will always be false
+        bool: True if the function is virtual
+        """
+        return self._virtual
+
+    @is_virtual.setter
+    def is_virtual(self, v: bool):
+        self._virtual = v
+
+    @property
+    def is_override(self) -> bool:
+        """
+        Note for Solidity < 0.6.0 it will always be false
+        bool: True if the function overrides a base function
+        """
+        return len(self._overrides) > 0
+
+    @property
+    def overridden_by(self) -> List["FunctionContract"]:
+        """
+        List["FunctionContract"]: List of functions in child contracts that override this function
+        This may include distinct instances of the same function due to inheritance
+        """
+        return self._overridden_by
+
+    @property
+    def overrides(self) -> List["FunctionContract"]:
+        """
+        List["FunctionContract"]: List of functions in parent contracts that this function overrides
+        This may include distinct instances of the same function due to inheritance
+        """
+        return self._overrides
+
     # endregion
     ###################################################################################
     ###################################################################################

slither/core/slither_core.py

@@ -22,7 +22,7 @@
 from slither.slithir.variables import Constant
 from slither.utils.colors import red
 from slither.utils.sarif import read_triage_info
-from slither.utils.source_mapping import get_definition, get_references, get_implementation
+from slither.utils.source_mapping import get_definition, get_references, get_all_implementations
 
 logger = logging.getLogger("Slither")
 logging.basicConfig()
@@ -204,41 +204,53 @@ def offset_to_objects(self, filename_str: str, offset: int) -> Set[SourceMapping
     def _compute_offsets_from_thing(self, thing: SourceMapping):
         definition = get_definition(thing, self.crytic_compile)
         references = get_references(thing)
-        implementation = get_implementation(thing)
+        implementations = get_all_implementations(thing, self.contracts)
 
         for offset in range(definition.start, definition.end + 1):
-
             if (
-                isinstance(thing, TopLevel)
+                isinstance(thing, (TopLevel, Contract))
                 or (
                     isinstance(thing, FunctionContract)
                     and thing.contract_declarer == thing.contract
                 )
                 or (isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract))
             ):
+
                 self._offset_to_objects[definition.filename][offset].add(thing)
 
             self._offset_to_definitions[definition.filename][offset].add(definition)
-            self._offset_to_implementations[definition.filename][offset].add(implementation)
+            self._offset_to_implementations[definition.filename][offset].update(implementations)
             self._offset_to_references[definition.filename][offset] |= set(references)
 
         for ref in references:
             for offset in range(ref.start, ref.end + 1):
-
+                is_declared_function = (
+                    isinstance(thing, FunctionContract)
+                    and thing.contract_declarer == thing.contract
+                )
                 if (
                     isinstance(thing, TopLevel)
-                    or (
-                        isinstance(thing, FunctionContract)
-                        and thing.contract_declarer == thing.contract
-                    )
+                    or is_declared_function
                     or (
                         isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract)
                     )
                 ):
                     self._offset_to_objects[definition.filename][offset].add(thing)
 
-                self._offset_to_definitions[ref.filename][offset].add(definition)
-                self._offset_to_implementations[ref.filename][offset].add(implementation)
+                if is_declared_function:
+                    # Only show the nearest lexical definition for declared contract-level functions
+                    if (
+                        thing.contract.source_mapping.start
+                        < offset
+                        < thing.contract.source_mapping.end
+                    ):
+
+                        self._offset_to_definitions[ref.filename][offset].add(definition)
+
+                else:
+                    self._offset_to_definitions[ref.filename][offset].add(definition)
+
+                self._offset_to_implementations[ref.filename][offset].update(implementations)
                 self._offset_to_references[ref.filename][offset] |= set(references)
 
     def _compute_offsets_to_ref_impl_decl(self):  # pylint: disable=too-many-branches
@@ -251,15 +263,18 @@ def _compute_offsets_to_ref_impl_decl(self):  # pylint: disable=too-many-branche
             for contract in compilation_unit.contracts:
                 self._compute_offsets_from_thing(contract)
 
-                for function in contract.functions:
+                for function in contract.functions_declared:
                     self._compute_offsets_from_thing(function)
                     for variable in function.local_variables:
                         self._compute_offsets_from_thing(variable)
-                for modifier in contract.modifiers:
+                for modifier in contract.modifiers_declared:
                     self._compute_offsets_from_thing(modifier)
                     for variable in modifier.local_variables:
                         self._compute_offsets_from_thing(variable)
 
+                for var in contract.state_variables:
+                    self._compute_offsets_from_thing(var)
+
                 for st in contract.structures:
                     self._compute_offsets_from_thing(st)
 
@@ -268,6 +283,10 @@ def _compute_offsets_to_ref_impl_decl(self):  # pylint: disable=too-many-branche
 
                 for event in contract.events:
                     self._compute_offsets_from_thing(event)
+
+                for typ in contract.type_aliases:
+                    self._compute_offsets_from_thing(typ)
+
             for enum in compilation_unit.enums_top_level:
                 self._compute_offsets_from_thing(enum)
             for event in compilation_unit.events_top_level:
@@ -276,6 +295,14 @@ def _compute_offsets_to_ref_impl_decl(self):  # pylint: disable=too-many-branche
                 self._compute_offsets_from_thing(function)
             for st in compilation_unit.structures_top_level:
                 self._compute_offsets_from_thing(st)
+            for var in compilation_unit.variables_top_level:
+                self._compute_offsets_from_thing(var)
+            for typ in compilation_unit.type_aliases.values():
+                self._compute_offsets_from_thing(typ)
+            for err in compilation_unit.custom_errors:
+                self._compute_offsets_from_thing(err)
+            for event in compilation_unit.events_top_level:
+                self._compute_offsets_from_thing(event)
             for import_directive in compilation_unit.import_directives:
                 self._compute_offsets_from_thing(import_directive)
             for pragma in compilation_unit.pragma_directives:

slither/printers/summary/declaration.py

@@ -21,18 +21,20 @@ def output(self, _filename: str) -> Output:
             txt += "\n# Contracts\n"
             for contract in compilation_unit.contracts:
                 txt += f"# {contract.name}\n"
-                txt += f"\t- Declaration: {get_definition(contract, compilation_unit.core.crytic_compile).to_detailed_str()}\n"
-                txt += f"\t- Implementation: {get_implementation(contract).to_detailed_str()}\n"
+                contract_def = get_definition(contract, compilation_unit.core.crytic_compile)
+                txt += f"\t- Declaration: {contract_def.to_detailed_str()}\n"
+                txt += f"\t- Implementation(s): {[x.to_detailed_str() for x in list(self.slither.offset_to_implementations(contract.source_mapping.filename.absolute, contract_def.start))]}\n"
                 txt += (
                     f"\t- References: {[x.to_detailed_str() for x in get_references(contract)]}\n"
                 )
 
                 txt += "\n\t## Function\n"
 
-                for func in contract.functions:
+                for func in contract.functions_declared:
                     txt += f"\t\t- {func.canonical_name}\n"
-                    txt += f"\t\t\t- Declaration: {get_definition(func, compilation_unit.core.crytic_compile).to_detailed_str()}\n"
-                    txt += f"\t\t\t- Implementation: {get_implementation(func).to_detailed_str()}\n"
+                    function_def = get_definition(func, compilation_unit.core.crytic_compile)
+                    txt += f"\t\t\t- Declaration: {function_def.to_detailed_str()}\n"
+                    txt += f"\t\t\t- Implementation(s): {[x.to_detailed_str() for x in list(self.slither.offset_to_implementations(func.source_mapping.filename.absolute, function_def.start))]}\n"
                     txt += f"\t\t\t- References: {[x.to_detailed_str() for x in get_references(func)]}\n"
 
                 txt += "\n\t## State variables\n"

slither/slithir/convert.py

@@ -1209,7 +1209,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call,
         internalcall.set_expression(ins.expression)
         return internalcall
 
-    raise Exception(f"Not extracted {type(ins.called)} {ins}")
+    raise SlithIRError(f"Not extracted {type(ins.called)} {ins}")
 
 
 # endregion
@@ -2014,6 +2014,9 @@ def _find_source_mapping_references(irs: List[Operation]) -> None:
         if isinstance(ir, NewContract):
             ir.contract_created.references.append(ir.expression.source_mapping)
 
+        if isinstance(ir, HighLevelCall):
+            ir.function.references.append(ir.expression.source_mapping)
+
 
 # endregion
 ###################################################################################

slither/solc_parsing/declarations/contract.py

@@ -1,6 +1,6 @@
 import logging
 import re
-from typing import Any, List, Dict, Callable, TYPE_CHECKING, Union, Set, Sequence
+from typing import Any, List, Dict, Callable, TYPE_CHECKING, Union, Set, Sequence, Tuple
 
 from slither.core.declarations import (
     Modifier,
@@ -64,7 +64,8 @@ def __init__(
         # use to remap inheritance id
         self._remapping: Dict[str, str] = {}
 
-        self.baseContracts: List[str] = []
+        # (referencedDeclaration, offset)
+        self.baseContracts: List[Tuple[int, str]] = []
         self.baseConstructorContractsCalled: List[str] = []
         self._linearized_base_contracts: List[int]
 
@@ -174,6 +175,9 @@ def _parse_contract_info(self) -> None:
         self._contract.is_fully_implemented = attributes["fullyImplemented"]
         self._linearized_base_contracts = attributes["linearizedBaseContracts"]
 
+        if "abstract" in attributes:
+            self._contract.is_abstract = attributes["abstract"]
+
         # Parse base contract information
         self._parse_base_contract_info()
 
@@ -201,7 +205,9 @@ def _parse_base_contract_info(self) -> None:  # pylint: disable=too-many-branche
 
                     # Obtain our contract reference and add it to our base contract list
                     referencedDeclaration = base_contract["baseName"]["referencedDeclaration"]
-                    self.baseContracts.append(referencedDeclaration)
+                    self.baseContracts.append(
+                        (referencedDeclaration, base_contract["baseName"]["src"])
+                    )
 
                     # If we have defined arguments in our arguments object, this is a constructor invocation.
                     # (note: 'arguments' can be [], which is not the same as None. [] implies a constructor was
@@ -233,7 +239,10 @@ def _parse_base_contract_info(self) -> None:  # pylint: disable=too-many-branche
                     referencedDeclaration = base_contract_items[0]["attributes"][
                         "referencedDeclaration"
                     ]
-                    self.baseContracts.append(referencedDeclaration)
+
+                    self.baseContracts.append(
+                        (referencedDeclaration, base_contract_items[0]["src"])
+                    )
 
                     # If we have an 'attributes'->'arguments' which is None, this is not a constructor call.
                     if (