From 6074ab1c3821e7d040512582756be789844b06c4 Mon Sep 17 00:00:00 2001 From: Alexis Date: Fri, 19 Apr 2024 10:54:37 +0200 Subject: [PATCH 1/8] Remove calls to isinstance to improve performances. --- slither/visitors/expression/expression.py | 79 +++++++++-------------- 1 file changed, 29 insertions(+), 50 deletions(-) diff --git a/slither/visitors/expression/expression.py b/slither/visitors/expression/expression.py index 41886a1023..860ca28503 100644 --- a/slither/visitors/expression/expression.py +++ b/slither/visitors/expression/expression.py @@ -1,4 +1,5 @@ import logging +from functools import cache from slither.core.expressions.assignment_operation import AssignmentOperation from slither.core.expressions.binary_operation import BinaryOperation @@ -21,6 +22,28 @@ logger = logging.getLogger("ExpressionVisitor") +@cache +def get_visitor_mapping(): + """Returns a visitor mapping from expression type to visiting functions.""" + return { + AssignmentOperation: '_visit_assignement_operation', + BinaryOperation: '_visit_binary_operation', + CallExpression: '_visit_call_expression', + ConditionalExpression: '_visit_conditional_expression', + ElementaryTypeNameExpression: '_visit_elementary_type_name_expression', + Identifier: '_visit_identifier', + IndexAccess: '_visit_index_access', + Literal: '_visit_literal', + MemberAccess: '_visit_member_access', + NewArray: '_visit_new_array', + NewContract: '_visit_new_contract', + NewElementaryType: '_visit_new_elementary_type', + TupleExpression: '_visit_tuple_expression', + TypeConversion: '_visit_type_conversion', + UnaryOperation: '_visit_unary_operation' + } + + # pylint: disable=too-few-public-methods class ExpressionVisitor: def __init__(self, expression: Expression) -> None: @@ -35,60 +58,16 @@ def expression(self) -> Expression: # visit an expression # call pre_visit, visit_expression_name, post_visit - # pylint: disable=too-many-branches def _visit_expression(self, expression: Expression) -> None: self._pre_visit(expression) - if isinstance(expression, AssignmentOperation): - self._visit_assignement_operation(expression) - - elif isinstance(expression, BinaryOperation): - self._visit_binary_operation(expression) - - elif isinstance(expression, CallExpression): - self._visit_call_expression(expression) - - elif isinstance(expression, ConditionalExpression): - self._visit_conditional_expression(expression) - - elif isinstance(expression, ElementaryTypeNameExpression): - self._visit_elementary_type_name_expression(expression) - - elif isinstance(expression, Identifier): - self._visit_identifier(expression) - - elif isinstance(expression, IndexAccess): - self._visit_index_access(expression) - - elif isinstance(expression, Literal): - self._visit_literal(expression) - - elif isinstance(expression, MemberAccess): - self._visit_member_access(expression) - - elif isinstance(expression, NewArray): - self._visit_new_array(expression) - - elif isinstance(expression, NewContract): - self._visit_new_contract(expression) - - elif isinstance(expression, NewElementaryType): - self._visit_new_elementary_type(expression) - - elif isinstance(expression, TupleExpression): - self._visit_tuple_expression(expression) + if expression is not None: + visitor_method = get_visitor_mapping().get(expression.__class__) + if not visitor_method: + raise SlitherError(f"Expression not handled: {expression}") - elif isinstance(expression, TypeConversion): - self._visit_type_conversion(expression) - - elif isinstance(expression, UnaryOperation): - self._visit_unary_operation(expression) - - elif expression is None: - pass - - else: - raise SlitherError(f"Expression not handled: {expression}") + visitor = getattr(self, visitor_method) + visitor(expression) self._post_visit(expression) From 6afe440924369910cd33890256ad6afb7677ce1a Mon Sep 17 00:00:00 2001 From: Alexis Date: Fri, 19 Apr 2024 10:54:52 +0200 Subject: [PATCH 2/8] Memoize calls to __str__ --- slither/core/slither_core.py | 10 +- slither/core/solidity_types/array_type.py | 4 +- .../core/solidity_types/elementary_type.py | 4 +- slither/core/solidity_types/mapping_type.py | 4 +- .../slither_compilation_unit_solc.py | 127 ++++++++++-------- 5 files changed, 92 insertions(+), 57 deletions(-) diff --git a/slither/core/slither_core.py b/slither/core/slither_core.py index 8eca260fac..727c2e44c6 100644 --- a/slither/core/slither_core.py +++ b/slither/core/slither_core.py @@ -35,6 +35,14 @@ def _relative_path_format(path: str) -> str: return path.split("..")[-1].strip(".").strip("/") +def empty_tuple_list(): + return [(-1, -1)] + + +def tuple_dict(): + return defaultdict(empty_tuple_list) + + # pylint: disable=too-many-instance-attributes,too-many-public-methods class SlitherCore(Context): """ @@ -80,7 +88,7 @@ def __init__(self) -> None: # Maps from file to detector name to the start/end ranges for that detector. # Infinity is used to signal a detector has no end range. self._ignore_ranges: Dict[str, Dict[str, List[Tuple[int, ...]]]] = defaultdict( - lambda: defaultdict(lambda: [(-1, -1)]) + tuple_dict ) self._compilation_units: List[SlitherCompilationUnit] = [] diff --git a/slither/core/solidity_types/array_type.py b/slither/core/solidity_types/array_type.py index 04a458d685..2541258913 100644 --- a/slither/core/solidity_types/array_type.py +++ b/slither/core/solidity_types/array_type.py @@ -1,3 +1,4 @@ +from functools import cache from typing import Union, Optional, Tuple, Any, TYPE_CHECKING from slither.core.expressions.expression import Expression @@ -66,6 +67,7 @@ def storage_size(self) -> Tuple[int, bool]: return elem_size * int(str(self._length_value)), True return 32, True + @cache def __str__(self) -> str: if self._length: return str(self._type) + f"[{str(self._length_value)}]" @@ -77,4 +79,4 @@ def __eq__(self, other: Any) -> bool: return self._type == other.type and self._length_value == other.length_value def __hash__(self) -> int: - return hash(str(self)) + return hash(self._type) diff --git a/slither/core/solidity_types/elementary_type.py b/slither/core/solidity_types/elementary_type.py index a9f45c8d81..63c3071ede 100644 --- a/slither/core/solidity_types/elementary_type.py +++ b/slither/core/solidity_types/elementary_type.py @@ -1,4 +1,5 @@ import itertools +from functools import cache from typing import Tuple, Optional, Any from slither.core.solidity_types.type import Type @@ -216,6 +217,7 @@ def max(self) -> int: return MaxValues[self.name] raise SlitherException(f"{self.name} does not have a max value") + @cache def __str__(self) -> str: return self._type @@ -225,4 +227,4 @@ def __eq__(self, other: Any) -> bool: return self.type == other.type def __hash__(self) -> int: - return hash(str(self)) + return hash(self._type) diff --git a/slither/core/solidity_types/mapping_type.py b/slither/core/solidity_types/mapping_type.py index 9741569edf..e7adb461cb 100644 --- a/slither/core/solidity_types/mapping_type.py +++ b/slither/core/solidity_types/mapping_type.py @@ -1,3 +1,4 @@ +from functools import cache from typing import Union, Tuple, TYPE_CHECKING, Any from slither.core.solidity_types.type import Type @@ -35,6 +36,7 @@ def storage_size(self) -> Tuple[int, bool]: def is_dynamic(self) -> bool: return True + @cache def __str__(self) -> str: return f"mapping({str(self._from)} => {str(self._to)})" @@ -44,4 +46,4 @@ def __eq__(self, other: Any) -> bool: return self.type_from == other.type_from and self.type_to == other.type_to def __hash__(self) -> int: - return hash(str(self)) + return hash(f"{self._from}.{self._to}") diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index 36efeef33a..b3bb0d46a9 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -1,3 +1,4 @@ +import concurrent.futures from collections import defaultdict import json import logging @@ -73,6 +74,64 @@ def _handle_import_aliases( ) +def generate_slithir_contract(contract): + contract.add_constructor_variables() + + for func in contract.functions + contract.modifiers: + try: + func.generate_slithir_and_analyze() + + except AttributeError as e: + # This can happen for example if there is a call to an interface + # And the interface is redefined due to contract's name reuse + # But the available version misses some functions + # self._underlying_contract_to_parser[contract].log_incorrect_parsing( + # f"Impossible to generate IR for {contract.name}.{func.name} ({func.source_mapping}):\n {e}" + # ) + # TODO(dm) + pass + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to generate IR for {contract.name}.{func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{contract.name}.{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + try: + contract.convert_expression_to_slithir_ssa() + except Exception as exc: + logger.error( + f"\nFailed to convert IR to SSA for {contract.name} contract. Please open an issue https://github.com/crytic/slither/issues.\n " + ) + raise exc + + +def generate_slithir_function(func): + try: + func.generate_slithir_and_analyze() + except AttributeError as e: + logger.error( + f"Impossible to generate IR for top level function {func.name} ({func.source_mapping}):\n {e}" + ) + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to generate IR for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + + try: + func.generate_slithir_ssa({}) + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to convert IR to SSA for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + + class SlitherCompilationUnitSolc(CallerContextExpression): # pylint: disable=too-many-instance-attributes def __init__(self, compilation_unit: SlitherCompilationUnit) -> None: @@ -803,60 +862,22 @@ def _analyze_variables_modifiers_functions(self, contract: ContractSolc) -> None contract.set_is_analyzed(True) def _convert_to_slithir(self) -> None: - for contract in self._compilation_unit.contracts: - contract.add_constructor_variables() - - for func in contract.functions + contract.modifiers: - try: - func.generate_slithir_and_analyze() - - except AttributeError as e: - # This can happens for example if there is a call to an interface - # And the interface is redefined due to contract's name reuse - # But the available version misses some functions - self._underlying_contract_to_parser[contract].log_incorrect_parsing( - f"Impossible to generate IR for {contract.name}.{func.name} ({func.source_mapping}):\n {e}" - ) - except Exception as e: - func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) - logger.error( - f"\nFailed to generate IR for {contract.name}.{func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{contract.name}.{func.name} ({func.source_mapping}):\n " - f"{func_expressions}" - ) - raise e - try: - contract.convert_expression_to_slithir_ssa() - except Exception as e: - logger.error( - f"\nFailed to convert IR to SSA for {contract.name} contract. Please open an issue https://github.com/crytic/slither/issues.\n " - ) - raise e - - for func in self._compilation_unit.functions_top_level: - try: - func.generate_slithir_and_analyze() - except AttributeError as e: - logger.error( - f"Impossible to generate IR for top level function {func.name} ({func.source_mapping}):\n {e}" - ) - except Exception as e: - func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) - logger.error( - f"\nFailed to generate IR for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " - f"{func_expressions}" - ) - raise e - - try: - func.generate_slithir_ssa({}) - except Exception as e: - func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) - logger.error( - f"\nFailed to convert IR to SSA for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " - f"{func_expressions}" - ) - raise e + generate_slithir_contract(contract) + + for function in self._compilation_unit.functions_top_level: + generate_slithir_function(function) + # + # with concurrent.futures.ProcessPoolExecutor() as executor: + # + # futures = [ + # executor.submit(generate_slithir_contract, contract) for contract in self._compilation_unit.contracts + # ] + [ + # executor.submit(generate_slithir_function, function) for function in self._compilation_unit.functions_top_level + # ] + # + # for future in concurrent.futures.as_completed(futures): + # future.result() self._compilation_unit.propagate_function_calls() for contract in self._compilation_unit.contracts: From ae6fc40b62c8c4bac7bbc71f4e485e7316bc13d7 Mon Sep 17 00:00:00 2001 From: Alexis Date: Fri, 19 Apr 2024 11:26:43 +0200 Subject: [PATCH 3/8] Remove unneeded metaclass --- slither/core/source_mapping/source_mapping.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index fceab78559..078d5afab1 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/slither/core/source_mapping/source_mapping.py @@ -1,5 +1,4 @@ import re -from abc import ABCMeta from typing import Dict, Union, List, Tuple, TYPE_CHECKING, Optional, Any from Crypto.Hash import SHA1 @@ -183,7 +182,7 @@ def _convert_source_mapping( return new_source -class SourceMapping(Context, metaclass=ABCMeta): +class SourceMapping(Context): def __init__(self) -> None: super().__init__() self.source_mapping: Optional[Source] = None From bb842783674034666df8031a80b5fb716c1b53c8 Mon Sep 17 00:00:00 2001 From: Alexis Date: Fri, 19 Apr 2024 11:52:53 +0200 Subject: [PATCH 4/8] Prevent a call to isinstance in _filter_state_variables_written --- slither/core/declarations/function.py | 9 +++------ slither/core/expressions/identifier.py | 3 +++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index 958a7d2199..4829f21023 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -94,14 +94,11 @@ class FunctionType(Enum): def _filter_state_variables_written(expressions: List["Expression"]): ret = [] + for expression in expressions: - if isinstance(expression, Identifier): - ret.append(expression) - if isinstance(expression, UnaryOperation): - ret.append(expression.expression) - if isinstance(expression, MemberAccess): + if isinstance(expression, (Identifier, UnaryOperation, MemberAccess)): ret.append(expression.expression) - if isinstance(expression, IndexAccess): + elif isinstance(expression, IndexAccess): ret.append(expression.expression_left) return ret diff --git a/slither/core/expressions/identifier.py b/slither/core/expressions/identifier.py index 5cd29a9f5d..493620ab18 100644 --- a/slither/core/expressions/identifier.py +++ b/slither/core/expressions/identifier.py @@ -78,3 +78,6 @@ def value( def __str__(self) -> str: return str(self._value) + + def expression(self): + return self From 59caf4bb3fc10b2e04173836eb6ae0f9f3a1d4c4 Mon Sep 17 00:00:00 2001 From: Alexis Date: Fri, 19 Apr 2024 14:08:05 +0200 Subject: [PATCH 5/8] Revert "Memoize calls to __str__" This reverts commit 6afe440924369910cd33890256ad6afb7677ce1a. --- slither/core/slither_core.py | 10 +- slither/core/solidity_types/array_type.py | 4 +- .../core/solidity_types/elementary_type.py | 4 +- slither/core/solidity_types/mapping_type.py | 4 +- .../slither_compilation_unit_solc.py | 127 ++++++++---------- 5 files changed, 57 insertions(+), 92 deletions(-) diff --git a/slither/core/slither_core.py b/slither/core/slither_core.py index 727c2e44c6..8eca260fac 100644 --- a/slither/core/slither_core.py +++ b/slither/core/slither_core.py @@ -35,14 +35,6 @@ def _relative_path_format(path: str) -> str: return path.split("..")[-1].strip(".").strip("/") -def empty_tuple_list(): - return [(-1, -1)] - - -def tuple_dict(): - return defaultdict(empty_tuple_list) - - # pylint: disable=too-many-instance-attributes,too-many-public-methods class SlitherCore(Context): """ @@ -88,7 +80,7 @@ def __init__(self) -> None: # Maps from file to detector name to the start/end ranges for that detector. # Infinity is used to signal a detector has no end range. self._ignore_ranges: Dict[str, Dict[str, List[Tuple[int, ...]]]] = defaultdict( - tuple_dict + lambda: defaultdict(lambda: [(-1, -1)]) ) self._compilation_units: List[SlitherCompilationUnit] = [] diff --git a/slither/core/solidity_types/array_type.py b/slither/core/solidity_types/array_type.py index 2541258913..04a458d685 100644 --- a/slither/core/solidity_types/array_type.py +++ b/slither/core/solidity_types/array_type.py @@ -1,4 +1,3 @@ -from functools import cache from typing import Union, Optional, Tuple, Any, TYPE_CHECKING from slither.core.expressions.expression import Expression @@ -67,7 +66,6 @@ def storage_size(self) -> Tuple[int, bool]: return elem_size * int(str(self._length_value)), True return 32, True - @cache def __str__(self) -> str: if self._length: return str(self._type) + f"[{str(self._length_value)}]" @@ -79,4 +77,4 @@ def __eq__(self, other: Any) -> bool: return self._type == other.type and self._length_value == other.length_value def __hash__(self) -> int: - return hash(self._type) + return hash(str(self)) diff --git a/slither/core/solidity_types/elementary_type.py b/slither/core/solidity_types/elementary_type.py index 63c3071ede..a9f45c8d81 100644 --- a/slither/core/solidity_types/elementary_type.py +++ b/slither/core/solidity_types/elementary_type.py @@ -1,5 +1,4 @@ import itertools -from functools import cache from typing import Tuple, Optional, Any from slither.core.solidity_types.type import Type @@ -217,7 +216,6 @@ def max(self) -> int: return MaxValues[self.name] raise SlitherException(f"{self.name} does not have a max value") - @cache def __str__(self) -> str: return self._type @@ -227,4 +225,4 @@ def __eq__(self, other: Any) -> bool: return self.type == other.type def __hash__(self) -> int: - return hash(self._type) + return hash(str(self)) diff --git a/slither/core/solidity_types/mapping_type.py b/slither/core/solidity_types/mapping_type.py index e7adb461cb..9741569edf 100644 --- a/slither/core/solidity_types/mapping_type.py +++ b/slither/core/solidity_types/mapping_type.py @@ -1,4 +1,3 @@ -from functools import cache from typing import Union, Tuple, TYPE_CHECKING, Any from slither.core.solidity_types.type import Type @@ -36,7 +35,6 @@ def storage_size(self) -> Tuple[int, bool]: def is_dynamic(self) -> bool: return True - @cache def __str__(self) -> str: return f"mapping({str(self._from)} => {str(self._to)})" @@ -46,4 +44,4 @@ def __eq__(self, other: Any) -> bool: return self.type_from == other.type_from and self.type_to == other.type_to def __hash__(self) -> int: - return hash(f"{self._from}.{self._to}") + return hash(str(self)) diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index b3bb0d46a9..36efeef33a 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -1,4 +1,3 @@ -import concurrent.futures from collections import defaultdict import json import logging @@ -74,64 +73,6 @@ def _handle_import_aliases( ) -def generate_slithir_contract(contract): - contract.add_constructor_variables() - - for func in contract.functions + contract.modifiers: - try: - func.generate_slithir_and_analyze() - - except AttributeError as e: - # This can happen for example if there is a call to an interface - # And the interface is redefined due to contract's name reuse - # But the available version misses some functions - # self._underlying_contract_to_parser[contract].log_incorrect_parsing( - # f"Impossible to generate IR for {contract.name}.{func.name} ({func.source_mapping}):\n {e}" - # ) - # TODO(dm) - pass - except Exception as e: - func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) - logger.error( - f"\nFailed to generate IR for {contract.name}.{func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{contract.name}.{func.name} ({func.source_mapping}):\n " - f"{func_expressions}" - ) - raise e - try: - contract.convert_expression_to_slithir_ssa() - except Exception as exc: - logger.error( - f"\nFailed to convert IR to SSA for {contract.name} contract. Please open an issue https://github.com/crytic/slither/issues.\n " - ) - raise exc - - -def generate_slithir_function(func): - try: - func.generate_slithir_and_analyze() - except AttributeError as e: - logger.error( - f"Impossible to generate IR for top level function {func.name} ({func.source_mapping}):\n {e}" - ) - except Exception as e: - func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) - logger.error( - f"\nFailed to generate IR for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " - f"{func_expressions}" - ) - raise e - - try: - func.generate_slithir_ssa({}) - except Exception as e: - func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) - logger.error( - f"\nFailed to convert IR to SSA for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " - f"{func_expressions}" - ) - raise e - - class SlitherCompilationUnitSolc(CallerContextExpression): # pylint: disable=too-many-instance-attributes def __init__(self, compilation_unit: SlitherCompilationUnit) -> None: @@ -862,22 +803,60 @@ def _analyze_variables_modifiers_functions(self, contract: ContractSolc) -> None contract.set_is_analyzed(True) def _convert_to_slithir(self) -> None: + for contract in self._compilation_unit.contracts: - generate_slithir_contract(contract) - - for function in self._compilation_unit.functions_top_level: - generate_slithir_function(function) - # - # with concurrent.futures.ProcessPoolExecutor() as executor: - # - # futures = [ - # executor.submit(generate_slithir_contract, contract) for contract in self._compilation_unit.contracts - # ] + [ - # executor.submit(generate_slithir_function, function) for function in self._compilation_unit.functions_top_level - # ] - # - # for future in concurrent.futures.as_completed(futures): - # future.result() + contract.add_constructor_variables() + + for func in contract.functions + contract.modifiers: + try: + func.generate_slithir_and_analyze() + + except AttributeError as e: + # This can happens for example if there is a call to an interface + # And the interface is redefined due to contract's name reuse + # But the available version misses some functions + self._underlying_contract_to_parser[contract].log_incorrect_parsing( + f"Impossible to generate IR for {contract.name}.{func.name} ({func.source_mapping}):\n {e}" + ) + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to generate IR for {contract.name}.{func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{contract.name}.{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + try: + contract.convert_expression_to_slithir_ssa() + except Exception as e: + logger.error( + f"\nFailed to convert IR to SSA for {contract.name} contract. Please open an issue https://github.com/crytic/slither/issues.\n " + ) + raise e + + for func in self._compilation_unit.functions_top_level: + try: + func.generate_slithir_and_analyze() + except AttributeError as e: + logger.error( + f"Impossible to generate IR for top level function {func.name} ({func.source_mapping}):\n {e}" + ) + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to generate IR for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e + + try: + func.generate_slithir_ssa({}) + except Exception as e: + func_expressions = "\n".join([f"\t{ex}" for ex in func.expressions]) + logger.error( + f"\nFailed to convert IR to SSA for top level function {func.name}. Please open an issue https://github.com/crytic/slither/issues.\n{func.name} ({func.source_mapping}):\n " + f"{func_expressions}" + ) + raise e self._compilation_unit.propagate_function_calls() for contract in self._compilation_unit.contracts: From 85b39ea915a64e7e5949f857c798a8e22858d2cc Mon Sep 17 00:00:00 2001 From: Alexis Date: Fri, 19 Apr 2024 14:28:33 +0200 Subject: [PATCH 6/8] Add missing expression types --- slither/visitors/expression/expression.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/slither/visitors/expression/expression.py b/slither/visitors/expression/expression.py index 860ca28503..f60fb2b5a0 100644 --- a/slither/visitors/expression/expression.py +++ b/slither/visitors/expression/expression.py @@ -17,6 +17,9 @@ from slither.core.expressions.tuple_expression import TupleExpression from slither.core.expressions.type_conversion import TypeConversion from slither.core.expressions.unary_operation import UnaryOperation +from slither.core.expressions.super_call_expression import SuperCallExpression +from slither.core.expressions.super_identifier import SuperIdentifier +from slither.core.expressions.self_identifier import SelfIdentifier from slither.exceptions import SlitherError logger = logging.getLogger("ExpressionVisitor") @@ -40,7 +43,10 @@ def get_visitor_mapping(): NewElementaryType: '_visit_new_elementary_type', TupleExpression: '_visit_tuple_expression', TypeConversion: '_visit_type_conversion', - UnaryOperation: '_visit_unary_operation' + UnaryOperation: '_visit_unary_operation', + SelfIdentifier: '_visit_identifier', + SuperIdentifier: '_visit_identifier', + SuperCallExpression: '_visit_call_expression', } From 68adb6e05465dfc2ecf546d57e92abfddde81294 Mon Sep 17 00:00:00 2001 From: Alexis Date: Mon, 22 Apr 2024 11:09:47 +0200 Subject: [PATCH 7/8] Continue performance improvments --- slither/core/declarations/custom_error.py | 1 + slither/core/declarations/import_directive.py | 2 + slither/core/declarations/pragma_directive.py | 1 + .../core/solidity_types/elementary_type.py | 2 +- slither/core/solidity_types/type_alias.py | 1 + slither/core/source_mapping/source_mapping.py | 42 +++++++++++++------ slither/utils/source_mapping.py | 28 ++----------- slither/visitors/expression/expression.py | 36 ++++++++-------- 8 files changed, 58 insertions(+), 55 deletions(-) diff --git a/slither/core/declarations/custom_error.py b/slither/core/declarations/custom_error.py index 234873eaca..6e2cf142ff 100644 --- a/slither/core/declarations/custom_error.py +++ b/slither/core/declarations/custom_error.py @@ -17,6 +17,7 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit") -> None: self._solidity_signature: Optional[str] = None self._full_name: Optional[str] = None + self._pattern = "error" @property def name(self) -> str: diff --git a/slither/core/declarations/import_directive.py b/slither/core/declarations/import_directive.py index 19ea2cff96..440b09e9cd 100644 --- a/slither/core/declarations/import_directive.py +++ b/slither/core/declarations/import_directive.py @@ -16,6 +16,8 @@ def __init__(self, filename: Path, scope: "FileScope") -> None: # Map local name -> original name self.renaming: Dict[str, str] = {} + self._pattern = "import" + @property def filename(self) -> str: """ diff --git a/slither/core/declarations/pragma_directive.py b/slither/core/declarations/pragma_directive.py index cd790d5a47..90c1da2ddf 100644 --- a/slither/core/declarations/pragma_directive.py +++ b/slither/core/declarations/pragma_directive.py @@ -11,6 +11,7 @@ def __init__(self, directive: List[str], scope: "FileScope") -> None: super().__init__() self._directive = directive self.scope: "FileScope" = scope + self._pattern = "pragma" @property def directive(self) -> List[str]: diff --git a/slither/core/solidity_types/elementary_type.py b/slither/core/solidity_types/elementary_type.py index a9f45c8d81..61729b06a2 100644 --- a/slither/core/solidity_types/elementary_type.py +++ b/slither/core/solidity_types/elementary_type.py @@ -225,4 +225,4 @@ def __eq__(self, other: Any) -> bool: return self.type == other.type def __hash__(self) -> int: - return hash(str(self)) + return hash(self._type) diff --git a/slither/core/solidity_types/type_alias.py b/slither/core/solidity_types/type_alias.py index ead9b5394f..c22cd257ef 100644 --- a/slither/core/solidity_types/type_alias.py +++ b/slither/core/solidity_types/type_alias.py @@ -15,6 +15,7 @@ def __init__(self, underlying_type: ElementaryType, name: str) -> None: super().__init__() self.name = name self.underlying_type = underlying_type + self._pattern = "type" @property def type(self) -> ElementaryType: diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index 078d5afab1..8dda25a241 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/slither/core/source_mapping/source_mapping.py @@ -98,21 +98,29 @@ def __str__(self) -> str: return f"{filename_short}{lines}" def __hash__(self) -> int: - return hash(str(self)) + return hash( + ( + self.start, + self.length, + self.filename.relative, + self.end, + ) + ) def __eq__(self, other: Any) -> bool: - if not isinstance(other, type(self)): + try: + return ( + self.start == other.start + and self.length == other.length + and self.filename == other.filename + and self.is_dependency == other.is_dependency + and self.lines == other.lines + and self.starting_column == other.starting_column + and self.ending_column == other.ending_column + and self.end == other.end + ) + except AttributeError: return NotImplemented - return ( - self.start == other.start - and self.length == other.length - and self.filename == other.filename - and self.is_dependency == other.is_dependency - and self.lines == other.lines - and self.starting_column == other.starting_column - and self.ending_column == other.ending_column - and self.end == other.end - ) def _compute_line( @@ -188,6 +196,8 @@ def __init__(self) -> None: self.source_mapping: Optional[Source] = None self.references: List[Source] = [] + self._pattern: Union[str, None] = None + def set_offset( self, offset: Union["Source", str], compilation_unit: "SlitherCompilationUnit" ) -> None: @@ -203,3 +213,11 @@ def add_reference_from_raw_source( ) -> None: s = _convert_source_mapping(offset, compilation_unit) self.references.append(s) + + @property + def pattern(self) -> str: + if self._pattern is None: + # Add " " to look after the first solidity keyword + return f" {self.name}" # pylint: disable=no-member + + return self._pattern diff --git a/slither/utils/source_mapping.py b/slither/utils/source_mapping.py index 9bf772894e..180c842f72 100644 --- a/slither/utils/source_mapping.py +++ b/slither/utils/source_mapping.py @@ -2,37 +2,17 @@ from crytic_compile import CryticCompile from slither.core.declarations import ( Contract, - Function, - Enum, - Event, - Import, - Pragma, - Structure, - CustomError, FunctionContract, ) -from slither.core.solidity_types import Type, TypeAlias from slither.core.source_mapping.source_mapping import Source, SourceMapping -from slither.core.variables.variable import Variable from slither.exceptions import SlitherError def get_definition(target: SourceMapping, crytic_compile: CryticCompile) -> Source: - if isinstance(target, (Contract, Function, Enum, Event, Structure, Variable)): - # Add " " to look after the first solidity keyword - pattern = " " + target.name - elif isinstance(target, Import): - pattern = "import" - elif isinstance(target, Pragma): - pattern = "pragma" # todo maybe return with the while pragma statement - elif isinstance(target, CustomError): - pattern = "error" - elif isinstance(target, TypeAlias): - pattern = "type" - elif isinstance(target, Type): - raise SlitherError("get_definition_generic not implemented for types") - else: - raise SlitherError(f"get_definition_generic not implemented for {type(target)}") + try: + pattern = target.pattern + except AttributeError as exc: + raise SlitherError(f"get_definition_generic not implemented for {type(target)}") from exc file_content = crytic_compile.src_content_for_file(target.source_mapping.filename.absolute) txt = file_content[ diff --git a/slither/visitors/expression/expression.py b/slither/visitors/expression/expression.py index f60fb2b5a0..f727273864 100644 --- a/slither/visitors/expression/expression.py +++ b/slither/visitors/expression/expression.py @@ -29,24 +29,24 @@ def get_visitor_mapping(): """Returns a visitor mapping from expression type to visiting functions.""" return { - AssignmentOperation: '_visit_assignement_operation', - BinaryOperation: '_visit_binary_operation', - CallExpression: '_visit_call_expression', - ConditionalExpression: '_visit_conditional_expression', - ElementaryTypeNameExpression: '_visit_elementary_type_name_expression', - Identifier: '_visit_identifier', - IndexAccess: '_visit_index_access', - Literal: '_visit_literal', - MemberAccess: '_visit_member_access', - NewArray: '_visit_new_array', - NewContract: '_visit_new_contract', - NewElementaryType: '_visit_new_elementary_type', - TupleExpression: '_visit_tuple_expression', - TypeConversion: '_visit_type_conversion', - UnaryOperation: '_visit_unary_operation', - SelfIdentifier: '_visit_identifier', - SuperIdentifier: '_visit_identifier', - SuperCallExpression: '_visit_call_expression', + AssignmentOperation: "_visit_assignement_operation", + BinaryOperation: "_visit_binary_operation", + CallExpression: "_visit_call_expression", + ConditionalExpression: "_visit_conditional_expression", + ElementaryTypeNameExpression: "_visit_elementary_type_name_expression", + Identifier: "_visit_identifier", + IndexAccess: "_visit_index_access", + Literal: "_visit_literal", + MemberAccess: "_visit_member_access", + NewArray: "_visit_new_array", + NewContract: "_visit_new_contract", + NewElementaryType: "_visit_new_elementary_type", + TupleExpression: "_visit_tuple_expression", + TypeConversion: "_visit_type_conversion", + UnaryOperation: "_visit_unary_operation", + SelfIdentifier: "_visit_identifier", + SuperIdentifier: "_visit_identifier", + SuperCallExpression: "_visit_call_expression", } From 82f295f2d072f214ebf3f25f9a91fc019fbb2229 Mon Sep 17 00:00:00 2001 From: Alexis Date: Mon, 22 Apr 2024 11:33:03 +0200 Subject: [PATCH 8/8] Replace cache with lru_cache to keep Py3.8 compatibility --- slither/visitors/expression/expression.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/visitors/expression/expression.py b/slither/visitors/expression/expression.py index f727273864..83dd1be51a 100644 --- a/slither/visitors/expression/expression.py +++ b/slither/visitors/expression/expression.py @@ -1,5 +1,5 @@ import logging -from functools import cache +from functools import lru_cache from slither.core.expressions.assignment_operation import AssignmentOperation from slither.core.expressions.binary_operation import BinaryOperation @@ -25,7 +25,7 @@ logger = logging.getLogger("ExpressionVisitor") -@cache +@lru_cache() def get_visitor_mapping(): """Returns a visitor mapping from expression type to visiting functions.""" return {