From e2f028266bb88179705d4971fd501a34b99d2479 Mon Sep 17 00:00:00 2001 From: Carlos Galindo Date: Sat, 14 Mar 2020 14:15:47 +0100 Subject: [PATCH] Revert "Reduce complexity of control dependency computation" This reverts commit 14b8ae0e, which computed control dependencies faster but incorrectly. --- src/main/java/tfm/graphs/Graph.java | 3 +- .../tfm/graphs/augmented/ACFGBuilder.java | 3 +- src/main/java/tfm/graphs/cfg/CFG.java | 20 -- src/main/java/tfm/graphs/cfg/CFGBuilder.java | 3 +- .../graphs/pdg/ControlDependencyBuilder.java | 85 ++++++- .../java/tfm/graphs/pdg/DirectedTree.java | 24 -- src/main/java/tfm/graphs/pdg/PDGBuilder.java | 1 - .../tfm/graphs/pdg/PostdominatorTree.java | 222 ------------------ src/main/java/tfm/nodes/GraphNode.java | 7 +- 9 files changed, 80 insertions(+), 288 deletions(-) delete mode 100644 src/main/java/tfm/graphs/pdg/DirectedTree.java delete mode 100644 src/main/java/tfm/graphs/pdg/PostdominatorTree.java diff --git a/src/main/java/tfm/graphs/Graph.java b/src/main/java/tfm/graphs/Graph.java index e20452c..e3a1473 100644 --- a/src/main/java/tfm/graphs/Graph.java +++ b/src/main/java/tfm/graphs/Graph.java @@ -88,7 +88,8 @@ public abstract class Graph extends DirectedPseudograph, Arc> { @Override public String toString() { - return vertexSet().stream().sorted() + return vertexSet().stream() + .sorted(Comparator.comparingInt(GraphNode::getId)) .map(GraphNode::toString) .collect(Collectors.joining(System.lineSeparator())); } diff --git a/src/main/java/tfm/graphs/augmented/ACFGBuilder.java b/src/main/java/tfm/graphs/augmented/ACFGBuilder.java index 242a335..d250d09 100644 --- a/src/main/java/tfm/graphs/augmented/ACFGBuilder.java +++ b/src/main/java/tfm/graphs/augmented/ACFGBuilder.java @@ -243,7 +243,6 @@ public class ACFGBuilder extends CFGBuilder { methodDeclaration.getBody().get().accept(this, arg); returnList.stream().filter(node -> !hangingNodes.contains(node)).forEach(hangingNodes::add); nonExecHangingNodes.add(graph.getRootNode().get()); - GraphNode exitNode = connectTo(new EmptyStmt(), "Exit"); - ((ACFG) graph).setExitNode(exitNode); + connectTo(new EmptyStmt(), "Exit"); } } diff --git a/src/main/java/tfm/graphs/cfg/CFG.java b/src/main/java/tfm/graphs/cfg/CFG.java index 41c9697..296428d 100644 --- a/src/main/java/tfm/graphs/cfg/CFG.java +++ b/src/main/java/tfm/graphs/cfg/CFG.java @@ -20,7 +20,6 @@ import java.util.Set; */ public class CFG extends GraphWithRootNode { private boolean built = false; - protected GraphNode exitNode; public CFG() { super(); @@ -63,28 +62,9 @@ public class CFG extends GraphWithRootNode { return res; } - @Override - public boolean removeVertex(GraphNode graphNode) { - if (Objects.equals(graphNode, exitNode)) - return false; - return super.removeVertex(graphNode); - } - - public GraphNode getExitNode() { - return exitNode; - } - - protected void setExitNode(GraphNode exitNode) { - if (this.exitNode != null) - throw new IllegalStateException("Exit node already set!"); - this.exitNode = exitNode; - } - @Override public void build(MethodDeclaration method) { method.accept(newCFGBuilder(), null); - if (exitNode == null) - throw new IllegalStateException("Exit node missing!"); built = true; } diff --git a/src/main/java/tfm/graphs/cfg/CFGBuilder.java b/src/main/java/tfm/graphs/cfg/CFGBuilder.java index f686d67..56f0996 100644 --- a/src/main/java/tfm/graphs/cfg/CFGBuilder.java +++ b/src/main/java/tfm/graphs/cfg/CFGBuilder.java @@ -275,7 +275,6 @@ public class CFGBuilder extends VoidVisitorAdapter { hangingNodes.add(graph.getRootNode().get()); methodDeclaration.getBody().get().accept(this, arg); returnList.stream().filter(node -> !hangingNodes.contains(node)).forEach(hangingNodes::add); - GraphNode exitNode = connectTo(new EmptyStmt(), "Exit"); - graph.setExitNode(exitNode); + connectTo(new EmptyStmt(), "Exit"); } } diff --git a/src/main/java/tfm/graphs/pdg/ControlDependencyBuilder.java b/src/main/java/tfm/graphs/pdg/ControlDependencyBuilder.java index bfaad31..29fbe20 100644 --- a/src/main/java/tfm/graphs/pdg/ControlDependencyBuilder.java +++ b/src/main/java/tfm/graphs/pdg/ControlDependencyBuilder.java @@ -1,7 +1,16 @@ package tfm.graphs.pdg; +import tfm.arcs.Arc; +import tfm.graphs.augmented.PPDG; import tfm.graphs.cfg.CFG; import tfm.nodes.GraphNode; +import tfm.nodes.NodeFactory; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; /** * A simple but slow finder of control dependencies. @@ -29,21 +38,77 @@ class ControlDependencyBuilder { } public void analyze() { + Map, GraphNode> nodeMap = new HashMap<>(); assert cfg.getRootNode().isPresent(); assert pdg.getRootNode().isPresent(); + nodeMap.put(cfg.getRootNode().get(), pdg.getRootNode().get()); + Set> roots = new HashSet<>(cfg.vertexSet()); + roots.remove(cfg.getRootNode().get()); + Set> cfgNodes = new HashSet<>(cfg.vertexSet()); + cfgNodes.removeIf(node -> node.getInstruction().equals("Exit")); + + for (GraphNode node : cfgNodes) + registerNode(node, nodeMap); - boolean needsStartEndEdge = !cfg.containsEdge(cfg.getRootNode().get(), cfg.getExitNode()); - if (needsStartEndEdge) - cfg.addControlFlowEdge(cfg.getRootNode().get(), cfg.getExitNode()); + for (GraphNode src : cfgNodes) { + for (GraphNode dest : cfgNodes) { + if (src == dest) continue; + if (hasControlDependence(src, dest)) { + pdg.addControlDependencyArc(nodeMap.get(src), nodeMap.get(dest)); + roots.remove(dest); + } + } + } + // In the original definition, nodes were dependent by default on the Enter/Start node + for (GraphNode node : roots) + if (!node.getInstruction().equals("Exit")) + pdg.addControlDependencyArc(pdg.getRootNode().get(), nodeMap.get(node)); + } - PostdominatorTree tree = new PostdominatorTree(cfg); + public void registerNode(GraphNode node, Map, GraphNode> nodeMap) { + if (nodeMap.containsKey(node) || node.getInstruction().equals("Exit")) + return; + GraphNode clone = NodeFactory.graphNode(node.getId(), node.getInstruction(), node.getAstNode()); + nodeMap.put(node, clone); + pdg.addVertex(clone); + } - for (GraphNode src : pdg.vertexSet()) - for (GraphNode dest : tree.controlDependenciesOf(cfg, src)) - if (!src.equals(dest)) - pdg.addControlDependencyArc(src, dest); + public boolean hasControlDependence(GraphNode a, GraphNode b) { + int yes = 0; + Set list = cfg.outgoingEdgesOf(a); + // Nodes with less than 1 outgoing arc cannot control another node. + if (cfg.outDegreeOf(a) < 2) + return false; + for (Arc arc : cfg.outgoingEdgesOf(a)) { + GraphNode successor = cfg.getEdgeTarget(arc); + if (postdominates(successor, b)) + yes++; + } + int no = list.size() - yes; + return yes > 0 && no > 0; + } + + public boolean postdominates(GraphNode a, GraphNode b) { + return postdominates(a, b, new HashSet<>()); + } - if (needsStartEndEdge) - cfg.removeEdge(cfg.getRootNode().get(), cfg.getExitNode()); + private boolean postdominates(GraphNode a, GraphNode b, Set> visited) { + // Stop w/ success if a == b or a has already been visited + if (a.equals(b) || visited.contains(a)) + return true; + Set outgoing = cfg.outgoingEdgesOf(a); + // Limit the traversal if it is a PPDG + if (pdg instanceof PPDG) + outgoing = outgoing.stream().filter(Arc::isExecutableControlFlowArc).collect(Collectors.toSet()); + // Stop w/ failure if there are no edges to traverse from a + if (outgoing.isEmpty()) + return false; + // Find all possible paths starting from a, if ALL find b, then true, else false + visited.add(a); + for (Arc out : outgoing) { + if (!postdominates(cfg.getEdgeTarget(out), b, visited)) + return false; + } + return true; } } diff --git a/src/main/java/tfm/graphs/pdg/DirectedTree.java b/src/main/java/tfm/graphs/pdg/DirectedTree.java deleted file mode 100644 index c5b59d5..0000000 --- a/src/main/java/tfm/graphs/pdg/DirectedTree.java +++ /dev/null @@ -1,24 +0,0 @@ -package tfm.graphs.pdg; - -import org.jgrapht.graph.DirectedAcyclicGraph; - -import java.util.function.Supplier; - -public class DirectedTree extends DirectedAcyclicGraph { - protected V root; - - public DirectedTree(Supplier vertexSupplier, Supplier edgeSupplier, boolean weighted) { - super(vertexSupplier, edgeSupplier, weighted); - } - - public V getRoot() { - return root; - } - - @Override - public boolean addEdge(V sourceVertex, V targetVertex, E defaultEdge) { - if (inDegreeOf(targetVertex) >= 1) - throw new IllegalArgumentException("The target vertex already has a parent. This should be a tree!"); - return super.addEdge(sourceVertex, targetVertex, defaultEdge); - } -} diff --git a/src/main/java/tfm/graphs/pdg/PDGBuilder.java b/src/main/java/tfm/graphs/pdg/PDGBuilder.java index 070b89c..363e3a7 100644 --- a/src/main/java/tfm/graphs/pdg/PDGBuilder.java +++ b/src/main/java/tfm/graphs/pdg/PDGBuilder.java @@ -3,7 +3,6 @@ package tfm.graphs.pdg; import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.stmt.BlockStmt; import tfm.graphs.cfg.CFG; -import tfm.nodes.GraphNode; import java.util.Objects; diff --git a/src/main/java/tfm/graphs/pdg/PostdominatorTree.java b/src/main/java/tfm/graphs/pdg/PostdominatorTree.java deleted file mode 100644 index fac73cf..0000000 --- a/src/main/java/tfm/graphs/pdg/PostdominatorTree.java +++ /dev/null @@ -1,222 +0,0 @@ -package tfm.graphs.pdg; - -import org.jgrapht.graph.DefaultEdge; -import org.jgrapht.traverse.DepthFirstIterator; -import tfm.arcs.Arc; -import tfm.graphs.Graph; -import tfm.graphs.GraphWithRootNode; -import tfm.graphs.cfg.CFG; -import tfm.nodes.GraphNode; - -import java.util.*; -import java.util.stream.Collectors; - -public class PostdominatorTree extends DirectedTree, DefaultEdge> { - private Map routeMap; -// private Map, Set> servedByMap; - - public PostdominatorTree(CFG cfg) { - super(null, DefaultEdge::new, false); - // Vertices - cfg.vertexSet().forEach(this::addVertex); - // Edges - Map, GraphNode> map = immediatePostdominatorTree(cfg); - for (Map.Entry, GraphNode> entry : map.entrySet()) - addEdge(entry.getValue(), entry.getKey()); - // Set root - for (GraphNode node : vertexSet()) { - if (inDegreeOf(node) == 0) { - if (root == null) - root = node; - else throw new IllegalStateException("Multiple roots found!"); - } - } - if (root == null) - throw new IllegalStateException("No root found!"); - // Build route map and cache - routeMap = constructRomanChariots(cfg); -// servedByMap = constructCache(); - } - - private GraphNode parentOf(GraphNode node) { - Set edges = incomingEdgesOf(node); - if (edges.size() > 1) - throw new IllegalStateException("Node has multiple parents!"); - if (edges.isEmpty()) - throw new IllegalStateException("Node has no parents! Don't call this method on the root node!"); - return getEdgeSource(edges.iterator().next()); - } - - private Set> childrenOf(GraphNode node) { - return outgoingEdgesOf(node).stream().map(this::getEdgeTarget).collect(Collectors.toSet()); - } - - /** The set of nodes controlled by the arc */ - protected Set> cd(Graph graph, Arc arc) { - GraphNode u = graph.getEdgeSource(arc); - GraphNode v = graph.getEdgeTarget(arc); - if (v.equals(parentOf(u))) - return Collections.emptySet(); - if (!routeMap.containsKey(arc)) - throw new IllegalArgumentException("Arc is not valid!"); - return cd(routeMap.get(arc)); - } - - /** The nodes traversed by the route */ - private Set> cd(Route r) { - GraphNode w = r.dest; - Set> cdSet = new HashSet<>(); - for (GraphNode v = r.src; !v.equals(w) && !v.equals(root); v = parentOf(v)) - cdSet.add(v); - return cdSet; - } - - /** The set of nodes that is controlled by the node. - * Equivalent to the routes that serve the node. */ - protected Set> conds(CFG cfg, GraphNode w) { - Set> res = new HashSet<>(); - for (Arc arc : cfg.outgoingEdgesOf(w)) - if (routeMap.containsKey(arc)) - res.addAll(cd(cfg, arc)); - return res; - } - - private List> inTopDownOrder() { - Iterator> it = new DepthFirstIterator<>(this, root); - List> list = new LinkedList<>(); - it.forEachRemaining(node -> { - if (!list.contains(node)) - list.add(node); - }); - return list; - } - - public Set> controlDependenciesOf(CFG cfg, GraphNode node) { - return conds(cfg, node); - } - - private Map constructRomanChariots(CFG cfg) { - Map routes = new HashMap<>(); - for (GraphNode p : inTopDownOrder()) { - for (GraphNode u : childrenOf(p)) { - for (Arc arc : cfg.outgoingEdgesOf(u)) { - GraphNode v = cfg.getEdgeTarget(arc); - if (!v.equals(p)) { - // Append a `cd` set to end of routes - routes.put(arc, new Route(v, p)); - } - } - } - } - return routes; - } - - private static class Route { - GraphNode src; - GraphNode dest; - - public Route(GraphNode src, GraphNode dest) { - this.src = src; - this.dest = dest; - } - - @Override - public String toString() { - return String.format("[%d, %d)", src.getId(), dest.getId()); - } - } - - private Map, Set> constructCache() { - Map, Set> map = new HashMap<>(); - for (Route r : routeMap.values()) { - for (GraphNode n : cd(r)) { - if (!map.containsKey(n)) - map.put(n, new HashSet<>()); - map.get(n).add(r); - } - } - return map; - } - - private static Map, GraphNode> immediatePostdominatorTree(CFG cfg) { - Optional> optExitNode = cfg.vertexSet().stream().filter(gn -> gn.getInstruction().equals("Exit")).findFirst(); - if (!optExitNode.isPresent()) - throw new IllegalStateException("CFG lacks exit node"); - GraphNode exitNode = optExitNode.get(); - Map, GraphNode> postdoms = new HashMap<>(); - List> postorderList = postorder(cfg); - Map, Integer> sortingMap = new HashMap<>(); - for (int i = 0; i < postorderList.size(); i++) - sortingMap.put(postorderList.get(i), i); - postorderList.remove(exitNode); - postdoms.put(exitNode, exitNode); - boolean changed = true; - while (changed) { - changed = false; - for (GraphNode node : postorderList) { - GraphNode newIpostdom = null; - for (Arc arc : cfg.outgoingEdgesOf(node)) { - GraphNode successor = cfg.getEdgeTarget(arc); - if (newIpostdom == null && postdoms.containsKey(successor)) - newIpostdom = successor; - if (newIpostdom != null && postdoms.containsKey(successor)) - newIpostdom = reverseIntersect(successor, newIpostdom, postdoms, sortingMap); - } - if (postdoms.get(node) != newIpostdom) { - postdoms.put(node, newIpostdom); - changed = true; - } - } - } - postdoms.remove(exitNode); - return postdoms; - } - - private static GraphNode reverseIntersect(final GraphNode b1, final GraphNode b2, - final Map, GraphNode> postdoms, - final Map, Integer> sortingMap) { - GraphNode finger1 = b1; - GraphNode finger2 = b2; - while (finger1 != finger2) { - while (compare(finger1, finger2, sortingMap) > 0) - finger1 = postdoms.get(finger1); - while (compare(finger2, finger1, sortingMap) > 0) - finger2 = postdoms.get(finger2); - } - return finger1; - } - - private static int compare(GraphNode b1, GraphNode b2, Map, Integer> sortingMap) { - return Integer.compare(sortingMap.get(b1), sortingMap.get(b2)); - } - - private static List> postorder(GraphWithRootNode graph) { - Optional> rootNode = graph.getRootNode(); - if (!rootNode.isPresent()) - throw new IllegalStateException("CFG lacks root node"); - return postorder(graph, rootNode.get()); - } - - private static List postorder(org.jgrapht.Graph graph, V startVertex) { - List dfsOrder = new LinkedList<>(); - Iterator it = new DepthFirstIterator<>(graph, startVertex); - it.forEachRemaining(dfsOrder::add); - - List postorder = new LinkedList<>(); - for (V v : dfsOrder) - if (!postorder.contains(v)) - postorder.add(v); - return postorder; - } - - @Override - public String toString() { - StringBuilder builder = new StringBuilder("PostdominatorTree"); - for (DefaultEdge edge : edgeSet()) - builder.append(getEdgeSource(edge).getId()) - .append(" -> ") - .append(getEdgeTarget(edge).getId()) - .append('\n'); - return builder.toString(); - } -} diff --git a/src/main/java/tfm/nodes/GraphNode.java b/src/main/java/tfm/nodes/GraphNode.java index bd4e741..cb642f1 100644 --- a/src/main/java/tfm/nodes/GraphNode.java +++ b/src/main/java/tfm/nodes/GraphNode.java @@ -25,7 +25,7 @@ import java.util.Set; * It is immutable. * @param The type of the AST represented by this node. */ -public class GraphNode implements Comparable> { +public class GraphNode { private final int id; private final String instruction; @@ -136,9 +136,4 @@ public class GraphNode implements Comparable> { public String getInstruction() { return instruction; } - - @Override - public int compareTo(@NotNull GraphNode o) { - return Integer.compare(id, o.id); - } } -- GitLab