Java: Minor TypeFlow precision improvement and refactor.

Author: aschackmull

java/ql/src/semmle/code/java/dataflow/TypeFlow.qll

@@ -76,9 +76,14 @@ private predicate joinStep0(TypeFlowNode n1, TypeFlowNode n2) {
   or
   n2.asExpr().(ConditionalExpr).getFalseExpr() = n1.asExpr()
   or
-  n2.asField().getAnAssignedValue() = n1.asExpr()
+  exists(Field f, Expr e |
+    f = n2.asField() and
+    f.getAnAssignedValue() = e and
+    e = n1.asExpr() and
+    not e.(FieldAccess).getField() = f
+  )
   or
-  n2.asSsa().(BaseSsaPhiNode).getAPhiInput() = n1.asSsa()
+  n2.asSsa().(BaseSsaPhiNode).getAnUltimateLocalDefinition() = n1.asSsa()
   or
   exists(ReturnStmt ret |
     n2.asMethod() = ret.getEnclosingCallable() and ret.getResult() = n1.asExpr()
@@ -89,7 +94,9 @@ private predicate joinStep0(TypeFlowNode n1, TypeFlowNode n2) {
   exists(Argument arg, Parameter p |
     privateParamArg(p, arg) and
     n1.asExpr() = arg and
-    n2.asSsa().(BaseSsaImplicitInit).isParameterDefinition(p)
+    n2.asSsa().(BaseSsaImplicitInit).isParameterDefinition(p) and
+    // skip trivial recursion
+    not arg = n2.asSsa().getAUse()
   )
 }
 
@@ -121,9 +128,18 @@ private predicate step(TypeFlowNode n1, TypeFlowNode n2) {
 private predicate isNull(TypeFlowNode n) {
   n.asExpr() instanceof NullLiteral
   or
+  exists(LocalVariableDeclExpr decl |
+    n.asSsa().(BaseSsaUpdate).getDefiningExpr() = decl and
+    not decl.hasImplicitInit() and
+    not exists(decl.getInit())
+  )
+  or
   exists(TypeFlowNode mid | isNull(mid) and step(mid, n))
   or
-  forex(TypeFlowNode mid | joinStep0(mid, n) | isNull(mid))
+  forex(TypeFlowNode mid | joinStep0(mid, n) | isNull(mid)) and
+  // Fields that are never assigned a non-null value are probably set by
+  // reflection and are thus not always null.
+  not exists(n.asField())
 }
 
 /**
@@ -155,6 +171,15 @@ private predicate joinStepRank(int r, TypeFlowNode n1, TypeFlowNode n2) {
 
 private int lastRank(TypeFlowNode n) { result = max(int r | joinStepRank(r, _, n)) }
 
+private predicate exactTypeBase(TypeFlowNode n, RefType t) {
+  exists(ClassInstanceExpr e |
+    n.asExpr() = e and
+    e.getType() = t and
+    not e instanceof FunctionalExpr and
+    exists(RefType sub | sub.getASourceSupertype() = t.getSourceDeclaration())
+  )
+}
+
 private predicate exactTypeRank(int r, TypeFlowNode n, RefType t) {
   forall(TypeFlowNode mid | joinStepRank(r, mid, n) | exactType(mid, t)) and
   joinStepRank(r, _, n)
@@ -171,12 +196,7 @@ private predicate exactTypeJoin(int r, TypeFlowNode n, RefType t) {
  * non-trivial lower bound, that is, `t` has a subtype.
  */
 private predicate exactType(TypeFlowNode n, RefType t) {
-  exists(ClassInstanceExpr e |
-    n.asExpr() = e and
-    e.getType() = t and
-    not e instanceof FunctionalExpr and
-    exists(RefType sub | sub.getASourceSupertype() = t.getSourceDeclaration())
-  )
+  exactTypeBase(n, t)
   or
   exists(TypeFlowNode mid | exactType(mid, t) and step(mid, n))
   or
@@ -289,6 +309,24 @@ private predicate instanceOfGuarded(VarAccess va, RefType t) {
   )
 }
 
+/**
+ * Holds if `n` has type `t` and this information is discarded, such that `t`
+ * might be a better type bound for nodes where `n` flows to.
+ */
+private predicate typeFlowBase(TypeFlowNode n, RefType t) {
+  exists(RefType srctype |
+    upcast(n, srctype) or
+    upcastEnhancedForStmt(n.asSsa(), srctype) or
+    downcastSuccessor(n.asExpr(), srctype) or
+    instanceOfGuarded(n.asExpr(), srctype) or
+    n.asExpr().(FunctionalExpr).getConstructedType() = srctype
+  |
+    t = srctype.(BoundedType).getAnUltimateUpperBoundType()
+    or
+    t = srctype and not srctype instanceof BoundedType
+  )
+}
+
 /**
  * Holds if `t` is a bound that holds on one of the incoming edges to `n` and
  * thus is a candidate bound for `n`.
@@ -320,15 +358,7 @@ private predicate typeFlowJoin(int r, TypeFlowNode n, RefType t) {
  * likely to be better than the static type of `n`.
  */
 private predicate typeFlow(TypeFlowNode n, RefType t) {
-  upcast(n, t)
-  or
-  upcastEnhancedForStmt(n.asSsa(), t)
-  or
-  downcastSuccessor(n.asExpr(), t)
-  or
-  instanceOfGuarded(n.asExpr(), t)
-  or
-  n.asExpr().(FunctionalExpr).getConstructedType() = t
+  typeFlowBase(n, t)
   or
   exists(TypeFlowNode mid | typeFlow(mid, t) and step(mid, n))
   or
@@ -363,17 +393,13 @@ private module TypeFlowBounds {
    */
   cached
   predicate fieldTypeFlow(Field f, RefType t, boolean exact) {
-    exists(TypeFlowNode n, RefType srctype |
+    exists(TypeFlowNode n |
       n.asField() = f and
       (
-        exactType(n, srctype) and exact = true
+        exactType(n, t) and exact = true
         or
-        not exactType(n, _) and bestTypeFlow(n, srctype) and exact = false
+        not exactType(n, _) and bestTypeFlow(n, t) and exact = false
       )
-    |
-      t = srctype.(BoundedType).getAnUltimateUpperBoundType()
-      or
-      t = srctype and not srctype instanceof BoundedType
     )
   }
 
@@ -384,17 +410,13 @@ private module TypeFlowBounds {
    */
   cached
   predicate exprTypeFlow(Expr e, RefType t, boolean exact) {
-    exists(TypeFlowNode n, RefType srctype |
+    exists(TypeFlowNode n |
       n.asExpr() = e and
       (
-        exactType(n, srctype) and exact = true
+        exactType(n, t) and exact = true
         or
-        not exactType(n, _) and bestTypeFlow(n, srctype) and exact = false
+        not exactType(n, _) and bestTypeFlow(n, t) and exact = false
       )
-    |
-      t = srctype.(BoundedType).getAnUltimateUpperBoundType()
-      or
-      t = srctype and not srctype instanceof BoundedType
     )
   }
 }