Merge remote-tracking branch 'origin/GT-3074-dragonmacher-fg-grouping-exception'

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/graph/FGPrimaryViewer.java

@@ -16,10 +16,14 @@
 package ghidra.app.plugin.core.functiongraph.graph;
 
 import java.awt.Dimension;
+import java.util.Set;
 
 import ghidra.app.plugin.core.functiongraph.graph.vertex.FGVertex;
 import ghidra.app.plugin.core.functiongraph.graph.vertex.FGVertexTooltipProvider;
+import ghidra.graph.*;
 import ghidra.graph.viewer.*;
+import ghidra.graph.viewer.edge.PathHighlightListener;
+import ghidra.graph.viewer.edge.VisualGraphPathHighlighter;
 import ghidra.graph.viewer.layout.VisualGraphLayout;
 
 public class FGPrimaryViewer extends GraphViewer<FGVertex, FGEdge> {
@@ -36,4 +40,35 @@ public class FGPrimaryViewer extends GraphViewer<FGVertex, FGEdge> {
 	protected VisualGraphViewUpdater<FGVertex, FGEdge> createViewUpdater() {
 		return new FGViewUpdater(this, getVisualGraph());
 	}
+
+	// Overridden so that we can install our own path highlighter that knows how to work around
+	// source/sink vertices that have been grouped.  This allows us to use dominance algorithms
+	// that require sources/sinks
+	@Override
+	protected VisualGraphPathHighlighter<FGVertex, FGEdge> createPathHighlighter(
+			PathHighlightListener listener) {
+
+		return new VisualGraphPathHighlighter<>(getVisualGraph(), listener) {
+
+			protected GDirectedGraph<FGVertex, FGEdge> getDominanceGraph(
+					VisualGraph<FGVertex, FGEdge> graph, boolean forward) {
+
+				Set<FGVertex> sources =
+					forward ? GraphAlgorithms.getSources(graph) : GraphAlgorithms.getSinks(graph);
+				if (!sources.isEmpty()) {
+					return graph;
+				}
+
+				FunctionGraph functionGraph = (FunctionGraph) graph;
+				Set<FGEdge> dummyEdges =
+					forward ? functionGraph.createDummySources() : functionGraph.createDummySinks();
+				MutableGDirectedGraphWrapper<FGVertex, FGEdge> modifiedGraph =
+					new MutableGDirectedGraphWrapper<>(graph);
+				for (FGEdge e : dummyEdges) {
+					modifiedGraph.addEdge(e);
+				}
+				return modifiedGraph;
+			}
+		};
+	}
 }

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/graph/FunctionGraph.java

@@ -30,6 +30,7 @@ import ghidra.graph.viewer.layout.LayoutListener.ChangeType;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSet;
 import ghidra.program.model.listing.Function;
+import ghidra.program.model.symbol.RefType;
 import ghidra.program.util.ProgramSelection;
 
 /**
@@ -588,6 +589,60 @@ public class FunctionGraph extends GroupingVisualGraph<FGVertex, FGEdge> {
 		return newGraph;
 	}
 
+	/**
+	 * A method to create dummy edges (with dummy vertices).  This is used to add entry and 
+	 * exit vertices as needed when a user grouping operation has consumed the entries or exits.
+	 * The returned edge will connect the current vertex containing the entry to a new dummy 
+	 * vertex that is a source for the graph.   Calling this method does not mutate this graph.
+	 * 
+	 * @return the edge
+	 */
+	public Set<FGEdge> createDummySources() {
+
+		Set<FGEdge> dummyEdges = new HashSet<>();
+		Set<FGVertex> entries = getEntryPoints();
+		for (FGVertex entry : entries) {
+			AbstractFunctionGraphVertex abstractVertex = (AbstractFunctionGraphVertex) entry;
+			FGController controller = abstractVertex.getController();
+			ListingFunctionGraphVertex newEntry = new ListingFunctionGraphVertex(controller,
+				abstractVertex.getAddresses(), RefType.UNCONDITIONAL_JUMP, true);
+			newEntry.setVertexType(FGVertexType.ENTRY);
+			FGVertex groupVertex = getVertexForAddress(entry.getVertexAddress());
+			FGEdgeImpl edge =
+				new FGEdgeImpl(newEntry, groupVertex, RefType.UNCONDITIONAL_JUMP, options);
+			dummyEdges.add(edge);
+		}
+
+		return dummyEdges;
+	}
+
+	/**
+	 * A method to create dummy edges (with dummy vertices).  This is used to add entry and 
+	 * exit vertices as needed when a user grouping operation has consumed the entries or exits.
+	 * The returned edge will connect the current vertex containing the exit to a new dummy 
+	 * vertex that is a sink for the graph.   Calling this method does not mutate this graph.
+	 * 
+	 * @return the edge
+	 */
+	public Set<FGEdge> createDummySinks() {
+
+		Set<FGEdge> dummyEdges = new HashSet<>();
+		Set<FGVertex> exits = getExitPoints();
+		for (FGVertex exit : exits) {
+			AbstractFunctionGraphVertex abstractVertex = (AbstractFunctionGraphVertex) exit;
+			FGController controller = abstractVertex.getController();
+			ListingFunctionGraphVertex newExit = new ListingFunctionGraphVertex(controller,
+				abstractVertex.getAddresses(), RefType.UNCONDITIONAL_JUMP, true);
+			newExit.setVertexType(FGVertexType.EXIT);
+			FGVertex groupVertex = getVertexForAddress(exit.getVertexAddress());
+			FGEdgeImpl edge =
+				new FGEdgeImpl(groupVertex, newExit, RefType.UNCONDITIONAL_JUMP, options);
+			dummyEdges.add(edge);
+		}
+
+		return dummyEdges;
+	}
+
 //==================================================================================================
 // Overridden Methods
 //==================================================================================================

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/graph/vertex/AbstractFunctionGraphVertex.java

@@ -134,7 +134,7 @@ public abstract class AbstractFunctionGraphVertex implements FGVertex {
 		return doGetComponent();
 	}
 
-	FGController getController() {
+	public FGController getController() {
 		return controller;
 	}
 

Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java

@@ -15,7 +15,7 @@
  */
 package ghidra.app.plugin.core.functiongraph;
 
-import static ghidra.graph.viewer.GraphViewerUtils.getGraphScale;
+import static ghidra.graph.viewer.GraphViewerUtils.*;
 import static org.junit.Assert.*;
 
 import java.awt.*;
@@ -45,6 +45,7 @@ import edu.uci.ics.jung.graph.Graph;
 import edu.uci.ics.jung.visualization.VisualizationModel;
 import edu.uci.ics.jung.visualization.VisualizationViewer;
 import edu.uci.ics.jung.visualization.picking.PickedState;
+import generic.test.AbstractGenericTest;
 import generic.test.TestUtils;
 import ghidra.app.cmd.label.AddLabelCmd;
 import ghidra.app.cmd.label.SetLabelPrimaryCmd;
@@ -2324,6 +2325,14 @@ public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedInte
 		assertTrue("Unexpectedly received an empty FunctionGraphData", graphData.hasResults());
 	}
 
+	protected void swing(Runnable r) {
+		AbstractGenericTest.runSwing(r);
+	}
+
+	protected <T> T swing(Supplier<T> s) {
+		return AbstractGenericTest.runSwing(s);
+	}
+
 	static class DummyTransferable implements Transferable {
 
 		@Override

Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices2Test.java

@@ -17,19 +17,20 @@ package ghidra.app.plugin.core.functiongraph;
 
 import static org.junit.Assert.*;
 
-import java.util.HashSet;
-import java.util.Set;
+import java.util.*;
 
 import org.junit.Before;
 import org.junit.Test;
 
 import edu.uci.ics.jung.graph.Graph;
-import ghidra.app.plugin.core.functiongraph.graph.FGEdge;
-import ghidra.app.plugin.core.functiongraph.graph.FunctionGraph;
+import ghidra.app.plugin.core.functiongraph.graph.*;
 import ghidra.app.plugin.core.functiongraph.graph.vertex.FGVertex;
 import ghidra.app.plugin.core.functiongraph.graph.vertex.GroupedFunctionGraphVertex;
 import ghidra.app.plugin.core.functiongraph.mvc.FGController;
 import ghidra.app.plugin.core.functiongraph.mvc.FGData;
+import ghidra.graph.viewer.edge.VisualGraphPathHighlighter;
+import ghidra.program.model.symbol.RefType;
+import util.CollectionUtils;
 
 public class FunctionGraphGroupVertices2Test extends AbstractFunctionGraphTest {
 
@@ -407,8 +408,75 @@ public class FunctionGraphGroupVertices2Test extends AbstractFunctionGraphTest {
 		assertInGroup(v2, v3, v4);
 	}
 
+	@Test
+	public void testFindForwardScopedFlowWhenGroupRemovesSourceNode() {
+
+		//
+		// Test the case that grouping the entry node will create a group that has incoming 
+		// edges.  In this case, there is no source node in the graph.  This will cause an 
+		// exception if the code does not create a fake source node before passing the graph
+		// the the algorithm for calculating dominance.
+		//
+
+		create12345GraphWithTransaction();
+
+		FGVertex entry = vertex("100415a");
+		FGVertex v2 = vertex("1004178");
+		FGVertex v3 = vertex("1004192");
+
+		FunctionGraph graph = getFunctionGraph();
+		FGEdgeImpl edge = new FGEdgeImpl(v3, v2, RefType.UNCONDITIONAL_JUMP, graph.getOptions());
+		graph.addEdge(edge);
+
+		FGComponent graphComponent = getGraphComponent();
+		VisualGraphPathHighlighter<FGVertex, FGEdge> pathHighlighter =
+			graphComponent.getPathHighlighter();
+		pathHighlighter.setHoveredVertex(entry);
+		waitForPathHighligter();
+
+		Collection<FGEdge> edges = graph.getEdges();
+		assertHovered(edges);
+
+		pathHighlighter.setHoveredVertex(null);
+		assertHovered(Collections.emptySet());
+
+		GroupedFunctionGraphVertex group = group("Entry in Group", entry, v2);
+
+		pathHighlighter.setHoveredVertex(group);
+		waitForPathHighligter();
+		assertHovered(edges);
+	}
+
 //==================================================================================================
 // Private Methods
 //==================================================================================================
 
+	private void assertHovered(Collection<FGEdge> edges) {
+
+		FunctionGraph graph = getFunctionGraph();
+		Set<FGEdge> nonHoveredEdges = new HashSet<>(graph.getEdges());
+		Set<FGEdge> expectedEdges = CollectionUtils.asSet(edges);
+		nonHoveredEdges.removeAll(expectedEdges);
+
+		for (FGEdge e : expectedEdges) {
+			boolean isHovered = swing(() -> e.isInHoveredVertexPath());
+			assertTrue("Edge was not hovered: " + e, isHovered);
+		}
+
+		for (FGEdge e : nonHoveredEdges) {
+			boolean isHovered = swing(() -> e.isInHoveredVertexPath());
+			assertFalse("Edge hovered when it should not have been: " + e, isHovered);
+		}
+	}
+
+	private void waitForPathHighligter() {
+		waitForSwing();
+		FGComponent graphComponent = getGraphComponent();
+		VisualGraphPathHighlighter<FGVertex, FGEdge> highlighter =
+			graphComponent.getPathHighlighter();
+		waitForCondition(() -> !highlighter.isBusy(), "Timed-out waiting for Path Highlighter");
+		// waitForAnimation(); don't need to do this, as the edges are hovered while animating
+		waitForSwing();
+	}
+
 }

Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/graph/layout/TestFGLayoutProvider.java

@@ -227,7 +227,8 @@ public class TestFGLayoutProvider extends FGLayoutProvider {
 					break;
 				default:
 					if (!(parent.v instanceof GroupedFunctionGraphVertex)) {
-						Msg.debug(this, "\n\n\tMore than 2 edges????: " + parent);
+						// this can happen if a test adds another edge to a test vertex
+						Msg.debug(this, "\n\n\tMore than 2 edges?: " + parent);
 					}
 
 			}

Ghidra/Framework/Graph/src/main/java/ghidra/graph/MutableGDirectedGraphWrapper.java

@@ -102,7 +102,14 @@ public class MutableGDirectedGraphWrapper<V, E extends GEdge<V>> implements GDir
 	@SuppressWarnings("unchecked")
 	@Override
 	public void addEdge(E e) {
-		mutatedGraph.addEdge((DefaultGEdge<Object>) e);
+		if (e instanceof DefaultGEdge) {
+			mutatedGraph.addEdge((DefaultGEdge<Object>) e);
+			return;
+		}
+
+		V start = e.getStart();
+		V end = e.getEnd();
+		addDummyEdge(start, end);
 	}
 
 	@SuppressWarnings("unchecked")

Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/GraphViewer.java

@@ -29,6 +29,7 @@ import edu.uci.ics.jung.visualization.picking.MultiPickedState;
 import edu.uci.ics.jung.visualization.picking.PickedState;
 import generic.util.WindowUtilities;
 import ghidra.graph.VisualGraph;
+import ghidra.graph.viewer.edge.PathHighlightListener;
 import ghidra.graph.viewer.edge.VisualGraphPathHighlighter;
 import ghidra.graph.viewer.event.mouse.*;
 import ghidra.graph.viewer.event.picking.GPickedState;
@@ -91,14 +92,15 @@ public class GraphViewer<V extends VisualVertex, E extends VisualEdge<V>>
 
 	private void buildUpdater() {
 		viewUpdater = createViewUpdater();
-		pathHighlighter = new VisualGraphPathHighlighter<>(getVisualGraph(), hoverChange -> {
+		PathHighlightListener listener = hoverChange -> {
 			if (hoverChange) {
 				viewUpdater.animateEdgeHover();
 			}
 			else {
 				repaint();
 			}
-		});
+		};
+		pathHighlighter = createPathHighlighter(listener);
 
 		//
 		// The path highlighter is subordinate to the view updater in that the path highlighter
@@ -111,6 +113,11 @@ public class GraphViewer<V extends VisualVertex, E extends VisualEdge<V>>
 		pathHighlighter.setWorkPauser(() -> viewUpdater.isMutatingGraph());
 	}
 
+	protected VisualGraphPathHighlighter<V, E> createPathHighlighter(
+			PathHighlightListener listener) {
+		return new VisualGraphPathHighlighter<>(getVisualGraph(), listener);
+	}
+
 	protected VisualGraphViewUpdater<V, E> createViewUpdater() {
 		return new VisualGraphViewUpdater<>(this, getVisualGraph());
 	}

Ghidra/Framework/Graph/src/main/java/ghidra/graph/viewer/edge/VisualGraphPathHighlighter.java

@@ -149,8 +149,8 @@ public class VisualGraphPathHighlighter<V extends VisualVertex, E extends Visual
 			TaskMonitor timeoutMonitor = TimeoutTaskMonitor.timeoutIn(ALGORITHM_TIMEOUT,
 				TimeUnit.SECONDS, new TaskMonitorAdapter(true));
 
-			Set<V> sources = GraphAlgorithms.getSources(graph);
-			if (sources.isEmpty()) {
+			GDirectedGraph<V, E> dominanceGraph = getDominanceGraph(graph, true);
+			if (dominanceGraph == null) {
 				Msg.debug(this, "No sources found for graph; cannot calculate dominance: " +
 					graph.getClass().getSimpleName());
 				return null;
@@ -158,7 +158,7 @@ public class VisualGraphPathHighlighter<V extends VisualVertex, E extends Visual
 
 			try {
 				// note: calling the constructor performs the work
-				return new ChkDominanceAlgorithm<>(graph, timeoutMonitor);
+				return new ChkDominanceAlgorithm<>(dominanceGraph, timeoutMonitor);
 			}
 			catch (CancelledException e) {
 				// shouldn't happen
@@ -170,6 +170,17 @@ public class VisualGraphPathHighlighter<V extends VisualVertex, E extends Visual
 		return dominanceFuture;
 	}
 
+	protected GDirectedGraph<V, E> getDominanceGraph(VisualGraph<V, E> visualGraph,
+			boolean forward) {
+
+		Set<V> sources = GraphAlgorithms.getSources(visualGraph);
+		if (!sources.isEmpty()) {
+			return visualGraph;
+		}
+
+		return null;
+	}
+
 	private CompletableFuture<ChkDominanceAlgorithm<V, E>> lazyCreatePostDominanceFuture() {
 
 		// lazy-load