Fix path appending from cache

Author: t1mlange

soot-infoflow/src/soot/jimple/infoflow/data/SourceContextAndPath.java

@@ -1,9 +1,6 @@
 package soot.jimple.infoflow.data;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 import heros.solver.Pair;
 import soot.jimple.Stmt;
@@ -90,19 +87,19 @@ public SourceContextAndPath extendPath(SourceContextAndPath other) {
 		if (this.path == null || other.path == null || other.path.size() <= this.path.size())
 			return null;
 
-		ArrayList<Abstraction> buf = new ArrayList<>(other.path.size() - this.path.size());
+		Stack<Abstraction> pathStack = new Stack<>();
 		Abstraction lastAbs = this.getLastAbstraction();
 		boolean foundCommonAbs = false;
 
 		// Collect all additional abstractions on the cached path
 		Iterator<Abstraction> pathIt = other.path.reverseIterator();
 		while (pathIt.hasNext()) {
 			Abstraction next = pathIt.next();
-			if (next == lastAbs) {
+			if (next == lastAbs || (next.neighbors != null && next.neighbors.contains(lastAbs))) {
 				foundCommonAbs = true;
 				break;
 			}
-			buf.add(next);
+			pathStack.push(next);
 		}
 
 		// If the paths do not have a common abstraction, there's probably something wrong...
@@ -111,15 +108,15 @@ public SourceContextAndPath extendPath(SourceContextAndPath other) {
 
 		// Append the additional abstractions to the new taint propagation path
 		SourceContextAndPath extendedScap = clone();
-		for (Abstraction abs : buf)
-			extendedScap.path.add(abs);
+		while (!pathStack.isEmpty())
+			extendedScap.path.add(pathStack.pop());
 
 		int newCallStackCapacity = other.getCallStackSize() - this.getCallStackSize();
 		// Sanity Check: The callStack of other should always be larger than the one of this
 		if (newCallStackCapacity < 0)
 			return null;
 		if (newCallStackCapacity > 0) {
-			ArrayList<Stmt> callStackBuf = new ArrayList<>(newCallStackCapacity);
+			Stack<Stmt> callStackBuf = new Stack<>();
 			Stmt topStmt = this.callStack == null ? null : this.callStack.getLast();
 
 			// Collect all additional statements on the call stack...
@@ -128,15 +125,15 @@ public SourceContextAndPath extendPath(SourceContextAndPath other) {
 				Stmt next = callStackIt.next();
 				if (next == topStmt)
 					break;
-				callStackBuf.add(next);
+				callStackBuf.push(next);
 			}
 
 			if (callStackBuf.size() > 0) {
 				if (extendedScap.callStack == null)
 					extendedScap.callStack = new ExtensibleList<>();
 				// ...and append them.
-				for (Stmt stmt : callStackBuf)
-					extendedScap.callStack.add(stmt);
+				while (!callStackBuf.isEmpty())
+					extendedScap.callStack.add(callStackBuf.pop());
 			}
 		}
 

soot-infoflow/src/soot/jimple/infoflow/data/pathBuilders/ContextSensitivePathBuilder.java

@@ -33,7 +33,9 @@ public class ContextSensitivePathBuilder extends ConcurrentAbstractionPathBuilde
 	protected ConcurrentIdentityHashMultiMap<Abstraction, SourceContextAndPath> pathCache = new ConcurrentIdentityHashMultiMap<>();
 
 	// Set holds all paths that reach an already cached subpath
-	protected ConcurrentHashSet<Pair<SourceContextAndPath, SourceContextAndPath>> deferredPaths = new ConcurrentHashSet<>();
+	protected ConcurrentHashSet<SourceContextAndPath> deferredPaths = new ConcurrentHashSet<>();
+	// Set holds all paths that reach a source
+	protected ConcurrentHashSet<SourceContextAndPath> sourceReachingScaps = new ConcurrentHashSet<>();
 
 	/**
 	 * Creates a new instance of the {@link ContextSensitivePathBuilder} class
@@ -78,12 +80,6 @@ public void run() {
 			final Set<SourceContextAndPath> paths = pathCache.get(abstraction);
 			Abstraction pred = abstraction.getPredecessor();
 
-			// Skip abstractions that don't contain any new information. This might
-			// be the case when a turn unit was added to the abstraction.
-			while (pred != null && pred.getCurrentStmt() == null && pred.getCorrespondingCallSite() == null) {
-				pred = pred.getPredecessor();
-			}
-
 			if (pred != null && paths != null) {
 				for (SourceContextAndPath scap : paths) {
 					// Process the predecessor
@@ -110,7 +106,7 @@ private void processAndQueue(Abstraction pred, SourceContextAndPath scap) {
 			case CACHED:
 				// In case we already know the subpath, we do append the path after the path
 				// builder terminated
-				deferredPaths.add(new Pair<>(scap, p.getScap()));
+				deferredPaths.add(scap);
 				break;
 			case INFEASIBLE_OR_MAX_PATHS_REACHED:
 				// Nothing to do
@@ -128,7 +124,8 @@ private ProcessingResult processPredecessor(SourceContextAndPath scap, Abstracti
 				if (extendedScap == null)
 					return ProcessingResult.INFEASIBLE_OR_MAX_PATHS_REACHED();
 
-				checkForSource(pred, extendedScap);
+				if (checkForSource(pred, extendedScap))
+					sourceReachingScaps.add(extendedScap);
 				return pathCache.put(pred, extendedScap) ? ProcessingResult.NEW()
 						: ProcessingResult.CACHED(extendedScap);
 			}
@@ -176,7 +173,8 @@ private ProcessingResult processPredecessor(SourceContextAndPath scap, Abstracti
 			}
 
 			// Add the new path
-			checkForSource(pred, extendedScap);
+			if (checkForSource(pred, extendedScap))
+				sourceReachingScaps.add(extendedScap);
 
 			final int maxPaths = config.getPathConfiguration().getMaxPathsPerAbstraction();
 			if (maxPaths > 0) {
@@ -289,27 +287,14 @@ public void computeTaintPaths(Set<AbstractionAtSink> res) {
 	}
 
 	/**
-	 * Uses the cached path to extend the current path
-	 *
-	 * @param scap SourceContextAndPath of the current abstraction
-	 * @param cachedScap cached SourceContextAndPath to extend scap
+	 * Tries to fill up deferred paths toward a source.
 	 */
-	protected void buildFullPathFromCache(SourceContextAndPath scap, SourceContextAndPath cachedScap) {
-		// Try to extend scap with cachedScap
-		Stack<Pair<SourceContextAndPath, SourceContextAndPath>> workStack = new Stack<>();
-		workStack.push(new Pair<>(scap, cachedScap));
-		while (!workStack.isEmpty()) {
-			Pair<SourceContextAndPath, SourceContextAndPath> p = workStack.pop();
-			scap = p.getO1();
-			cachedScap = p.getO2();
-
-			SourceContextAndPath extendedScap = scap.extendPath(cachedScap);
-			if (extendedScap != null) {
-				Abstraction last = extendedScap.getLastAbstraction();
-				// Try to build the path further using the cache if we didn't reach a source
-				if (!checkForSource(last, extendedScap))
-					for (SourceContextAndPath preds : pathCache.get(last.getPredecessor()))
-						workStack.push(new Pair<>(extendedScap, preds));
+	protected void buildPathsFromCache() {
+		for (SourceContextAndPath deferredScap : deferredPaths) {
+			for (SourceContextAndPath sourceScap : sourceReachingScaps) {
+				SourceContextAndPath fullScap = deferredScap.extendPath(sourceScap);
+				if (fullScap != null)
+					checkForSource(fullScap.getLastAbstraction(), fullScap);
 			}
 		}
 	}
@@ -318,9 +303,7 @@ protected void buildFullPathFromCache(SourceContextAndPath scap, SourceContextAn
 	 * Method that is called when the taint paths have been computed
 	 */
 	protected void onTaintPathsComputed() {
-		for (Pair<SourceContextAndPath, SourceContextAndPath> deferredPair : deferredPaths) {
-			buildFullPathFromCache(deferredPair.getO1(), deferredPair.getO2());
-		}
+		buildPathsFromCache();
 	}
 
 	/**