Merge pull request #239 from max/virtual-dispatch

Call-graph API cleanup

ql/src/RedundantCode/NegativeLengthCheck.ql

@@ -16,7 +16,7 @@ where
   (len = Builtin::len() or len = Builtin::cap()) and
   (
     exists(RelationalComparisonExpr rel | rel = cmp |
-      rel.getLesserOperand() = len.getACallExpr() and
+      rel.getLesserOperand() = len.getACall().asExpr() and
       rel.getGreaterOperand().getIntValue() = ub and
       (
         ub < 0
@@ -27,7 +27,7 @@ where
     )
     or
     exists(EqualityTestExpr eq | eq = cmp |
-      eq.getAnOperand() = len.getACallExpr() and
+      eq.getAnOperand() = len.getACall().asExpr() and
       eq.getAnOperand().getIntValue() = ub and
       ub < 0 and
       r = "equal"

ql/src/semmle/go/Expr.qll

@@ -442,14 +442,6 @@ class CallExpr extends CallOrConversionExpr {
   /** Gets the expression representing the function being called. */
   Expr getCalleeExpr() { result = getChildExpr(0) }
 
-  /** Holds if this call is of the form `base.method(...)`. */
-  predicate calls(Expr base, string method) {
-    exists(SelectorExpr callee | callee = getCalleeExpr().stripParens() |
-      callee.getBase() = base and
-      method = callee.getSelector().getName()
-    )
-  }
-
   /** Gets the `i`th argument expression of this call (0-based). */
   Expr getArgument(int i) {
     i >= 0 and
@@ -462,39 +454,17 @@ class CallExpr extends CallOrConversionExpr {
   /** Gets the number of argument expressions of this call. */
   int getNumArgument() { result = count(getAnArgument()) }
 
-  /** Gets the name of the invoked function or method. */
+  /** Gets the name of the invoked function or method if it can be determined syntactically. */
   string getCalleeName() {
-    result = getCalleeExpr().stripParens().(Ident).getName() or
-    calls(_, result)
+    exists(Expr callee | callee = getCalleeExpr().stripParens() |
+      result = callee.(Ident).getName()
+      or
+      result = callee.(SelectorExpr).getSelector().getName()
+    )
   }
 
   /** Gets the declared target of this call. */
-  Function getTarget() { this = result.getACallExpr() }
-
-  /**
-   * Gets the definition of a possible target of this call.
-   *
-   * For non-virtual calls, there is at most one possible call target (but there may be none if the
-   * target has no declaration).
-   *
-   * For virtual calls, we look up possible targets in all types that implement the receiver
-   * interface type.
-   */
-  FuncDef getACallee() {
-    result = getTarget().(DeclaredFunction).getFuncDecl()
-    or
-    exists(SelectorExpr sel, InterfaceType declaredRecv, Type actualRecv |
-      sel = getCalleeExpr().stripParens() and
-      declaredRecv = sel.getBase().getType().getUnderlyingType() and
-      actualRecv.implements(declaredRecv)
-    |
-      result = actualRecv
-            .(PointerType)
-            .getBaseType()
-            .(NamedType)
-            .getMethodDecl(sel.getSelector().getName())
-    )
-  }
+  Function getTarget() { getCalleeExpr() = result.getAReference() }
 
   override predicate mayHaveOwnSideEffects() {
     getTarget().mayHaveSideEffects() or

ql/src/semmle/go/Scopes.qll

@@ -276,11 +276,13 @@ class Field extends Variable {
 
 /** A built-in or declared function. */
 class Function extends ValueEntity, @functionobject {
-  /** Gets an expression representing a call to this function. */
-  CallExpr getACallExpr() { result.getCalleeExpr() = getAReference() }
-
   /** Gets a call to this function. */
-  DataFlow::CallNode getACall() { result.getExpr() = getACallExpr() }
+  pragma[nomagic]
+  DataFlow::CallNode getACall() {
+    this = result.getTarget()
+    or
+    this.(DeclaredFunction).getFuncDecl() = result.getACallee()
+  }
 
   /** Holds if this function has no observable side effects. */
   predicate mayHaveSideEffects() { none() }

ql/src/semmle/go/StringOps.qll

@@ -214,13 +214,16 @@ module StringOps {
      * width and precision specifiers, but not including `*` specifiers or explicit argument
      * indices.
      */
+    pragma[noinline]
     private string getFormatComponentRegex() {
-      exists(string literal, string opt_flag, string opt_width, string operator, string verb |
+      exists(string literal, string opt_flag, string width, string prec, string opt_width_and_prec, string operator, string verb |
         literal = "([^%]|%%)+" and
         opt_flag = "[-+ #0]?" and
-        opt_width = "((\\d*|\\*)(\\.(\\d*|\\*))?)?" and
+        width = "\\d+|\\*" and
+        prec = "\\.(\\d+|\\*)" and
+        opt_width_and_prec = "(" + width + ")?(" + prec + ")?" and
         operator = "[bcdeEfFgGoOpqstTxXUv]" and
-        verb = "(%" + opt_flag + opt_width + operator + ")"
+        verb = "(%" + opt_flag + opt_width_and_prec + operator + ")"
       |
         result = "(" + literal + "|" + verb + ")"
       )

ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll

@@ -7,7 +7,7 @@ DataFlowCallable viableImpl(DataFlowCall ma) { result = viableCallable(ma) }
  * Gets a function that might be called by `call`.
  */
 DataFlowCallable viableCallable(CallExpr ma) {
-  result = ma.getACallee()
+  result = DataFlow::exprNode(ma).(DataFlow::CallNode).getACallee()
 }
 
 /**

ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll

@@ -264,11 +264,36 @@ class CallNode extends ExprNode {
   /** Gets the declared target of this call */
   Function getTarget() { result = expr.getTarget() }
 
-  /** Get the definition of a possible target of this call. See `CallExpr.getACallee`. */
-  FuncDef getACallee() { result = expr.getACallee() }
+  private DataFlow::Node getACalleeSource() {
+    result.getASuccessor*() = getCalleeNode()
+  }
+
+  /**
+   * Gets the definition of a possible target of this call.
+   *
+   * For non-virtual calls, there is at most one possible call target (but there may be none if the
+   * target has no declaration).
+   *
+   * For virtual calls, we look up possible targets in all types that implement the receiver
+   * interface type.
+   */
+  FuncDef getACallee() {
+    result = getTarget().(DeclaredFunction).getFuncDecl()
+    or
+    exists(DataFlow::Node calleeSource | calleeSource = getACalleeSource() |
+      result = calleeSource.asExpr()
+      or
+      exists(Method m, InterfaceType declaredRecv, Type actualRecv |
+        calleeSource = m.getARead() and
+        declaredRecv = m.getReceiverType().(NamedType).getBaseType() and
+        actualRecv.implements(declaredRecv) and
+        result = actualRecv.getMethod(m.getName()).(DeclaredFunction).getFuncDecl()
+      )
+    )
+  }
 
   /** Gets the name of the function or method being called, if it can be determined. */
-  string getCalleeName() { result = expr.getTarget().getName() }
+  string getCalleeName() { result = expr.getTarget().getName() or result = expr.getCalleeName() }
 
   /** Gets the data flow node specifying the function to be called. */
   Node getCalleeNode() { result = exprNode(expr.getCalleeExpr()) }
@@ -315,10 +340,7 @@ class CallNode extends ExprNode {
 
   /** Gets the data flow node corresponding to the receiver of this call, if any. */
   Node getReceiver() {
-    exists(MethodReadNode mrn |
-      mrn.getASuccessor*() = this.getCalleeNode() and
-      result = mrn.getReceiver()
-    )
+    result = getACalleeSource().(MethodReadNode).getReceiver()
   }
 }
 
@@ -328,7 +350,7 @@ class MethodCallNode extends CallNode {
 
   override Method getTarget() { result = expr.getTarget() }
 
-  override MethodDecl getACallee() { result = expr.getACallee() }
+  override MethodDecl getACallee() { result = super.getACallee() }
 }
 
 /** A representation of a receiver initialization. */

ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.expected

@@ -0,0 +1,2 @@
+missingCall
+spuriousCall

ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.ql

@@ -0,0 +1,11 @@
+import go
+
+query predicate missingCall(DeclaredFunction f, DataFlow::CallNode call) {
+  call.getACallee() = f.getFuncDecl() and
+  not call = f.getACall()
+}
+
+query predicate spuriousCall(DeclaredFunction f, DataFlow::CallNode call) {
+  call = f.getACall() and
+  exists(FuncDecl fd | fd = f.getFuncDecl() | not call.getACallee() = fd)
+}

ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected

@@ -0,0 +1,2 @@
+missingCallee
+spuriousCallee

ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.ql

@@ -0,0 +1,25 @@
+import go
+
+/**
+ * Gets a string `val` such that there is a comment on the same line as `l`
+ * that contains the substring `key: val`.
+ */
+string metadata(Locatable l, string key) {
+  exists(string f, int line, Comment c, string kv |
+    l.hasLocationInfo(f, line, _, _, _) and
+    c.hasLocationInfo(f, line, _, _, _) and
+    kv = c.getText().regexpFind("\\b(\\w+: \\S+)", _, _) and
+    key = kv.regexpCapture("(\\w+): (\\S+)", 1) and
+    result = kv.regexpCapture("(\\w+): (\\S+)", 2)
+  )
+}
+
+query predicate missingCallee(DataFlow::CallNode call, FuncDef callee) {
+  metadata(call.asExpr(), "callee") = metadata(callee, "name") and
+  not call.getACallee() = callee
+}
+
+query predicate spuriousCallee(DataFlow::CallNode call, FuncDef callee) {
+  call.getACallee() = callee and
+  not metadata(call.asExpr(), "callee") = metadata(callee, "name")
+}

ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go

@@ -0,0 +1,35 @@
+package main
+
+type I interface {
+	m() // name: I.m
+}
+
+type s1 struct{}
+
+type s2 struct{}
+
+type s3 struct{}
+
+type mybool bool
+
+func (s1) m() {} // name: s1.m
+
+func (*s2) m() {} // name: s2.m
+
+func (s3) m(int) {} // name: s3.m
+
+func (mybool) m() {} // name: mybool.m
+
+func test(x *s1, y s2, z s3, b mybool, i I) {
+	x.m()                              // callee: s1.m
+	y.m()                              // callee: s2.m
+	z.m(0)                             // callee: s3.m
+	b.m()                              // callee: mybool.m
+	i.m()                              // callee: s1.m callee: s2.m callee: mybool.m
+	s1.m(*x)                           // callee: s1.m
+	s3.m(z, 0)                         // callee: s3.m
+	mybool.m(b)                        // callee: mybool.m
+	(func() {})()                      // name: func(){} callee: func(){}
+	id := func(x int) int { return x } // name: id
+	id(1)                              // callee: id
+}