Skip to content

Commit dc94503

Browse files
committed
Shared: use a call bit when tracking reachability to/from a discriminator
1 parent 5324305 commit dc94503

File tree

2 files changed

+100
-30
lines changed

2 files changed

+100
-30
lines changed

java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.expected

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,27 @@ nodes
77
| A.java:15:29:15:29 | x : String | semmle.label | x : String |
88
| A.java:17:20:17:27 | source(...) : String | semmle.label | source(...) : String |
99
| A.java:18:20:18:38 | apply(...) : String | semmle.label | apply(...) : String |
10+
| A.java:18:20:18:38 | apply(...) : String | semmle.label | apply(...) : String |
1011
| A.java:18:33:18:37 | step0 : String | semmle.label | step0 : String |
1112
| A.java:19:20:19:38 | apply(...) : String | semmle.label | apply(...) : String |
1213
| A.java:19:33:19:37 | step1 : String | semmle.label | step1 : String |
14+
| A.java:19:33:19:37 | step1 : String | semmle.label | step1 : String |
1315
| A.java:21:10:21:14 | step2 | semmle.label | step2 |
1416
| A.java:26:8:26:8 | x : String | semmle.label | x : String |
17+
| A.java:26:8:26:8 | x : String | semmle.label | x : String |
18+
| A.java:26:13:26:81 | ...?...:... : String | semmle.label | ...?...:... : String |
1519
| A.java:26:13:26:81 | ...?...:... : String | semmle.label | ...?...:... : String |
1620
| A.java:26:35:26:56 | propagateState(...) : String | semmle.label | propagateState(...) : String |
1721
| A.java:26:50:26:50 | x : String | semmle.label | x : String |
1822
| A.java:26:60:26:81 | propagateState(...) : String | semmle.label | propagateState(...) : String |
1923
| A.java:26:75:26:75 | x : String | semmle.label | x : String |
2024
| A.java:28:20:28:27 | source(...) : String | semmle.label | source(...) : String |
2125
| A.java:29:20:29:38 | apply(...) : String | semmle.label | apply(...) : String |
26+
| A.java:29:20:29:38 | apply(...) : String | semmle.label | apply(...) : String |
2227
| A.java:29:33:29:37 | step0 : String | semmle.label | step0 : String |
2328
| A.java:30:20:30:38 | apply(...) : String | semmle.label | apply(...) : String |
2429
| A.java:30:33:30:37 | step1 : String | semmle.label | step1 : String |
30+
| A.java:30:33:30:37 | step1 : String | semmle.label | step1 : String |
2531
| A.java:32:10:32:14 | step2 | semmle.label | step2 |
2632
edges
2733
| A.java:14:9:14:9 | x : String | A.java:14:29:14:29 | x : String |
@@ -30,13 +36,16 @@ edges
3036
| A.java:15:29:15:29 | x : String | A.java:15:14:15:35 | propagateState(...) : String |
3137
| A.java:17:20:17:27 | source(...) : String | A.java:18:33:18:37 | step0 : String |
3238
| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String |
39+
| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String |
3340
| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String |
3441
| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String |
3542
| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String |
43+
| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String |
3644
| A.java:19:20:19:38 | apply(...) : String | A.java:21:10:21:14 | step2 |
3745
| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String |
3846
| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String |
3947
| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String |
48+
| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String |
4049
| A.java:26:8:26:8 | x : String | A.java:26:50:26:50 | x : String |
4150
| A.java:26:8:26:8 | x : String | A.java:26:75:26:75 | x : String |
4251
| A.java:26:35:26:56 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String |
@@ -45,23 +54,26 @@ edges
4554
| A.java:26:75:26:75 | x : String | A.java:26:60:26:81 | propagateState(...) : String |
4655
| A.java:28:20:28:27 | source(...) : String | A.java:29:33:29:37 | step0 : String |
4756
| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String |
57+
| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String |
4858
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String |
59+
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String |
60+
| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String |
4961
| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String |
5062
| A.java:30:20:30:38 | apply(...) : String | A.java:32:10:32:14 | step2 |
5163
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String |
64+
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String |
65+
| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String |
5266
| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String |
5367
subpaths
5468
| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String | A.java:14:14:14:35 | propagateState(...) : String | A.java:18:20:18:38 | apply(...) : String |
5569
| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String | A.java:15:14:15:35 | propagateState(...) : String | A.java:18:20:18:38 | apply(...) : String |
5670
| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String | A.java:14:14:14:35 | propagateState(...) : String | A.java:19:20:19:38 | apply(...) : String |
5771
| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String | A.java:15:14:15:35 | propagateState(...) : String | A.java:19:20:19:38 | apply(...) : String |
5872
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:29:20:29:38 | apply(...) : String |
73+
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:29:20:29:38 | apply(...) : String |
74+
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:30:20:30:38 | apply(...) : String |
5975
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:30:20:30:38 | apply(...) : String |
6076
spuriousFlow
61-
| A.java:14:14:14:35 | propagateState(...) : String | B | A |
62-
| A.java:15:14:15:35 | propagateState(...) : String | A | B |
63-
| A.java:26:35:26:56 | propagateState(...) : String | B | A |
64-
| A.java:26:60:26:81 | propagateState(...) : String | A | B |
6577
#select
6678
| A.java:17:20:17:27 | source(...) : String | A.java:17:20:17:27 | source(...) : String | A.java:21:10:21:14 | step2 | Flow |
6779
| A.java:28:20:28:27 | source(...) : String | A.java:28:20:28:27 | source(...) : String | A.java:32:10:32:14 | step2 | Flow |

