Address more parts of Anders review.

Author: criemen

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll

@@ -1734,7 +1734,7 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPath ap) {
-  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) |
+  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc, node) |
     localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and
   cc = mid.getCallContext() and
   ap = mid.getAp()

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll

@@ -1734,7 +1734,7 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPath ap) {
-  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) |
+  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc, node) |
     localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and
   cc = mid.getCallContext() and
   ap = mid.getAp()

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll

@@ -1734,7 +1734,7 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPath ap) {
-  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) |
+  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc, node) |
     localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and
   cc = mid.getCallContext() and
   ap = mid.getAp()

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll

@@ -1734,7 +1734,7 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPath ap) {
-  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) |
+  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc, node) |
     localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and
   cc = mid.getCallContext() and
   ap = mid.getAp()

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll

@@ -1734,7 +1734,7 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPath ap) {
-  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) |
+  exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc, node) |
     localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and
   cc = mid.getCallContext() and
   ap = mid.getAp()

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll

@@ -120,13 +120,16 @@ private module ImplCommon {
         int i, ArgumentNode arg, CallContext outercc, DataFlowCall call
       ) {
         exists(DataFlowCallable c | argumentOf(call, i, arg, c) |
+      (
           outercc = TAnyCallContext()
           or
           outercc = TSomeCall(getAParameter(c), _)
           or
           exists(DataFlowCall other | outercc = TSpecificCall(other, _, _) |
             recordDataFlowCallSite(other, c)
           )
+      ) and
+      not isUnreachableInCall(arg, outercc.(CallContextSpecificCall).getCall())
         )
       }
 
@@ -180,14 +183,16 @@ private module ImplCommon {
         exists(Node mid |
           parameterValueFlow(p, mid, cc) and
           step(mid, node) and
-          compatibleTypes(p.getType(), node.getType())
+          compatibleTypes(p.getType(), node.getType()) and
+          not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall())
         )
         or
         // flow through a callable
         exists(Node arg |
           parameterValueFlow(p, arg, cc) and
           argumentValueFlowsThrough(arg, node, cc) and
-          compatibleTypes(p.getType(), node.getType())
+      compatibleTypes(p.getType(), node.getType()) and
+      not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall())
         )
       }
 
@@ -220,6 +225,7 @@ private module ImplCommon {
           argumentValueFlowsThrough0(call, arg, kind, cc)
         |
           out = getAnOutNode(call, kind) and
+      not isUnreachableInCall(out, cc.(CallContextSpecificCall).getCall()) and
           compatibleTypes(arg.getType(), out.getType())
         )
       }
@@ -675,16 +681,14 @@ private module ImplCommon {
     abstract predicate validFor(Node n);
   }
 
-  LocalCallContext getMatchingLocalCallContext(CallContext ctx) {
-    (
-      not ctx instanceof CallContextSpecificCall or
-      not exists(TSpecificLocalCall(ctx.(CallContextSpecificCall).getCall()))
-  ) and
-  exists(LocalCallContextAny l | result = l)
-  or
-  exists(LocalCallContextSpecificCall l |
-    ctx.(CallContextSpecificCall).getCall() = l.getCall() and result = l
-  )
+/**
+ * Gets a matching local call context given the call context and a node which is in
+ * the callable the call is targeting.
+ */
+LocalCallContext getMatchingLocalCallContext(CallContext ctx, Node n) {
+  if hasUnreachableNode(ctx.(CallContextSpecificCall).getCall(), n.getEnclosingCallable())
+  then result.(LocalCallContextSpecificCall).getCall() = ctx.(CallContextSpecificCall).getCall()
+  else result instanceof LocalCallContextAny
 }
 
 class LocalCallContextAny extends LocalCallContext, TAnyLocalCall {

java/ql/test/library-tests/dataflow/call-sensitivity/A.java

@@ -3,6 +3,14 @@ public class A {
 	public static void sink(Object o) {
 	}
 
+	public Object flowThrough(Object o, boolean cond) {
+		if (cond) {
+			return o;
+		} else {
+			return null;
+		}
+	}
+
 	public void callSinkIfTrue(Object o, boolean cond) {
 		if (cond) {
 			sink(o);
@@ -49,6 +57,7 @@ public void f1() {
 		callSinkIfFalse(new Integer(2), true);
 		callSinkFromLoop(new Integer(3), false);
 		localCallSensitivity(new Integer(4), false);
+		sink(flowThrough(new Integer(4), false));
 		// should exhibit flow
 		callSinkIfTrue(new Integer(1), true);
 		callSinkIfFalse(new Integer(2), false);
@@ -57,6 +66,7 @@ public void f1() {
 		localCallSensitivity2(new Integer(4), true, true);
 		localCallSensitivity2(new Integer(4), false, true);
 		localCallSensitivity2(new Integer(4), true, false);
+		sink(flowThrough(new Integer(4), true));
 		// expected false positive
 		localCallSensitivity2(new Integer(4), false, false);
 	}
@@ -69,11 +79,13 @@ public void f2() {
 		callSinkIfFalse(new Integer(5), t);
 		callSinkFromLoop(new Integer(6), f);
 		localCallSensitivity(new Integer(4), f);
+		sink(flowThrough(new Integer(4), f));
 		// should exhibit flow
 		callSinkIfTrue(new Integer(4), t);
 		callSinkIfFalse(new Integer(5), f);
 		callSinkFromLoop(new Integer(6), t);
 		localCallSensitivity(new Integer(4), t);
+		sink(flowThrough(new Integer(4), t));
 	}
 
 	public void f3(InterfaceA b) {

java/ql/test/library-tests/dataflow/call-sensitivity/A2.java

@@ -0,0 +1,42 @@
+public class A2 {
+
+	public static void sink(Object o) {
+	}
+
+	public void m() {
+
+	}
+
+	public void callsite(InterfaceB intF) {
+		B b = new B();
+		// in both possible implementations of foo, this callsite is relevant
+		// in IntA, it improves virtual dispatch,
+		// and in IntB, it improves the dataflow analysis.
+		intF.foo(b, new Integer(5), false);
+	}
+
+	private class B extends A2 {
+		@Override
+		public void m() {
+
+		}
+	}
+
+	private class IntA implements InterfaceB {
+		@Override
+		public void foo(A2 obj, Object o, boolean cond) {
+			obj.m();
+			sink(o);
+		}
+	}
+
+	private class IntB implements InterfaceB {
+		@Override
+		public void foo(A2 obj, Object o, boolean cond) {
+			if (cond) {
+				sink(o);
+			}
+		}
+	}
+
+}

java/ql/test/library-tests/dataflow/call-sensitivity/InterfaceB.java

@@ -0,0 +1,3 @@
+public interface InterfaceB {
+	public void foo(A2 a, Object o, boolean cond);
+}