Binary: Make the instruction transformation framework skip dead instructions.

Author: MathiasVP

binary/ql/lib/semmle/code/binary/ast/ir/internal/Instruction2/Instruction2.qll

@@ -152,7 +152,7 @@ module InstructionInput implements Transform<Instruction1>::TransformInputSig {
   }
 
   private predicate isStackPointerVariable(Instruction1::Variable v) {
-    v.toString() = "rsp" // TODO: Something else here
+    v instanceof Instruction1::StackPointer
   }
 
   /** Holds if `def2 = def1 + k`. */
@@ -242,6 +242,12 @@ module InstructionInput implements Transform<Instruction1>::TransformInputSig {
     exists(TTranslatedLoad(instr))
     or
     exists(TTranslatedStore(instr))
+    or
+    exists(Ssa::Definition def |
+      def.getInstruction() = instr and
+      def.getSourceVariable() instanceof Instruction1::TempVariable and
+      forex(Instruction1::Operand op | op = def.getARead() | isRemovedInstruction(op.getUse())) // TODO: Recursion through forex is bad for performance
+    )
   }
 
   abstract class TranslatedElement extends TTranslatedElement {

binary/ql/lib/semmle/code/binary/ast/ir/internal/InstructionSig.qll

@@ -54,6 +54,12 @@ signature module InstructionSig {
     Operand getAnAccess();
   }
 
+  class StackPointer extends Variable;
+
+  class FramePointer extends Variable;
+
+  class TempVariable extends Variable;
+
   class BasicBlock {
     ControlFlowNode getNode(int index);
 

binary/ql/lib/semmle/code/binary/ast/ir/internal/TransformInstruction/TransformInstruction.qll

@@ -156,6 +156,22 @@ module Transform<InstructionSig Input> {
       Operand getAnAccess() { result.getVariable() = this }
     }
 
+    class StackPointer extends Variable {
+      StackPointer() { this.asOldVariable() instanceof Input::StackPointer }
+    }
+
+    class FramePointer extends Variable {
+      FramePointer() { this.asOldVariable() instanceof Input::FramePointer }
+    }
+
+    class TempVariable extends Variable {
+      TempVariable() {
+        this.asOldVariable() instanceof Input::TempVariable
+        or
+        this.isNewVariable(_, _)
+      }
+    }
+
     final private class FinalTranslatedElement = TransformInput::TranslatedElement;
 
     private class TranslatedElement extends FinalTranslatedElement {
@@ -574,6 +590,22 @@ module Transform<InstructionSig Input> {
       result = any(TranslatedElement te).getInstructionSuccessor(old, succType)
     }
 
+    private Input::Instruction getASuccessorIfRemoved(Input::Instruction i) {
+      TransformInput::isRemovedInstruction(i) and
+      result = i.getASuccessor()
+    }
+
+    private Input::Instruction getSuccessorFromNonRemoved(Input::Instruction i, SuccessorType t) {
+      result = i.getSuccessor(t) and
+      not TransformInput::isRemovedInstruction(i)
+      or
+      result = getASuccessorIfRemoved(getSuccessorFromNonRemoved(i, t))
+    }
+
+    private Input::Instruction getNonRemovedSuccessor(Input::Instruction i, SuccessorType t) {
+      result = getSuccessorFromNonRemoved(i, t) and not TransformInput::isRemovedInstruction(result)
+    }
+
     private class OldInstruction extends TOldInstruction, Instruction {
       Input::Instruction old;
 
@@ -590,7 +622,7 @@ module Transform<InstructionSig Input> {
       override Instruction getSuccessor(SuccessorType succType) {
         exists(Input::Instruction oldSucc |
           not exists(getInstructionSuccessor(old, _)) and
-          oldSucc = old.getSuccessor(succType) and
+          oldSucc = getNonRemovedSuccessor(old, succType) and
           result = getNewInstruction(oldSucc)
         )
         or