diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index f9bf153064..fb67228e97 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -78,6 +78,8 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit", scope: "FileScope # do not contain private variables inherited from contract self._variables: Dict[str, "StateVariable"] = {} self._variables_ordered: List["StateVariable"] = [] + # Reference id -> variable declaration (only available for compact AST) + self._state_variables_by_ref_id: Dict[int, "StateVariable"] = {} self._modifiers: Dict[str, "Modifier"] = {} self._functions: Dict[str, "FunctionContract"] = {} self._linearizedBaseContracts: List[int] = [] @@ -404,6 +406,12 @@ def type_aliases_as_dict(self) -> Dict[str, "TypeAliasContract"]: # region Variables ################################################################################### ################################################################################### + @property + def state_variables_by_ref_id(self) -> Dict[int, "StateVariable"]: + """ + Returns the state variables by reference id (only available for compact AST). + """ + return self._state_variables_by_ref_id @property def variables(self) -> List["StateVariable"]: diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index a118b1e650..154d844d18 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -357,6 +357,8 @@ def parse_state_variables(self) -> None: self._variables_parser.append(var_parser) assert var.name + if var_parser.reference_id is not None: + self._contract.state_variables_by_ref_id[var_parser.reference_id] = var self._contract.variables_as_dict[var.name] = var self._contract.add_variables_ordered([var]) diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index a0bce044c2..c192b4efee 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -486,13 +486,18 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) t = None + referenced_declaration = None if caller_context.is_compact_ast: value = expression["name"] t = expression["typeDescriptions"]["typeString"] + if "referencedDeclaration" in expression: + referenced_declaration = expression["referencedDeclaration"] else: value = expression["attributes"]["value"] if "type" in expression["attributes"]: t = expression["attributes"]["type"] + if "referencedDeclaration" in expression["attributes"]: + referenced_declaration = expression["attributes"]["referencedDeclaration"] if t: found = re.findall(r"[struct|enum|function|modifier] \(([\[\] ()a-zA-Z0-9\.,_]*)\)", t) @@ -501,10 +506,6 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression) value = value + "(" + found[0] + ")" value = filter_name(value) - if "referencedDeclaration" in expression: - referenced_declaration = expression["referencedDeclaration"] - else: - referenced_declaration = None var, was_created = find_variable(value, caller_context, referenced_declaration) if was_created: var.set_offset(src, caller_context.compilation_unit) diff --git a/slither/solc_parsing/expressions/find_variable.py b/slither/solc_parsing/expressions/find_variable.py index 3bbdd268ef..b7e28a8912 100644 --- a/slither/solc_parsing/expressions/find_variable.py +++ b/slither/solc_parsing/expressions/find_variable.py @@ -54,10 +54,24 @@ def _find_variable_from_ref_declaration( referenced_declaration: Optional[int], all_contracts: List["Contract"], all_functions: List["Function"], + function_parser: Optional["FunctionSolc"], + contract_declarer: Optional["Contract"], ) -> Optional[Union[Contract, Function]]: + """ + Reference declarations take the highest priority, but they are not available for legacy AST. + """ if referenced_declaration is None: return None - # id of the contracts is the referenced declaration + # We look for variable declared with the referencedDeclaration attribute + if function_parser is not None and referenced_declaration in function_parser.variables_renamed: + return function_parser.variables_renamed[referenced_declaration].underlying_variable + + if ( + contract_declarer is not None + and referenced_declaration in contract_declarer.state_variables_by_ref_id + ): + return contract_declarer.state_variables_by_ref_id[referenced_declaration] + # Ccontracts ids are the referenced declaration # This is not true for the functions, as we dont always have the referenced_declaration # But maybe we could? (TODO) for contract_candidate in all_contracts: @@ -72,14 +86,9 @@ def _find_variable_from_ref_declaration( def _find_variable_in_function_parser( var_name: str, function_parser: Optional["FunctionSolc"], - referenced_declaration: Optional[int] = None, ) -> Optional[Variable]: if function_parser is None: return None - # We look for variable declared with the referencedDeclaration attr - func_variables_renamed = function_parser.variables_renamed - if referenced_declaration and referenced_declaration in func_variables_renamed: - return func_variables_renamed[referenced_declaration].underlying_variable # If not found, check for name func_variables = function_parser.underlying_function.variables_as_dict if var_name in func_variables: @@ -368,20 +377,6 @@ def find_variable( if var_name in current_scope.renaming: var_name = current_scope.renaming[var_name] - # Use ret0/ret1 to help mypy - ret0 = _find_variable_from_ref_declaration( - referenced_declaration, direct_contracts, direct_functions - ) - if ret0: - return ret0, False - - function_parser: Optional[FunctionSolc] = ( - caller_context if isinstance(caller_context, FunctionSolc) else None - ) - ret1 = _find_variable_in_function_parser(var_name, function_parser, referenced_declaration) - if ret1: - return ret1, False - contract: Optional[Contract] = None contract_declarer: Optional[Contract] = None if isinstance(caller_context, ContractSolc): @@ -395,6 +390,24 @@ def find_variable( else: assert isinstance(underlying_func, FunctionTopLevel) + function_parser: Optional[FunctionSolc] = ( + caller_context if isinstance(caller_context, FunctionSolc) else None + ) + # Use ret0/ret1 to help mypy + ret0 = _find_variable_from_ref_declaration( + referenced_declaration, + direct_contracts, + direct_functions, + function_parser, + contract_declarer, + ) + if ret0: + return ret0, False + + ret1 = _find_variable_in_function_parser(var_name, function_parser) + if ret1: + return ret1, False + ret = _find_in_contract(var_name, contract, contract_declarer, is_super, is_identifier_path) if ret: return ret, False @@ -445,6 +458,8 @@ def find_variable( referenced_declaration, list(current_scope.contracts.values()), list(current_scope.functions), + None, + None, ) if ret: return ret, False diff --git a/tests/unit/core/test_data/name_resolution/shadowing_compact.sol b/tests/unit/core/test_data/name_resolution/shadowing_compact.sol new file mode 100644 index 0000000000..c96130ae92 --- /dev/null +++ b/tests/unit/core/test_data/name_resolution/shadowing_compact.sol @@ -0,0 +1,8 @@ +pragma solidity 0.8.0; +contract B { + uint public x = 21; + function a() public { + uint u = 2 * x; + uint x; + } +} \ No newline at end of file diff --git a/tests/unit/core/test_data/name_resolution/shadowing_legacy_post_0_5_0.sol b/tests/unit/core/test_data/name_resolution/shadowing_legacy_post_0_5_0.sol new file mode 100644 index 0000000000..4a7e3a47f2 --- /dev/null +++ b/tests/unit/core/test_data/name_resolution/shadowing_legacy_post_0_5_0.sol @@ -0,0 +1,8 @@ +pragma solidity 0.5.0; +contract B { + uint public x = 21; + function a() public { + uint u = 2 * x; + uint x; + } +} \ No newline at end of file diff --git a/tests/unit/core/test_data/name_resolution/shadowing_legacy_pre_0_5_0.sol b/tests/unit/core/test_data/name_resolution/shadowing_legacy_pre_0_5_0.sol new file mode 100644 index 0000000000..e523fd2fdd --- /dev/null +++ b/tests/unit/core/test_data/name_resolution/shadowing_legacy_pre_0_5_0.sol @@ -0,0 +1,8 @@ +pragma solidity 0.4.12; +contract B { + uint public x = 21; + function a() public { + uint u = 2 * x; + uint x; + } +} diff --git a/tests/unit/core/test_name_resolution.py b/tests/unit/core/test_name_resolution.py new file mode 100644 index 0000000000..8756baaba1 --- /dev/null +++ b/tests/unit/core/test_name_resolution.py @@ -0,0 +1,45 @@ +from pathlib import Path + +from slither import Slither + + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" +NAME_RESOLUTION_TEST_ROOT = Path(TEST_DATA_DIR, "name_resolution") + + +def _sort_references_lines(refs: list) -> list: + return sorted([ref.lines[0] for ref in refs]) + + +def test_name_resolution_compact(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.0") + slither = Slither( + Path(NAME_RESOLUTION_TEST_ROOT, "shadowing_compact.sol").as_posix(), solc=solc_path + ) + contract = slither.get_contract_from_name("B")[0] + x = contract.get_state_variable_from_name("x") + assert _sort_references_lines(x.references) == [5] + + +def test_name_resolution_legacy_post_0_5_0(solc_binary_path) -> None: + solc_path = solc_binary_path("0.5.0") + slither = Slither( + Path(NAME_RESOLUTION_TEST_ROOT, "shadowing_legacy_post_0_5_0.sol").as_posix(), + solc=solc_path, + ) + contract = slither.get_contract_from_name("B")[0] + x = contract.get_state_variable_from_name("x") + assert _sort_references_lines(x.references) == [5] + + +def test_name_resolution_legacy_pre_0_5_0(solc_binary_path) -> None: + solc_path = solc_binary_path("0.4.12") + slither = Slither( + Path(NAME_RESOLUTION_TEST_ROOT, "shadowing_legacy_pre_0_5_0.sol").as_posix(), + solc=solc_path, + force_legacy=True, + ) + contract = slither.get_contract_from_name("B")[0] + function = contract.get_function_from_signature("a()") + x = function.get_local_variable_from_name("x") + assert _sort_references_lines(x.references) == [5]