lib/ir/project: propagate control flow for call returns

This patch does two things:

1. It allows the re-targeting of jumps for which no known true condition
   is available. Without a known condition, only blocks that consist of
   a single, unconditional jump can be skipped.
2. It allows the re-targeting of call returns in the same way that we
   already do it for unconditional jumps. For calls we never have a
   known condition as side-effects may invalidate any knowledge we have
   after the execution of all DEFs in the block.

Example:

Before the optimization we might have code like this:

  BLK [blk_0040a9c4]
    DEF [instr_0040a9c4_0] ra:4 = 0x40a9cc:4
    JMP [instr_0040a9c4_1] call sub_00403f80 ret blk_0040a9cc
  BLK [blk_0040a9cc]
    JMP [instr_0040a9cc_1] Jump to blk_0040a9d0
  BLK [blk_0040a9d0]
    DEF [instr_0040a9d0_0] a0:4 = ((0x43:4 << 0x10:4) + 0xffffb730:4)
    JMP [instr_0040a9d0_1] Jump to blk_0040a9d4

whereas after the optimization it becomes:

  BLK [blk_0040a9c4]
    DEF [instr_0040a9c4_0] ra:4 = 0x40a9cc:4
    JMP [instr_0040a9c4_1] call sub_00403f80 ret blk_0040a9d0
  BLK [blk_0040a9d0]
    DEF [instr_0040a9d0_0] a0:4 = ((0x43:4 << 0x10:4) + 0xffffb730:4)
    JMP [instr_0040a9d0_1] Jump to blk_0040a9d4

Fixes: 2487aac ("remove dead code originating from control flow propagation (fkie-cad#384)")
Closes: fkie-cad#461
Reported-by: https://github.com/ElDavoo
Signed-off-by: Valentin Obst <[email protected]>

src/cwe_checker_lib/src/intermediate_representation/project/propagate_control_flow.rs

@@ -27,15 +27,50 @@ pub fn propagate_control_flow(project: &mut Project) {
             // Check whether we can propagate the control flow for outgoing jumps
             match &block.term.jmps[..] {
                 [Term {
-                    term: Jmp::Branch(target),
+                    tid: call_tid,
+                    term:
+                        Jmp::Call {
+                            target: _,
+                            return_: Some(return_target),
+                        },
+                }]
+                | [Term {
+                    tid: call_tid,
+                    term:
+                        Jmp::CallInd {
+                            target: _,
+                            return_: Some(return_target),
+                        },
+                }]
+                | [Term {
+                    tid: call_tid,
+                    term:
+                        Jmp::CallOther {
+                            description: _,
+                            return_: Some(return_target),
+                        },
+                }] => {
+                    if let Some(new_target) = find_target_for_retargetable_jump(
+                        return_target,
+                        &sub.term,
+                        // Call may have side-effects that invalidate our
+                        // knowledge about any condition we know to be true
+                        // after execution of all DEFs in a block.
+                        None,
+                    ) {
+                        jmps_to_retarget.insert(call_tid.clone(), new_target);
+                    }
+                }
+                [Term {
                     tid: jump_tid,
+                    term: Jmp::Branch(target),
                 }] => {
-                    if let Some(true_condition) = &known_conditional_result {
-                        if let Some(new_target) =
-                            find_target_for_retargetable_jump(target, &sub.term, true_condition)
-                        {
-                            jmps_to_retarget.insert(jump_tid.clone(), new_target);
-                        }
+                    if let Some(new_target) = find_target_for_retargetable_jump(
+                        target,
+                        &sub.term,
+                        known_conditional_result.as_ref(),
+                    ) {
+                        jmps_to_retarget.insert(jump_tid.clone(), new_target);
                     }
                 }
                 [Term {
@@ -50,14 +85,14 @@ pub fn propagate_control_flow(project: &mut Project) {
                     tid: jump_tid_else,
                 }] => {
                     if let Some(new_target) =
-                        find_target_for_retargetable_jump(if_target, &sub.term, condition)
+                        find_target_for_retargetable_jump(if_target, &sub.term, Some(condition))
                     {
                         jmps_to_retarget.insert(jump_tid_if.clone(), new_target);
                     }
                     if let Some(new_target) = find_target_for_retargetable_jump(
                         else_target,
                         &sub.term,
-                        &negate_condition(condition.clone()),
+                        Some(&negate_condition(condition.clone())),
                     ) {
                         jmps_to_retarget.insert(jump_tid_else.clone(), new_target);
                     }
@@ -85,7 +120,20 @@ fn retarget_jumps(project: &mut Project, mut jmps_to_retarget: HashMap<Tid, Tid>
             for jmp in blk.term.jmps.iter_mut() {
                 if let Some(new_target) = jmps_to_retarget.remove(&jmp.tid) {
                     match &mut jmp.term {
-                        Jmp::Branch(target) | Jmp::CBranch { target, .. } => *target = new_target,
+                        Jmp::Branch(target)
+                        | Jmp::CBranch { target, .. }
+                        | Jmp::Call {
+                            target: _,
+                            return_: Some(target),
+                        }
+                        | Jmp::CallInd {
+                            target: _,
+                            return_: Some(target),
+                        }
+                        | Jmp::CallOther {
+                            description: _,
+                            return_: Some(target),
+                        } => *target = new_target,
                         _ => panic!("Unexpected type of jump encountered."),
                     }
                 }
@@ -101,7 +149,7 @@ fn retarget_jumps(project: &mut Project, mut jmps_to_retarget: HashMap<Tid, Tid>
 fn find_target_for_retargetable_jump(
     target: &Tid,
     sub: &Sub,
-    true_condition: &Expression,
+    true_condition: Option<&Expression>,
 ) -> Option<Tid> {
     let mut visited_tids = BTreeSet::from([target.clone()]);
     let mut new_target = target;
@@ -129,27 +177,33 @@ fn find_target_for_retargetable_jump(
 /// If it can be predicted, return the target of the jump.
 fn check_for_retargetable_block<'a>(
     block: &'a Term<Blk>,
-    true_condition: &Expression,
+    true_condition: Option<&Expression>,
 ) -> Option<&'a Tid> {
     if !block.term.defs.is_empty() {
         return None;
     }
-    match &block.term.jmps[..] {
-        [Term {
-            term: Jmp::Branch(target),
-            ..
-        }] => Some(target),
-        [Term {
-            term:
-                Jmp::CBranch {
-                    target: if_target,
-                    condition,
-                },
-            ..
-        }, Term {
-            term: Jmp::Branch(else_target),
-            ..
-        }] => {
+    match (&block.term.jmps[..], true_condition) {
+        (
+            [Term {
+                term: Jmp::Branch(target),
+                ..
+            }],
+            _,
+        ) => Some(target),
+        (
+            [Term {
+                term:
+                    Jmp::CBranch {
+                        target: if_target,
+                        condition,
+                    },
+                ..
+            }, Term {
+                term: Jmp::Branch(else_target),
+                ..
+            }],
+            Some(true_condition),
+        ) => {
             if condition == true_condition {
                 Some(if_target)
             } else if *condition == negate_condition(true_condition.clone()) {