From 99b75ba309b3c9d3dc14c4af3b6380adc3c9f280 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Wed, 30 Aug 2023 10:46:25 +0000 Subject: [PATCH 01/31] Create draft PR for #325 From 3e392ccfe55a3cd555deea534dd43b0a29cea3ba Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Mon, 18 Sep 2023 12:08:26 +0200 Subject: [PATCH 02/31] Equalize last defintion in continue node and reconstruct for loop --- .../readability_based_refinement.py | 62 +++++++++++++++++-- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 048d4c098..48cc82344 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -2,7 +2,7 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Dict, Optional, Union +from typing import Dict, List, Optional, Union from decompiler.pipeline.stage import PipelineStage from decompiler.structures.ast.ast_nodes import ( @@ -19,7 +19,7 @@ ) from decompiler.structures.ast.syntaxtree import AbstractSyntaxTree from decompiler.structures.logic.logic_condition import LogicCondition -from decompiler.structures.pseudo import Assignment, Condition, Expression, Operation, Variable +from decompiler.structures.pseudo import Assignment, BinaryOperation, Condition, Constant, Expression, Operation, OperationType, Variable from decompiler.structures.visitors.assignment_visitor import AssignmentVisitor from decompiler.task import DecompilerTask from decompiler.util.options import Options @@ -277,6 +277,7 @@ def run(self): -> loop condition complexity < condition complexity -> possible modification complexity < modification complexity -> if condition is only a symbol: check condition type for allowed one + -> has a continue statement which must and can be equalized If 'force_for_loops' is enabled, the complexity options are ignored and every while loop after the initial transformation will be forced into a for loop with an empty declaration/modification @@ -287,9 +288,6 @@ def run(self): or self._invalid_simple_for_loop_condition_type(loop_node.condition): continue - if any(node.does_end_with_continue for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop()): - continue - if not self._force_for_loops and loop_node.condition.get_complexity(self._ast.condition_map) > self._condition_max_complexity: continue @@ -300,6 +298,10 @@ def run(self): continue if not self._force_for_loops and continuation.instruction.complexity > self._modification_max_complexity: continue + if (equalizable_continue_nodes := self._get_continue_nodes_with_equalizable_definition(loop_node, continuation, variable_init)) is None: + break + for node in equalizable_continue_nodes: + self._remove_continuation_from_last_definition(node, continuation, variable_init) self._replace_with_for_loop(loop_node, continuation, variable_init) break @@ -315,6 +317,54 @@ def run(self): ) ) + def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNode, continuation: AstInstruction, variable_init: AstInstruction) -> List[CodeNode]: + """ + Finds code nodes containing continue statements and a definition of the continuation instruction, which can be easily equalized. + + Returns None if there is no definition or equalizing would not be easy enough. + """ + equalizable_nodes = [] + for code_node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop(): + if not code_node.does_end_with_continue: + continue + if not self._is_expression_simple_binary_operation(continuation.instruction.value): + return None + if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: + return None + if not (self._is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) \ + or isinstance(code_node.instructions[last_definition_index].value, Constant)): + return None + equalizable_nodes.append(code_node) + return equalizable_nodes + + def _is_expression_simple_binary_operation(self, expression: Expression) -> bool: + """Checks if an expression is a binary operation with plus or minus and a constant.""" + if not isinstance(expression, BinaryOperation): + return False + if not expression.operation in [OperationType.plus, OperationType.minus]: + return False + if not any(isinstance(operand, Constant) for operand in expression.operands): + return False + return True + + def _remove_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): + """Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant.""" + last_definition = code_node.instructions[_get_last_definition_index_of(code_node, variable_init.instruction.destination)] + continuation_operand = continuation.instruction.value.right if isinstance(continuation.instruction.value.right, Constant) else continuation.instruction.value.left + + if isinstance(last_definition.value, BinaryOperation) and self._is_expression_simple_binary_operation(last_definition.value): + last_definition_operand = last_definition.value.right if isinstance(last_definition.value.right, Constant) else last_definition.value.left + + if continuation.instruction.value.operation == last_definition.value.operation: + last_definition_operand.value -= continuation_operand.value + else: + last_definition_operand.value += continuation_operand.value + elif isinstance(last_definition.value, Constant): + if continuation.instruction.value.operation == OperationType.plus: + last_definition.value.value -= continuation_operand.value + else: + last_definition.value.value += continuation_operand.value + def _replace_with_for_loop(self, loop_node: WhileLoopNode, continuation: AstInstruction, init: AstInstruction): """ Replaces a given WhileLoopNode with a ForLoopNode. @@ -347,7 +397,7 @@ def _replace_with_for_loop(self, loop_node: WhileLoopNode, continuation: AstInst ) continuation.node.instructions.remove(continuation.instruction) self._ast.clean_up() - + def _invalid_simple_for_loop_condition_type(self, logic_condition) -> bool: """ Checks if a logic condition is only a symbol, if true checks condition type of symbol for forbidden ones""" if not logic_condition.is_symbol or not self._forbidden_condition_types: From 635faaeb66bd09ca5c40551f79d1680d3606fe81 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Wed, 20 Sep 2023 11:00:17 +0200 Subject: [PATCH 03/31] Changed and added tests --- .../test_readability_based_refinement.py | 223 +++++++++++++++++- 1 file changed, 218 insertions(+), 5 deletions(-) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index eecd157a3..0e41bbcda 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -7,6 +7,7 @@ WhileLoopReplacer, WhileLoopVariableRenamer, _find_continuation_instruction, + _get_last_definition_index_of, _has_deep_requirement, _initialization_reaches_loop_node, ) @@ -2080,8 +2081,10 @@ def test_declaration_listop(self, ast_call_for_loop): if isinstance(node, ForLoopNode): assert node.declaration.destination.operands[0].name == "i" - def test_skip_for_loop_recovery_if_continue_in_while(self): + def test_for_loop_recovery_if_continue_in_while_1(self): """ + Test for loop recovery if a continue occurs in a while loop and the last definition is a simple binary operation + a = 0 while(a < 10) { if(a == 2) { @@ -2127,9 +2130,68 @@ def test_skip_for_loop_recovery_if_continue_in_while(self): ) WhileLoopReplacer(ast, _generate_options()).run() - assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + condition_nodes = list(ast.get_condition_nodes_post_order()) + last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] + assert last_definition.value.right.value == 1 + + def test_for_loop_recovery_if_continue_in_while_2(self): + """ + Test for loop recovery if a continue occurs in a while loop and the last definition is a contant assignment + + a = 0 + while(a < 10) { + if(a == 2) { + a = 4 + continue + } + a = a + 1 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) + } + ) + + true_branch = ast._add_code_node( + [ + Assignment(Variable("a"), Constant(4)), + Continue() + ] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))]) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration) + ] + ) - def test_skip_for_loop_recovery_if_continue_in_nested_while(self): + WhileLoopReplacer(ast, _generate_options()).run() + assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + condition_nodes = list(ast.get_condition_nodes_post_order()) + last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] + assert last_definition.value.value == 3 + + def test_for_loop_recovery_if_continue_in_nested_while(self): """ while(a < 5) { a = a + b @@ -2188,5 +2250,156 @@ def test_skip_for_loop_recovery_if_continue_in_nested_while(self): ) WhileLoopReplacer(ast, _generate_options()).run() - loop_nodes = list(ast.get_loop_nodes_post_order()) - assert not isinstance(loop_nodes[0], ForLoopNode) and isinstance(loop_nodes[1], ForLoopNode) \ No newline at end of file + assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + condition_nodes = list(ast.get_condition_nodes_post_order()) + last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("b"))] + assert last_definition.value.right.value == 1 + + def test_skip_for_loop_recovery_if_continue_in_while_1(self): + """ + Test skip of for loop recovery if a continue occurs in a while loop, because the continuation instruction is no simple binary operation + + a = 0 + while(a < 10) { + if(a == 2) { + a = a + 2 + continue + } + a = a * 2 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) + } + ) + + true_branch = ast._add_code_node( + [ + Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(2)])), + Continue() + ] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.multiply, [Variable("a"), Constant(2)]))]) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration) + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + + def test_skip_for_loop_recovery_if_continue_in_while_2(self): + """ + Test skip of for loop recovery if a continue occurs in a while loop, because the last definition is no simple binary operation + + a = 0 + while(a < 10) { + if(a == 2) { + a = a * 2 + continue + } + a = a + 1 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) + } + ) + + true_branch = ast._add_code_node( + [ + Assignment(Variable("a"), BinaryOperation(OperationType.multiply, [Variable("a"), Constant(2)])), + Continue() + ] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))]) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration) + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + def test_skip_for_loop_recovery_if_continue_in_while_3(self): + """ + Test skip of for loop recovery if a continue occurs in a while loop, because no last definition exists + + a = 0 + while(a < 10) { + if(a == 2) { + continue + } + a = a + 1 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) + } + ) + + true_branch = ast._add_code_node([Continue()]) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))]) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration) + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) From 6767cf7b95361acec9f2daf354d0fde8a6854024 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Wed, 20 Sep 2023 11:55:39 +0200 Subject: [PATCH 04/31] Changed condition in remove_continuation + comments --- .../readability_based_refinement.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 48cc82344..633164f07 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -319,9 +319,12 @@ def run(self): def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNode, continuation: AstInstruction, variable_init: AstInstruction) -> List[CodeNode]: """ - Finds code nodes containing continue statements and a definition of the continuation instruction, which can be easily equalized. + Finds code nodes of a while loop containing continue statements and a definition of the continuation instruction, which can be easily equalized. - Returns None if there is no definition or equalizing would not be easy enough. + :param loop_node: While-loop to search in + :param continuation: Instruction defining the for-loops modification + :param variable_init: Instruction defining the for-loops declaration + :return: List of continue code nodes that has equalizable definitions, None if at least one continue node does not match the requirements """ equalizable_nodes = [] for code_node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop(): @@ -348,18 +351,24 @@ def _is_expression_simple_binary_operation(self, expression: Expression) -> bool return True def _remove_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): - """Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant.""" + """ + Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant. + + :param code_node: Code node whose last instruction is to be changed + :param continuation: Instruction defining the for-loops modification + :param variable_init: Instruction defining the for-loops declaration + """ last_definition = code_node.instructions[_get_last_definition_index_of(code_node, variable_init.instruction.destination)] continuation_operand = continuation.instruction.value.right if isinstance(continuation.instruction.value.right, Constant) else continuation.instruction.value.left - if isinstance(last_definition.value, BinaryOperation) and self._is_expression_simple_binary_operation(last_definition.value): + if isinstance(last_definition.value, BinaryOperation): last_definition_operand = last_definition.value.right if isinstance(last_definition.value.right, Constant) else last_definition.value.left if continuation.instruction.value.operation == last_definition.value.operation: last_definition_operand.value -= continuation_operand.value else: last_definition_operand.value += continuation_operand.value - elif isinstance(last_definition.value, Constant): + else: if continuation.instruction.value.operation == OperationType.plus: last_definition.value.value -= continuation_operand.value else: From b34d013d95748b864496b3b59120fece941ca128 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 12 Oct 2023 09:19:16 +0200 Subject: [PATCH 05/31] Syntax and other small changes --- .../readability_based_refinement.py | 31 +++++++++---------- .../test_readability_based_refinement.py | 4 +-- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 633164f07..c9975aaa8 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -301,7 +301,7 @@ def run(self): if (equalizable_continue_nodes := self._get_continue_nodes_with_equalizable_definition(loop_node, continuation, variable_init)) is None: break for node in equalizable_continue_nodes: - self._remove_continuation_from_last_definition(node, continuation, variable_init) + self._substract_continuation_from_last_definition(node, continuation, variable_init) self._replace_with_for_loop(loop_node, continuation, variable_init) break @@ -327,33 +327,32 @@ def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNo :return: List of continue code nodes that has equalizable definitions, None if at least one continue node does not match the requirements """ equalizable_nodes = [] - for code_node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop(): - if not code_node.does_end_with_continue: - continue + for code_node in (node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue): if not self._is_expression_simple_binary_operation(continuation.instruction.value): return None if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: return None - if not (self._is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) \ - or isinstance(code_node.instructions[last_definition_index].value, Constant)): + if not ( + self._is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) + or isinstance(code_node.instructions[last_definition_index].value, Constant) + ): return None equalizable_nodes.append(code_node) return equalizable_nodes def _is_expression_simple_binary_operation(self, expression: Expression) -> bool: """Checks if an expression is a binary operation with plus or minus and a constant.""" - if not isinstance(expression, BinaryOperation): - return False - if not expression.operation in [OperationType.plus, OperationType.minus]: - return False - if not any(isinstance(operand, Constant) for operand in expression.operands): - return False - return True + return ( + isinstance(expression, BinaryOperation) + and expression.operation in {OperationType.plus, OperationType.minus} + and any(isinstance(operand, Constant) for operand in expression.operands) + ) - def _remove_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): + def _substract_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): """ - Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant. - + Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, + defining the same value as the continuation instruction in the given code node. + :param code_node: Code node whose last instruction is to be changed :param continuation: Instruction defining the for-loops modification :param variable_init: Instruction defining the for-loops declaration diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index 0e41bbcda..0ee815295 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -2134,7 +2134,7 @@ def test_for_loop_recovery_if_continue_in_while_1(self): condition_nodes = list(ast.get_condition_nodes_post_order()) last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] - assert last_definition.value.right.value == 1 + assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)])) def test_for_loop_recovery_if_continue_in_while_2(self): """ @@ -2189,7 +2189,7 @@ def test_for_loop_recovery_if_continue_in_while_2(self): condition_nodes = list(ast.get_condition_nodes_post_order()) last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] - assert last_definition.value.value == 3 + assert last_definition == Assignment(Variable("a"), Constant(3)) def test_for_loop_recovery_if_continue_in_nested_while(self): """ From ec1ef9de09412c70e94f55846e895077a36f9df1 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Mon, 16 Oct 2023 12:59:22 +0200 Subject: [PATCH 06/31] Added check for uniform variables in continuation and last definition --- .../readability_based_refinement.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index c9975aaa8..315129e27 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -328,7 +328,10 @@ def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNo """ equalizable_nodes = [] for code_node in (node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue): - if not self._is_expression_simple_binary_operation(continuation.instruction.value): + if not ( + self._is_expression_simple_binary_operation(continuation.instruction.value) + and continuation.instruction.destination == (continuation_used_var := self._get_variable_in_binary_operation(continuation.instruction.value)) + ): return None if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: return None @@ -337,6 +340,10 @@ def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNo or isinstance(code_node.instructions[last_definition_index].value, Constant) ): return None + if not isinstance(code_node.instructions[last_definition_index].value, Constant): + if not continuation_used_var == self._get_variable_in_binary_operation(code_node.instructions[last_definition_index].value): + return None + equalizable_nodes.append(code_node) return equalizable_nodes @@ -347,6 +354,15 @@ def _is_expression_simple_binary_operation(self, expression: Expression) -> bool and expression.operation in {OperationType.plus, OperationType.minus} and any(isinstance(operand, Constant) for operand in expression.operands) ) + + def _get_variable_in_binary_operation(self, binaryoperation: BinaryOperation) -> Variable: + """Returns the used variable of a binary operation if available.""" + if isinstance(binaryoperation.left, Variable): + return binaryoperation.left + elif isinstance(binaryoperation.right, Variable): + return binaryoperation.right + else: + return None def _substract_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): """ From 6997fd5eab5030e05190b82e1c8e08ec5d617e1c Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Wed, 18 Oct 2023 13:09:29 +0200 Subject: [PATCH 07/31] Moved check for uniform variables, added check for commutativity --- .../readability_based_refinement.py | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 315129e27..3401ed97c 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -328,41 +328,33 @@ def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNo """ equalizable_nodes = [] for code_node in (node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue): - if not ( - self._is_expression_simple_binary_operation(continuation.instruction.value) - and continuation.instruction.destination == (continuation_used_var := self._get_variable_in_binary_operation(continuation.instruction.value)) - ): + if not self._is_assignment_simple_increment(continuation.instruction): return None if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: return None if not ( - self._is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) + self._is_assignment_simple_increment(code_node.instructions[last_definition_index]) or isinstance(code_node.instructions[last_definition_index].value, Constant) ): return None - if not isinstance(code_node.instructions[last_definition_index].value, Constant): - if not continuation_used_var == self._get_variable_in_binary_operation(code_node.instructions[last_definition_index].value): - return None - equalizable_nodes.append(code_node) return equalizable_nodes - def _is_expression_simple_binary_operation(self, expression: Expression) -> bool: - """Checks if an expression is a binary operation with plus or minus and a constant.""" + def _is_assignment_simple_increment(self, assignment: Assignment) -> bool: + """ + Checks if an assignment is a simple incrementation. Meaning that it defines and uses the same variable and which included expression is a binary operation with the used + variable and a constant and uses plus or minus as operation. The minus operation is not commutative and is therefore only defined as simple if the variable is the first + operand and the constant the second. + """ return ( - isinstance(expression, BinaryOperation) - and expression.operation in {OperationType.plus, OperationType.minus} - and any(isinstance(operand, Constant) for operand in expression.operands) + isinstance(assignment.value, BinaryOperation) + and assignment.value.operation in {OperationType.plus, OperationType.minus} + and any(isinstance(operand, Constant) for operand in assignment.value.operands) + and ( + assignment.destination == assignment.value.left + or (assignment.destination == assignment.value.right and assignment.value.operation == OperationType.plus) + ) ) - - def _get_variable_in_binary_operation(self, binaryoperation: BinaryOperation) -> Variable: - """Returns the used variable of a binary operation if available.""" - if isinstance(binaryoperation.left, Variable): - return binaryoperation.left - elif isinstance(binaryoperation.right, Variable): - return binaryoperation.right - else: - return None def _substract_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): """ From fe90534adb2167c0ada4c28a325cf7b9cca7a64d Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Mon, 13 Nov 2023 12:20:22 +0100 Subject: [PATCH 08/31] Unify instructions, applied changing cases --- .../readability_based_refinement.py | 77 +++++++++++-------- decompiler/structures/pseudo/operations.py | 9 +++ 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 3401ed97c..b80768351 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -19,7 +19,7 @@ ) from decompiler.structures.ast.syntaxtree import AbstractSyntaxTree from decompiler.structures.logic.logic_condition import LogicCondition -from decompiler.structures.pseudo import Assignment, BinaryOperation, Condition, Constant, Expression, Operation, OperationType, Variable +from decompiler.structures.pseudo import Assignment, BinaryOperation, Condition, Constant, Expression, Operation, OperationType, UnaryOperation, Variable from decompiler.structures.visitors.assignment_visitor import AssignmentVisitor from decompiler.task import DecompilerTask from decompiler.util.options import Options @@ -328,34 +328,59 @@ def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNo """ equalizable_nodes = [] for code_node in (node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue): - if not self._is_assignment_simple_increment(continuation.instruction): + if not self._is_expression_simple_binary_operation(continuation.instruction.value): return None if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: return None if not ( - self._is_assignment_simple_increment(code_node.instructions[last_definition_index]) - or isinstance(code_node.instructions[last_definition_index].value, Constant) + isinstance(code_node.instructions[last_definition_index].value, Constant) + or self._is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) + and self._get_variable_in_binary_operation(continuation.instruction.value) + == self._get_variable_in_binary_operation(code_node.instructions[last_definition_index].value) ): return None + + self._unify_binary_operation(continuation.instruction.value) + if not isinstance(code_node.instructions[last_definition_index].value, Constant): + self._unify_binary_operation(code_node.instructions[last_definition_index].value) + equalizable_nodes.append(code_node) return equalizable_nodes - def _is_assignment_simple_increment(self, assignment: Assignment) -> bool: - """ - Checks if an assignment is a simple incrementation. Meaning that it defines and uses the same variable and which included expression is a binary operation with the used - variable and a constant and uses plus or minus as operation. The minus operation is not commutative and is therefore only defined as simple if the variable is the first - operand and the constant the second. - """ + def _is_expression_simple_binary_operation(self, expression: Expression) -> bool: + """Checks if an expression is a simple binary operation. Meaning it includes a variable and a constant and uses plus or minus as operation type.""" return ( - isinstance(assignment.value, BinaryOperation) - and assignment.value.operation in {OperationType.plus, OperationType.minus} - and any(isinstance(operand, Constant) for operand in assignment.value.operands) - and ( - assignment.destination == assignment.value.left - or (assignment.destination == assignment.value.right and assignment.value.operation == OperationType.plus) - ) + isinstance(expression, BinaryOperation) + and expression.operation in {OperationType.plus, OperationType.minus} + and any(isinstance(operand, Constant) for operand in expression.subexpressions()) + and any(isinstance(operand, Variable) for operand in expression.subexpressions()) ) + def _get_variable_in_binary_operation(self, binaryoperation: BinaryOperation) -> Variable: + """Returns the used variable of a binary operation if available.""" + for operand in binaryoperation.subexpressions(): + if isinstance(operand, Variable): + return operand + return None + + def _count_unaryoperations_negations(self, expression: Expression) -> int: + """Counts the amount of UnaryOperation negations of an expression.""" + negations = 0 + for subexpression in expression.subexpressions(): + if isinstance(subexpression, UnaryOperation) and subexpression.operation == OperationType.negate: + negations += 1 + return negations + + def _unify_binary_operation(self, binaryoperation: BinaryOperation): + """Brings a simple binary operation into a unified representation like 'var + const'.""" + print(binaryoperation) + if not binaryoperation.operation == OperationType.plus: + binaryoperation.operation = OperationType.plus + binaryoperation.substitute(binaryoperation.right, UnaryOperation(OperationType.negate, [binaryoperation.right])) + + if any(isinstance(operand, Constant) for operand in binaryoperation.left.subexpressions()): + binaryoperation.swap_operands() + def _substract_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): """ Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, @@ -366,20 +391,10 @@ def _substract_continuation_from_last_definition(self, code_node: CodeNode, cont :param variable_init: Instruction defining the for-loops declaration """ last_definition = code_node.instructions[_get_last_definition_index_of(code_node, variable_init.instruction.destination)] - continuation_operand = continuation.instruction.value.right if isinstance(continuation.instruction.value.right, Constant) else continuation.instruction.value.left - - if isinstance(last_definition.value, BinaryOperation): - last_definition_operand = last_definition.value.right if isinstance(last_definition.value.right, Constant) else last_definition.value.left - - if continuation.instruction.value.operation == last_definition.value.operation: - last_definition_operand.value -= continuation_operand.value - else: - last_definition_operand.value += continuation_operand.value - else: - if continuation.instruction.value.operation == OperationType.plus: - last_definition.value.value -= continuation_operand.value - else: - last_definition.value.value += continuation_operand.value + last_definition.substitute(last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right])) + + if self._count_unaryoperations_negations(continuation.instruction.value.left) % 2 != 0: + last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [last_definition.value])) def _replace_with_for_loop(self, loop_node: WhileLoopNode, continuation: AstInstruction, init: AstInstruction): """ diff --git a/decompiler/structures/pseudo/operations.py b/decompiler/structures/pseudo/operations.py index 28d6ab9a6..971c35a60 100644 --- a/decompiler/structures/pseudo/operations.py +++ b/decompiler/structures/pseudo/operations.py @@ -446,6 +446,11 @@ def right(self) -> Expression: """Return the right-hand-side operand.""" return self._operands[1] + @Operation.operation.setter + def operation(self, value): + """Setter function for operation.""" + self._operation = value + def copy(self) -> BinaryOperation: """Generate a deep copy of the current binary operation.""" return self.__class__(self._operation, [operand.copy() for operand in self._operands], self._type.copy(), self.tags) @@ -454,6 +459,10 @@ def accept(self, visitor: DataflowObjectVisitorInterface[T]) -> T: """Invoke the appropriate visitor for this Operation.""" return visitor.visit_binary_operation(self) + def swap_operands(self): + """Swaps left-hand-side with right-hand-side operand.""" + self._operands[0], self._operands[1] = self._operands[1], self._operands[0] + class Call(Operation): """Class representing a Call operation""" From fd1076f836ff0a09b3f66f07b1bd151fef4f3d14 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 14 Nov 2023 11:22:55 +0100 Subject: [PATCH 09/31] Moved methods in new structure --- .../loop_utility_methods.py | 84 +++++++++++++++++- .../readability_based_refinement.py | 85 +------------------ 2 files changed, 85 insertions(+), 84 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index f9390fecf..cbd801925 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -1,12 +1,12 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Dict, Optional +from typing import Dict, List, Optional -from decompiler.structures.ast.ast_nodes import AbstractSyntaxTreeNode, CaseNode, CodeNode, ConditionNode, LoopNode, SeqNode, SwitchNode +from decompiler.structures.ast.ast_nodes import AbstractSyntaxTreeNode, CaseNode, CodeNode, ConditionNode, LoopNode, SeqNode, SwitchNode, WhileLoopNode from decompiler.structures.ast.syntaxtree import AbstractSyntaxTree from decompiler.structures.logic.logic_condition import LogicCondition -from decompiler.structures.pseudo import Assignment, Condition, Variable +from decompiler.structures.pseudo import Assignment, BinaryOperation, Condition, Constant, Expression, OperationType, UnaryOperation, Variable from decompiler.structures.visitors.assignment_visitor import AssignmentVisitor @@ -210,3 +210,81 @@ def _requirement_without_reinitialization(ast: AbstractSyntaxTree, node: Abstrac return True elif variable in assignment.requirements: return True + +def _get_continue_nodes_with_equalizable_definition(loop_node: WhileLoopNode, continuation: AstInstruction, variable_init: AstInstruction) -> List[CodeNode]: + """ + Finds code nodes of a while loop containing continue statements and a definition of the continuation instruction, which can be easily equalized. + + :param loop_node: While-loop to search in + :param continuation: Instruction defining the for-loops modification + :param variable_init: Instruction defining the for-loops declaration + :return: List of continue code nodes that has equalizable definitions, None if at least one continue node does not match the requirements + """ + equalizable_nodes = [] + for code_node in (node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue): + if not _is_expression_simple_binary_operation(continuation.instruction.value): + return None + if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: + return None + if not ( + isinstance(code_node.instructions[last_definition_index].value, Constant) + or _is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) + and _get_variable_in_binary_operation(continuation.instruction.value) + == _get_variable_in_binary_operation(code_node.instructions[last_definition_index].value) + ): + return None + + _unify_binary_operation(continuation.instruction.value) + if not isinstance(code_node.instructions[last_definition_index].value, Constant): + _unify_binary_operation(code_node.instructions[last_definition_index].value) + + equalizable_nodes.append(code_node) + return equalizable_nodes + +def _is_expression_simple_binary_operation(expression: Expression) -> bool: + """Checks if an expression is a simple binary operation. Meaning it includes a variable and a constant and uses plus or minus as operation type.""" + return ( + isinstance(expression, BinaryOperation) + and expression.operation in {OperationType.plus, OperationType.minus} + and any(isinstance(operand, Constant) for operand in expression.subexpressions()) + and any(isinstance(operand, Variable) for operand in expression.subexpressions()) + ) + +def _get_variable_in_binary_operation(binaryoperation: BinaryOperation) -> Variable: + """Returns the used variable of a binary operation if available.""" + for operand in binaryoperation.subexpressions(): + if isinstance(operand, Variable): + return operand + return None + +def _count_unaryoperations_negations(expression: Expression) -> int: + """Counts the amount of UnaryOperation negations of an expression.""" + negations = 0 + for subexpression in expression.subexpressions(): + if isinstance(subexpression, UnaryOperation) and subexpression.operation == OperationType.negate: + negations += 1 + return negations + +def _unify_binary_operation(binaryoperation: BinaryOperation): + """Brings a simple binary operation into a unified representation like 'var + const'.""" + if not binaryoperation.operation == OperationType.plus: + binaryoperation.operation = OperationType.plus + binaryoperation.substitute(binaryoperation.right, UnaryOperation(OperationType.negate, [binaryoperation.right])) + + if any(isinstance(operand, Constant) for operand in binaryoperation.left.subexpressions()): + binaryoperation.swap_operands() + +def _substract_continuation_from_last_definition(code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): + """ + Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, + defining the same value as the continuation instruction in the given code node. + + :param code_node: Code node whose last instruction is to be changed + :param continuation: Instruction defining the for-loops modification + :param variable_init: Instruction defining the for-loops declaration + """ + last_definition = code_node.instructions[_get_last_definition_index_of(code_node, variable_init.instruction.destination)] + last_definition.substitute(last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right])) + + if _count_unaryoperations_negations(continuation.instruction.value.left) % 2 != 0: + last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [last_definition.value])) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 366a57dbf..4612cc842 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -6,10 +6,12 @@ from decompiler.pipeline.controlflowanalysis.loop_utility_methods import ( AstInstruction, _find_continuation_instruction, + _get_continue_nodes_with_equalizable_definition, _get_variable_initialisation, _initialization_reaches_loop_node, _is_single_instruction_loop_node, _single_defininition_reaches_node, + _substract_continuation_from_last_definition, ) from decompiler.pipeline.stage import PipelineStage from decompiler.structures.ast.ast_nodes import ConditionNode, DoWhileLoopNode, ForLoopNode, WhileLoopNode @@ -101,10 +103,10 @@ def run(self): continue if not self._force_for_loops and continuation.instruction.complexity > self._modification_max_complexity: continue - if (equalizable_continue_nodes := self._get_continue_nodes_with_equalizable_definition(loop_node, continuation, variable_init)) is None: + if (equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation, variable_init)) is None: break for node in equalizable_continue_nodes: - self._substract_continuation_from_last_definition(node, continuation, variable_init) + _substract_continuation_from_last_definition(node, continuation, variable_init) self._replace_with_for_loop(loop_node, continuation, variable_init) break @@ -120,85 +122,6 @@ def run(self): ), ) - def _get_continue_nodes_with_equalizable_definition(self, loop_node: WhileLoopNode, continuation: AstInstruction, variable_init: AstInstruction) -> List[CodeNode]: - """ - Finds code nodes of a while loop containing continue statements and a definition of the continuation instruction, which can be easily equalized. - - :param loop_node: While-loop to search in - :param continuation: Instruction defining the for-loops modification - :param variable_init: Instruction defining the for-loops declaration - :return: List of continue code nodes that has equalizable definitions, None if at least one continue node does not match the requirements - """ - equalizable_nodes = [] - for code_node in (node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue): - if not self._is_expression_simple_binary_operation(continuation.instruction.value): - return None - if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: - return None - if not ( - isinstance(code_node.instructions[last_definition_index].value, Constant) - or self._is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) - and self._get_variable_in_binary_operation(continuation.instruction.value) - == self._get_variable_in_binary_operation(code_node.instructions[last_definition_index].value) - ): - return None - - self._unify_binary_operation(continuation.instruction.value) - if not isinstance(code_node.instructions[last_definition_index].value, Constant): - self._unify_binary_operation(code_node.instructions[last_definition_index].value) - - equalizable_nodes.append(code_node) - return equalizable_nodes - - def _is_expression_simple_binary_operation(self, expression: Expression) -> bool: - """Checks if an expression is a simple binary operation. Meaning it includes a variable and a constant and uses plus or minus as operation type.""" - return ( - isinstance(expression, BinaryOperation) - and expression.operation in {OperationType.plus, OperationType.minus} - and any(isinstance(operand, Constant) for operand in expression.subexpressions()) - and any(isinstance(operand, Variable) for operand in expression.subexpressions()) - ) - - def _get_variable_in_binary_operation(self, binaryoperation: BinaryOperation) -> Variable: - """Returns the used variable of a binary operation if available.""" - for operand in binaryoperation.subexpressions(): - if isinstance(operand, Variable): - return operand - return None - - def _count_unaryoperations_negations(self, expression: Expression) -> int: - """Counts the amount of UnaryOperation negations of an expression.""" - negations = 0 - for subexpression in expression.subexpressions(): - if isinstance(subexpression, UnaryOperation) and subexpression.operation == OperationType.negate: - negations += 1 - return negations - - def _unify_binary_operation(self, binaryoperation: BinaryOperation): - """Brings a simple binary operation into a unified representation like 'var + const'.""" - print(binaryoperation) - if not binaryoperation.operation == OperationType.plus: - binaryoperation.operation = OperationType.plus - binaryoperation.substitute(binaryoperation.right, UnaryOperation(OperationType.negate, [binaryoperation.right])) - - if any(isinstance(operand, Constant) for operand in binaryoperation.left.subexpressions()): - binaryoperation.swap_operands() - - def _substract_continuation_from_last_definition(self, code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): - """ - Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, - defining the same value as the continuation instruction in the given code node. - - :param code_node: Code node whose last instruction is to be changed - :param continuation: Instruction defining the for-loops modification - :param variable_init: Instruction defining the for-loops declaration - """ - last_definition = code_node.instructions[_get_last_definition_index_of(code_node, variable_init.instruction.destination)] - last_definition.substitute(last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right])) - - if self._count_unaryoperations_negations(continuation.instruction.value.left) % 2 != 0: - last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [last_definition.value])) - def _replace_with_for_loop(self, loop_node: WhileLoopNode, continuation: AstInstruction, init: AstInstruction): """ Replaces a given WhileLoopNode with a ForLoopNode. From 2e2fdf2187a5337ecb571f68c44a673522086a7f Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 14 Nov 2023 11:34:59 +0100 Subject: [PATCH 10/31] Clean up tests for new structure --- .../test_readability_based_refinement.py | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index 340d8046e..7c7c7c8c2 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -849,41 +849,6 @@ def test_separated_by_loop_node_4(self, ast_while_in_else): assert _initialization_reaches_loop_node(init_code_node, inner_while) is False - def test_for_loop_variable_generation(self): - renamer = ForLoopVariableRenamer( - AbstractSyntaxTree(SeqNode(LogicCondition.initialize_true(LogicCondition.generate_new_context())), {}), - ["i", "j", "k", "l", "m", "n"] - ) - assert [renamer._get_variable_name() for _ in range(14)] == [ - "i", - "j", - "k", - "l", - "m", - "n", - "i1", - "j1", - "k1", - "l1", - "m1", - "n1", - "i2", - "j2", - ] - - def test_while_loop_variable_generation(self): - renamer = WhileLoopVariableRenamer( - AbstractSyntaxTree(SeqNode(LogicCondition.initialize_true(LogicCondition.generate_new_context())), {}) - ) - assert [renamer._get_variable_name() for _ in range(5)] == ["counter", "counter1", "counter2", "counter3", "counter4"] - - def test_declaration_listop(self, ast_call_for_loop): - """Test renaming with ListOperation as Declaration""" - ForLoopVariableRenamer(ast_call_for_loop, ["i"]).rename() - for node in ast_call_for_loop: - if isinstance(node, ForLoopNode): - assert node.declaration.destination.operands[0].name == "i" - def test_for_loop_recovery_if_continue_in_while_1(self): """ Test for loop recovery if a continue occurs in a while loop and the last definition is a simple binary operation From 560b5d0b41b8e528631b158e4283daf4c77d375c Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 14 Nov 2023 12:05:49 +0100 Subject: [PATCH 11/31] Changed current tests to new recovery output --- .../test_readability_based_refinement.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index 7c7c7c8c2..aa1bc140e 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -901,7 +901,7 @@ def test_for_loop_recovery_if_continue_in_while_1(self): condition_nodes = list(ast.get_condition_nodes_post_order()) last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] - assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)])) + assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("a"), Constant(2)]), Constant(1)])) def test_for_loop_recovery_if_continue_in_while_2(self): """ @@ -956,7 +956,7 @@ def test_for_loop_recovery_if_continue_in_while_2(self): condition_nodes = list(ast.get_condition_nodes_post_order()) last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] - assert last_definition == Assignment(Variable("a"), Constant(3)) + assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.minus, [Constant(4), Constant(1)])) def test_for_loop_recovery_if_continue_in_nested_while(self): """ @@ -1024,7 +1024,7 @@ def test_for_loop_recovery_if_continue_in_nested_while(self): condition_nodes = list(ast.get_condition_nodes_post_order()) last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("b"))] - assert last_definition.value.right.value == 1 + assert last_definition == Assignment(Variable("b"), BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("b"), Constant(2)]), Constant(1)])) def test_skip_for_loop_recovery_if_continue_in_while_1(self): """ From 72be886dcfed94ddbaf78e0c902a87a2239c1b33 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 14 Nov 2023 12:17:48 +0100 Subject: [PATCH 12/31] Black formatting --- .../loop_utility_methods.py | 45 +++++++++-- .../readability_based_refinement.py | 4 +- .../test_readability_based_refinement.py | 80 ++++++++++--------- 3 files changed, 84 insertions(+), 45 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index cbd801925..076754754 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -3,10 +3,28 @@ from dataclasses import dataclass from typing import Dict, List, Optional -from decompiler.structures.ast.ast_nodes import AbstractSyntaxTreeNode, CaseNode, CodeNode, ConditionNode, LoopNode, SeqNode, SwitchNode, WhileLoopNode +from decompiler.structures.ast.ast_nodes import ( + AbstractSyntaxTreeNode, + CaseNode, + CodeNode, + ConditionNode, + LoopNode, + SeqNode, + SwitchNode, + WhileLoopNode, +) from decompiler.structures.ast.syntaxtree import AbstractSyntaxTree from decompiler.structures.logic.logic_condition import LogicCondition -from decompiler.structures.pseudo import Assignment, BinaryOperation, Condition, Constant, Expression, OperationType, UnaryOperation, Variable +from decompiler.structures.pseudo import ( + Assignment, + BinaryOperation, + Condition, + Constant, + Expression, + OperationType, + UnaryOperation, + Variable, +) from decompiler.structures.visitors.assignment_visitor import AssignmentVisitor @@ -211,7 +229,10 @@ def _requirement_without_reinitialization(ast: AbstractSyntaxTree, node: Abstrac elif variable in assignment.requirements: return True -def _get_continue_nodes_with_equalizable_definition(loop_node: WhileLoopNode, continuation: AstInstruction, variable_init: AstInstruction) -> List[CodeNode]: + +def _get_continue_nodes_with_equalizable_definition( + loop_node: WhileLoopNode, continuation: AstInstruction, variable_init: AstInstruction +) -> List[CodeNode]: """ Finds code nodes of a while loop containing continue statements and a definition of the continuation instruction, which can be easily equalized. @@ -221,7 +242,9 @@ def _get_continue_nodes_with_equalizable_definition(loop_node: WhileLoopNode, co :return: List of continue code nodes that has equalizable definitions, None if at least one continue node does not match the requirements """ equalizable_nodes = [] - for code_node in (node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue): + for code_node in ( + node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue + ): if not _is_expression_simple_binary_operation(continuation.instruction.value): return None if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: @@ -241,6 +264,7 @@ def _get_continue_nodes_with_equalizable_definition(loop_node: WhileLoopNode, co equalizable_nodes.append(code_node) return equalizable_nodes + def _is_expression_simple_binary_operation(expression: Expression) -> bool: """Checks if an expression is a simple binary operation. Meaning it includes a variable and a constant and uses plus or minus as operation type.""" return ( @@ -250,13 +274,15 @@ def _is_expression_simple_binary_operation(expression: Expression) -> bool: and any(isinstance(operand, Variable) for operand in expression.subexpressions()) ) + def _get_variable_in_binary_operation(binaryoperation: BinaryOperation) -> Variable: """Returns the used variable of a binary operation if available.""" for operand in binaryoperation.subexpressions(): - if isinstance(operand, Variable): - return operand + if isinstance(operand, Variable): + return operand return None + def _count_unaryoperations_negations(expression: Expression) -> int: """Counts the amount of UnaryOperation negations of an expression.""" negations = 0 @@ -265,6 +291,7 @@ def _count_unaryoperations_negations(expression: Expression) -> int: negations += 1 return negations + def _unify_binary_operation(binaryoperation: BinaryOperation): """Brings a simple binary operation into a unified representation like 'var + const'.""" if not binaryoperation.operation == OperationType.plus: @@ -274,6 +301,7 @@ def _unify_binary_operation(binaryoperation: BinaryOperation): if any(isinstance(operand, Constant) for operand in binaryoperation.left.subexpressions()): binaryoperation.swap_operands() + def _substract_continuation_from_last_definition(code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): """ Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, @@ -284,7 +312,8 @@ def _substract_continuation_from_last_definition(code_node: CodeNode, continuati :param variable_init: Instruction defining the for-loops declaration """ last_definition = code_node.instructions[_get_last_definition_index_of(code_node, variable_init.instruction.destination)] - last_definition.substitute(last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right])) - + last_definition.substitute( + last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right]) + ) if _count_unaryoperations_negations(continuation.instruction.value.left) % 2 != 0: last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [last_definition.value])) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 4612cc842..69f7d3262 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -103,7 +103,9 @@ def run(self): continue if not self._force_for_loops and continuation.instruction.complexity > self._modification_max_complexity: continue - if (equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation, variable_init)) is None: + if ( + equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation, variable_init) + ) is None: break for node in equalizable_continue_nodes: _substract_continuation_from_last_definition(node, continuation, variable_init) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index aa1bc140e..bd9a9c062 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -900,8 +900,13 @@ def test_for_loop_recovery_if_continue_in_while_1(self): assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) condition_nodes = list(ast.get_condition_nodes_post_order()) - last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] - assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("a"), Constant(2)]), Constant(1)])) + last_definition = condition_nodes[0].true_branch_child.instructions[ + _get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a")) + ] + assert last_definition == Assignment( + Variable("a"), + BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("a"), Constant(2)]), Constant(1)]), + ) def test_for_loop_recovery_if_continue_in_while_2(self): """ @@ -921,23 +926,20 @@ def test_for_loop_recovery_if_continue_in_while_2(self): root := SeqNode(true_value), condition_map={ logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), - logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) - } + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), + }, ) - true_branch = ast._add_code_node( - [ - Assignment(Variable("a"), Constant(4)), - Continue() - ] - ) + true_branch = ast._add_code_node([Assignment(Variable("a"), Constant(4)), Continue()]) if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) while_loop_body = ast.factory.create_seq_node() - while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))]) + while_loop_iteration = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))] + ) ast._add_node(while_loop) ast._add_node(while_loop_body) @@ -947,7 +949,7 @@ def test_for_loop_recovery_if_continue_in_while_2(self): (root, while_loop), (while_loop, while_loop_body), (while_loop_body, if_condition), - (while_loop_body, while_loop_iteration) + (while_loop_body, while_loop_iteration), ] ) @@ -955,7 +957,9 @@ def test_for_loop_recovery_if_continue_in_while_2(self): assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) condition_nodes = list(ast.get_condition_nodes_post_order()) - last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a"))] + last_definition = condition_nodes[0].true_branch_child.instructions[ + _get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a")) + ] assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.minus, [Constant(4), Constant(1)])) def test_for_loop_recovery_if_continue_in_nested_while(self): @@ -1023,8 +1027,13 @@ def test_for_loop_recovery_if_continue_in_nested_while(self): assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) condition_nodes = list(ast.get_condition_nodes_post_order()) - last_definition = condition_nodes[0].true_branch_child.instructions[_get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("b"))] - assert last_definition == Assignment(Variable("b"), BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("b"), Constant(2)]), Constant(1)])) + last_definition = condition_nodes[0].true_branch_child.instructions[ + _get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("b")) + ] + assert last_definition == Assignment( + Variable("b"), + BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("b"), Constant(2)]), Constant(1)]), + ) def test_skip_for_loop_recovery_if_continue_in_while_1(self): """ @@ -1044,15 +1053,12 @@ def test_skip_for_loop_recovery_if_continue_in_while_1(self): root := SeqNode(true_value), condition_map={ logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), - logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) - } + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), + }, ) true_branch = ast._add_code_node( - [ - Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(2)])), - Continue() - ] + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(2)])), Continue()] ) if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) @@ -1060,7 +1066,9 @@ def test_skip_for_loop_recovery_if_continue_in_while_1(self): while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) while_loop_body = ast.factory.create_seq_node() - while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.multiply, [Variable("a"), Constant(2)]))]) + while_loop_iteration = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.multiply, [Variable("a"), Constant(2)]))] + ) ast._add_node(while_loop) ast._add_node(while_loop_body) @@ -1070,13 +1078,12 @@ def test_skip_for_loop_recovery_if_continue_in_while_1(self): (root, while_loop), (while_loop, while_loop_body), (while_loop_body, if_condition), - (while_loop_body, while_loop_iteration) + (while_loop_body, while_loop_iteration), ] ) WhileLoopReplacer(ast, _generate_options()).run() assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) - def test_skip_for_loop_recovery_if_continue_in_while_2(self): """ @@ -1096,15 +1103,12 @@ def test_skip_for_loop_recovery_if_continue_in_while_2(self): root := SeqNode(true_value), condition_map={ logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), - logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) - } + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), + }, ) true_branch = ast._add_code_node( - [ - Assignment(Variable("a"), BinaryOperation(OperationType.multiply, [Variable("a"), Constant(2)])), - Continue() - ] + [Assignment(Variable("a"), BinaryOperation(OperationType.multiply, [Variable("a"), Constant(2)])), Continue()] ) if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) @@ -1112,7 +1116,9 @@ def test_skip_for_loop_recovery_if_continue_in_while_2(self): while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) while_loop_body = ast.factory.create_seq_node() - while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))]) + while_loop_iteration = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))] + ) ast._add_node(while_loop) ast._add_node(while_loop_body) @@ -1122,7 +1128,7 @@ def test_skip_for_loop_recovery_if_continue_in_while_2(self): (root, while_loop), (while_loop, while_loop_body), (while_loop_body, if_condition), - (while_loop_body, while_loop_iteration) + (while_loop_body, while_loop_iteration), ] ) @@ -1146,8 +1152,8 @@ def test_skip_for_loop_recovery_if_continue_in_while_3(self): root := SeqNode(true_value), condition_map={ logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), - logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]) - } + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), + }, ) true_branch = ast._add_code_node([Continue()]) @@ -1157,7 +1163,9 @@ def test_skip_for_loop_recovery_if_continue_in_while_3(self): while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) while_loop_body = ast.factory.create_seq_node() - while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))]) + while_loop_iteration = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]))] + ) ast._add_node(while_loop) ast._add_node(while_loop_body) @@ -1167,7 +1175,7 @@ def test_skip_for_loop_recovery_if_continue_in_while_3(self): (root, while_loop), (while_loop, while_loop_body), (while_loop_body, if_condition), - (while_loop_body, while_loop_iteration) + (while_loop_body, while_loop_iteration), ] ) From 7799db5bed29f87fa7eac6dc6c83ac3b9afe2f10 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Wed, 15 Nov 2023 13:39:00 +0100 Subject: [PATCH 13/31] Unified form only needed for continuation --- .../pipeline/controlflowanalysis/loop_utility_methods.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 076754754..14058fecd 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -258,9 +258,6 @@ def _get_continue_nodes_with_equalizable_definition( return None _unify_binary_operation(continuation.instruction.value) - if not isinstance(code_node.instructions[last_definition_index].value, Constant): - _unify_binary_operation(code_node.instructions[last_definition_index].value) - equalizable_nodes.append(code_node) return equalizable_nodes From b4cfe639c4d53275b3c9cfaee885f908ac814bf9 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 16 Nov 2023 10:11:44 +0100 Subject: [PATCH 14/31] Replaced setter functions with substitutes --- .../loop_utility_methods.py | 19 +++++++++++-------- decompiler/structures/pseudo/operations.py | 9 --------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 14058fecd..8620e667a 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -26,6 +26,9 @@ Variable, ) from decompiler.structures.visitors.assignment_visitor import AssignmentVisitor +from decompiler.pipeline.controlflowanalysis.expression_simplification.rules.sub_to_add import SubToAdd +from decompiler.pipeline.controlflowanalysis.expression_simplification.rules.term_order import TermOrder +from decompiler.pipeline.controlflowanalysis.expression_simplification.rules.rule import SimplificationRule @dataclass @@ -257,7 +260,7 @@ def _get_continue_nodes_with_equalizable_definition( ): return None - _unify_binary_operation(continuation.instruction.value) + _unify_binary_operation_in_assignment(continuation.instruction) equalizable_nodes.append(code_node) return equalizable_nodes @@ -289,14 +292,14 @@ def _count_unaryoperations_negations(expression: Expression) -> int: return negations -def _unify_binary_operation(binaryoperation: BinaryOperation): - """Brings a simple binary operation into a unified representation like 'var + const'.""" - if not binaryoperation.operation == OperationType.plus: - binaryoperation.operation = OperationType.plus - binaryoperation.substitute(binaryoperation.right, UnaryOperation(OperationType.negate, [binaryoperation.right])) +def _unify_binary_operation_in_assignment(assignment: Assignment): + """Brings a simple binary operation of an assignment into a unified representation like 'var = -var + const' instead of 'var = const - var'.""" + if not assignment.value.operation == OperationType.plus: + assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.left, assignment.value.right])) + assignment.substitute(assignment.value.right, UnaryOperation(OperationType.negate, [assignment.value.right])) - if any(isinstance(operand, Constant) for operand in binaryoperation.left.subexpressions()): - binaryoperation.swap_operands() + if any(isinstance(operand, Constant) for operand in assignment.value.left.subexpressions()): + assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.right, assignment.value.left])) def _substract_continuation_from_last_definition(code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): diff --git a/decompiler/structures/pseudo/operations.py b/decompiler/structures/pseudo/operations.py index 7615b824d..1b8c88d89 100644 --- a/decompiler/structures/pseudo/operations.py +++ b/decompiler/structures/pseudo/operations.py @@ -456,11 +456,6 @@ def right(self) -> Expression: """Return the right-hand-side operand.""" return self._operands[1] - @Operation.operation.setter - def operation(self, value): - """Setter function for operation.""" - self._operation = value - def copy(self) -> BinaryOperation: """Generate a deep copy of the current binary operation.""" return self.__class__(self._operation, [operand.copy() for operand in self._operands], self._type.copy(), self.tags) @@ -469,10 +464,6 @@ def accept(self, visitor: DataflowObjectVisitorInterface[T]) -> T: """Invoke the appropriate visitor for this Operation.""" return visitor.visit_binary_operation(self) - def swap_operands(self): - """Swaps left-hand-side with right-hand-side operand.""" - self._operands[0], self._operands[1] = self._operands[1], self._operands[0] - class Call(Operation): """Class representing a Call operation""" From 67f325a0dbfed0b8e1e513aeb505194e9ab4875d Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 16 Nov 2023 10:13:57 +0100 Subject: [PATCH 15/31] Clean up imports --- .../pipeline/controlflowanalysis/loop_utility_methods.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 8620e667a..86fe8e501 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -26,9 +26,6 @@ Variable, ) from decompiler.structures.visitors.assignment_visitor import AssignmentVisitor -from decompiler.pipeline.controlflowanalysis.expression_simplification.rules.sub_to_add import SubToAdd -from decompiler.pipeline.controlflowanalysis.expression_simplification.rules.term_order import TermOrder -from decompiler.pipeline.controlflowanalysis.expression_simplification.rules.rule import SimplificationRule @dataclass From 0c2d455282447adb95aea18743b6fcb690485fc6 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 16 Nov 2023 10:18:04 +0100 Subject: [PATCH 16/31] Implemented tests --- .../test_readability_based_refinement.py | 254 ++++++++++++++++++ 1 file changed, 254 insertions(+) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index bd9a9c062..192507f5b 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -22,6 +22,7 @@ ImportedFunctionSymbol, ListOperation, OperationType, + UnaryOperation, Variable, ) from decompiler.structures.pseudo.operations import OperationType @@ -962,6 +963,208 @@ def test_for_loop_recovery_if_continue_in_while_2(self): ] assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.minus, [Constant(4), Constant(1)])) + def test_for_loop_recovery_if_continue_in_while_3(self): + """ + Test for loop recovery if a continue occurs in a while loop and the continuation-statement and the last definition + are simple binary operations that use the same variable but another than the iteration variable. + + a = 1 + while(a < 10){ + if(num2 > num1){ + a = num1 + 3 + continue + } + a = num1 + 2 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.greater, [Variable("num2"), Variable("num1")]), + }, + ) + + true_branch = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num1"), Constant(3)])), Continue()] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num1"), Constant(2)]))] + ) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration), + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + condition_nodes = list(ast.get_condition_nodes_post_order()) + last_definition = condition_nodes[0].true_branch_child.instructions[ + _get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a")) + ] + assert last_definition == Assignment( + Variable("a"), + BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("num1"), Constant(3)]), Constant(2)]), + ) + + def test_for_loop_recovery_if_continue_in_while_4(self): + """ + Test for loop recovery if a continue occurs in a while loop, the last definition is a simple binary operation and + the continuation-instruction has a negated used variable. + + a = 0 + while(a < 10) { + if(a == 2) { + a = a + 1 + continue + } + a = -a + 10 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), + }, + ) + + true_branch = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)])), Continue()] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node( + [ + Assignment( + Variable("a"), + BinaryOperation(OperationType.plus, [UnaryOperation(OperationType.negate, [Variable("a")]), Constant(10)]), + ) + ] + ) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration), + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + condition_nodes = list(ast.get_condition_nodes_post_order()) + last_definition = condition_nodes[0].true_branch_child.instructions[ + _get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a")) + ] + assert last_definition == Assignment( + Variable("a"), + UnaryOperation( + OperationType.negate, + [BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]), Constant(10)])], + ), + ) + + def test_for_loop_recovery_if_continue_in_while_5(self): + """ + Test for loop recovery if a continue occurs in a while loop, the last definition is a simple binary operation, + the continuation-instruction has a negated constant and a not unified form like 'var = var + const'. + + a = 0; + while(a < 10) { + if(a == 2) { + a = a + 1 + continue + } + a = -10 - -a + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), + }, + ) + + true_branch = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)])), Continue()] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node( + [ + Assignment( + Variable("a"), + BinaryOperation(OperationType.minus, [Constant(-10), UnaryOperation(OperationType.negate, [Variable("a")])]), + ) + ] + ) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration), + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + + for loop_node in list(ast.get_loop_nodes_post_order()): + assert isinstance(loop_node, ForLoopNode) + assert loop_node.modification == Assignment( + Variable("a"), + BinaryOperation( + OperationType.plus, + [UnaryOperation(OperationType.negate, [UnaryOperation(OperationType.negate, [Variable("a")])]), Constant(-10)], + ), + ) + + condition_nodes = list(ast.get_condition_nodes_post_order()) + last_definition = condition_nodes[0].true_branch_child.instructions[ + _get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a")) + ] + assert last_definition == Assignment( + Variable("a"), + BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)]), Constant(-10)]), + ) + def test_for_loop_recovery_if_continue_in_nested_while(self): """ while(a < 5) { @@ -1181,3 +1384,54 @@ def test_skip_for_loop_recovery_if_continue_in_while_3(self): WhileLoopReplacer(ast, _generate_options()).run() assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + def test_skip_for_loop_recovery_if_continue_in_while_4(self): + """ + Test skip of for loop recovery if a continue occurs in a while loop, the continuation-statement and the last definition + uses different variables. + + a = 0 + while(a < 10){ + if(num2 > num1){ + a = num2 + 3 + continue + } + a = num1 + 2 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.greater, [Variable("num2"), Variable("num1")]), + }, + ) + + true_branch = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num2"), Constant(3)])), Continue()] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num1"), Constant(2)]))] + ) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration), + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) From a7f4d76ad01b47d238633a23bf468020839ee904 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 16 Nov 2023 10:26:48 +0100 Subject: [PATCH 17/31] Fix comments --- .../test_readability_based_refinement.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index 192507f5b..4723237e5 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -852,7 +852,7 @@ def test_separated_by_loop_node_4(self, ast_while_in_else): def test_for_loop_recovery_if_continue_in_while_1(self): """ - Test for loop recovery if a continue occurs in a while loop and the last definition is a simple binary operation + Test for loop recovery if a continue occurs in a while loop and the last definition is a simple binary operation. a = 0 while(a < 10) { @@ -911,7 +911,7 @@ def test_for_loop_recovery_if_continue_in_while_1(self): def test_for_loop_recovery_if_continue_in_while_2(self): """ - Test for loop recovery if a continue occurs in a while loop and the last definition is a contant assignment + Test for loop recovery if a continue occurs in a while loop and the last definition is a constant. a = 0 while(a < 10) { @@ -1167,6 +1167,8 @@ def test_for_loop_recovery_if_continue_in_while_5(self): def test_for_loop_recovery_if_continue_in_nested_while(self): """ + Test for loop recovery if a continue occurs in a nested while loop and the last definition is a simple binary operation. + while(a < 5) { a = a + b while(b < 10) { @@ -1240,7 +1242,7 @@ def test_for_loop_recovery_if_continue_in_nested_while(self): def test_skip_for_loop_recovery_if_continue_in_while_1(self): """ - Test skip of for loop recovery if a continue occurs in a while loop, because the continuation instruction is no simple binary operation + Test skip of for loop recovery if a continue occurs in a while loop, because the continuation instruction is no simple binary operation. a = 0 while(a < 10) { @@ -1290,7 +1292,7 @@ def test_skip_for_loop_recovery_if_continue_in_while_1(self): def test_skip_for_loop_recovery_if_continue_in_while_2(self): """ - Test skip of for loop recovery if a continue occurs in a while loop, because the last definition is no simple binary operation + Test skip of for loop recovery if a continue occurs in a while loop, because the last definition is no simple binary operation. a = 0 while(a < 10) { @@ -1340,7 +1342,7 @@ def test_skip_for_loop_recovery_if_continue_in_while_2(self): def test_skip_for_loop_recovery_if_continue_in_while_3(self): """ - Test skip of for loop recovery if a continue occurs in a while loop, because no last definition exists + Test skip of for loop recovery if a continue occurs in a while loop, because no last definition exists. a = 0 while(a < 10) { @@ -1387,8 +1389,8 @@ def test_skip_for_loop_recovery_if_continue_in_while_3(self): def test_skip_for_loop_recovery_if_continue_in_while_4(self): """ - Test skip of for loop recovery if a continue occurs in a while loop, the continuation-statement and the last definition - uses different variables. + Test skip of for loop recovery if a continue occurs in a while loop and the continuation-statement and the last definition + use different variables. a = 0 while(a < 10){ From 55ead2ed5271a1e2c8d9c48d4788528c13fd5339 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 16 Nov 2023 11:10:13 +0100 Subject: [PATCH 18/31] Shortened if statement --- decompiler/pipeline/controlflowanalysis/loop_utility_methods.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 86fe8e501..b2ea2e520 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -312,5 +312,5 @@ def _substract_continuation_from_last_definition(code_node: CodeNode, continuati last_definition.substitute( last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right]) ) - if _count_unaryoperations_negations(continuation.instruction.value.left) % 2 != 0: + if _count_unaryoperations_negations(continuation.instruction.value.left) % 2: last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [last_definition.value])) From 29ec9fa1d70f2931a1ebdc4af55ab1acf7c52736 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Wed, 6 Dec 2023 13:02:11 +0100 Subject: [PATCH 19/31] Updated suggestions --- .../loop_utility_methods.py | 35 ++++++++++--------- .../readability_based_refinement.py | 6 ++-- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index b2ea2e520..7d372a763 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -231,7 +231,7 @@ def _requirement_without_reinitialization(ast: AbstractSyntaxTree, node: Abstrac def _get_continue_nodes_with_equalizable_definition( - loop_node: WhileLoopNode, continuation: AstInstruction, variable_init: AstInstruction + loop_node: WhileLoopNode, continuation: AstInstruction ) -> List[CodeNode]: """ Finds code nodes of a while loop containing continue statements and a definition of the continuation instruction, which can be easily equalized. @@ -247,7 +247,7 @@ def _get_continue_nodes_with_equalizable_definition( ): if not _is_expression_simple_binary_operation(continuation.instruction.value): return None - if (last_definition_index := _get_last_definition_index_of(code_node, variable_init.instruction.destination)) == -1: + if (last_definition_index := _get_last_definition_index_of(code_node, continuation.instruction.destination)) == -1: return None if not ( isinstance(code_node.instructions[last_definition_index].value, Constant) @@ -267,28 +267,29 @@ def _is_expression_simple_binary_operation(expression: Expression) -> bool: return ( isinstance(expression, BinaryOperation) and expression.operation in {OperationType.plus, OperationType.minus} - and any(isinstance(operand, Constant) for operand in expression.subexpressions()) - and any(isinstance(operand, Variable) for operand in expression.subexpressions()) + and any(isinstance(operand, Constant) or _is_negated_constant_variable(operand, Constant) for operand in expression.operands) + and any(isinstance(operand, Variable) or _is_negated_constant_variable(operand, Variable) for operand in expression.operands) + ) + + +def _is_negated_constant_variable(operand: Expression, expression: Constant | Variable) -> bool: + return ( + isinstance(operand, UnaryOperation) + and operand.operation == OperationType.negate + and isinstance(operand.operand, expression) ) def _get_variable_in_binary_operation(binaryoperation: BinaryOperation) -> Variable: """Returns the used variable of a binary operation if available.""" - for operand in binaryoperation.subexpressions(): + for operand in binaryoperation.operands: if isinstance(operand, Variable): return operand + if _is_negated_constant_variable(operand, Variable): + return operand.operand return None -def _count_unaryoperations_negations(expression: Expression) -> int: - """Counts the amount of UnaryOperation negations of an expression.""" - negations = 0 - for subexpression in expression.subexpressions(): - if isinstance(subexpression, UnaryOperation) and subexpression.operation == OperationType.negate: - negations += 1 - return negations - - def _unify_binary_operation_in_assignment(assignment: Assignment): """Brings a simple binary operation of an assignment into a unified representation like 'var = -var + const' instead of 'var = const - var'.""" if not assignment.value.operation == OperationType.plus: @@ -299,7 +300,7 @@ def _unify_binary_operation_in_assignment(assignment: Assignment): assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.right, assignment.value.left])) -def _substract_continuation_from_last_definition(code_node: CodeNode, continuation: AstInstruction, variable_init: AstInstruction): +def _substract_continuation_from_last_definition(code_node: CodeNode, continuation: AstInstruction): """ Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, defining the same value as the continuation instruction in the given code node. @@ -308,9 +309,9 @@ def _substract_continuation_from_last_definition(code_node: CodeNode, continuati :param continuation: Instruction defining the for-loops modification :param variable_init: Instruction defining the for-loops declaration """ - last_definition = code_node.instructions[_get_last_definition_index_of(code_node, variable_init.instruction.destination)] + last_definition = code_node.instructions[_get_last_definition_index_of(code_node, continuation.instruction.destination)] last_definition.substitute( last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right]) ) - if _count_unaryoperations_negations(continuation.instruction.value.left) % 2: + if _is_negated_constant_variable(continuation.instruction.value.left, Variable): last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [last_definition.value])) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 69f7d3262..491d54092 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -104,11 +104,11 @@ def run(self): if not self._force_for_loops and continuation.instruction.complexity > self._modification_max_complexity: continue if ( - equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation, variable_init) + equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation) ) is None: - break + continue for node in equalizable_continue_nodes: - _substract_continuation_from_last_definition(node, continuation, variable_init) + _substract_continuation_from_last_definition(node, continuation) self._replace_with_for_loop(loop_node, continuation, variable_init) break From 4efd39abb4febef261b1afd3638ab04a855710a5 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Wed, 6 Dec 2023 13:08:30 +0100 Subject: [PATCH 20/31] Black formatting, added comment --- .../controlflowanalysis/loop_utility_methods.py | 11 +++-------- .../readability_based_refinement.py | 4 +--- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 7d372a763..943ff224c 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -230,9 +230,7 @@ def _requirement_without_reinitialization(ast: AbstractSyntaxTree, node: Abstrac return True -def _get_continue_nodes_with_equalizable_definition( - loop_node: WhileLoopNode, continuation: AstInstruction -) -> List[CodeNode]: +def _get_continue_nodes_with_equalizable_definition(loop_node: WhileLoopNode, continuation: AstInstruction) -> List[CodeNode]: """ Finds code nodes of a while loop containing continue statements and a definition of the continuation instruction, which can be easily equalized. @@ -273,11 +271,8 @@ def _is_expression_simple_binary_operation(expression: Expression) -> bool: def _is_negated_constant_variable(operand: Expression, expression: Constant | Variable) -> bool: - return ( - isinstance(operand, UnaryOperation) - and operand.operation == OperationType.negate - and isinstance(operand.operand, expression) - ) + """Checks if an operand (constant or variable) is negated.""" + return isinstance(operand, UnaryOperation) and operand.operation == OperationType.negate and isinstance(operand.operand, expression) def _get_variable_in_binary_operation(binaryoperation: BinaryOperation) -> Variable: diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 491d54092..3d640eac5 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -103,9 +103,7 @@ def run(self): continue if not self._force_for_loops and continuation.instruction.complexity > self._modification_max_complexity: continue - if ( - equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation) - ) is None: + if (equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation)) is None: continue for node in equalizable_continue_nodes: _substract_continuation_from_last_definition(node, continuation) From 13dc250cad11964266c1de3f3dd63a92f62bfe91 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 7 Dec 2023 11:36:06 +0100 Subject: [PATCH 21/31] Fix substitute in unify --- decompiler/pipeline/controlflowanalysis/loop_utility_methods.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 943ff224c..de21e9beb 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -289,7 +289,7 @@ def _unify_binary_operation_in_assignment(assignment: Assignment): """Brings a simple binary operation of an assignment into a unified representation like 'var = -var + const' instead of 'var = const - var'.""" if not assignment.value.operation == OperationType.plus: assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.left, assignment.value.right])) - assignment.substitute(assignment.value.right, UnaryOperation(OperationType.negate, [assignment.value.right])) + assignment.value.substitute(assignment.value.right, UnaryOperation(OperationType.negate, [assignment.value.right])) if any(isinstance(operand, Constant) for operand in assignment.value.left.subexpressions()): assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.right, assignment.value.left])) From 3176e982a38aeb0600fcac3d6dc82f0085982f7e Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 7 Dec 2023 11:51:22 +0100 Subject: [PATCH 22/31] Operate on last definitions --- .../loop_utility_methods.py | 19 ++++++++++--------- .../readability_based_refinement.py | 8 ++++---- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index de21e9beb..b09410053 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -230,14 +230,14 @@ def _requirement_without_reinitialization(ast: AbstractSyntaxTree, node: Abstrac return True -def _get_continue_nodes_with_equalizable_definition(loop_node: WhileLoopNode, continuation: AstInstruction) -> List[CodeNode]: +def _get_equalizable_last_definitions(loop_node: WhileLoopNode, continuation: AstInstruction) -> List[CodeNode]: """ - Finds code nodes of a while loop containing continue statements and a definition of the continuation instruction, which can be easily equalized. + Finds equalizable last definitions of the continuation instruction in the code nodes of a while loop containing continue statements. :param loop_node: While-loop to search in :param continuation: Instruction defining the for-loops modification :param variable_init: Instruction defining the for-loops declaration - :return: List of continue code nodes that has equalizable definitions, None if at least one continue node does not match the requirements + :return: List of equalizable last definitions, None if at least one continue node does not match the requirements """ equalizable_nodes = [] for code_node in ( @@ -247,16 +247,18 @@ def _get_continue_nodes_with_equalizable_definition(loop_node: WhileLoopNode, co return None if (last_definition_index := _get_last_definition_index_of(code_node, continuation.instruction.destination)) == -1: return None + else: + last_definition = code_node.instructions[last_definition_index] if not ( - isinstance(code_node.instructions[last_definition_index].value, Constant) - or _is_expression_simple_binary_operation(code_node.instructions[last_definition_index].value) + isinstance(last_definition.value, Constant) + or _is_expression_simple_binary_operation(last_definition.value) and _get_variable_in_binary_operation(continuation.instruction.value) - == _get_variable_in_binary_operation(code_node.instructions[last_definition_index].value) + == _get_variable_in_binary_operation(last_definition.value) ): return None _unify_binary_operation_in_assignment(continuation.instruction) - equalizable_nodes.append(code_node) + equalizable_nodes.append(last_definition) return equalizable_nodes @@ -295,7 +297,7 @@ def _unify_binary_operation_in_assignment(assignment: Assignment): assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.right, assignment.value.left])) -def _substract_continuation_from_last_definition(code_node: CodeNode, continuation: AstInstruction): +def _substract_continuation_from_last_definition(last_definition: Assignment, continuation: AstInstruction): """ Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, defining the same value as the continuation instruction in the given code node. @@ -304,7 +306,6 @@ def _substract_continuation_from_last_definition(code_node: CodeNode, continuati :param continuation: Instruction defining the for-loops modification :param variable_init: Instruction defining the for-loops declaration """ - last_definition = code_node.instructions[_get_last_definition_index_of(code_node, continuation.instruction.destination)] last_definition.substitute( last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right]) ) diff --git a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py index 3d640eac5..f979803c7 100644 --- a/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py +++ b/decompiler/pipeline/controlflowanalysis/readability_based_refinement.py @@ -6,7 +6,7 @@ from decompiler.pipeline.controlflowanalysis.loop_utility_methods import ( AstInstruction, _find_continuation_instruction, - _get_continue_nodes_with_equalizable_definition, + _get_equalizable_last_definitions, _get_variable_initialisation, _initialization_reaches_loop_node, _is_single_instruction_loop_node, @@ -103,10 +103,10 @@ def run(self): continue if not self._force_for_loops and continuation.instruction.complexity > self._modification_max_complexity: continue - if (equalizable_continue_nodes := _get_continue_nodes_with_equalizable_definition(loop_node, continuation)) is None: + if (equalizable_last_definitions := _get_equalizable_last_definitions(loop_node, continuation)) is None: continue - for node in equalizable_continue_nodes: - _substract_continuation_from_last_definition(node, continuation) + for last_definition in equalizable_last_definitions: + _substract_continuation_from_last_definition(last_definition, continuation) self._replace_with_for_loop(loop_node, continuation, variable_init) break From 0d5b1df53a1f4e2ba743c78df9902d447d79ab9c Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Thu, 7 Dec 2023 12:02:07 +0100 Subject: [PATCH 23/31] Updated comments --- .../pipeline/controlflowanalysis/loop_utility_methods.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index b09410053..890cbb29a 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -236,7 +236,6 @@ def _get_equalizable_last_definitions(loop_node: WhileLoopNode, continuation: As :param loop_node: While-loop to search in :param continuation: Instruction defining the for-loops modification - :param variable_init: Instruction defining the for-loops declaration :return: List of equalizable last definitions, None if at least one continue node does not match the requirements """ equalizable_nodes = [] @@ -302,9 +301,8 @@ def _substract_continuation_from_last_definition(last_definition: Assignment, co Substracts the value of the continuation instruction from the last definition, which must be a simple binary operation or a constant, defining the same value as the continuation instruction in the given code node. - :param code_node: Code node whose last instruction is to be changed + :param last_definition: Last definition that is to be changed :param continuation: Instruction defining the for-loops modification - :param variable_init: Instruction defining the for-loops declaration """ last_definition.substitute( last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right]) From 795c68e365466fe97f776a1f62deb30526b85467 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 12 Dec 2023 16:46:39 +0100 Subject: [PATCH 24/31] Only reconstruct if defined and used variables are the same --- .../loop_utility_methods.py | 3 +- .../test_readability_based_refinement.py | 78 +++---------------- 2 files changed, 11 insertions(+), 70 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 890cbb29a..66a63650b 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -251,7 +251,8 @@ def _get_equalizable_last_definitions(loop_node: WhileLoopNode, continuation: As if not ( isinstance(last_definition.value, Constant) or _is_expression_simple_binary_operation(last_definition.value) - and _get_variable_in_binary_operation(continuation.instruction.value) + and continuation.instruction.destination + == _get_variable_in_binary_operation(continuation.instruction.value) == _get_variable_in_binary_operation(last_definition.value) ): return None diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index 4723237e5..4ea3afd99 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -964,66 +964,6 @@ def test_for_loop_recovery_if_continue_in_while_2(self): assert last_definition == Assignment(Variable("a"), BinaryOperation(OperationType.minus, [Constant(4), Constant(1)])) def test_for_loop_recovery_if_continue_in_while_3(self): - """ - Test for loop recovery if a continue occurs in a while loop and the continuation-statement and the last definition - are simple binary operations that use the same variable but another than the iteration variable. - - a = 1 - while(a < 10){ - if(num2 > num1){ - a = num1 + 3 - continue - } - a = num1 + 2 - } - """ - true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) - ast = AbstractSyntaxTree( - root := SeqNode(true_value), - condition_map={ - logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), - logic_cond("x2", context): Condition(OperationType.greater, [Variable("num2"), Variable("num1")]), - }, - ) - - true_branch = ast._add_code_node( - [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num1"), Constant(3)])), Continue()] - ) - if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) - - init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) - - while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) - while_loop_body = ast.factory.create_seq_node() - while_loop_iteration = ast._add_code_node( - [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num1"), Constant(2)]))] - ) - ast._add_node(while_loop) - ast._add_node(while_loop_body) - - ast._add_edges_from( - [ - (root, init_code_node), - (root, while_loop), - (while_loop, while_loop_body), - (while_loop_body, if_condition), - (while_loop_body, while_loop_iteration), - ] - ) - - WhileLoopReplacer(ast, _generate_options()).run() - assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) - - condition_nodes = list(ast.get_condition_nodes_post_order()) - last_definition = condition_nodes[0].true_branch_child.instructions[ - _get_last_definition_index_of(condition_nodes[0].true_branch_child, Variable("a")) - ] - assert last_definition == Assignment( - Variable("a"), - BinaryOperation(OperationType.minus, [BinaryOperation(OperationType.plus, [Variable("num1"), Constant(3)]), Constant(2)]), - ) - - def test_for_loop_recovery_if_continue_in_while_4(self): """ Test for loop recovery if a continue occurs in a while loop, the last definition is a simple binary operation and the continuation-instruction has a negated used variable. @@ -1091,7 +1031,7 @@ def test_for_loop_recovery_if_continue_in_while_4(self): ), ) - def test_for_loop_recovery_if_continue_in_while_5(self): + def test_for_loop_recovery_if_continue_in_while_4(self): """ Test for loop recovery if a continue occurs in a while loop, the last definition is a simple binary operation, the continuation-instruction has a negated constant and a not unified form like 'var = var + const'. @@ -1389,16 +1329,16 @@ def test_skip_for_loop_recovery_if_continue_in_while_3(self): def test_skip_for_loop_recovery_if_continue_in_while_4(self): """ - Test skip of for loop recovery if a continue occurs in a while loop and the continuation-statement and the last definition - use different variables. + Test skip of for loop recovery if a continue occurs in a while loop and the continuation-statement defines + and uses different variables. a = 0 while(a < 10){ - if(num2 > num1){ - a = num2 + 3 + if(a == 2) { + a = a + 3 continue } - a = num1 + 2 + a = b + 2 } """ true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) @@ -1406,12 +1346,12 @@ def test_skip_for_loop_recovery_if_continue_in_while_4(self): root := SeqNode(true_value), condition_map={ logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), - logic_cond("x2", context): Condition(OperationType.greater, [Variable("num2"), Variable("num1")]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), }, ) true_branch = ast._add_code_node( - [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num2"), Constant(3)])), Continue()] + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(3)])), Continue()] ) if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) @@ -1420,7 +1360,7 @@ def test_skip_for_loop_recovery_if_continue_in_while_4(self): while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) while_loop_body = ast.factory.create_seq_node() while_loop_iteration = ast._add_code_node( - [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("num1"), Constant(2)]))] + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("b"), Constant(2)]))] ) ast._add_node(while_loop) ast._add_node(while_loop_body) From d4d5dd71349e498350a56973e82ba41c6da8b388 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 19 Dec 2023 10:42:00 +0100 Subject: [PATCH 25/31] Added skip testcase for continuation with only constant --- .../test_readability_based_refinement.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index 4ea3afd99..fe0e7239a 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -1377,3 +1377,52 @@ def test_skip_for_loop_recovery_if_continue_in_while_4(self): WhileLoopReplacer(ast, _generate_options()).run() assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + def test_skip_for_loop_recovery_if_continue_in_while_5(self): + """ + Test skip of for loop recovery if a continue occurs in a while loop, because the continuation instruction is + an assignment with a constant and is therefore not a simple binary operation. + + a = 0 + while(a < 10){ + if(a == 2) { + a = a + 1 + continue + } + a = 4 + } + """ + true_value = LogicCondition.initialize_true(context := LogicCondition.generate_new_context()) + ast = AbstractSyntaxTree( + root := SeqNode(true_value), + condition_map={ + logic_cond("x1", context): Condition(OperationType.less, [Variable("a"), Constant(10)]), + logic_cond("x2", context): Condition(OperationType.equal, [Variable("a"), Constant(2)]), + }, + ) + + true_branch = ast._add_code_node( + [Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)])), Continue()] + ) + if_condition = ast._add_condition_node_with(logic_cond("x2", context), true_branch) + + init_code_node = ast._add_code_node([Assignment(Variable("a"), Constant(0))]) + + while_loop = ast.factory.create_while_loop_node(logic_cond("x1", context)) + while_loop_body = ast.factory.create_seq_node() + while_loop_iteration = ast._add_code_node([Assignment(Variable("a"), Constant(4))]) + ast._add_node(while_loop) + ast._add_node(while_loop_body) + + ast._add_edges_from( + [ + (root, init_code_node), + (root, while_loop), + (while_loop, while_loop_body), + (while_loop_body, if_condition), + (while_loop_body, while_loop_iteration), + ] + ) + + WhileLoopReplacer(ast, _generate_options()).run() + assert not any(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) From 6a9e4bc2cd2fab14bb21835fb0bf385f15cf9b7f Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 19 Dec 2023 11:49:53 +0100 Subject: [PATCH 26/31] Adjusted check for simple binary operation and variable usage --- .../loop_utility_methods.py | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 66a63650b..0b29535bd 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -238,23 +238,18 @@ def _get_equalizable_last_definitions(loop_node: WhileLoopNode, continuation: As :param continuation: Instruction defining the for-loops modification :return: List of equalizable last definitions, None if at least one continue node does not match the requirements """ + if not (_is_assignment_with_simple_binary_operation(continuation.instruction)): + return None + equalizable_nodes = [] for code_node in ( node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue ): - if not _is_expression_simple_binary_operation(continuation.instruction.value): - return None if (last_definition_index := _get_last_definition_index_of(code_node, continuation.instruction.destination)) == -1: return None else: last_definition = code_node.instructions[last_definition_index] - if not ( - isinstance(last_definition.value, Constant) - or _is_expression_simple_binary_operation(last_definition.value) - and continuation.instruction.destination - == _get_variable_in_binary_operation(continuation.instruction.value) - == _get_variable_in_binary_operation(last_definition.value) - ): + if not (isinstance(last_definition.value, Constant) or _is_assignment_with_simple_binary_operation(last_definition)): return None _unify_binary_operation_in_assignment(continuation.instruction) @@ -262,13 +257,17 @@ def _get_equalizable_last_definitions(loop_node: WhileLoopNode, continuation: As return equalizable_nodes -def _is_expression_simple_binary_operation(expression: Expression) -> bool: - """Checks if an expression is a simple binary operation. Meaning it includes a variable and a constant and uses plus or minus as operation type.""" +def _is_assignment_with_simple_binary_operation(assignment: Assignment) -> bool: + """ + Checks if an assignment has a simple binary operation as value and the used and defined variable is the same. A simple binary + operation means that it includes a variable and a constant and uses plus or minus as operation type. + """ return ( - isinstance(expression, BinaryOperation) - and expression.operation in {OperationType.plus, OperationType.minus} - and any(isinstance(operand, Constant) or _is_negated_constant_variable(operand, Constant) for operand in expression.operands) - and any(isinstance(operand, Variable) or _is_negated_constant_variable(operand, Variable) for operand in expression.operands) + isinstance(assignment.value, BinaryOperation) + and assignment.value.operation in {OperationType.plus, OperationType.minus} + and any(isinstance(operand, Constant) or _is_negated_constant_variable(operand, Constant) for operand in assignment.value.operands) + and any(isinstance(operand, Variable) or _is_negated_constant_variable(operand, Variable) for operand in assignment.value.operands) + and assignment.destination == _get_variable_in_binary_operation(assignment.value) ) From 459504d18f841aeb8303d50e55540d75df7d3949 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 19 Dec 2023 12:37:04 +0100 Subject: [PATCH 27/31] Fix order of check for continue nodes --- .../loop_utility_methods.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 0b29535bd..739cf23b9 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -236,19 +236,25 @@ def _get_equalizable_last_definitions(loop_node: WhileLoopNode, continuation: As :param loop_node: While-loop to search in :param continuation: Instruction defining the for-loops modification - :return: List of equalizable last definitions, None if at least one continue node does not match the requirements + :return: List of equalizable last definitions, Empty list if no continue nodes or no equalizable nodes + :return: None if at least one continue node does not match the requirements """ + if not ( + continue_nodes := [ + node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue + ] + ): + return continue_nodes + if not (_is_assignment_with_simple_binary_operation(continuation.instruction)): return None equalizable_nodes = [] - for code_node in ( - node for node in loop_node.body.get_descendant_code_nodes_interrupting_ancestor_loop() if node.does_end_with_continue - ): + for code_node in continue_nodes: if (last_definition_index := _get_last_definition_index_of(code_node, continuation.instruction.destination)) == -1: return None - else: - last_definition = code_node.instructions[last_definition_index] + + last_definition = code_node.instructions[last_definition_index] if not (isinstance(last_definition.value, Constant) or _is_assignment_with_simple_binary_operation(last_definition)): return None From aa026576608a2834084bbcd845f53a2bafbfbc8d Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 19 Dec 2023 12:44:10 +0100 Subject: [PATCH 28/31] Merged substitutes --- .../pipeline/controlflowanalysis/loop_utility_methods.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 739cf23b9..9199feb84 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -295,8 +295,10 @@ def _get_variable_in_binary_operation(binaryoperation: BinaryOperation) -> Varia def _unify_binary_operation_in_assignment(assignment: Assignment): """Brings a simple binary operation of an assignment into a unified representation like 'var = -var + const' instead of 'var = const - var'.""" if not assignment.value.operation == OperationType.plus: - assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.left, assignment.value.right])) - assignment.value.substitute(assignment.value.right, UnaryOperation(OperationType.negate, [assignment.value.right])) + assignment.substitute( + assignment.value, + BinaryOperation(OperationType.plus, [assignment.value.left, UnaryOperation(OperationType.negate, assignment.value.right)]), + ) if any(isinstance(operand, Constant) for operand in assignment.value.left.subexpressions()): assignment.substitute(assignment.value, BinaryOperation(OperationType.plus, [assignment.value.right, assignment.value.left])) From 83370e49a32628014157c6465240c0f67a12665b Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 19 Dec 2023 12:48:14 +0100 Subject: [PATCH 29/31] Fix missing brackets --- decompiler/pipeline/controlflowanalysis/loop_utility_methods.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index 9199feb84..ea1ed2936 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -297,7 +297,7 @@ def _unify_binary_operation_in_assignment(assignment: Assignment): if not assignment.value.operation == OperationType.plus: assignment.substitute( assignment.value, - BinaryOperation(OperationType.plus, [assignment.value.left, UnaryOperation(OperationType.negate, assignment.value.right)]), + BinaryOperation(OperationType.plus, [assignment.value.left, UnaryOperation(OperationType.negate, [assignment.value.right])]), ) if any(isinstance(operand, Constant) for operand in assignment.value.left.subexpressions()): From 9dc780e83f6991f7593f4d3ac672712029d7a0a3 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 19 Dec 2023 13:25:08 +0100 Subject: [PATCH 30/31] Added check for correct continuation instruction --- .../controlflowanalysis/test_readability_based_refinement.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py index fe0e7239a..1bf2a57ee 100644 --- a/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py +++ b/tests/pipeline/controlflowanalysis/test_readability_based_refinement.py @@ -898,7 +898,10 @@ def test_for_loop_recovery_if_continue_in_while_1(self): ) WhileLoopReplacer(ast, _generate_options()).run() - assert all(isinstance(loop_node, ForLoopNode) for loop_node in list(ast.get_loop_nodes_post_order())) + + for loop_node in list(ast.get_loop_nodes_post_order()): + assert isinstance(loop_node, ForLoopNode) + assert loop_node.modification == Assignment(Variable("a"), BinaryOperation(OperationType.plus, [Variable("a"), Constant(1)])) condition_nodes = list(ast.get_condition_nodes_post_order()) last_definition = condition_nodes[0].true_branch_child.instructions[ From 9515867963e2603d3ae608ffff6dd5e7b9cddb72 Mon Sep 17 00:00:00 2001 From: fnhartmann Date: Tue, 19 Dec 2023 13:53:42 +0100 Subject: [PATCH 31/31] Merged substitutes --- .../pipeline/controlflowanalysis/loop_utility_methods.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py index ea1ed2936..99de6adfd 100644 --- a/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py +++ b/decompiler/pipeline/controlflowanalysis/loop_utility_methods.py @@ -312,8 +312,8 @@ def _substract_continuation_from_last_definition(last_definition: Assignment, co :param last_definition: Last definition that is to be changed :param continuation: Instruction defining the for-loops modification """ - last_definition.substitute( - last_definition.value, BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right]) - ) + substracted_binary_operation = BinaryOperation(OperationType.minus, [last_definition.value, continuation.instruction.value.right]) if _is_negated_constant_variable(continuation.instruction.value.left, Variable): - last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [last_definition.value])) + last_definition.substitute(last_definition.value, UnaryOperation(OperationType.negate, [substracted_binary_operation])) + else: + last_definition.substitute(last_definition.value, substracted_binary_operation)