diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/DecompilerController.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/DecompilerController.java index 719e625875..4db01954ba 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/DecompilerController.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/DecompilerController.java @@ -309,6 +309,12 @@ public class DecompilerController { //@formatter:on } + // for testing + void setCache(Cache cache) { + this.decompilerCache.invalidateAll(); + this.decompilerCache = cache; + } + public void clearCache() { decompilerCache.invalidateAll(); } diff --git a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/decompiler/component/DecompilerCachingTest.java b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/decompiler/component/DecompilerCachingTest.java index 7ba195b376..bf24e02683 100644 --- a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/decompiler/component/DecompilerCachingTest.java +++ b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/decompiler/component/DecompilerCachingTest.java @@ -15,8 +15,7 @@ */ package ghidra.app.decompiler.component; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import java.util.*; import java.util.Map.Entry; @@ -43,14 +42,11 @@ import ghidra.program.model.listing.Function; import ghidra.program.model.mem.MemoryAccessException; import ghidra.program.util.ProgramLocation; import ghidra.test.*; -import mockit.Mock; -import mockit.MockUp; public class DecompilerCachingTest extends AbstractGhidraHeadedIntegrationTest { private TestEnv env; private PluginTool tool; - private DecompilePlugin decompilePlugin; private CodeBrowserPlugin codeBrowser; private ProgramDB program; private List
functionAddrs = new ArrayList<>(); @@ -58,34 +54,26 @@ public class DecompilerCachingTest extends AbstractGhidraHeadedIntegrationTest { public Cache cache; private ToyProgramBuilder builder; - // partial fake of DecompilerController to take control of the buildCache() method. - public class FakeDecompilerController extends MockUp { - @Mock - public Cache buildCache() { - //@formatter:off - cache = CacheBuilder - .newBuilder() - .maximumSize(3) - .recordStats() - .build() - ; - //@formatter:on - return cache; - } - } - @Before public void setUp() throws Exception { - // the magic of JMockit will cause our FakeDecompilerController to get used instead - // of the real one, regardless of where it gets constructed. - new FakeDecompilerController(); - setErrorGUIEnabled(false); env = new TestEnv(); tool = env.getTool(); initializeTool(); + + //@formatter:off + cache = CacheBuilder + .newBuilder() + .maximumSize(3) + .recordStats() + .build() + ; + //@formatter:on + + DecompilerController controller = decompilerProvider.getController(); + controller.setCache(cache); } @After @@ -258,8 +246,6 @@ public class DecompilerCachingTest extends AbstractGhidraHeadedIntegrationTest { private void installPlugins() throws PluginException { tool.addPlugin(CodeBrowserPlugin.class.getName()); tool.addPlugin(DecompilePlugin.class.getName()); - - decompilePlugin = env.getPlugin(DecompilePlugin.class); codeBrowser = env.getPlugin(CodeBrowserPlugin.class); } diff --git a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGController.java b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGController.java index fd1ae8a26a..9773f95e9c 100644 --- a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGController.java +++ b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGController.java @@ -21,6 +21,7 @@ import java.awt.geom.Point2D; import java.util.ArrayList; import java.util.List; import java.util.Set; +import java.util.function.BiConsumer; import javax.swing.JComponent; import javax.swing.SwingUtilities; @@ -85,6 +86,9 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi string -> provider.setClipboardStringContent(string); private Cache cache; + private BiConsumer fgDataDisposeListener = (data, evicted) -> { + // dummy + }; public FGController(FGProvider provider, FunctionGraphPlugin plugin) { this.provider = provider; @@ -110,6 +114,7 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi } data.dispose(); + fgDataDisposeListener.accept(data, true); return true; } @@ -288,7 +293,7 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi /** * Sets the message that will appear in the lower part of the graph. - * + * * @param message the message to display */ public void setStatusMessage(String message) { @@ -355,7 +360,7 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi //================================================================================================== // Grouping Methods (to be moved in a later refactoring) -//================================================================================================== +//================================================================================================== public void groupSelectedVertices() { groupSelectedVertices(null); @@ -413,7 +418,7 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi //================================================================================================== // End Group Methods -//================================================================================================== +//================================================================================================== //================================================================================================== // Methods call by the providers @@ -795,7 +800,7 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi * BLEH!: I don't like clearing the cache this way...another options is to mark all cached * values as stale, somehow. If we did this, then when the view reuses the cached data, it could * signal to the user that the graph is out-of-date. - * + * * @param program the program */ public void invalidateAllCacheForProgram(Program program) { @@ -882,6 +887,7 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi Function function = data.getFunction(); if (function != null && cache.getIfPresent(function) != data) { data.dispose(); + fgDataDisposeListener.accept(data, false); return true; } return false; @@ -1012,7 +1018,7 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi /** * Will broadcast the given vertex location to the external system - * + * * @param location the location coming from the vertex */ public void synchronizeProgramLocationToVertex(ProgramLocation location) { @@ -1033,17 +1039,30 @@ public class FGController implements ProgramLocationListener, ProgramSelectionLi .newBuilder() .maximumSize(5) .removalListener(listener) - // Note: using soft values means that sometimes our data is reclaimed by the + // Note: using soft values means that sometimes our data is reclaimed by the // Garbage Collector. We don't want that, we wish to call dispose() on the data - //.softValues() + //.softValues() .build(); //@formatter:on } - private void cacheValueRemoved(RemovalNotification notification) { + // for testing + void setCache(Cache cache) { + this.cache.invalidateAll(); + this.cache = cache; + } + + // open for testing + void cacheValueRemoved(RemovalNotification notification) { disposeGraphDataIfNotInUse(notification.getValue()); } + void setFGDataDisposedListener(BiConsumer listener) { + this.fgDataDisposeListener = listener != null ? listener : (data, evicted) -> { + // dummy + }; + } + //================================================================================================== // Inner Classes //================================================================================================== diff --git a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGData.java b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGData.java index 3347d2bdcb..31b68c5877 100644 --- a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGData.java +++ b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/mvc/FGData.java @@ -101,6 +101,6 @@ public class FGData { @Override public String toString() { - return "FunctionGraphData[" + function.getName() + "]"; + return "FunctionGraphData[" + function.getName() + "@" + function.getEntryPoint() + "]"; } } diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java similarity index 98% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java index da967ade97..7c91d7d701 100644 --- a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java +++ b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java @@ -33,7 +33,6 @@ import org.junit.*; import docking.*; import docking.action.*; -import docking.menu.ActionState; import docking.menu.MultiStateDockingAction; import docking.test.AbstractDockingTest; import docking.widgets.EventTrigger; @@ -75,7 +74,6 @@ import ghidra.program.util.ProgramLocation; import ghidra.program.util.ProgramSelection; import ghidra.test.*; import ghidra.util.Msg; -import ghidra.util.exception.AssertException; import ghidra.util.task.RunManager; public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedIntegrationTest { @@ -541,13 +539,13 @@ public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedInte protected void waitForBusyRunManager(FGController controller) { FGModel model = controller.getModel(); - long start = System.nanoTime(); +// long start = System.nanoTime(); waitForSwing(); RunManager runManager = (RunManager) TestUtils.getInstanceField("runManager", model); waitForCondition(() -> !runManager.isInProgress()); - long end = System.nanoTime(); - long total = end - start; +// long end = System.nanoTime(); +// long total = end - start; // Msg.debug(this, // "Run manager wait time: " + TimeUnit.MILLISECONDS.convert(total, TimeUnit.NANOSECONDS)); } @@ -1999,26 +1997,6 @@ public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedInte waitForSwing(); } - private void setMinCrossLayout() { - Object actionManager = getInstanceField("actionManager", graphProvider); - @SuppressWarnings("unchecked") - final MultiStateDockingAction action = - (MultiStateDockingAction) getInstanceField("layoutAction", - actionManager); - runSwing(() -> { - List> states = action.getAllActionStates(); - for (ActionState state : states) { - FGLayoutProvider layoutProvider = state.getUserData(); - if (layoutProvider.getLayoutName().contains("MinCross")) { - action.setCurrentActionState(state); - return; - } - } - - throw new AssertException("Unable to find MinCross layout"); - }); - } - protected FGData triggerPersistenceAndReload(String functionAddress) { // // Ideally, we would like to save, close and re-open the program so that we can get diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices1Test.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices1Test.java similarity index 100% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices1Test.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices1Test.java diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices2Test.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices2Test.java similarity index 100% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices2Test.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices2Test.java diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices3Test.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices3Test.java similarity index 100% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices3Test.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphGroupVertices3Test.java diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java similarity index 100% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin2Test.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin2Test.java similarity index 100% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin2Test.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin2Test.java diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphTestSuite.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphTestSuite.java similarity index 94% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphTestSuite.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphTestSuite.java index 9b1cce6ed7..4bef4cba9b 100644 --- a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphTestSuite.java +++ b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/FunctionGraphTestSuite.java @@ -19,6 +19,8 @@ import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; +import ghidra.app.plugin.core.functiongraph.mvc.FunctionGraphCacheTest; + //@formatter:off @RunWith(Suite.class) @SuiteClasses({ diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/graph/layout/TestFGLayoutProvider.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/graph/layout/TestFGLayoutProvider.java similarity index 100% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/graph/layout/TestFGLayoutProvider.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/graph/layout/TestFGLayoutProvider.java diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphCacheTest.java b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphCacheTest.java similarity index 68% rename from Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphCacheTest.java rename to Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphCacheTest.java index 3bf84eab7d..df0eb113ed 100644 --- a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphCacheTest.java +++ b/Ghidra/Features/FunctionGraph/src/test.slow/java/ghidra/app/plugin/core/functiongraph/mvc/FunctionGraphCacheTest.java @@ -13,10 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package ghidra.app.plugin.core.functiongraph; +package ghidra.app.plugin.core.functiongraph.mvc; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import java.util.*; @@ -25,69 +24,53 @@ import org.junit.Test; import com.google.common.cache.*; -import ghidra.app.plugin.core.functiongraph.mvc.FGController; -import ghidra.app.plugin.core.functiongraph.mvc.FGData; +import ghidra.app.plugin.core.functiongraph.AbstractFunctionGraphTest; import ghidra.program.model.address.Address; import ghidra.program.model.listing.Function; -import mockit.*; public class FunctionGraphCacheTest extends AbstractFunctionGraphTest { private Cache cache; private List
disposedFunctionData = Collections.synchronizedList(new ArrayList<>()); private List
evictedFromCache = Collections.synchronizedList(new ArrayList<>()); - // partial fake of FGController to take control of the buildCache() method and spy - // on the two methods that might dispose a FunctionGrahpData object. - public class FakeFunctionGraphController extends MockUp { - - @Mock - public Cache buildCache(RemovalListener listener) { - - //@formatter:off - cache = CacheBuilder - .newBuilder() - .maximumSize(3) - .removalListener(listener) - .recordStats() - .build() - ; - //@formatter:on - return cache; - } - - @Mock - boolean disposeIfNotInCache(Invocation invocation, FGData data) { - boolean result = invocation.proceed(data); - Function function = data.getFunction(); - if (function == null) { - return result; - } - Address address = data.getFunction().getEntryPoint(); - if (result) { - disposedFunctionData.add(address); - } - return result; - } - - @Mock - boolean disposeGraphDataIfNotInUse(Invocation invocation, FGData data) { - boolean result = invocation.proceed(data); - evictedFromCache.add(data.getFunction().getEntryPoint()); - if (result) { - disposedFunctionData.add(data.getFunction().getEntryPoint()); - } - return result; - } - } - @Override @Before public void setUp() throws Exception { - // the magic of JMockit will cause our FakeDecompilerController to get used instead - // of the real one, regardless of where it gets constructed. - new FakeFunctionGraphController(); - super.setUp(); + + FGController controller = getFunctionGraphController(); + + // go to an function address that is not used by this test (all tests use functions 0-2) + goToAddress(functionAddrs.get(3)); + controller.clear(); + waitForSwing(); + + RemovalListener listener = controller::cacheValueRemoved; + + //@formatter:off + cache = CacheBuilder + .newBuilder() + .maximumSize(3) + .removalListener(listener) + .recordStats() + .build() + ; + //@formatter:on + + controller.setCache(cache); + controller.setFGDataDisposedListener((data, evicted) -> { + Function function = data.getFunction(); + if (function == null) { + return; + } + Address address = data.getFunction().getEntryPoint(); + disposedFunctionData.add(address); + if (evicted) { + evictedFromCache.add(address); + } + }); + + goToAddress(getStartingAddress()); } @Test @@ -103,6 +86,7 @@ public class FunctionGraphCacheTest extends AbstractFunctionGraphTest { @Test public void testBackToOldFunctionIsCacheHit() { + goToAddress(functionAddrs.get(0)); goToAddress(functionAddrs.get(1)); CacheStats stats1 = cache.stats(); @@ -141,10 +125,11 @@ public class FunctionGraphCacheTest extends AbstractFunctionGraphTest { goToAddress(functionAddrs.get(0)); goToAddress(functionAddrs.get(1)); goToAddress(functionAddrs.get(2)); + assertEquals(3, cache.size()); cache.invalidateAll(); - assertEquals(3, evictedFromCache.size()); + assertEquals(2, evictedFromCache.size()); assertEquals(2, disposedFunctionData.size()); assertTrue(disposedFunctionData.contains(getAddress(functionAddrs.get(0)))); assertTrue(disposedFunctionData.contains(getAddress(functionAddrs.get(1))));