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.

Sign up for GitHub

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

Dominator deadcode problem fix #1984

Merged
merged 12 commits into from
Oct 12, 2023
Merged

Dominator deadcode problem fix #1984

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c919cda
added reachability field for node which indicates whether the node is…
Tiko7454 Jun 22, 2023
7b9024f
the reachability is updated as the last step of cfg parsing
Tiko7454 Jun 22, 2023
49a3117
now we intersect only reachable fathers' dominators
Tiko7454 Jun 22, 2023
d75d8d8
changed by black
Tiko7454 Jun 22, 2023
6e15671
fixed dfs
Tiko7454 Jun 26, 2023
cee0c6b
added nullcheck for functions not having entry point (non implemented…
Tiko7454 Jun 26, 2023
7a35d8f
Merge branch 'dev' into dominator-deadcode-problem-fix
Tiko7454 Jun 26, 2023
d05ceff
Merge branch 'dev' into dominator-deadcode-problem-fix
Tiko7454 Jul 6, 2023
185680d
Merge branch 'dev' into dominator-deadcode-problem-fix
Tiko7454 Jul 10, 2023
2687867
Merge branch 'dev' into dominator-deadcode-problem-fix
Tiko7454 Aug 21, 2023
6f27239
added type annotation
Tiko7454 Sep 11, 2023
86f45c9
Merge branch 'dev' into dominator-deadcode-problem-fix
Tiko7454 Sep 11, 2023
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
9 changes: 9 additions & 0 deletions slither/core/cfg/node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ def __init__(
self.file_scope: "FileScope" = file_scope
self._function: Optional["Function"] = None

self._is_reachable: bool = False

###################################################################################
###################################################################################
# region General's properties
Expand Down Expand Up @@ -234,6 +236,13 @@ def set_function(self, function: "Function") -> None:
def function(self) -> "Function":
return self._function

@property
def is_reachable(self) -> bool:
return self._is_reachable

def set_is_reachable(self, new_is_reachable: bool) -> None:
self._is_reachable = new_is_reachable

# endregion
###################################################################################
###################################################################################
Expand Down
15 changes: 12 additions & 3 deletions slither/core/dominators/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,16 @@
def intersection_predecessor(node: "Node") -> Set["Node"]:
if not node.fathers:
return set()
ret = node.fathers[0].dominators
for pred in node.fathers[1:]:
ret = ret.intersection(pred.dominators)
if not any(father.is_reachable for father in node.fathers):
return set()

ret = set()
for pred in node.fathers:
ret = ret.union(pred.dominators)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhh I am confused, can you explain why are we doing the union over the predecessor dominators, and then their interaction?

It seem to me that this loop should be removed - unless I am missing something here

Copy link
Contributor Author

@Tiko7454 Tiko7454 Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the safest way to start set intersections. I could store the first set as the base and start intersecting all sets with that but the first set may not exist. So the safest way is to get every element from every set, and start to intersect every set with that big set

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha that makes sense :)


for pred in node.fathers:
if pred.is_reachable:
ret = ret.intersection(pred.dominators)
return ret


Expand Down Expand Up @@ -84,6 +91,8 @@ def compute_dominance_frontier(nodes: List["Node"]) -> None:
for node in nodes:
if len(node.fathers) >= 2:
for father in node.fathers:
if not father.is_reachable:
continue
runner = father
# Corner case: if there is a if without else
# we need to add update the conditional node
Expand Down
10 changes: 10 additions & 0 deletions slither/solc_parsing/declarations/function.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ def analyze_content(self) -> None:

self._remove_alone_endif()

if self._function.entry_point:
self._update_reachability(self._function.entry_point)

# endregion
###################################################################################
###################################################################################
Expand Down Expand Up @@ -1100,6 +1103,13 @@ def _parse_unchecked_block(self, block: Dict, node: NodeSolc, scope):
node = self._parse_statement(statement, node, new_scope)
return node

def _update_reachability(self, node: Node):
Tiko7454 marked this conversation as resolved.
Show resolved Hide resolved
if node.is_reachable:
return
node.set_is_reachable(True)
for son in node.sons:
self._update_reachability(son)

def _parse_cfg(self, cfg: Dict) -> None:

assert cfg[self.get_key()] == "Block"
Expand Down
Loading