Add IR instructions to model implicit pointer dereferences.

github · Jan 21, 2020 · ba9d2fb · ba9d2fb
1 parent efc5f10
commit ba9d2fb
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 25 deletions.
diff --git a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll
@@ -262,6 +262,24 @@ newtype TControlFlowNode =
    * A control-flow node that represents the implicit max bound of a simple slice expression.
    */
   MkImplicitMaxSliceBound(SliceExpr sl) { not exists(sl.getMax()) } or
+  /**
+   * A control-flow node that represents the implicit dereference of the base in a field/method
+   * access.
+   */
+  MkImplicitDeref(Expr e) {
+    e.getType().getUnderlyingType() instanceof PointerType and
+    exists(SelectorExpr sel | e = sel.getBase() |
+      // field accesses through a pointer always implicitly dereference
+      sel = any(Field f).getAReference()
+      or
+      // method accesses only dereference if the receiver is _not_ a pointer
+      exists(Method m, Type tp |
+        sel = m.getAReference() and
+        tp = m.getReceiver().getType().getUnderlyingType() and
+        not tp instanceof PointerType
+      )
+    )
+  } or
   /**
    * A control-flow node that represents the start of the execution of a function or file.
    */
@@ -1614,21 +1632,36 @@ module CFG {
     }
   }
 
-  private class SelectorExprTree extends PostOrderTree, SelectorExpr {
+  private class SelectorExprTree extends ControlFlowTree, SelectorExpr {
     SelectorExprTree() { getBase() instanceof ValueExpr }
 
-    override ControlFlow::Node getNode() { result = mkExprOrSkipNode(this) }
+    override predicate firstNode(ControlFlow::Node first) {
+      firstNode(getBase(), first)
+    }
 
-    override Completion getCompletion() {
-      result = Done()
+    override predicate lastNode(ControlFlow::Node last, Completion cmpl) {
+      ControlFlowTree.super.lastNode(last, cmpl)
       or
       // panic due to `nil` dereference
-      getBase() instanceof ValueExpr and
-      this.(ReferenceExpr).isRvalue() and
-      result = Panic()
+      last = MkImplicitDeref(this.getBase()) and
+      cmpl = Panic()
+      or
+      last = mkExprOrSkipNode(this) and
+      cmpl = Done()
     }
 
-    override ControlFlowTree getChildTree(int i) { i = 0 and result = getBase() }
+    override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) {
+      lastNode(getBase(), pred, normalCompletion()) and
+      (
+        succ = MkImplicitDeref(this.getBase())
+        or
+        not exists(MkImplicitDeref(this.getBase())) and
+        succ = mkExprOrSkipNode(this)
+      )
+      or
+      pred = MkImplicitDeref(this.getBase()) and
+      succ = mkExprOrSkipNode(this)
+    }
   }
 
   private class SendStmtTree extends ControlFlowTree, SendStmt {

diff --git a/ql/src/semmle/go/controlflow/IR.qll b/ql/src/semmle/go/controlflow/IR.qll
@@ -46,7 +46,8 @@ module IR {
       this instanceof MkCaseCheckNode or
       this instanceof MkImplicitLowerSliceBound or
       this instanceof MkImplicitUpperSliceBound or
-      this instanceof MkImplicitMaxSliceBound
+      this instanceof MkImplicitMaxSliceBound or
+      this instanceof MkImplicitDeref
     }
 
     /** Holds if this instruction reads the value of variable or constant `v`. */
@@ -169,6 +170,8 @@ module IR {
       this instanceof MkImplicitUpperSliceBound and result = "implicit upper bound"
       or
       this instanceof MkImplicitMaxSliceBound and result = "implicit maximum"
+      or
+      this instanceof MkImplicitDeref and result = "implicit dereference"
     }
   }
 
@@ -229,6 +232,21 @@ module IR {
     }
   }
 
