From 7571944b9c071c335b9d86ab250a9660841df04b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:18:03 +0000 Subject: [PATCH 01/12] Create draft PR for #374 From 2403e9ce1a417db0b9817c0236e9b103421307ac Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 13 Dec 2023 16:33:58 +0100 Subject: [PATCH 02/12] Improve else-if chaining --- decompiler/backend/codevisitor.py | 16 ++++++-- tests/backend/test_codegenerator.py | 62 +++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index 66058b239..1cded02f3 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -71,9 +71,19 @@ def visit_condition_node(self, node: ast_nodes.ConditionNode) -> str: if node.false_branch is None: return f"if ({self._condition_string(node.condition)}) {{{true_str}}}" false_str = self.visit(node.false_branch_child) - if isinstance(node.false_branch_child, ast_nodes.ConditionNode): - return f"if ({self._condition_string(node.condition)}){{{true_str}}} else {false_str}" - return f"if ({self._condition_string(node.condition)}){{{true_str}}} else{{{false_str}}}" + if isinstance(node.true_branch_child, ast_nodes.ConditionNode) or isinstance(node.false_branch_child, ast_nodes.ConditionNode): + negate_condition = isinstance(node.true_branch_child, ast_nodes.ConditionNode) and ( + not isinstance(node.false_branch_child, ast_nodes.ConditionNode) or len(false_str) > len(true_str) + ) + + condition = node.condition + if negate_condition: + true_str, false_str = false_str, true_str + condition = ~condition + + return f"if ({self._condition_string(condition)}) {{{true_str}}} else {false_str}" + else: + return f"if ({self._condition_string(node.condition)}) {{{true_str}}} else {{{false_str}}}" def visit_true_node(self, node: ast_nodes.TrueNode) -> str: """Generate code for the given TrueNode by evaluating its child (Wrapper).""" diff --git a/tests/backend/test_codegenerator.py b/tests/backend/test_codegenerator.py index 234c049a6..180d2a070 100644 --- a/tests/backend/test_codegenerator.py +++ b/tests/backend/test_codegenerator.py @@ -256,6 +256,68 @@ def test_function_with_ifelse(self): self._task(ast, params=[var_a.copy(), var_b.copy()]), ) + def test_function_with_ifelseif(self): + context = LogicCondition.generate_new_context() + root = SeqNode(LogicCondition.initialize_true(context)) + ast = AbstractSyntaxTree(root, { + x1_symbol(context): Condition(OperationType.less, [var_a, const_3]), + x2_symbol(context): Condition(OperationType.less, [var_a, const_5]) + }) + + x2_true_node = ast._add_code_node([instructions.Return([const_1])]) + x2_false_node = ast._add_code_node([instructions.Return([const_2])]) + x1_true_node = ast._add_code_node([instructions.Return([const_0])]) + x1_false_node = ast._add_condition_node_with( + condition=x2_symbol(ast.factory.logic_context), + true_branch=x2_true_node, + false_branch=x2_false_node + ) + condition_node = ast._add_condition_node_with( + condition=x1_symbol(ast.factory.logic_context), + true_branch=x1_true_node, + false_branch=x1_false_node + ) + + ast._add_edges_from([(root, condition_node)]) + + assert self._regex_matches( + r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%<%3%\)%{%return%0%;%}%else +if%\(%a%<%5%\)%{%return%1%;%}%else%{%return%2%;%}%}%$".replace( + "%", "\\s*" + ), + self._task(ast, params=[var_a.copy(), var_b.copy()]), + ) + + def test_function_with_ifelseif_swapped(self): + context = LogicCondition.generate_new_context() + root = SeqNode(LogicCondition.initialize_true(context)) + ast = AbstractSyntaxTree(root, { + x1_symbol(context): Condition(OperationType.greater_or_equal, [var_a, const_3]), + x2_symbol(context): Condition(OperationType.less, [var_a, const_5]) + }) + + x2_true_node = ast._add_code_node([instructions.Return([const_1])]) + x2_false_node = ast._add_code_node([instructions.Return([const_2])]) + x1_true_node = ast._add_condition_node_with( + condition=x2_symbol(ast.factory.logic_context), + true_branch=x2_true_node, + false_branch=x2_false_node + ) + x1_false_node = ast._add_code_node([instructions.Return([const_0])]) + condition_node = ast._add_condition_node_with( + condition=x1_symbol(ast.factory.logic_context), + true_branch=x1_true_node, + false_branch=x1_false_node + ) + + ast._add_edges_from([(root, condition_node)]) + + assert self._regex_matches( + r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%<%3%\)%{%return%0%;%}%else +if%\(%a%<%5%\)%{%return%1%;%}%else%{%return%2%;%}%}%$".replace( + "%", "\\s*" + ), + self._task(ast, params=[var_a.copy(), var_b.copy()]), + ) + def test_function_with_switch(self): root_switch_node = SwitchNode( expression=var_a.copy(), reaching_condition=LogicCondition.initialize_true(LogicCondition.generate_new_context()) From fa50ae5a8a127cc3fada69e80993748c9c8045a6 Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 13 Dec 2023 16:38:02 +0100 Subject: [PATCH 03/12] black --- decompiler/backend/codevisitor.py | 2 +- tests/backend/test_codegenerator.py | 38 ++++++++++++++--------------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index 1cded02f3..7da89a5cc 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -73,7 +73,7 @@ def visit_condition_node(self, node: ast_nodes.ConditionNode) -> str: false_str = self.visit(node.false_branch_child) if isinstance(node.true_branch_child, ast_nodes.ConditionNode) or isinstance(node.false_branch_child, ast_nodes.ConditionNode): negate_condition = isinstance(node.true_branch_child, ast_nodes.ConditionNode) and ( - not isinstance(node.false_branch_child, ast_nodes.ConditionNode) or len(false_str) > len(true_str) + not isinstance(node.false_branch_child, ast_nodes.ConditionNode) or len(false_str) > len(true_str) ) condition = node.condition diff --git a/tests/backend/test_codegenerator.py b/tests/backend/test_codegenerator.py index 180d2a070..1ddfa07ff 100644 --- a/tests/backend/test_codegenerator.py +++ b/tests/backend/test_codegenerator.py @@ -259,23 +259,22 @@ def test_function_with_ifelse(self): def test_function_with_ifelseif(self): context = LogicCondition.generate_new_context() root = SeqNode(LogicCondition.initialize_true(context)) - ast = AbstractSyntaxTree(root, { - x1_symbol(context): Condition(OperationType.less, [var_a, const_3]), - x2_symbol(context): Condition(OperationType.less, [var_a, const_5]) - }) + ast = AbstractSyntaxTree( + root, + { + x1_symbol(context): Condition(OperationType.less, [var_a, const_3]), + x2_symbol(context): Condition(OperationType.less, [var_a, const_5]), + }, + ) x2_true_node = ast._add_code_node([instructions.Return([const_1])]) x2_false_node = ast._add_code_node([instructions.Return([const_2])]) x1_true_node = ast._add_code_node([instructions.Return([const_0])]) x1_false_node = ast._add_condition_node_with( - condition=x2_symbol(ast.factory.logic_context), - true_branch=x2_true_node, - false_branch=x2_false_node + condition=x2_symbol(ast.factory.logic_context), true_branch=x2_true_node, false_branch=x2_false_node ) condition_node = ast._add_condition_node_with( - condition=x1_symbol(ast.factory.logic_context), - true_branch=x1_true_node, - false_branch=x1_false_node + condition=x1_symbol(ast.factory.logic_context), true_branch=x1_true_node, false_branch=x1_false_node ) ast._add_edges_from([(root, condition_node)]) @@ -290,23 +289,22 @@ def test_function_with_ifelseif(self): def test_function_with_ifelseif_swapped(self): context = LogicCondition.generate_new_context() root = SeqNode(LogicCondition.initialize_true(context)) - ast = AbstractSyntaxTree(root, { - x1_symbol(context): Condition(OperationType.greater_or_equal, [var_a, const_3]), - x2_symbol(context): Condition(OperationType.less, [var_a, const_5]) - }) + ast = AbstractSyntaxTree( + root, + { + x1_symbol(context): Condition(OperationType.greater_or_equal, [var_a, const_3]), + x2_symbol(context): Condition(OperationType.less, [var_a, const_5]), + }, + ) x2_true_node = ast._add_code_node([instructions.Return([const_1])]) x2_false_node = ast._add_code_node([instructions.Return([const_2])]) x1_true_node = ast._add_condition_node_with( - condition=x2_symbol(ast.factory.logic_context), - true_branch=x2_true_node, - false_branch=x2_false_node + condition=x2_symbol(ast.factory.logic_context), true_branch=x2_true_node, false_branch=x2_false_node ) x1_false_node = ast._add_code_node([instructions.Return([const_0])]) condition_node = ast._add_condition_node_with( - condition=x1_symbol(ast.factory.logic_context), - true_branch=x1_true_node, - false_branch=x1_false_node + condition=x1_symbol(ast.factory.logic_context), true_branch=x1_true_node, false_branch=x1_false_node ) ast._add_edges_from([(root, condition_node)]) From 50e0bbdffd96bb6aa4a5abef3fbf2a0a4e5430ce Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 10 Jan 2024 15:17:09 +0100 Subject: [PATCH 04/12] Remove check to swap if else branch based on length --- decompiler/backend/codevisitor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index 7da89a5cc..cf3bb18da 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -72,8 +72,8 @@ def visit_condition_node(self, node: ast_nodes.ConditionNode) -> str: return f"if ({self._condition_string(node.condition)}) {{{true_str}}}" false_str = self.visit(node.false_branch_child) if isinstance(node.true_branch_child, ast_nodes.ConditionNode) or isinstance(node.false_branch_child, ast_nodes.ConditionNode): - negate_condition = isinstance(node.true_branch_child, ast_nodes.ConditionNode) and ( - not isinstance(node.false_branch_child, ast_nodes.ConditionNode) or len(false_str) > len(true_str) + negate_condition = isinstance(node.true_branch_child, ast_nodes.ConditionNode) and not isinstance( + node.false_branch_child, ast_nodes.ConditionNode ) condition = node.condition From d1ce3bf28d18fa34d3603593dd73d8dde0e10fe5 Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 10 Jan 2024 15:59:05 +0100 Subject: [PATCH 05/12] Implement configurable swapping of if-else branches based on size --- decompiler/backend/codevisitor.py | 31 +++++++++++++++++++++++-------- decompiler/util/default.json | 20 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index cf3bb18da..0fd7ec1e8 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -39,6 +39,7 @@ def __init__(self, task: DecompilerTask): self._int_repr_scope: int = task.options.getint("code-generator.int_representation_scope", fallback=256) self._neg_hex_as_twos_complement: bool = task.options.getboolean("code-generator.negative_hex_as_twos_complement", fallback=True) self._aggressive_array_detection: bool = task.options.getboolean("code-generator.aggressive_array_detection", fallback=False) + self._preferred_true_branch: str = task.options.getstring("code-generator.preferred_true_branch", fallback="none") self.task = task def visit_seq_node(self, node: ast_nodes.SeqNode) -> str: @@ -70,16 +71,30 @@ def visit_condition_node(self, node: ast_nodes.ConditionNode) -> str: true_str = self.visit(node.true_branch_child) if node.false_branch is None: return f"if ({self._condition_string(node.condition)}) {{{true_str}}}" + false_str = self.visit(node.false_branch_child) - if isinstance(node.true_branch_child, ast_nodes.ConditionNode) or isinstance(node.false_branch_child, ast_nodes.ConditionNode): - negate_condition = isinstance(node.true_branch_child, ast_nodes.ConditionNode) and not isinstance( - node.false_branch_child, ast_nodes.ConditionNode - ) - condition = node.condition - if negate_condition: - true_str, false_str = false_str, true_str - condition = ~condition + condition = node.condition + true_child = node.true_branch_child + false_child = node.false_branch_child + + def swap_branches(): + nonlocal condition, true_child, false_child, true_str, false_str + condition = ~condition + true_child, false_child = false_child, true_child + true_str, false_str = false_str, true_str + + length_comparisons = { + "none": False, + "smallest": len(true_str) > len(false_str), + "largest": len(true_str) < len(false_str) + } + if length_comparisons[self._preferred_true_branch]: + swap_branches() + + if isinstance(true_child, ast_nodes.ConditionNode) or isinstance(false_child, ast_nodes.ConditionNode): + if not isinstance(false_child, ast_nodes.ConditionNode): + swap_branches() return f"if ({self._condition_string(condition)}) {{{true_str}}} else {false_str}" else: diff --git a/decompiler/util/default.json b/decompiler/util/default.json index 25f5be1c9..c266a8246 100644 --- a/decompiler/util/default.json +++ b/decompiler/util/default.json @@ -418,6 +418,26 @@ "is_hidden_from_cli": false, "argument_name": "--variable-declarations-per-line" }, + { + "dest": "code-generator.preferred_true_branch", + "default": "smallest", + "title": "Preferred type of true branch in if-else", + "type": "string", + "enum": [ + "smallest", + "largest", + "none" + ], + "enumDescriptions": [ + "smallest", + "largest", + "none" + ], + "description": "Swap branches of if-else structures based on the given criteria", + "is_hidden_from_gui": false, + "is_hidden_from_cli": false, + "argument_name": "--preferred_true_branch" + }, { "dest": "pattern-independent-restructuring.switch_reconstruction", "default": true, From aaaf9653e64a1a9675e9474f8e46fbe393d450ac Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:02:03 +0100 Subject: [PATCH 06/12] black --- decompiler/backend/codevisitor.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index 0fd7ec1e8..14985b2c7 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -84,11 +84,7 @@ def swap_branches(): true_child, false_child = false_child, true_child true_str, false_str = false_str, true_str - length_comparisons = { - "none": False, - "smallest": len(true_str) > len(false_str), - "largest": len(true_str) < len(false_str) - } + length_comparisons = {"none": False, "smallest": len(true_str) > len(false_str), "largest": len(true_str) < len(false_str)} if length_comparisons[self._preferred_true_branch]: swap_branches() From 5e2dfad4d6ba615101a6346ef156e6f7c9956997 Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 17 Jan 2024 11:50:20 +0100 Subject: [PATCH 07/12] Improve documentation --- decompiler/util/default.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/decompiler/util/default.json b/decompiler/util/default.json index c266a8246..5ef9251af 100644 --- a/decompiler/util/default.json +++ b/decompiler/util/default.json @@ -429,9 +429,9 @@ "none" ], "enumDescriptions": [ - "smallest", - "largest", - "none" + "Swap branches, so that the true branch is the larger one length wise.", + "Swap branches, so that the true branch is the smaller one length wise.", + "Don't swap branches based on length." ], "description": "Swap branches of if-else structures based on the given criteria", "is_hidden_from_gui": false, From d0d603aa45126cd879cb471f47ecef1555da28f6 Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 17 Jan 2024 12:25:36 +0100 Subject: [PATCH 08/12] Update tests --- tests/backend/test_codegenerator.py | 66 ++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/tests/backend/test_codegenerator.py b/tests/backend/test_codegenerator.py index 1ddfa07ff..8d2445350 100644 --- a/tests/backend/test_codegenerator.py +++ b/tests/backend/test_codegenerator.py @@ -1,5 +1,5 @@ import re -from typing import Dict, List +from typing import Dict, List, Optional import decompiler.structures.pseudo.instructions as instructions import decompiler.structures.pseudo.operations as operations @@ -99,6 +99,7 @@ def _generate_options( twos_complement: bool = True, array_detection: bool = False, var_declarations_per_line: int = 1, + preferred_true_branch: str = "smallest", ): options = Options() options.set("code-generator.max_complexity", max_complx) @@ -111,15 +112,17 @@ def _generate_options( options.set("code-generator.negative_hex_as_twos_complement", twos_complement) options.set("code-generator.aggressive_array_detection", array_detection) options.set("code-generator.variable_declarations_per_line", var_declarations_per_line) + options.set("code-generator.preferred_true_branch", preferred_true_branch) return options class TestCodeGeneration: @staticmethod - def _task(ast: AbstractSyntaxTree, params: List[DataflowObject] = None, return_type: Type = int32): + def _task(ast: AbstractSyntaxTree, params: List[DataflowObject] = None, return_type: Type = int32, options: Optional[Options] = None): if not params: params = [] - options = _generate_options(max_complx=100, compounding=False) + if not options: + options = _generate_options(compounding=False) return DecompilerTask("test_function", None, ast=ast, options=options, function_parameters=params, function_return_type=return_type) @staticmethod @@ -286,7 +289,37 @@ def test_function_with_ifelseif(self): self._task(ast, params=[var_a.copy(), var_b.copy()]), ) - def test_function_with_ifelseif_swapped(self): + def test_function_with_ifelseif_prioritize_elseif_over_length(self): + context = LogicCondition.generate_new_context() + root = SeqNode(LogicCondition.initialize_true(context)) + ast = AbstractSyntaxTree( + root, + { + x1_symbol(context): Condition(OperationType.less, [var_a, const_3]), + x2_symbol(context): Condition(OperationType.less, [var_a, const_5]), + }, + ) + + x2_true_node = ast._add_code_node([instructions.Return([const_1])]) + x2_false_node = ast._add_code_node([instructions.Return([const_2])]) + x1_true_node = ast._add_code_node([instructions.Return([const_0])]) + x1_false_node = ast._add_condition_node_with( + condition=x2_symbol(ast.factory.logic_context), true_branch=x2_true_node, false_branch=x2_false_node + ) + condition_node = ast._add_condition_node_with( + condition=x1_symbol(ast.factory.logic_context), true_branch=x1_true_node, false_branch=x1_false_node + ) + + ast._add_edges_from([(root, condition_node)]) + + assert self._regex_matches( + r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%<%3%\)%{%return%0%;%}%else +if%\(%a%<%5%\)%{%return%1%;%}%else%{%return%2%;%}%}%$".replace( + "%", "\\s*" + ), + self._task(ast, params=[var_a.copy(), var_b.copy()], options=_generate_options(preferred_true_branch="largest")), + ) + + def test_function_with_ifelseif_swapped_because_elseif(self): context = LogicCondition.generate_new_context() root = SeqNode(LogicCondition.initialize_true(context)) ast = AbstractSyntaxTree( @@ -302,6 +335,29 @@ def test_function_with_ifelseif_swapped(self): x1_true_node = ast._add_condition_node_with( condition=x2_symbol(ast.factory.logic_context), true_branch=x2_true_node, false_branch=x2_false_node ) + x1_false_node = ast._add_code_node([instructions.Comment("Long comment to pad branch length..."), instructions.Return([const_0])]) + condition_node = ast._add_condition_node_with( + condition=x1_symbol(ast.factory.logic_context), true_branch=x1_true_node, false_branch=x1_false_node + ) + + ast._add_edges_from([(root, condition_node)]) + + assert self._regex_matches( + r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%<%3%\)%{%\/\*%Long comment to pad branch length...%\*\/%return%0%;%}%else +if%\(%a%<%5%\)%{%return%1%;%}%else%{%return%2%;%}%}%$".replace( + "%", "\\s*" + ), + self._task(ast, params=[var_a.copy(), var_b.copy()]), + ) + + def test_function_with_ifelseif_swapped_because_length(self): + context = LogicCondition.generate_new_context() + root = SeqNode(LogicCondition.initialize_true(context)) + ast = AbstractSyntaxTree( + root, + {x1_symbol(context): Condition(OperationType.greater_or_equal, [var_a, const_3])}, + ) + + x1_true_node = ast._add_code_node([instructions.Comment("Long comment to pad branch length..."), instructions.Return([const_1])]) x1_false_node = ast._add_code_node([instructions.Return([const_0])]) condition_node = ast._add_condition_node_with( condition=x1_symbol(ast.factory.logic_context), true_branch=x1_true_node, false_branch=x1_false_node @@ -310,7 +366,7 @@ def test_function_with_ifelseif_swapped(self): ast._add_edges_from([(root, condition_node)]) assert self._regex_matches( - r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%<%3%\)%{%return%0%;%}%else +if%\(%a%<%5%\)%{%return%1%;%}%else%{%return%2%;%}%}%$".replace( + r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%>=%3%\)%{%return%0%;%}%else%{%\/\*%Long comment to pad branch length...%\*\/%return%1%;%}%}%$".replace( "%", "\\s*" ), self._task(ast, params=[var_a.copy(), var_b.copy()]), From 514332536b160f9ea1b9aab983e707839c69ffc4 Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 17 Jan 2024 12:31:35 +0100 Subject: [PATCH 09/12] Fix bug --- decompiler/backend/codevisitor.py | 2 +- tests/backend/test_codegenerator.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index 14985b2c7..572cdd521 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -94,7 +94,7 @@ def swap_branches(): return f"if ({self._condition_string(condition)}) {{{true_str}}} else {false_str}" else: - return f"if ({self._condition_string(node.condition)}) {{{true_str}}} else {{{false_str}}}" + return f"if ({self._condition_string(condition)}) {{{true_str}}} else {{{false_str}}}" def visit_true_node(self, node: ast_nodes.TrueNode) -> str: """Generate code for the given TrueNode by evaluating its child (Wrapper).""" diff --git a/tests/backend/test_codegenerator.py b/tests/backend/test_codegenerator.py index 8d2445350..91403f60d 100644 --- a/tests/backend/test_codegenerator.py +++ b/tests/backend/test_codegenerator.py @@ -256,7 +256,7 @@ def test_function_with_ifelse(self): r"^%int +test_function\(%int +a%,%int +b%\)%{%int%c;%if%\(%c%<%5%\)%{%c%=%5%;%return%c%;%}%else%{%return%0%;%}%}%$".replace( "%", "\\s*" ), - self._task(ast, params=[var_a.copy(), var_b.copy()]), + self._task(ast, params=[var_a.copy(), var_b.copy()], options=_generate_options(preferred_true_branch="none")), ) def test_function_with_ifelseif(self): @@ -366,7 +366,7 @@ def test_function_with_ifelseif_swapped_because_length(self): ast._add_edges_from([(root, condition_node)]) assert self._regex_matches( - r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%>=%3%\)%{%return%0%;%}%else%{%\/\*%Long comment to pad branch length...%\*\/%return%1%;%}%}%$".replace( + r"^%int +test_function\(%int +a%,%int +b%\)%{%if%\(%a%<%3%\)%{%return%0%;%}%else%{%\/\*%Long comment to pad branch length...%\*\/%return%1%;%}%}%$".replace( "%", "\\s*" ), self._task(ast, params=[var_a.copy(), var_b.copy()]), From 5ae3cb5295ceb293534e4ad11dc803a7ab173b37 Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 17 Jan 2024 12:54:04 +0100 Subject: [PATCH 10/12] Rework swapping code --- decompiler/backend/codevisitor.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index 572cdd521..64537d2cc 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -78,20 +78,23 @@ def visit_condition_node(self, node: ast_nodes.ConditionNode) -> str: true_child = node.true_branch_child false_child = node.false_branch_child - def swap_branches(): - nonlocal condition, true_child, false_child, true_str, false_str - condition = ~condition - true_child, false_child = false_child, true_child - true_str, false_str = false_str, true_str + swap_branches = None + + # if only one branch is a condition node we want to swap + if isinstance(false_child, ast_nodes.ConditionNode) != isinstance(true_child, ast_nodes.ConditionNode): + swap_branches = swap_branches or not isinstance(false_child, ast_nodes.ConditionNode) - length_comparisons = {"none": False, "smallest": len(true_str) > len(false_str), "largest": len(true_str) < len(false_str)} - if length_comparisons[self._preferred_true_branch]: - swap_branches() + length_comparisons = {"none": None, "smallest": len(true_str) > len(false_str), "largest": len(true_str) < len(false_str)} + # if we haven't already decided on swapping (swap_branches is None), decide by length + if swap_branches is None: + swap_branches = length_comparisons[self._preferred_true_branch] - if isinstance(true_child, ast_nodes.ConditionNode) or isinstance(false_child, ast_nodes.ConditionNode): - if not isinstance(false_child, ast_nodes.ConditionNode): - swap_branches() + if swap_branches: + condition = ~condition + true_str, false_str = false_str, true_str + true_child, false_child = false_child, true_child + if isinstance(false_child, ast_nodes.ConditionNode): return f"if ({self._condition_string(condition)}) {{{true_str}}} else {false_str}" else: return f"if ({self._condition_string(condition)}) {{{true_str}}} else {{{false_str}}}" From f2b409250e1f12b08cabfcbc626858e750262a75 Mon Sep 17 00:00:00 2001 From: rihi <19492038+rihi@users.noreply.github.com> Date: Thu, 18 Jan 2024 11:18:03 +0100 Subject: [PATCH 11/12] Slight improvements to code --- decompiler/backend/codevisitor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/decompiler/backend/codevisitor.py b/decompiler/backend/codevisitor.py index 64537d2cc..ad064012b 100644 --- a/decompiler/backend/codevisitor.py +++ b/decompiler/backend/codevisitor.py @@ -80,13 +80,13 @@ def visit_condition_node(self, node: ast_nodes.ConditionNode) -> str: swap_branches = None - # if only one branch is a condition node we want to swap + # if only one branch is a condition node, we want to decide swapping by which branch is a condition node if isinstance(false_child, ast_nodes.ConditionNode) != isinstance(true_child, ast_nodes.ConditionNode): - swap_branches = swap_branches or not isinstance(false_child, ast_nodes.ConditionNode) + swap_branches = not isinstance(false_child, ast_nodes.ConditionNode) - length_comparisons = {"none": None, "smallest": len(true_str) > len(false_str), "largest": len(true_str) < len(false_str)} # if we haven't already decided on swapping (swap_branches is None), decide by length if swap_branches is None: + length_comparisons = {"none": None, "smallest": len(true_str) > len(false_str), "largest": len(true_str) < len(false_str)} swap_branches = length_comparisons[self._preferred_true_branch] if swap_branches: From 320271073f16fa328b90e0659d46a465a837f20d Mon Sep 17 00:00:00 2001 From: Rihi <19492038+rihi@users.noreply.github.com> Date: Wed, 24 Jan 2024 14:18:16 +0100 Subject: [PATCH 12/12] Fix swapped documentation Co-authored-by: ebehner --- decompiler/util/default.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decompiler/util/default.json b/decompiler/util/default.json index 5ef9251af..e2eedadac 100644 --- a/decompiler/util/default.json +++ b/decompiler/util/default.json @@ -429,8 +429,8 @@ "none" ], "enumDescriptions": [ - "Swap branches, so that the true branch is the larger one length wise.", "Swap branches, so that the true branch is the smaller one length wise.", + "Swap branches, so that the true branch is the larger one length wise.", "Don't swap branches based on length." ], "description": "Swap branches of if-else structures based on the given criteria",