Merge pull request #7321 from hvitved/csharp/cil/unique-type

C#: Avoid CIL instructions with multiple types

Author: hvitved

csharp/ql/lib/semmle/code/cil/CallableReturns.qll

@@ -25,7 +25,7 @@ private module Cached {
   cached
   predicate alwaysThrowsException(Method m, Type t) {
     alwaysThrowsMethod(m) and
-    forex(Throw ex | ex = m.getImplementation().getAnInstruction() | t = ex.getExpr().getType())
+    forex(Throw ex | ex = m.getImplementation().getAnInstruction() | t = ex.getExceptionType())
   }
 }
 

csharp/ql/lib/semmle/code/cil/ConsistencyChecks.qll

@@ -612,7 +612,7 @@ class ExprMissingType extends InstructionViolation {
     not instruction instanceof Opcodes::Ldvirtftn and
     not instruction instanceof Opcodes::Arglist and
     not instruction instanceof Opcodes::Refanytype and
-    instruction.getPushCount() = 1 and
+    instruction.getPushCount() >= 1 and
     count(instruction.getType()) != 1
   }
 

csharp/ql/lib/semmle/code/cil/ControlFlow.qll

@@ -56,6 +56,32 @@ class ControlFlowNode extends @cil_controlflow_node {
     )
   }
 
+  /**
+   * Gets the type of the `i`th operand. Unlike `getOperand(i).getType()`, this
+   * predicate takes into account when there are multiple possible operands with
+   * different types.
+   */
+  Type getOperandType(int i) {
+    strictcount(this.getOperand(i)) = 1 and
+    result = this.getOperand(i).getType()
+    or
+    strictcount(this.getOperand(i)) = 2 and
+    exists(ControlFlowNode op1, ControlFlowNode op2, Type t2 |
+      op1 = this.getOperand(i) and
+      op2 = this.getOperand(i) and
+      op1 != op2 and
+      result = op1.getType() and
+      t2 = op2.getType()
+    |
+      result = t2
+      or
+      result.(PrimitiveType).getUnderlyingType().getConversionIndex() >
+        t2.(PrimitiveType).getUnderlyingType().getConversionIndex()
+      or
+      op2 instanceof NullLiteral
+    )
+  }
+
   /** Gets an operand of this instruction, if any. */
   ControlFlowNode getAnOperand() { result = this.getOperand(_) }
 
@@ -102,7 +128,12 @@ class ControlFlowNode extends @cil_controlflow_node {
   /** Gets the method containing this control flow node. */
   MethodImplementation getImplementation() { none() }
 
-  /** Gets the type of the item pushed onto the stack, if any. */
+  /**
+   * Gets the type of the item pushed onto the stack, if any.
+   *
+   * If called via `ControlFlowNode::getOperand(i).getType()`, consider using
+   * `ControlFlowNode::getOperandType(i)` instead.
+   */
   cached
   Type getType() { none() }
 

csharp/ql/lib/semmle/code/cil/InstructionGroups.qll

@@ -73,8 +73,8 @@ class ComparisonOperation extends BinaryExpr, @cil_comparison_operation {
 class BinaryArithmeticExpr extends BinaryExpr, @cil_binary_arithmetic_operation {
   override Type getType() {
     exists(Type t0, Type t1 |
-      t0 = this.getOperand(0).getType().getUnderlyingType() and
-      t1 = this.getOperand(1).getType().getUnderlyingType()
+      t0 = this.getOperandType(0).getUnderlyingType() and
+      t1 = this.getOperandType(1).getUnderlyingType()
     |
       t0 = t1 and result = t0
       or
@@ -242,6 +242,9 @@ class Return extends Instruction, @cil_ret {
 class Throw extends Instruction, DotNet::Throw, @cil_throw_any {
   override Expr getExpr() { result = this.getOperand(0) }
 
+  /** Gets the type of the exception being thrown. */
+  Type getExceptionType() { result = this.getOperandType(0) }
+
   override predicate canFlowNext() { none() }
 }
 

csharp/ql/lib/semmle/code/cil/Instructions.qll

@@ -199,9 +199,9 @@ module Opcodes {
     override string getOpcodeName() { result = "neg" }
 
     override NumericType getType() {
-      result = this.getOperand().getType()
+      result = this.getOperandType(0)
       or
-      this.getOperand().getType() instanceof Enum and result instanceof IntType
+      this.getOperandType(0) instanceof Enum and result instanceof IntType
     }
   }
 
@@ -260,7 +260,7 @@ module Opcodes {
 
     override int getPushCount() { result = 2 } // This is the only instruction that pushes 2 items
 
-    override Type getType() { result = this.getOperand(0).getType() }
+    override Type getType() { result = this.getOperandType(0) }
   }
 
   /** A `ret` instruction. */
@@ -887,7 +887,7 @@ module Opcodes {
   class Ldelem_ref extends ReadArrayElement, @cil_ldelem_ref {
     override string getOpcodeName() { result = "ldelem.ref" }
 
-    override Type getType() { result = this.getArray().getType() }
+    override Type getType() { result = this.getOperandType(1) }
   }
 
   /** An `ldelema` instruction. */