Merge pull request #33 from github/fixes

improve callgraph resolution, and other fixes

Author: erik-krogh

ql/src/codeql_ql/ast/Ast.qll

@@ -56,6 +56,15 @@ class AstNode extends TAstNode {
    * Gets the primary QL class for the ast node.
    */
   string getAPrimaryQlClass() { result = "???" }
+
+  /**
+   * Gets the predicate that contains this AST node.
+   */
+  pragma[noinline]
+  Predicate getEnclosingPredicate() {
+    not this instanceof Predicate and
+    toGenerated(result) = toGenerated(this).getParent+()
+  }
 }
 
 /** A toplevel QL program, i.e. a file. */
@@ -145,7 +154,14 @@ class Predicate extends TPredicate, AstNode {
   /**
    * Gets the number of parameters.
    */
-  int getArity() { result = count(getParameter(_)) }
+  int getArity() {
+    not this.(ClasslessPredicate).getAlias() instanceof PredicateExpr and
+    result = count(getParameter(_))
+    or
+    exists(PredicateExpr alias | alias = this.(ClasslessPredicate).getAlias() |
+      result = alias.getArity()
+    )
+  }
 
   /**
    * Gets the return type (if any) of the predicate.
@@ -990,6 +1006,21 @@ class Float extends Literal {
   float getValue() { result = lit.getChild().(Generated::Float).getValue().toFloat() }
 }
 
+/** A boolean literal */
+class Boolean extends Literal {
+  Generated::Bool bool;
+
+  Boolean() { lit.getChild() = bool }
+
+  /** Holds if the value is `true` */
+  predicate isTrue() { bool.getChild() instanceof Generated::True }
+
+  /** Holds if the value is `false` */
+  predicate isFalse() { bool.getChild() instanceof Generated::False }
+
+  override string getAPrimaryQlClass() { result = "Boolean" }
+}
+
 /** A comparison symbol, such as `"<"` or `"="`. */
 class ComparisonSymbol extends string {
   ComparisonSymbol() {
@@ -1315,6 +1346,19 @@ class ExprAggregate extends TExprAggregate, Expr {
       pred = indexedMember("getOrderBy", i) and result = this.getOrderBy(i)
     )
   }
+
+  override Type getType() {
+    exists(PrimitiveType prim | prim = result |
+      kind.regexpMatch("(strict)?count|sum|min|max|rank") and
+      result.getName() = "int"
+      or
+      kind.regexpMatch("(strict)?concat") and
+      result.getName() = "string"
+    )
+    or
+    not kind = ["count", "strictcount"] and
+    result = getExpr(0).getType()
+  }
 }
 
 /** An aggregate expression, such as `count` or `sum`. */
@@ -1365,12 +1409,21 @@ class Aggregate extends TAggregate, Expr {
 
   override string getAPrimaryQlClass() { result = "Aggregate[" + kind + "]" }
 
-  override PrimitiveType getType() {
-    kind.regexpMatch("(strict)?count|sum|min|max|rank") and
-    result.getName() = "int"
+  override Type getType() {
+    exists(PrimitiveType prim | prim = result |
+      kind.regexpMatch("(strict)?count|sum|min|max|rank") and
+      result.getName() = "int"
+      or
+      kind.regexpMatch("(strict)?concat") and
+      result.getName() = "string"
+    )
+    or
+    kind = ["any", "min", "max"] and
+    not exists(getExpr(_)) and
+    result = getArgument(0).getTypeExpr().getResolvedType()
     or
-    kind.regexpMatch("(strict)?concat") and
-    result.getName() = "string"
+    not kind = ["count", "strictcount"] and
+    result = getExpr(0).getType()
   }
 
   override AstNode getAChild(string pred) {
@@ -1499,6 +1552,13 @@ class ThisAccess extends Identifier {
   override string getAPrimaryQlClass() { result = "ThisAccess" }
 }
 
+/** A use of `super`. */
+class Super extends TSuper, Expr {
+  Super() { this = TSuper(_) }
+
+  override string getAPrimaryQlClass() { result = "SuperAccess" }
+}
+
 /** An access to `result`. */
 class ResultAccess extends Identifier {
   ResultAccess() { any(Generated::Result r).getParent() = id }

ql/src/codeql_ql/ast/internal/AstNodes.qll

@@ -24,6 +24,7 @@ newtype TAstNode =
   TExprAggregate(Generated::Aggregate agg) {
     agg.getChild(_) instanceof Generated::ExprAggregateBody
   } or
+  TSuper(Generated::SuperRef sup) or
   TIdentifier(Generated::Variable var) or
   TAsExpr(Generated::AsExpr asExpr) { asExpr.getChild(1) instanceof Generated::VarName } or
   TPredicateCall(Generated::CallOrUnqualAggExpr call) or
@@ -63,7 +64,7 @@ class TBinOpExpr = TAddSubExpr or TMulDivModExpr;
 
 class TExpr =
   TBinOpExpr or TLiteral or TAggregate or TExprAggregate or TIdentifier or TInlineCast or TCall or
-      TUnaryExpr or TExprAnnotation or TDontCare or TRange or TSet or TAsExpr;
+      TUnaryExpr or TExprAnnotation or TDontCare or TRange or TSet or TAsExpr or TSuper;
 
 class TCall = TPredicateCall or TMemberCall or TNoneCall or TAnyCall;
 
@@ -154,6 +155,8 @@ Generated::AstNode toGenerated(AST::AstNode n) {
   n = TNoneCall(result)
   or
   n = TAnyCall(result)
+  or
+  n = TSuper(result)
 }
 
 class TPredicate = TCharPred or TClasslessPredicate or TClassPredicate;

ql/src/codeql_ql/ast/internal/Predicate.qll

@@ -79,6 +79,13 @@ private module Cached {
       t = mc.getBase().getType() and
       p = t.getClassPredicate(mc.getMemberName(), mc.getNumberOfArguments())
     )
+    or
+    // super calls
+    exists(Super sup, ClassType type |
+      mc.getBase() = sup and
+      sup.getEnclosingPredicate().(ClassPredicate).getParent().getType() = type and
+      p = type.getASuperType().getClassPredicate(mc.getMemberName(), mc.getNumberOfArguments())
+    )
   }
 
   cached
@@ -233,6 +240,7 @@ module PredConsistency {
   query predicate noResolveCall(Call c) {
     not resolveCall(c, _) and
     not c instanceof NoneCall and
+    not c instanceof AnyCall and
     not c.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
   }
 
@@ -243,7 +251,12 @@ module PredConsistency {
   }
 
   query predicate multipleResolveCall(Call call, int c, PredicateOrBuiltin p) {
-    c = strictcount(PredicateOrBuiltin p0 | resolveCall(call, p0)) and
+    c =
+      strictcount(PredicateOrBuiltin p0 |
+        resolveCall(call, p0) and
+        // aliases are expected to resolve to multiple.
+        not exists(p0.getDeclaration().(ClasslessPredicate).getAlias())
+      ) and
     c > 1 and
     resolveCall(call, p)
   }

ql/test/callgraph/Foo.qll

@@ -0,0 +1,33 @@
+import ql
+
+predicate foo() { none() }
+
+query predicate test() { foo() }
+
+class Foo extends AstNode {
+  predicate bar() { none() }
+
+  predicate baz() { bar() }
+}
+
+class Sub extends Foo {
+  override predicate baz() { super.baz() }
+}
+
+query predicate test2() { any(Foo f).bar() }
+
+module Aliases {
+  predicate myThing2(int i, int j) { i = 2 and j = 3 }
+
+  predicate myThing0() { any() }
+
+  predicate alias0 = myThing0/0;
+
+  predicate alias2 = myThing2/2;
+
+  query predicate test3() {
+    alias2(3, 4) // doesn't work.
+    or
+    alias0() // <- works
+  }
+}

ql/test/callgraph/callgraph.expected

@@ -0,0 +1,8 @@
+| Foo.qll:5:26:5:30 | PredicateCall | Foo.qll:3:1:3:26 | foo |
+| Foo.qll:10:21:10:25 | PredicateCall | Foo.qll:8:3:8:28 | ClassPredicate |
+| Foo.qll:14:30:14:40 | MemberCall | Foo.qll:10:3:10:27 | ClassPredicate |
+| Foo.qll:17:27:17:42 | MemberCall | Foo.qll:8:3:8:28 | ClassPredicate |
+| Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:20:3:20:54 | myThing2 |
+| Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:26:3:26:32 | alias2 |
+| Foo.qll:31:5:31:12 | PredicateCall | Foo.qll:22:3:22:32 | myThing0 |
+| Foo.qll:31:5:31:12 | PredicateCall | Foo.qll:24:3:24:32 | alias0 |

ql/test/callgraph/callgraph.ql

@@ -0,0 +1,3 @@
+import ql
+
+query AstNode getTarget(Call call) { result = call.getTarget().getDeclaration() }

ql/test/printAst/Foo.qll

@@ -22,4 +22,6 @@ predicate calls(Foo f) {
   f = any(Foo f)
   or
   2 = 1 + (2 + (3 + 4))
+  or
+  true = false
 }