Merge pull request github#2000 from AndreiDiaconu1/ircsharp-fixes

C# IR: Minor fixes and changes

Author: calumgrant

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -194,8 +194,9 @@ predicate needsLoad(Expr expr) {
  * Holds if we should ignore the `Load` instruction for `expr` when generating IR.
  */
 private predicate ignoreLoad(Expr expr) {
-  // No load needed for the qualifier
-  // in an array access
+  // No load needed for the qualifier of an array access,
+  // since we use the instruction `ElementsAddress`
+  // to get the address of the first element in an array
   expr = any(ArrayAccess aa).getQualifier()
   or
   // No load is needed for the lvalue in an assignment such as:
@@ -211,6 +212,9 @@ private predicate ignoreLoad(Expr expr) {
   // address is the final value of the expression
   expr.getParent() instanceof AddressOfExpr
   or
+  // A property access does not need a load since it is a call
+  expr instanceof PropertyAccess
+  or
   // If expr is a variable access used as the qualifier for a field access and
   // its target variable is a value type variable,
   // ignore the load since the address of a variable that is a value type is

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedExpr.qll

@@ -414,7 +414,7 @@ abstract class TranslatedCrementOperation extends TranslatedNonConstantExpr {
       this.getOpcode() instanceof Opcode::PointerAdd or
       this.getOpcode() instanceof Opcode::PointerSub
     ) and
-    result = 4 //max(getResultType().(PointerType).getSize())
+    result = Language::getTypeSize(this.getResultType().(PointerType).getReferentType())
   }
 
   final TranslatedExpr getOperand() { result = getTranslatedExpr(expr.getOperand()) }
@@ -578,7 +578,6 @@ class TranslatedArrayAccess extends TranslatedNonConstantExpr {
       (
         // The successor of a `PointerAdd` is an `ElementsAddress` if
         // that `PointerAdd` is not the last `PointerAdd` instruction.
-        index < getRank() - 1 and
         tag = PointerAddTag(index) and
         result = this.getInstruction(ElementsAddressTag(index + 1))
         or
@@ -607,10 +606,7 @@ class TranslatedArrayAccess extends TranslatedNonConstantExpr {
     result = this.getInstruction(PointerAddTag(child.getAST().getIndex()))
   }
 
-  override Instruction getResult() {
-    result = this.getInstruction(PointerAddTag(getRank() - 1)) //and
-    //result.getResultType() = expr.getType()
-  }
+  override Instruction getResult() { result = this.getInstruction(PointerAddTag(getRank() - 1)) }
 
   override predicate hasInstruction(
     Opcode opcode, InstructionTag tag, Type resultType, boolean isLValue
@@ -663,12 +659,14 @@ class TranslatedArrayAccess extends TranslatedNonConstantExpr {
   }
 
   override int getInstructionElementSize(InstructionTag tag) {
-    tag = PointerAddTag(_) and
-    // TODO: Fix sizes once we have type sizes
-    result = 4
+    exists(int index |
+      inBounds(index) and
+      tag = PointerAddTag(index) and
+      result = Language::getTypeSize(expr.getQualifier().getType().(ArrayType).getElementType())
+    )
   }
 
-  private TranslatedExpr getBaseOperand() { result = getTranslatedExpr(expr.getChild(-1)) }
+  private TranslatedExpr getBaseOperand() { result = getTranslatedExpr(expr.getQualifier()) }
 
   private TranslatedExpr getOffsetOperand(int index) {
     this.inBounds(index) and
@@ -1248,16 +1246,14 @@ class TranslatedBinaryOperation extends TranslatedSingleInstructionExpr {
         opcode instanceof Opcode::PointerSub or
         opcode instanceof Opcode::PointerDiff
       ) and
-      result = 8 //max(getPointerOperand().getResultType().(PointerType).getReferentType().getSize()) TODO: SIZE AGAIN
+      result = Language::getTypeSize(this.getPointerOperand().getResultType())
     )
   }
 
-  //  private TranslatedExpr getPointerOperand() {
-  //    if swapOperandsOnOp() then
-  //      result = this.getRightOperand()
-  //    else
-  //      result = this.getLeftOperand()
-  //  }
+  private TranslatedExpr getPointerOperand() {
+    if swapOperandsOnOp() then result = this.getRightOperand() else result = this.getLeftOperand()
+  }
+
   private predicate swapOperandsOnOp() {
     // Swap the operands on a pointer add 'i + p', so that the pointer operand
     // always comes first. Note that we still evaluate the operands
@@ -1444,9 +1440,19 @@ class TranslatedAssignOperation extends TranslatedAssignment {
   }
 
   private Opcode getOpcode() {
-    expr instanceof AssignAddExpr and result instanceof Opcode::Add
+    expr instanceof AssignAddExpr and
+    (
+      if expr.getRValue().getType() instanceof PointerType
+      then result instanceof Opcode::PointerAdd
+      else result instanceof Opcode::Add
+    )
     or
-    expr instanceof AssignSubExpr and result instanceof Opcode::Sub
+    expr instanceof AssignSubExpr and
+    (
+      if expr.getRValue().getType() instanceof PointerType
+      then result instanceof Opcode::PointerSub
+      else result instanceof Opcode::Sub
+    )
     or
     expr instanceof AssignMulExpr and result instanceof Opcode::Mul
     or
@@ -1462,10 +1468,7 @@ class TranslatedAssignOperation extends TranslatedAssignment {
     or
     expr instanceof AssignLShiftExpr and result instanceof Opcode::ShiftLeft
     or
-    expr instanceof AssignRShiftExpr and result instanceof Opcode::ShiftRight // or
-    // TODO: THE CASES ABOVE DEAL WITH POINTERS
-    //    expr instanceof AssignPointerAddExpr and result instanceof Opcode::PointerAdd or
-    //    expr instanceof AssignPointerSubExpr and result instanceof Opcode::PointerSub
+    expr instanceof AssignRShiftExpr and result instanceof Opcode::ShiftRight
   }
 
   override predicate hasInstruction(
@@ -1500,11 +1503,13 @@ class TranslatedAssignOperation extends TranslatedAssignment {
   override int getInstructionElementSize(InstructionTag tag) {
     tag = AssignOperationOpTag() and
     exists(Opcode opcode |
-      opcode = getOpcode() and
-      // TODO: ADD AND SUB FOR POITNER ARITH (WAS POINTERADD AND POINTERSUB)
-      (opcode instanceof Opcode::Add or opcode instanceof Opcode::Sub)
+      opcode = this.getOpcode() and
+      (
+        opcode instanceof Opcode::PointerAdd or
+        opcode instanceof Opcode::PointerSub
+      )
     ) and
-    result = 8 //max(getResultType().(PointerType).getReferentType().getSize()) TODO: DEAL WITH SIZE
+    result = Language::getTypeSize(getResultType().(PointerType).getReferentType())
   }
 
   override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedInitialization.qll

@@ -249,6 +249,11 @@ abstract class TranslatedElementInitialization extends TranslatedElement {
     )
   }
 
+  override int getInstructionElementSize(InstructionTag tag) {
+    tag = getElementAddressTag() and
+    result = Language::getTypeSize(getElementType())
+  }
+
   override string getInstructionConstantValue(InstructionTag tag) {
     tag = getElementIndexTag() and
     result = getElementIndex().toString()