Skip to content

Commit bc3d141

Browse files
authored
Don't update CHACallGraph internal target cache from CallGraph getter methods (#1473)
We should only update `targetCache` when computing the call graph, not when reading its current state. When debugging another issue, I ran into very confusing behavior where the debugger invoked `toString()` on a `CHACallGraph` to display its state, which ended up updating `targetCache` (whose state I was trying to debug) by calling an accessor method.
1 parent a44aa59 commit bc3d141

File tree

1 file changed

+24
-4
lines changed

1 file changed

+24
-4
lines changed

core/src/main/java/com/ibm/wala/ipa/callgraph/cha/CHACallGraph.java

+24-4
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,13 @@ public IClassHierarchy getClassHierarchy() {
148148

149149
private final Map<CallSiteReference, Set<IMethod>> targetCache = HashMapFactory.make();
150150

151-
private Iterator<IMethod> getPossibleTargets(CallSiteReference site) {
151+
/**
152+
* Gets the possible targets of a call site, caching the result if it has not been computed.
153+
*
154+
* @param site the call site
155+
* @return an iterator of possible targets
156+
*/
157+
private Iterator<IMethod> getOrUpdatePossibleTargets(CallSiteReference site) {
152158
Set<IMethod> result = targetCache.get(site);
153159
if (result == null) {
154160
if (site.isDispatch()) {
@@ -166,11 +172,25 @@ private Iterator<IMethod> getPossibleTargets(CallSiteReference site) {
166172
return result.iterator();
167173
}
168174

175+
/**
176+
* Gets the possible targets of a call site from the cache.
177+
*
178+
* @param site the call site
179+
* @return an iterator of possible targets
180+
*/
181+
private Iterator<IMethod> getPossibleTargetsFromCache(CallSiteReference site) {
182+
Set<IMethod> result = targetCache.get(site);
183+
if (result == null) {
184+
return Collections.emptyIterator();
185+
}
186+
return result.iterator();
187+
}
188+
169189
@Override
170190
public Set<CGNode> getPossibleTargets(CGNode node, CallSiteReference site) {
171191
return Iterator2Collection.toSet(
172192
new MapIterator<>(
173-
new FilterIterator<>(getPossibleTargets(site), this::isRelevantMethod),
193+
new FilterIterator<>(getPossibleTargetsFromCache(site), this::isRelevantMethod),
174194
object -> {
175195
try {
176196
return findOrCreateNode(object, Everywhere.EVERYWHERE);
@@ -183,7 +203,7 @@ public Set<CGNode> getPossibleTargets(CGNode node, CallSiteReference site) {
183203

184204
@Override
185205
public int getNumberOfTargets(CGNode node, CallSiteReference site) {
186-
return IteratorUtil.count(getPossibleTargets(site));
206+
return IteratorUtil.count(getPossibleTargetsFromCache(site));
187207
}
188208

189209
@Override
@@ -259,7 +279,7 @@ private void closure() throws CancelException {
259279
while (!newNodes.isEmpty()) {
260280
CGNode n = newNodes.pop();
261281
for (CallSiteReference site : Iterator2Iterable.make(n.iterateCallSites())) {
262-
Iterator<IMethod> methods = getPossibleTargets(site);
282+
Iterator<IMethod> methods = getOrUpdatePossibleTargets(site);
263283
while (methods.hasNext()) {
264284
IMethod target = methods.next();
265285
if (isRelevantMethod(target)) {

0 commit comments

Comments
 (0)