Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add missing references in the source mapping API #2371

Merged
merged 7 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 15 additions & 0 deletions slither/core/slither_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ def _compute_offsets_to_ref_impl_decl(self): # pylint: disable=too-many-branche
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)

Expand All @@ -268,6 +271,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:
Expand All @@ -276,6 +283,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:
Expand Down
5 changes: 4 additions & 1 deletion slither/slithir/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
###################################################################################
Expand Down
14 changes: 10 additions & 4 deletions slither/solc_parsing/declarations/contract.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -201,7 +202,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
Expand Down Expand Up @@ -233,7 +236,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 (
Expand Down
9 changes: 6 additions & 3 deletions slither/solc_parsing/slither_compilation_unit_solc.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,16 @@ def resolve_remapping_and_renaming(contract_parser: ContractSolc, want: str) ->
else:
missing_inheritance = i

# Resolve immediate base contracts.
for i in contract_parser.baseContracts:
# Resolve immediate base contracts and attach references.
for (i, src) in contract_parser.baseContracts:
if i in contract_parser.remapping:
target = resolve_remapping_and_renaming(contract_parser, i)
fathers.append(target)
target.add_reference_from_raw_source(src, self.compilation_unit)
elif i in self._contracts_by_id:
fathers.append(self._contracts_by_id[i])
target = self._contracts_by_id[i]
fathers.append(target)
target.add_reference_from_raw_source(src, self.compilation_unit)
else:
missing_inheritance = i

Expand Down
17 changes: 15 additions & 2 deletions slither/utils/source_mapping.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
from typing import List
from crytic_compile import CryticCompile
from slither.core.declarations import Contract, Function, Enum, Event, Import, Pragma, Structure
from slither.core.solidity_types.type import Type
from slither.core.declarations import (
Contract,
Function,
Enum,
Event,
Import,
Pragma,
Structure,
CustomError,
)
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
Expand All @@ -15,6 +24,10 @@ def get_definition(target: SourceMapping, crytic_compile: CryticCompile) -> Sour
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:
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/core/test_data/src_mapping/TopLevelReferences.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
type T is uint256;
uint constant U = 1;
error V(T);
event W(T);

contract E {
type X is int256;
function f() public {
T t = T.wrap(U);
if (T.unwrap(t) == 0) {
revert V(t);
}
emit W(t);
X x = X.wrap(1);
}
}
76 changes: 71 additions & 5 deletions tests/unit/core/test_source_mapping.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
from pathlib import Path

import pytest
from slither import Slither
from slither.core.declarations import Function
from slither.core.declarations import Function, CustomErrorTopLevel, EventTopLevel
from slither.core.solidity_types.type_alias import TypeAliasTopLevel, TypeAliasContract
from slither.core.variables.top_level_variable import TopLevelVariable

TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data"
SRC_MAPPING_TEST_ROOT = Path(TEST_DATA_DIR, "src_mapping")


def test_source_mapping(solc_binary_path):
solc_path = solc_binary_path("0.6.12")
# Ensure issue fixed in https://github.com/crytic/crytic-compile/pull/554 does not regress in Slither's reference lookup.
@pytest.mark.parametrize("solc_version", ["0.6.12", "0.8.7", "0.8.8"])
def test_source_mapping_inheritance(solc_binary_path, solc_version):
solc_path = solc_binary_path(solc_version)
file = Path(SRC_MAPPING_TEST_ROOT, "inheritance.sol").as_posix()
slither = Slither(file, solc=solc_path)

# 3 reference to A in inheritance `contract $ is A`
assert {(x.start, x.end) for x in slither.offset_to_references(file, 9)} == {
(121, 122),
(185, 186),
(299, 300),
}

# Check if A.f() is at the offset 27
functions = slither.offset_to_objects(file, 27)
assert len(functions) == 1
Expand Down Expand Up @@ -113,6 +123,62 @@ def test_references_user_defined_types_when_casting(solc_binary_path):
assert lines == [12, 18]


def test_source_mapping_top_level_defs(solc_binary_path):
solc_path = solc_binary_path("0.8.24")
file = Path(SRC_MAPPING_TEST_ROOT, "TopLevelReferences.sol").as_posix()
slither = Slither(file, solc=solc_path)

# Check if T is at the offset 5
types = slither.offset_to_objects(file, 5)
assert len(types) == 1
type_ = types.pop()
assert isinstance(type_, TypeAliasTopLevel)
assert type_.name == "T"

assert {(x.start, x.end) for x in slither.offset_to_references(file, 5)} == {
(48, 49),
(60, 61),
(134, 135),
(140, 141),
(163, 164),
}

# Check if U is at the offset 33
constants = slither.offset_to_objects(file, 33)
assert len(constants) == 1
constant = constants.pop()
assert isinstance(constant, TopLevelVariable)
assert constant.name == "U"
assert {(x.start, x.end) for x in slither.offset_to_references(file, 33)} == {(147, 148)}

# Check if V is at the offset 46
errors = slither.offset_to_objects(file, 46)
assert len(errors) == 1
error = errors.pop()
assert isinstance(error, CustomErrorTopLevel)
assert error.name == "V"
assert {(x.start, x.end) for x in slither.offset_to_references(file, 46)} == {(202, 203)}

# Check if W is at the offset 58
events = slither.offset_to_objects(file, 58)
assert len(events) == 1
event = events.pop()
assert isinstance(event, EventTopLevel)
assert event.name == "W"
assert {(x.start, x.end) for x in slither.offset_to_references(file, 58)} == {(231, 232)}

# Check if X is at the offset 87
types = slither.offset_to_objects(file, 87)
assert len(types) == 1
type_ = types.pop()
assert isinstance(type_, TypeAliasContract)
assert type_.name == "X"
assert {(x.start, x.end) for x in slither.offset_to_references(file, 87)} == {
(245, 246),
(251, 252),
}


def test_references_self_identifier():
"""
Tests that shadowing state variables with local variables does not affect references.
Expand Down
Loading