Improve find_variable

Fix dupplicate parsers
Minor code improvements
Prevent the CI to run on solc 0.8

slither/slithir/convert.py

@@ -13,7 +13,6 @@ from slither.core.declarations import (
     Structure,
 )
 from slither.core.declarations.function_contract import FunctionContract
-from slither.core.declarations.solidity_variables import SolidityImportPlaceHolder
 from slither.core.expressions import Identifier, Literal
 from slither.core.solidity_types import (
     ArrayType,
@@ -26,10 +25,9 @@ from slither.core.solidity_types import (
 from slither.core.solidity_types.elementary_type import Int as ElementaryTypeInt
 from slither.core.solidity_types.type import Type
 from slither.core.variables.function_type_variable import FunctionTypeVariable
-from slither.core.variables.variable import Variable
 from slither.core.variables.state_variable import StateVariable
-from slither.slithir.operations.codesize import CodeSize
-from slither.slithir.variables import TupleVariable
+from slither.core.variables.variable import Variable
+from slither.slithir.exceptions import SlithIRError
 from slither.slithir.operations import (
     Assignment,
     Balance,
@@ -63,6 +61,7 @@ from slither.slithir.operations import (
     Unpack,
     Nop,
 )
+from slither.slithir.operations.codesize import CodeSize
 from slither.slithir.tmp_operations.argument import Argument, ArgumentType
 from slither.slithir.tmp_operations.tmp_call import TmpCall
 from slither.slithir.tmp_operations.tmp_new_array import TmpNewArray
@@ -70,10 +69,10 @@ from slither.slithir.tmp_operations.tmp_new_contract import TmpNewContract
 from slither.slithir.tmp_operations.tmp_new_elementary_type import TmpNewElementaryType
 from slither.slithir.tmp_operations.tmp_new_structure import TmpNewStructure
 from slither.slithir.variables import Constant, ReferenceVariable, TemporaryVariable
-from slither.visitors.slithir.expression_to_slithir import ExpressionToSlithIR
+from slither.slithir.variables import TupleVariable
 from slither.utils.function import get_function_id
 from slither.utils.type import export_nested_types_from_variable
-from slither.slithir.exceptions import SlithIRError
+from slither.visitors.slithir.expression_to_slithir import ExpressionToSlithIR
 
 if TYPE_CHECKING:
     from slither.core.cfg.node import Node
@@ -433,7 +432,6 @@ def _convert_type_contract(ir, slither):
         assignment.lvalue.set_type(ElementaryType("string"))
         return assignment
 
-
     raise SlithIRError(f"type({contract.name}).{ir.variable_right} is unknown")
 
 

slither/solc_parsing/declarations/contract.py

@@ -1,7 +1,7 @@
 import logging
-from typing import List, Dict, Callable, TYPE_CHECKING, Union
+from typing import List, Dict, Callable, TYPE_CHECKING, Union, Set
 
-from slither.core.declarations import Modifier, Event, EnumContract, StructureContract
+from slither.core.declarations import Modifier, Event, EnumContract, StructureContract, Function
 from slither.core.declarations.contract import Contract
 from slither.core.declarations.function_contract import FunctionContract
 from slither.core.variables.state_variable import StateVariable
@@ -301,7 +301,7 @@ class ContractSolc:
         self._modifiers_no_params.append(modif_parser)
         self._modifiers_parser.append(modif_parser)
 
-        self._slither_parser.add_functions_parser(modif_parser)
+        self._slither_parser.add_function_or_modifier_parser(modif_parser)
 
     def parse_modifiers(self):
         for modifier in self._modifiersNotParsed:
@@ -319,7 +319,7 @@ class ContractSolc:
         self._functions_no_params.append(func_parser)
         self._functions_parser.append(func_parser)
 
-        self._slither_parser.add_functions_parser(func_parser)
+        self._slither_parser.add_function_or_modifier_parser(func_parser)
 
     def parse_functions(self):
 
@@ -395,6 +395,47 @@ class ContractSolc:
             self.log_incorrect_parsing(f"Missing params {e}")
         self._functions_no_params = []
 
+    def _analyze_params_element(  # pylint: disable=too-many-arguments
+        self,
+        Cls: Callable,
+        Cls_parser: Callable,
+        element_parser: FunctionSolc,
+        explored_reference_id: Set[int],
+        parser: List[FunctionSolc],
+        all_elements: Dict[str, Function],
+    ):
+        elem = Cls(self.slither)
+        elem.set_contract(self._contract)
+        underlying_function = element_parser.underlying_function
+        # TopLevel function are not analyzed here
+        assert isinstance(underlying_function, FunctionContract)
+        elem.set_contract_declarer(underlying_function.contract_declarer)
+        elem.set_offset(
+            element_parser.function_not_parsed["src"], self._contract.slither,
+        )
+
+        elem_parser = Cls_parser(
+            elem, element_parser.function_not_parsed, self, self.slither_parser
+        )
+        if (
+            element_parser.referenced_declaration
+            and element_parser.referenced_declaration in explored_reference_id
+        ):
+            # Already added from other fathers
+            return
+        if element_parser.referenced_declaration:
+            explored_reference_id.add(element_parser.referenced_declaration)
+        elem_parser.analyze_params()
+        if isinstance(elem, Modifier):
+            self._contract.slither.add_modifier(elem)
+        else:
+            self._contract.slither.add_function(elem)
+
+        self._slither_parser.add_function_or_modifier_parser(elem_parser)
+
+        all_elements[elem.canonical_name] = elem
+        parser.append(elem_parser)
+
     def _analyze_params_elements(  # pylint: disable=too-many-arguments,too-many-locals
         self,
         elements_no_params: List[FunctionSolc],
@@ -417,34 +458,15 @@ class ContractSolc:
         """
         all_elements = {}
 
+        explored_reference_id = set()
         try:
             for father in self._contract.inheritance:
                 father_parser = self._slither_parser.underlying_contract_to_parser[father]
                 for element_parser in getter(father_parser):
-                    elem = Cls(self.slither)
-                    elem.set_contract(self._contract)
-                    underlying_function = element_parser.underlying_function
-                    # TopLevel function are not analyzed here
-                    assert isinstance(underlying_function, FunctionContract)
-                    elem.set_contract_declarer(underlying_function.contract_declarer)
-                    elem.set_offset(
-                        element_parser.function_not_parsed["src"], self._contract.slither,
+                    self._analyze_params_element(
+                        Cls, Cls_parser, element_parser, explored_reference_id, parser, all_elements
                     )
 
-                    elem_parser = Cls_parser(
-                        elem, element_parser.function_not_parsed, self, self.slither_parser
-                    )
-                    elem_parser.analyze_params()
-                    if isinstance(elem, Modifier):
-                        self._contract.slither.add_modifier(elem)
-                    else:
-                        self._contract.slither.add_function(elem)
-
-                    self._slither_parser.add_functions_parser(elem_parser)
-
-                    all_elements[elem.canonical_name] = elem
-                    parser.append(elem_parser)
-
             accessible_elements = self._contract.available_elements_from_inheritances(
                 all_elements, getter_available
             )

slither/solc_parsing/expressions/expression_parsing.py

@@ -1,6 +1,6 @@
 import logging
 import re
-from typing import Dict, TYPE_CHECKING, Optional, Union, List
+from typing import Dict, TYPE_CHECKING, Optional, Union, List, Tuple
 
 from slither.core.declarations import Event, Enum, Structure
 from slither.core.declarations.contract import Contract
@@ -38,7 +38,6 @@ from slither.core.expressions.super_identifier import SuperIdentifier
 from slither.core.expressions.tuple_expression import TupleExpression
 from slither.core.expressions.type_conversion import TypeConversion
 from slither.core.expressions.unary_operation import UnaryOperation, UnaryOperationType
-from slither.core.slither_core import SlitherCore
 from slither.core.solidity_types import (
     ArrayType,
     ElementaryType,
@@ -46,6 +45,7 @@ from slither.core.solidity_types import (
     MappingType,
 )
 from slither.core.variables.variable import Variable
+from slither.exceptions import SlitherError
 from slither.solc_parsing.exceptions import ParsingError, VariableNotFound
 from slither.solc_parsing.solidity_types.type_parsing import UnknownType, parse_type
 
@@ -53,6 +53,8 @@ if TYPE_CHECKING:
     from slither.core.expressions.expression import Expression
     from slither.solc_parsing.declarations.function import FunctionSolc
     from slither.solc_parsing.declarations.contract import ContractSolc
+    from slither.core.slither_core import SlitherCore
+    from slither.solc_parsing.slitherSolc import SlitherSolc
 
 logger = logging.getLogger("ExpressionParsing")
 
@@ -95,7 +97,10 @@ def _find_variable_from_ref_declaration(
         if contract_candidate.id == referenced_declaration:
             return contract_candidate
     for function_candidate in all_functions_parser:
-        if function_candidate.referenced_declaration == referenced_declaration:
+        if (
+            function_candidate.referenced_declaration == referenced_declaration
+            and not function_candidate.underlying_function.is_shadowed
+        ):
             return function_candidate.underlying_function
     return None
 
@@ -217,6 +222,47 @@ def _find_in_contract(
     return None
 
 
+def _find_variable_init(
+    caller_context: CallerContext,
+) -> Tuple[List[Contract], Union[List["FunctionSolc"]], "SlitherCore", "SlitherSolc"]:
+    from slither.solc_parsing.slitherSolc import SlitherSolc
+    from slither.solc_parsing.declarations.contract import ContractSolc
+    from slither.solc_parsing.declarations.function import FunctionSolc
+
+    direct_contracts: List[Contract]
+    direct_functions_parser: List[FunctionSolc]
+
+    if isinstance(caller_context, SlitherSolc):
+        direct_contracts = []
+        direct_functions_parser = []
+        sl = caller_context.core
+        sl_parser = caller_context
+    elif isinstance(caller_context, ContractSolc):
+        direct_contracts = [caller_context.underlying_contract]
+        direct_functions_parser = caller_context.functions_parser + caller_context.modifiers_parser
+        sl = caller_context.slither
+        sl_parser = caller_context.slither_parser
+    elif isinstance(caller_context, FunctionSolc):
+        if caller_context.contract_parser:
+            direct_contracts = [caller_context.contract_parser.underlying_contract]
+            direct_functions_parser = (
+                caller_context.contract_parser.functions_parser
+                + caller_context.contract_parser.modifiers_parser
+            )
+        else:
+            # Top level functions
+            direct_contracts = []
+            direct_functions_parser = []
+        sl = caller_context.slither
+        sl_parser = caller_context.slither_parser
+    else:
+        raise SlitherError(
+            f"{type(caller_context)} ({caller_context} is not valid for find_variable"
+        )
+
+    return direct_contracts, direct_functions_parser, sl, sl_parser
+
+
 def find_variable(
     var_name: str,
     caller_context: CallerContext,
@@ -225,9 +271,8 @@ def find_variable(
 ) -> Union[
     Variable, Function, Contract, SolidityVariable, SolidityFunction, Event, Enum, Structure,
 ]:
-    from slither.solc_parsing.declarations.contract import ContractSolc
     from slither.solc_parsing.declarations.function import FunctionSolc
-    from slither.solc_parsing.slitherSolc import SlitherSolc
+    from slither.solc_parsing.declarations.contract import ContractSolc
 
     # variable are looked from the contract declarer
     # functions can be shadowed, but are looked from the contract instance, rather than the contract declarer
@@ -244,14 +289,22 @@ def find_variable(
     # for events it's unclear what should be the behavior, as they can be shadowed, but there is not impact
     # structure/enums cannot be shadowed
 
-    sl: SlitherCore = caller_context.core if isinstance(
-        caller_context, SlitherSolc
-    ) else caller_context.slither
-    sl_parser: SlitherSolc = caller_context if isinstance(
-        caller_context, SlitherSolc
-    ) else caller_context.slither_parser
+    direct_contracts, direct_functions_parser, sl, sl_parser = _find_variable_init(caller_context)
+
     all_contracts = sl.contracts
-    all_functions_parser = sl_parser.all_functions_parser
+    all_functions_parser = sl_parser.all_functions_and_modifiers_parser
+
+    # Look for all references delcaration
+    # First look only in the context of function/contract
+    # Then look everywhere
+    # Because functions are copied between contracts, two functions can have the same ref
+    # So we need to first look with respect to the direct context
+
+    ret = _find_variable_from_ref_declaration(
+        referenced_declaration, direct_contracts, direct_functions_parser
+    )
+    if ret:
+        return ret
 
     ret = _find_variable_from_ref_declaration(
         referenced_declaration, all_contracts, all_functions_parser
@@ -266,10 +319,6 @@ def find_variable(
     if ret:
         return ret
 
-    ret = _find_top_level(var_name, sl)
-    if ret:
-        return ret
-
     contract: Optional[Contract] = None
     contract_declarer: Optional[Contract] = None
     if isinstance(caller_context, ContractSolc):
@@ -302,6 +351,11 @@ def find_variable(
     if var_name in SOLIDITY_FUNCTIONS:
         return SolidityFunction(var_name)
 
+    # Top level must be at the end, if nothing else was found
+    ret = _find_top_level(var_name, sl)
+    if ret:
+        return ret
+
     raise VariableNotFound("Variable not found: {} (context {})".format(var_name, caller_context))
 
 

slither/solc_parsing/slitherSolc.py

@@ -43,7 +43,7 @@ class SlitherSolc:
         self._is_compact_ast = False
         self._core: SlitherCore = core
 
-        self._all_functions_parser: List[FunctionSolc] = []
+        self._all_functions_and_modifier_parser: List[FunctionSolc] = []
 
         self._top_level_contracts_counter = 0
 
@@ -52,11 +52,11 @@ class SlitherSolc:
         return self._core
 
     @property
-    def all_functions_parser(self) -> List[FunctionSolc]:
-        return self._all_functions_parser
+    def all_functions_and_modifiers_parser(self) -> List[FunctionSolc]:
+        return self._all_functions_and_modifier_parser
 
-    def add_functions_parser(self, f: FunctionSolc):
-        self._all_functions_parser.append(f)
+    def add_function_or_modifier_parser(self, f: FunctionSolc):
+        self._all_functions_and_modifier_parser.append(f)
 
     @property
     def underlying_contract_to_parser(self) -> Dict[Contract, ContractSolc]:
@@ -217,7 +217,7 @@ class SlitherSolc:
 
                 self._core.functions_top_level.append(func)
                 self._functions_top_level_parser.append(func_parser)
-                self.add_functions_parser(func_parser)
+                self.add_function_or_modifier_parser(func_parser)
 
             else:
                 raise SlitherException(f"Top level {top_level_data[self.get_key()]} not supported")

slither/solc_parsing/solidity_types/type_parsing.py

@@ -1,23 +1,19 @@
 import logging
 import re
-from typing import List, TYPE_CHECKING, Union, Dict, Optional
+from typing import List, TYPE_CHECKING, Union, Dict
 
 from slither.core.declarations.function_contract import FunctionContract
+from slither.core.expressions.literal import Literal
+from slither.core.solidity_types.array_type import ArrayType
 from slither.core.solidity_types.elementary_type import (
     ElementaryType,
     ElementaryTypeName,
 )
+from slither.core.solidity_types.function_type import FunctionType
+from slither.core.solidity_types.mapping_type import MappingType
 from slither.core.solidity_types.type import Type
 from slither.core.solidity_types.user_defined_type import UserDefinedType
-from slither.core.solidity_types.array_type import ArrayType
-from slither.core.solidity_types.mapping_type import MappingType
-from slither.core.solidity_types.function_type import FunctionType
-
 from slither.core.variables.function_type_variable import FunctionTypeVariable
-
-
-from slither.core.expressions.literal import Literal
-
 from slither.solc_parsing.exceptions import ParsingError
 
 if TYPE_CHECKING:
@@ -241,7 +237,7 @@ def parse_type(t: Union[Dict, UnknownType], caller_context):
         all_enums = [item for sublist in all_enumss for item in sublist]
         all_enums += contract.slither.enums_top_level
         contracts = contract.slither.contracts
-        functions = contract.functions
+        functions = contract.functions + contract.modifiers
     else:
         raise ParsingError(f"Incorrect caller context: {type(caller_context)}")
 

slither/visitors/slithir/expression_to_slithir.py

@@ -6,7 +6,6 @@ from slither.core.declarations import (
     SolidityVariableComposed,
     SolidityFunction,
 )
-from slither.core.declarations.solidity_variables import SolidityImportPlaceHolder
 from slither.core.expressions import (
     AssignmentOperationType,
     UnaryOperationType,

tests/test_ast_parsing.py

@@ -404,6 +404,8 @@ XFAIL = [
 ]
 
 TESTED_SOLC_07 = ["0.7.0", "0.7.1", "0.7.2", "0.7.3", "0.7.4", "0.7.5"]
+
+
 def get_solc_versions() -> List[str]:
     """
     get a list of all the supported versions of solidity, sorted from earliest to latest
@@ -415,7 +417,14 @@ def get_solc_versions() -> List[str]:
     # there's an extra newline so just remove all empty strings
     solc_versions = [version for version in solc_versions if version != ""]
 
-    solc_versions = [version for version in solc_versions if (not version.startswith('0.7.')) or (version in TESTED_SOLC_07)]
+    # Dont test for newer 0.7 versions until explicity updated
+    # Dont test for 0.8 yet
+    solc_versions = [
+        version
+        for version in solc_versions
+        if ((not version.startswith("0.7.") or version.startswith("0.8.")))
+        or (version in TESTED_SOLC_07)
+    ]
     solc_versions.reverse()
     return solc_versions