+  /**
+   * Gets the effective base of a selector expression, taking implicit dereferences into account.
+   *
+   * For a selector expression `b.f`, this will either be the implicit dereference `*b`, or just
+   * `b` if there is no implicit dereferencing.
+   */
+  private Instruction selectorBase(SelectorExpr e) {
+    exists(Expr base | base = e.getBase() |
+      result = MkImplicitDeref(base)
+      or
+      not exists(MkImplicitDeref(base)) and
+      result = evalExprInstruction(base)
+    )
+  }
+
   /**
    * An IR instruction that reads the value of a field.
    *
@@ -244,7 +262,7 @@ module IR {
     }
 
     /** Gets the instruction computing the base value on which the field is read. */
-    Instruction getBase() { result = evalExprInstruction(e.getBase()) }
+    Instruction getBase() { result = selectorBase(e) }
 
     /** Gets the field being read. */
     Field getField() { e.getSelector() = result.getAReference() }
@@ -262,7 +280,7 @@ module IR {
     MethodReadInstruction() { e.getSelector() = method.getAReference() }
 
     /** Gets the instruction computing the receiver value on which the method is looked up. */
-    Instruction getReceiver() { result = evalExprInstruction(e.getBase()) }
+    Instruction getReceiver() { result = selectorBase(e) }
 
     /** Gets the method being looked up. */
     Method getMethod() { result = method }
@@ -1189,6 +1207,33 @@ module IR {
     }
   }
 
