Skip to content

Commit e6d4e87

Browse files
authored
Merge pull request #10416 from aschackmull/java/dispatch-confidence
Java: Remove low confidence dispatch for which we have a manual summary.
2 parents bc93a22 + 9714497 commit e6d4e87

File tree

3 files changed

+74
-4
lines changed

3 files changed

+74
-4
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: majorAnalysis
3+
---
4+
* The virtual dispatch relation used in data flow now favors summary models over source code for dispatch to interface methods from `java.util` unless there is evidence that a specific source implementation is reachable. This should provide increased precision for any projects that include, for example, custom `List` or `Map` implementations.

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,27 @@ private import semmle.code.java.dataflow.TypeFlow
88
private import semmle.code.java.dispatch.internal.Unification
99

1010
private module DispatchImpl {
11+
private predicate hasHighConfidenceTarget(Call c) {
12+
exists(SummarizedCallable sc |
13+
sc = c.getCallee().getSourceDeclaration() and not sc.isAutoGenerated()
14+
)
15+
or
16+
exists(Callable srcTgt |
17+
srcTgt = VirtualDispatch::viableCallable(c) and
18+
not VirtualDispatch::lowConfidenceDispatchTarget(c, srcTgt)
19+
)
20+
}
21+
22+
private Callable sourceDispatch(Call c) {
23+
result = VirtualDispatch::viableCallable(c) and
24+
if VirtualDispatch::lowConfidenceDispatchTarget(c, result)
25+
then not hasHighConfidenceTarget(c)
26+
else any()
27+
}
28+
1129
/** Gets a viable implementation of the target of the given `Call`. */
1230
DataFlowCallable viableCallable(DataFlowCall c) {
13-
result.asCallable() = VirtualDispatch::viableCallable(c.asCall())
31+
result.asCallable() = sourceDispatch(c.asCall())
1432
or
1533
result.asSummarizedCallable() = c.asCall().getCallee().getSourceDeclaration()
1634
}
@@ -22,7 +40,7 @@ private module DispatchImpl {
2240
*/
2341
private predicate mayBenefitFromCallContext(MethodAccess ma, Callable c, int i) {
2442
exists(Parameter p |
25-
2 <= strictcount(VirtualDispatch::viableImpl(ma)) and
43+
2 <= strictcount(sourceDispatch(ma)) and
2644
ma.getQualifier().(VarAccess).getVariable() = p and
2745
p.getPosition() = i and
2846
c.getAParameter() = p and
@@ -31,7 +49,7 @@ private module DispatchImpl {
3149
)
3250
or
3351
exists(OwnInstanceAccess ia |
34-
2 <= strictcount(VirtualDispatch::viableImpl(ma)) and
52+
2 <= strictcount(sourceDispatch(ma)) and
3553
(ia.isExplicit(ma.getQualifier()) or ia.isImplicitMethodQualifier(ma)) and
3654
i = -1 and
3755
c = ma.getEnclosingCallable()
@@ -47,7 +65,7 @@ private module DispatchImpl {
4765
private predicate relevantContext(Call ctx, int i) {
4866
exists(Callable c |
4967
mayBenefitFromCallContext(_, c, i) and
50-
c = VirtualDispatch::viableCallable(ctx)
68+
c = sourceDispatch(ctx)
5169
)
5270
}
5371

java/ql/lib/semmle/code/java/dispatch/VirtualDispatch.qll

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ private module Dispatch {
5454
cached
5555
Method viableImpl(MethodAccess ma) { result = ObjFlow::viableImpl_out(ma) }
5656

57+
/**
58+
* Holds if `m` is a viable implementation of the method called in `ma` for
59+
* which we only have imprecise open-world type-based dispatch resolution, and
60+
* the dispatch type is likely to yield implausible dispatch targets.
61+
*/
62+
cached
63+
predicate lowConfidenceDispatchTarget(MethodAccess ma, Method m) {
64+
m = viableImpl(ma) and lowConfidenceDispatch(ma)
65+
}
66+
5767
/**
5868
* INTERNAL: Use `viableImpl` instead.
5969
*
@@ -62,6 +72,44 @@ private module Dispatch {
6272
cached
6373
Method viableImpl_v3(MethodAccess ma) { result = DispatchFlow::viableImpl_out(ma) }
6474

75+
/**
76+
* Holds if the best type bounds for the qualifier of `ma` are likely to
77+
* contain implausible dispatch targets.
78+
*/
79+
private predicate lowConfidenceDispatch(VirtualMethodAccess ma) {
80+
exists(RefType t | hasQualifierType(ma, t, false) |
81+
lowConfidenceDispatchType(t.getSourceDeclaration())
82+
) and
83+
(
84+
not qualType(ma, _, _)
85+
or
86+
exists(RefType t | qualType(ma, t, false) |
87+
lowConfidenceDispatchType(t.getSourceDeclaration())
88+
)
89+
) and
90+
(
91+
not qualUnionType(ma, _, _)
92+
or
93+
exists(RefType t | qualUnionType(ma, t, false) |
94+
lowConfidenceDispatchType(t.getSourceDeclaration())
95+
)
96+
)
97+
}
98+
99+
private predicate lowConfidenceDispatchType(SrcRefType t) {
100+
t instanceof TypeObject
101+
or
102+
t instanceof FunctionalInterface
103+
or
104+
t.hasQualifiedName("java.io", "Serializable")
105+
or
106+
t.hasQualifiedName("java.lang", "Cloneable")
107+
or
108+
t.getPackage().hasName("java.util") and t instanceof Interface
109+
or
110+
t.hasQualifiedName("java.util", "Hashtable")
111+
}
112+
65113
/**
66114
* INTERNAL: Use `viableImpl` instead.
67115
*

0 commit comments

Comments
 (0)