shared/dataflow/codeql/dataflow/DataFlow.qll

Lines changed: 84 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -707,22 +707,6 @@ module DataFlowMake<InputSig Lang> {
707707
// non-sink PathNode with the same `(node, toString)` value and the same successors, but is transitively
708708
// reachable from a different set of PathNodes. (And conversely for sources).
709709
//
710-
/**
711-
* Gets a successor of `node`, taking `subpaths` into account.
712-
*/
713-
private InputPathNode getASuccessorLike(InputPathNode node) {
714-
Graph::edges(node, result)
715-
or
716-
Graph::subpaths(node, _, _, result) // arg -> out
717-
//
718-
// Note that there is no case for `arg -> param` or `ret -> out` for subpaths.
719-
// It is OK to collapse nodes inside a subpath while calls to that subpaths aren't collapsed and vice versa.
720-
}
721-
722-
private InputPathNode getAPredecessorLike(InputPathNode node) {
723-
node = getASuccessorLike(result)
724-
}
725-
726710
pragma[nomagic]
727711
private InputPathNode getAPathNode(Node node, string toString) {
728712
result.getNode() = node and
@@ -731,7 +715,11 @@ module DataFlowMake<InputSig Lang> {
731715

732716
private signature predicate collapseCandidateSig(Node node, string toString);
733717

734-
private signature InputPathNode stepSig(InputPathNode node);
718+
private signature predicate stepSig(InputPathNode node1, InputPathNode node2);
719+
720+
private signature predicate subpathStepSig(
721+
InputPathNode arg, InputPathNode param, InputPathNode ret, InputPathNode out
722+
);
735723

736724
/**
737725
* Performs a forward or backward pass computing which `(node, toString)` pairs can subsume their corresponding
@@ -744,12 +732,14 @@ module DataFlowMake<InputSig Lang> {
744732
* Comments are written as if this checks for outgoing edges and propagates backward, though the module is also
745733
* used to perform the opposite direction.
746734
*/
747-
private module MakeDiscriminatorPass<collapseCandidateSig/2 collapseCandidate, stepSig/1 step> {
735+
private module MakeDiscriminatorPass<
736+
collapseCandidateSig/2 collapseCandidate, stepSig/2 step, subpathStepSig/4 subpathStep>
737+
{
748738
/**
749739
* Gets the number of `(node, toString)` pairs reachable in one step from `pathNode`.
750740
*/
751741
private int getOutDegreeFromPathNode(InputPathNode pathNode) {
752-
result = count(Node node, string toString | step(pathNode) = getAPathNode(node, toString))
742+
result = count(Node node, string toString | step(pathNode, getAPathNode(node, toString)))
753743
}
754744

755745
/**
@@ -758,13 +748,46 @@ module DataFlowMake<InputSig Lang> {
758748
private int getOutDegreeFromNode(Node node, string toString) {
759749
result =
760750
strictcount(Node node2, string toString2 |
761-
step(getAPathNode(node, toString)) = getAPathNode(node2, toString2)
751+
step(getAPathNode(node, toString), getAPathNode(node2, toString2))
752+
)
753+
}
754+
755+
/**
756+
* Like `getOutDegreeFromPathNode` except counts `subpath` tuples.
757+
*/
758+
private int getSubpathOutDegreeFromPathNode(InputPathNode pathNode) {
759+
result =
760+
count(Node n1, string s1, Node n2, string s2, Node n3, string s3 |
761+
subpathStep(pathNode, getAPathNode(n1, s1), getAPathNode(n2, s2), getAPathNode(n3, s3))
762762
)
763763
}
764764

765+
/**
766+
* Like `getOutDegreeFromNode` except counts `subpath` tuples.
767+
*/
768+
private int getSubpathOutDegreeFromNode(Node node, string toString) {
769+
result =
770+
strictcount(Node n1, string s1, Node n2, string s2, Node n3, string s3 |
771+
subpathStep(getAPathNode(node, toString), getAPathNode(n1, s1), getAPathNode(n2, s2),
772+
getAPathNode(n3, s3))
773+
)
774+
}
775+
776+
/** Gets a successor of `node` including subpath flow-through. */
777+
InputPathNode stepEx(InputPathNode node) {
778+
step(node, result)
779+
or
780+
subpathStep(node, _, _, result) // assuming the input is pruned properly, all subpaths have flow-through
781+
}
782+
783+
InputPathNode enterSubpathStep(InputPathNode node) { subpathStep(node, result, _, _) }
784+
785+
InputPathNode exitSubpathStep(InputPathNode node) { subpathStep(_, _, node, result) }
786+
765787
/** Holds if `(node, toString)` cannot be collapsed (but was a candidate for being collapsed). */
766-
predicate discriminatedPair(Node node, string toString) {
788+
predicate discriminatedPair(Node node, string toString, boolean hasEnter) {
767789
collapseCandidate(node, toString) and
790+
hasEnter = false and
768791
(
769792
// Check if all corresponding PathNodes have the same successor sets when projected to `(node, toString)`.
770793
// To do this, we check that each successor set has the same size as the union of the succesor sets.
@@ -774,27 +797,62 @@ module DataFlowMake<InputSig Lang> {
774797
getOutDegreeFromPathNode(getAPathNode(node, toString)) <
775798
getOutDegreeFromNode(node, toString)
776799
or
800+
// Same as above but counting associated subpath triples instead
801+
getSubpathOutDegreeFromPathNode(getAPathNode(node, toString)) <
802+
getSubpathOutDegreeFromNode(node, toString)
803+
)
804+
or
805+
collapseCandidate(node, toString) and
806+
(
777807
// Retain flow state if one of the successors requires it to be retained
778-
discriminatedPathNode(step(getAPathNode(node, toString)))
808+
discriminatedPathNode(stepEx(getAPathNode(node, toString)), hasEnter)
809+
or
810+
// Enter a subpath
811+
discriminatedPathNode(enterSubpathStep(getAPathNode(node, toString)), _) and
812+
hasEnter = true
813+
or
814+
// Exit a subpath
815+
discriminatedPathNode(exitSubpathStep(getAPathNode(node, toString)), false) and
816+
hasEnter = false
779817
)
780818
}
781819

782820
/** Holds if `pathNode` cannot be collapsed. */
783-
predicate discriminatedPathNode(InputPathNode pathNode) {
821+
private predicate discriminatedPathNode(InputPathNode pathNode, boolean hasEnter) {
784822
exists(Node node, string toString |
785-
discriminatedPair(node, toString) and
823+
discriminatedPair(node, toString, hasEnter) and
786824
getAPathNode(node, toString) = pathNode
787825
)
788826
}
827+
828+
/** Holds if `(node, toString)` cannot be collapsed (but was a candidate for being collapsed). */
829+
predicate discriminatedPair(Node node, string toString) {
830+
discriminatedPair(node, toString, _)
831+
}
832+
833+
/** Holds if `pathNode` cannot be collapsed. */
834+
predicate discriminatedPathNode(InputPathNode pathNode) { discriminatedPathNode(pathNode, _) }
789835
}
790836

791837
private predicate initialCandidate(Node node, string toString) {
792838
exists(getAPathNode(node, toString))
793839
}
794840

795-
private module Pass1 = MakeDiscriminatorPass<initialCandidate/2, getASuccessorLike/1>;
841+
private module Pass1 =
842+
MakeDiscriminatorPass<initialCandidate/2, Graph::edges/2, Graph::subpaths/4>;
843+
844+
private predicate edgesRev(InputPathNode node1, InputPathNode node2) {
845+
Graph::edges(node2, node1)
846+
}
847+
848+
private predicate subpathsRev(
849+
InputPathNode n1, InputPathNode n2, InputPathNode n3, InputPathNode n4
850+
) {
851+
Graph::subpaths(n4, n3, n2, n1)
852+
}
796853

797-
private module Pass2 = MakeDiscriminatorPass<Pass1::discriminatedPair/2, getAPredecessorLike/1>;
854+
private module Pass2 =
855+
MakeDiscriminatorPass<Pass1::discriminatedPair/2, edgesRev/2, subpathsRev/4>;
798856

799857
private newtype TPathNode =
800858
TPreservedPathNode(InputPathNode node) { Pass2::discriminatedPathNode(node) } or

0 commit comments

Comments
 (0)