diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/FunctionComparison.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/FunctionComparison.java index ea047c6eb4..8743fee1b0 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/FunctionComparison.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/FunctionComparison.java @@ -114,19 +114,19 @@ public class FunctionComparison implements Comparable { String sourceName = getSource().getName(); String otherName = o.getSource().getName(); - - if (sourcePath.equals(otherPath)) { - if (sourceName.contentEquals(otherName)) { - return getSource().getBody() - .getMinAddress() - .compareTo(o.getSource().getBody().getMinAddress()); - } - else { - return sourceName.compareTo(otherName); - } + int result = sourcePath.compareTo(otherPath); + if (result != 0) { + return result; } - return sourcePath.compareTo(otherPath); + // equal paths + result = sourceName.compareTo(otherName); + if (result != 0) { + return result; + } + + // equal names + return getSource().getEntryPoint().compareTo(o.getSource().getEntryPoint()); } /** @@ -144,20 +144,21 @@ public class FunctionComparison implements Comparable { String o1Path = o1.getProgram().getDomainFile().getPathname(); String o2Path = o2.getProgram().getDomainFile().getPathname(); - - String o1Name = o1.getName(); - String o2Name = o2.getName(); - - if (o1Path.equals(o2Path)) { - if (o1Name.equals(o2Name)) { - return o1.getBody().getMinAddress().compareTo(o2.getBody().getMinAddress()); - } - else { - return o1Name.compareTo(o2Name); - } + int result = o1Path.compareTo(o2Path); + if (result != 0) { + return result; } - return o1Path.compareTo(o2Path); + // equal paths + String o1Name = o1.getName(); + String o2Name = o2.getName(); + result = o1Name.compareTo(o2Name); + if (result != 0) { + return result; + } + + // equal names + return o1.getEntryPoint().compareTo(o2.getEntryPoint()); } } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/MultiFunctionComparisonPanel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/MultiFunctionComparisonPanel.java index 168485c4be..9e4ba5891e 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/MultiFunctionComparisonPanel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functioncompare/MultiFunctionComparisonPanel.java @@ -89,19 +89,17 @@ public class MultiFunctionComparisonPanel extends FunctionComparisonPanel { */ @Override public void reload() { - SwingUtilities.invokeLater(() -> { - reloadSourceList(); - Function selectedSource = (Function) sourceFunctionsCBModel.getSelectedItem(); - reloadTargetList(selectedSource); - loadFunctions(selectedSource, (Function) targetFunctionsCBModel.getSelectedItem()); + reloadSourceList(); + Function selectedSource = (Function) sourceFunctionsCBModel.getSelectedItem(); + reloadTargetList(selectedSource); + loadFunctions(selectedSource, (Function) targetFunctionsCBModel.getSelectedItem()); - updateTabText(); + updateTabText(); - // Fire a notification to update the UI state; without this the - // actions would not be properly enabled/disabled - tool.contextChanged(provider); - tool.setStatusInfo("function comparisons updated"); - }); + // Fire a notification to update the UI state; without this the + // actions would not be properly enabled/disabled + tool.contextChanged(provider); + tool.setStatusInfo("function comparisons updated"); } /** diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/services/FunctionComparisonModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/services/FunctionComparisonModel.java index 8e6826fe7e..9ab14fd7b1 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/services/FunctionComparisonModel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/services/FunctionComparisonModel.java @@ -155,8 +155,7 @@ public class FunctionComparisonModel { Iterator iter = comparisons.iterator(); while (iter.hasNext()) { - // First remove any comparisons that have the function as its - // source + // First remove any comparisons that have the function as its source FunctionComparison fc = iter.next(); if (fc.getSource().equals(function)) { comparisonsToRemove.add(fc); diff --git a/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/functioncompare/CompareFunctionsSlowTest.java b/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/functioncompare/CompareFunctionsSlowTest.java index 03b680372f..45313c0e52 100644 --- a/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/functioncompare/CompareFunctionsSlowTest.java +++ b/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/functioncompare/CompareFunctionsSlowTest.java @@ -15,9 +15,7 @@ */ package ghidra.app.plugin.core.functioncompare; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import java.awt.Window; import java.util.Date; @@ -81,27 +79,26 @@ public class CompareFunctionsSlowTest extends AbstractGhidraHeadedIntegrationTes @Test public void testRemoveLastItem() throws Exception { Set functions = CompareFunctionsTestUtility.getFunctionsAsSet(foo); - provider = plugin.compareFunctions(functions); - provider = waitForComponentProvider(FunctionComparisonProvider.class); - plugin.removeFunction(foo, provider); + provider = compareFunctions(functions); + runSwing(() -> plugin.removeFunction(foo, provider)); assertFalse(provider.isVisible()); } @Test public void testCloseProgram() throws Exception { Set functions = CompareFunctionsTestUtility.getFunctionsAsSet(foo, bar); - provider = plugin.compareFunctions(functions); + provider = compareFunctions(functions); CompareFunctionsTestUtility.checkSourceFunctions(provider, foo, bar); CompareFunctionsTestUtility.checkTargetFunctions(provider, foo, foo, bar); CompareFunctionsTestUtility.checkTargetFunctions(provider, bar, foo, bar); - plugin.programClosed(program1); + runSwing(() -> plugin.programClosed(program1)); CompareFunctionsTestUtility.checkSourceFunctions(provider, bar); CompareFunctionsTestUtility.checkTargetFunctions(provider, bar, bar); - plugin.programClosed(program2); + runSwing(() -> plugin.programClosed(program2)); CompareFunctionsTestUtility.checkSourceFunctions(provider); } @@ -109,9 +106,7 @@ public class CompareFunctionsSlowTest extends AbstractGhidraHeadedIntegrationTes @Test public void testNextPreviousAction() { Set functions = CompareFunctionsTestUtility.getFunctionsAsSet(foo, bar); - provider = plugin.compareFunctions(functions); - provider.setVisible(true); - waitForSwing(); + provider = compareFunctions(functions); // Must do this or there will be no "active" provider in the actions // initiated below @@ -134,9 +129,7 @@ public class CompareFunctionsSlowTest extends AbstractGhidraHeadedIntegrationTes @Test public void testNextPreviousActionSwitchPanelFocus() { Set functions = CompareFunctionsTestUtility.getFunctionsAsSet(foo, bar); - provider = plugin.compareFunctions(functions); - provider.setVisible(true); - waitForSwing(); + provider = compareFunctions(functions); // Must do this or there will be no "active" provider in the actions // initiated below @@ -169,50 +162,46 @@ public class CompareFunctionsSlowTest extends AbstractGhidraHeadedIntegrationTes @Test public void testOpenFunctionTableActionForAdd() { Set functions = CompareFunctionsTestUtility.getFunctionsAsSet(foo, bar); - provider = plugin.compareFunctions(functions); - provider.setVisible(true); + provider = compareFunctions(functions); // Must do this or the context for the action initiated below will be // for the listing, not the comparison provider clickComponentProvider(provider); DockingActionIf openTableAction = getAction(plugin, "Add Functions To Comparison"); - performAction(openTableAction); + performAction(openTableAction, false); Window selectWindow = waitForWindowByTitleContaining("Select Functions"); assertNotNull(selectWindow); + selectWindow.setVisible(false); } @SuppressWarnings("unchecked") @Test public void testAddFunctionToExistingCompare() { Set functions = CompareFunctionsTestUtility.getFunctionsAsSet(foo); - provider = plugin.compareFunctions(functions); - provider.setVisible(true); - waitForSwing(); + provider = compareFunctions(functions); // Must do this or there will be no "active" provider in the actions // initiated below clickComponentProvider(provider); - assertTrue(provider.getModel().getSourceFunctions().size() == 1); + assertEquals(provider.getModel().getSourceFunctions().size(), 1); assertTrue(provider.getModel().getSourceFunctions().contains(foo)); DockingActionIf openTableAction = getAction(plugin, "Add Functions To Comparison"); - performAction(openTableAction); + performAction(openTableAction, false); TableChooserDialog chooser = waitForDialogComponent(TableChooserDialog.class); - assertNotNull(chooser); - GFilterTable table = (GFilterTable) getInstanceField("gFilterTable", chooser); - assertTrue(table.getModel().getRowCount() == 2); + assertEquals(table.getModel().getRowCount(), 2); clickTableCell(table.getTable(), 1, 0, 1); pressButtonByText(chooser, "OK"); waitForSwing(); - assertTrue(provider.getModel().getSourceFunctions().size() == 2); + assertEquals(provider.getModel().getSourceFunctions().size(), 2); assertTrue(provider.getModel().getSourceFunctions().contains(foo)); assertTrue(provider.getModel().getSourceFunctions().contains(bat)); } @@ -225,10 +214,9 @@ public class CompareFunctionsSlowTest extends AbstractGhidraHeadedIntegrationTes @Test public void testDeleteFunctionFromListing() { Set functions = CompareFunctionsTestUtility.getFunctionsAsSet(foo, bar); - provider = plugin.compareFunctions(functions); - provider.setVisible(true); + provider = compareFunctions(functions); - assertTrue(provider.getModel().getSourceFunctions().size() == 2); + assertEquals(provider.getModel().getSourceFunctions().size(), 2); assertTrue(provider.getModel().getSourceFunctions().contains(foo)); assertTrue(provider.getModel().getSourceFunctions().contains(bar)); @@ -236,14 +224,20 @@ public class CompareFunctionsSlowTest extends AbstractGhidraHeadedIntegrationTes ProgramLocation loc = new ProgramLocation(program1, addr); cbPlugin.goTo(loc); DockingActionIf deleteAction = getAction(functionPlugin, "Delete Function"); - performAction(deleteAction); - + performAction(deleteAction, cbPlugin.getProvider().getActionContext(null), true); waitForSwing(); - assertTrue(provider.getModel().getSourceFunctions().size() == 1); + assertEquals(provider.getModel().getSourceFunctions().size(), 1); assertTrue(provider.getModel().getSourceFunctions().contains(bar)); } + private FunctionComparisonProvider compareFunctions(Set functions) { + provider = runSwing(() -> plugin.compareFunctions(functions)); + provider.setVisible(true); + waitForSwing(); + return provider; + } + /** * Builds a program with 2 functions */ diff --git a/Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java b/Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java index 797a05fb98..8fc608e142 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/ComponentPlaceholder.java @@ -488,6 +488,10 @@ public class ComponentPlaceholder { /** Updates local actions for providers */ void contextChanged() { + if (componentProvider == null) { + return; // disposed + } + ActionContext actionContext = componentProvider.getActionContext(null); if (actionContext == null) { actionContext = new ActionContext(componentProvider, null);