From 812a0bcb16c0152d0382d0904df8cb69ef3dcf23 Mon Sep 17 00:00:00 2001 From: Cornelius Riemenschneider Date: Mon, 30 Sep 2019 13:50:44 +0200 Subject: [PATCH] Address some parts of Anders' review. --- .../java/dataflow/internal/DataFlowImpl.qll | 10 ++++-- .../java/dataflow/internal/DataFlowImpl2.qll | 10 ++++-- .../java/dataflow/internal/DataFlowImpl3.qll | 10 ++++-- .../java/dataflow/internal/DataFlowImpl4.qll | 10 ++++-- .../java/dataflow/internal/DataFlowImpl5.qll | 10 ++++-- .../dataflow/internal/DataFlowImplCommon.qll | 36 +++++++++---------- 6 files changed, 53 insertions(+), 33 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll index 4391241bb71..acdcd4dfe6d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll @@ -917,6 +917,7 @@ private predicate localFlowStepPlus( ) and node1 != node2 and cc.validFor(node1) and + not isUnreachableInCall(node1, cc.(LocalCallContextSpecificCall).getCall()) and nodeCand(node2, unbind(config)) or exists(Node mid | @@ -1733,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.matchesCallContext(cc) | + exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) | localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and cc = mid.getCallContext() and ap = mid.getAp() @@ -1744,8 +1745,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat ap = node.(AccessPathNilNode).getAp() ) or - not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and - ( jumpStep(mid.getNode(), node, mid.getConfiguration()) and cc instanceof CallContextAny and ap = mid.getAp() @@ -1755,6 +1754,8 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat mid.getAp() instanceof AccessPathNil and ap = node.(AccessPathNilNode).getAp() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( contentReadStep(mid, node, ap) and cc = mid.getCallContext() or exists(Content f, AccessPath ap0 | contentStoreStep(mid, node, ap0, f, cc) and push(ap0, f, ap)) @@ -2236,6 +2237,8 @@ private module FlowExploration { ap = TPartialNil(getErasedRepr(node.getType())) and config = mid.getConfiguration() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( partialPathStoreStep(mid, _, _, node, ap) and cc = mid.getCallContext() and config = mid.getConfiguration() @@ -2254,6 +2257,7 @@ private module FlowExploration { partialPathThroughCallable(mid, node, cc, ap, config) or valuePartialPathThroughCallable(mid, node, cc, ap, config) + ) } bindingset[result, i] diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll index 4391241bb71..acdcd4dfe6d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll @@ -917,6 +917,7 @@ private predicate localFlowStepPlus( ) and node1 != node2 and cc.validFor(node1) and + not isUnreachableInCall(node1, cc.(LocalCallContextSpecificCall).getCall()) and nodeCand(node2, unbind(config)) or exists(Node mid | @@ -1733,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.matchesCallContext(cc) | + exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) | localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and cc = mid.getCallContext() and ap = mid.getAp() @@ -1744,8 +1745,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat ap = node.(AccessPathNilNode).getAp() ) or - not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and - ( jumpStep(mid.getNode(), node, mid.getConfiguration()) and cc instanceof CallContextAny and ap = mid.getAp() @@ -1755,6 +1754,8 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat mid.getAp() instanceof AccessPathNil and ap = node.(AccessPathNilNode).getAp() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( contentReadStep(mid, node, ap) and cc = mid.getCallContext() or exists(Content f, AccessPath ap0 | contentStoreStep(mid, node, ap0, f, cc) and push(ap0, f, ap)) @@ -2236,6 +2237,8 @@ private module FlowExploration { ap = TPartialNil(getErasedRepr(node.getType())) and config = mid.getConfiguration() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( partialPathStoreStep(mid, _, _, node, ap) and cc = mid.getCallContext() and config = mid.getConfiguration() @@ -2254,6 +2257,7 @@ private module FlowExploration { partialPathThroughCallable(mid, node, cc, ap, config) or valuePartialPathThroughCallable(mid, node, cc, ap, config) + ) } bindingset[result, i] diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll index 4391241bb71..acdcd4dfe6d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll @@ -917,6 +917,7 @@ private predicate localFlowStepPlus( ) and node1 != node2 and cc.validFor(node1) and + not isUnreachableInCall(node1, cc.(LocalCallContextSpecificCall).getCall()) and nodeCand(node2, unbind(config)) or exists(Node mid | @@ -1733,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.matchesCallContext(cc) | + exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) | localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and cc = mid.getCallContext() and ap = mid.getAp() @@ -1744,8 +1745,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat ap = node.(AccessPathNilNode).getAp() ) or - not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and - ( jumpStep(mid.getNode(), node, mid.getConfiguration()) and cc instanceof CallContextAny and ap = mid.getAp() @@ -1755,6 +1754,8 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat mid.getAp() instanceof AccessPathNil and ap = node.(AccessPathNilNode).getAp() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( contentReadStep(mid, node, ap) and cc = mid.getCallContext() or exists(Content f, AccessPath ap0 | contentStoreStep(mid, node, ap0, f, cc) and push(ap0, f, ap)) @@ -2236,6 +2237,8 @@ private module FlowExploration { ap = TPartialNil(getErasedRepr(node.getType())) and config = mid.getConfiguration() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( partialPathStoreStep(mid, _, _, node, ap) and cc = mid.getCallContext() and config = mid.getConfiguration() @@ -2254,6 +2257,7 @@ private module FlowExploration { partialPathThroughCallable(mid, node, cc, ap, config) or valuePartialPathThroughCallable(mid, node, cc, ap, config) + ) } bindingset[result, i] diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll index 4391241bb71..acdcd4dfe6d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll @@ -917,6 +917,7 @@ private predicate localFlowStepPlus( ) and node1 != node2 and cc.validFor(node1) and + not isUnreachableInCall(node1, cc.(LocalCallContextSpecificCall).getCall()) and nodeCand(node2, unbind(config)) or exists(Node mid | @@ -1733,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.matchesCallContext(cc) | + exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) | localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and cc = mid.getCallContext() and ap = mid.getAp() @@ -1744,8 +1745,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat ap = node.(AccessPathNilNode).getAp() ) or - not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and - ( jumpStep(mid.getNode(), node, mid.getConfiguration()) and cc instanceof CallContextAny and ap = mid.getAp() @@ -1755,6 +1754,8 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat mid.getAp() instanceof AccessPathNil and ap = node.(AccessPathNilNode).getAp() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( contentReadStep(mid, node, ap) and cc = mid.getCallContext() or exists(Content f, AccessPath ap0 | contentStoreStep(mid, node, ap0, f, cc) and push(ap0, f, ap)) @@ -2236,6 +2237,8 @@ private module FlowExploration { ap = TPartialNil(getErasedRepr(node.getType())) and config = mid.getConfiguration() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( partialPathStoreStep(mid, _, _, node, ap) and cc = mid.getCallContext() and config = mid.getConfiguration() @@ -2254,6 +2257,7 @@ private module FlowExploration { partialPathThroughCallable(mid, node, cc, ap, config) or valuePartialPathThroughCallable(mid, node, cc, ap, config) + ) } bindingset[result, i] diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll index 4391241bb71..acdcd4dfe6d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll @@ -917,6 +917,7 @@ private predicate localFlowStepPlus( ) and node1 != node2 and cc.validFor(node1) and + not isUnreachableInCall(node1, cc.(LocalCallContextSpecificCall).getCall()) and nodeCand(node2, unbind(config)) or exists(Node mid | @@ -1733,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.matchesCallContext(cc) | + exists(LocalCallContext localCC | localCC = getMatchingLocalCallContext(cc) | localFlowBigStep(mid.getNode(), node, true, mid.getConfiguration(), localCC) and cc = mid.getCallContext() and ap = mid.getAp() @@ -1744,8 +1745,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat ap = node.(AccessPathNilNode).getAp() ) or - not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and - ( jumpStep(mid.getNode(), node, mid.getConfiguration()) and cc instanceof CallContextAny and ap = mid.getAp() @@ -1755,6 +1754,8 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat mid.getAp() instanceof AccessPathNil and ap = node.(AccessPathNilNode).getAp() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( contentReadStep(mid, node, ap) and cc = mid.getCallContext() or exists(Content f, AccessPath ap0 | contentStoreStep(mid, node, ap0, f, cc) and push(ap0, f, ap)) @@ -2236,6 +2237,8 @@ private module FlowExploration { ap = TPartialNil(getErasedRepr(node.getType())) and config = mid.getConfiguration() or + not isUnreachableInCall(node, cc.(CallContextSpecificCall).getCall()) and + ( partialPathStoreStep(mid, _, _, node, ap) and cc = mid.getCallContext() and config = mid.getConfiguration() @@ -2254,6 +2257,7 @@ private module FlowExploration { partialPathThroughCallable(mid, node, cc, ap, config) or valuePartialPathThroughCallable(mid, node, cc, ap, config) + ) } bindingset[result, i] diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll index 69917ccadbf..bca829ab395 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @@ -576,8 +576,9 @@ private module ImplCommon { } /** - * Record a call site in the dataflow graph if it either improves - * virtual dispatch or if we can remove unreachable edges by recoring this call site + * Holds if recording a dataflow call site either improves + * virtual dispatch or if we can remove unreachable edges in the dataflow graph + * by recoring this call site */ cached predicate recordDataFlowCallSite(DataFlowCall call, DataFlowCallable callable) { @@ -626,7 +627,7 @@ private module ImplCommon { * - `TSpecificCall(DataFlowCall call, int i)` : Flow entered through the `i`th * parameter at the given `call`. This call improves the set of viable * dispatch targets for at least one method call in the current callable - * or helps pruning unreachable nodes from the data flow graph. + * or helps to prune unreachable nodes from the data flow graph. * - `TSomeCall(ParameterNode p)` : Flow entered through parameter `p`. The * originating call does not improve the set of dispatch targets for any * method call in the current callable and was therefore not recorded. @@ -665,24 +666,29 @@ private module ImplCommon { } /** - * A call context which is used to restrict local data flow nodes + * A call context that is used to restrict local data flow nodes * to nodes which are actually reachable in a call context. */ abstract class LocalCallContext extends TLocalFlowCallContext { abstract string toString(); - abstract predicate matchesCallContext(CallContext ctx); - abstract predicate validFor(Node n); } - class LocalCallContextAny extends LocalCallContext, TAnyLocalCall { - override string toString() { result = "LocalCcAny" } - - override predicate matchesCallContext(CallContext ctx) { + 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 + ) +} + +class LocalCallContextAny extends LocalCallContext, TAnyLocalCall { + override string toString() { result = "LocalCcAny" } override predicate validFor(Node n) { any() } } @@ -701,13 +707,7 @@ private module ImplCommon { override string toString() { result = "LocalCcCall(" + call + ")" } - override predicate matchesCallContext(CallContext ctx) { - ctx.(CallContextSpecificCall).getCall() = call - } - - override predicate validFor(Node n) { - hasUnreachableNode(call, n.getEnclosingCallable()) - } + override predicate validFor(Node n) { hasUnreachableNode(call, n.getEnclosingCallable()) } } /** A callable tagged with a relevant return kind. */