+  /**
+   * An instruction implicitly dereferencing the base in a field or method reference through a
+   * pointer.
+   */
+  class EvalImplicitDerefInstruction extends Instruction, MkImplicitDeref {
+    Expr e;
+
+    EvalImplicitDerefInstruction() { this = MkImplicitDeref(e) }
+
+    /** Gets the operand that is being dereferenced. */
+    Expr getOperand() { result = e }
+
+    override Type getResultType() {
+      result = e.getType().getUnderlyingType().(PointerType).getBaseType()
+    }
+
+    override ControlFlow::Root getRoot() { result.isRootOf(e) }
+
+    override string toString() { result = "implicit dereference" }
+
+    override predicate hasLocationInfo(
+      string filepath, int startline, int startcolumn, int endline, int endcolumn
+    ) {
+      e.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    }
+  }
+
   /** A representation of the target of a write instruction. */
   class WriteTarget extends TWriteTarget {
     ControlFlow::Node w;
@@ -1294,7 +1339,7 @@ module IR {
 
     /** Gets the instruction computing the base value on which this field is accessed. */
     Instruction getBase() {
-      exists(SelectorExpr sel | this = MkLhs(_, sel) | result = evalExprInstruction(sel.getBase()))
+      exists(SelectorExpr sel | this = MkLhs(_, sel) | result = selectorBase(sel))
       or
       result = w.(InitLiteralStructFieldInstruction).getBase()
     }

diff --git a/...brary-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected b/...brary-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected
@@ -98,27 +98,23 @@
 | exprs.go:15:32:15:43 | init of key-value pair | exprs.go:15:49:15:55 | struct2 |
 | exprs.go:15:35:15:41 | struct1 | exprs.go:15:35:15:43 | selection of x |
 | exprs.go:15:35:15:43 | selection of x | exprs.go:15:32:15:43 | init of key-value pair |
-| exprs.go:15:35:15:43 | selection of x | exprs.go:26:1:26:1 | exit |
 | exprs.go:15:46:15:57 | init of key-value pair | exprs.go:15:2:15:8 | assignment to struct3 |
 | exprs.go:15:49:15:55 | struct2 | exprs.go:15:49:15:57 | selection of x |
 | exprs.go:15:49:15:57 | selection of x | exprs.go:15:46:15:57 | init of key-value pair |
-| exprs.go:15:49:15:57 | selection of x | exprs.go:26:1:26:1 | exit |
 | exprs.go:16:2:16:5 | assignment to arr1 | exprs.go:17:2:17:5 | skip |
 | exprs.go:16:2:16:5 | skip | exprs.go:16:10:16:26 | composite literal |
 | exprs.go:16:10:16:26 | composite literal | exprs.go:16:17:16:17 | element index |
 | exprs.go:16:17:16:17 | element index | exprs.go:16:17:16:23 | struct3 |
 | exprs.go:16:17:16:23 | struct3 | exprs.go:16:17:16:25 | selection of x |
 | exprs.go:16:17:16:25 | init of selection of x | exprs.go:16:2:16:5 | assignment to arr1 |
 | exprs.go:16:17:16:25 | selection of x | exprs.go:16:17:16:25 | init of selection of x |
-| exprs.go:16:17:16:25 | selection of x | exprs.go:26:1:26:1 | exit |
 | exprs.go:17:2:17:5 | assignment to arr2 | exprs.go:18:2:18:4 | skip |
 | exprs.go:17:2:17:5 | skip | exprs.go:17:10:17:40 | composite literal |
 | exprs.go:17:10:17:40 | composite literal | exprs.go:17:19:17:19 | element index |
 | exprs.go:17:19:17:19 | element index | exprs.go:17:19:17:25 | struct3 |
 | exprs.go:17:19:17:25 | struct3 | exprs.go:17:19:17:27 | selection of x |
 | exprs.go:17:19:17:27 | init of selection of x | exprs.go:17:30:17:30 | 2 |
 | exprs.go:17:19:17:27 | selection of x | exprs.go:17:19:17:27 | init of selection of x |
-| exprs.go:17:19:17:27 | selection of x | exprs.go:26:1:26:1 | exit |
 | exprs.go:17:30:17:30 | 2 | exprs.go:17:33:17:36 | arr1 |
 | exprs.go:17:30:17:39 | init of key-value pair | exprs.go:17:2:17:5 | assignment to arr2 |
 | exprs.go:17:33:17:36 | arr1 | exprs.go:17:38:17:38 | 0 |
@@ -210,7 +206,6 @@
 | exprs.go:29:9:29:19 | type assertion | exprs.go:29:9:29:21 | selection of x |
 | exprs.go:29:9:29:19 | type assertion | exprs.go:30:1:30:1 | exit |
 | exprs.go:29:9:29:21 | selection of x | exprs.go:29:2:29:21 | return statement |
-| exprs.go:29:9:29:21 | selection of x | exprs.go:30:1:30:1 | exit |
 | exprs.go:32:1:32:1 | entry | exprs.go:32:12:32:14 | argument corresponding to arg |
 | exprs.go:32:1:37:1 | function declaration | exprs.go:39:6:39:10 | skip |
 | exprs.go:32:6:32:10 | skip | exprs.go:32:1:37:1 | function declaration |
@@ -231,7 +226,6 @@
 | exprs.go:34:3:34:12 | return statement | exprs.go:37:1:37:1 | exit |
 | exprs.go:34:10:34:10 | p | exprs.go:34:10:34:12 | selection of x |
 | exprs.go:34:10:34:12 | selection of x | exprs.go:34:3:34:12 | return statement |
-| exprs.go:34:10:34:12 | selection of x | exprs.go:37:1:37:1 | exit |
 | exprs.go:36:2:36:10 | return statement | exprs.go:37:1:37:1 | exit |
 | exprs.go:36:9:36:10 | -... | exprs.go:36:2:36:10 | return statement |
 | exprs.go:39:1:39:1 | entry | exprs.go:39:12:39:14 | argument corresponding to arg |
@@ -260,7 +254,6 @@
 | exprs.go:44:3:44:12 | return statement | exprs.go:47:1:47:1 | exit |
 | exprs.go:44:10:44:10 | p | exprs.go:44:10:44:12 | selection of x |
 | exprs.go:44:10:44:12 | selection of x | exprs.go:44:3:44:12 | return statement |
-| exprs.go:44:10:44:12 | selection of x | exprs.go:47:1:47:1 | exit |
 | exprs.go:46:2:46:10 | return statement | exprs.go:47:1:47:1 | exit |
 | exprs.go:46:9:46:10 | -... | exprs.go:46:2:46:10 | return statement |
 | exprs.go:49:1:49:1 | entry | exprs.go:49:10:49:11 | argument corresponding to xs |
@@ -797,7 +790,6 @@
 | stmts5.go:8:2:8:3 | me | stmts5.go:8:2:8:7 | selection of val |
 | stmts5.go:8:2:8:7 | assignment to field val | stmts5.go:9:1:9:1 | exit |
 | stmts5.go:8:2:8:7 | selection of val | stmts5.go:8:12:8:16 | other |
-| stmts5.go:8:2:8:7 | selection of val | stmts5.go:9:1:9:1 | exit |
 | stmts5.go:8:2:8:16 | ... += ... | stmts5.go:8:2:8:7 | assignment to field val |
 | stmts5.go:8:12:8:16 | other | stmts5.go:8:2:8:16 | ... += ... |
 | stmts5.go:11:1:11:1 | entry | stmts5.go:11:20:12:1 | skip |
@@ -842,9 +834,10 @@
 | stmts7.go:19:7:19:13 | argument corresponding to methods | stmts7.go:19:7:19:13 | initialization of methods |
 | stmts7.go:19:7:19:13 | initialization of methods | stmts7.go:20:2:20:8 | methods |
 | stmts7.go:19:26:19:28 | skip | stmts7.go:19:1:21:1 | function declaration |
-| stmts7.go:20:2:20:8 | methods | stmts7.go:20:2:20:11 | selection of fn |
+| stmts7.go:20:2:20:8 | implicit dereference | stmts7.go:20:2:20:11 | selection of fn |
+| stmts7.go:20:2:20:8 | implicit dereference | stmts7.go:21:1:21:1 | exit |
+| stmts7.go:20:2:20:8 | methods | stmts7.go:20:2:20:8 | implicit dereference |
 | stmts7.go:20:2:20:11 | selection of fn | stmts7.go:20:2:20:13 | call to fn |
-| stmts7.go:20:2:20:11 | selection of fn | stmts7.go:21:1:21:1 | exit |
 | stmts7.go:20:2:20:13 | call to fn | stmts7.go:21:1:21:1 | exit |
 | stmts7.go:23:1:23:1 | entry | stmts7.go:23:16:23:23 | argument corresponding to callback |
 | stmts7.go:23:1:28:1 | function declaration | stmts7.go:0:0:0:0 | exit |
@@ -854,13 +847,13 @@
 | stmts7.go:24:2:24:20 | defer statement | stmts7.go:25:10:25:17 | callback |
 | stmts7.go:24:8:24:15 | callback | stmts7.go:24:8:24:18 | selection of fn |
 | stmts7.go:24:8:24:18 | selection of fn | stmts7.go:24:2:24:20 | defer statement |
-| stmts7.go:24:8:24:18 | selection of fn | stmts7.go:28:1:28:1 | exit |
 | stmts7.go:24:8:24:20 | call to fn | stmts7.go:28:1:28:1 | exit |
 | stmts7.go:25:2:25:23 | defer statement | stmts7.go:26:2:26:12 | selection of Println |
-| stmts7.go:25:8:25:21 | selection of fn | stmts7.go:24:8:24:20 | call to fn |
+| stmts7.go:25:8:25:18 | implicit dereference | stmts7.go:24:8:24:20 | call to fn |
+| stmts7.go:25:8:25:18 | implicit dereference | stmts7.go:25:8:25:21 | selection of fn |
 | stmts7.go:25:8:25:21 | selection of fn | stmts7.go:25:2:25:23 | defer statement |
 | stmts7.go:25:8:25:23 | call to fn | stmts7.go:24:8:24:20 | call to fn |
-| stmts7.go:25:9:25:17 | &... | stmts7.go:25:8:25:21 | selection of fn |
+| stmts7.go:25:9:25:17 | &... | stmts7.go:25:8:25:18 | implicit dereference |
 | stmts7.go:25:10:25:17 | callback | stmts7.go:25:9:25:17 | &... |
 | stmts7.go:26:2:26:12 | selection of Println | stmts7.go:26:14:26:30 | "print something" |
 | stmts7.go:26:2:26:31 | call to Println | stmts7.go:25:8:25:23 | call to fn |