diff --git a/Ghidra/Configurations/Public_Release/src/global/docs/ChangeHistory.html b/Ghidra/Configurations/Public_Release/src/global/docs/ChangeHistory.html index af41b69e2c..29a991442b 100644 --- a/Ghidra/Configurations/Public_Release/src/global/docs/ChangeHistory.html +++ b/Ghidra/Configurations/Public_Release/src/global/docs/ChangeHistory.html @@ -7,6 +7,50 @@ +

Ghidra 10.0.2 Change History (August 2021)

+

New Features

+ +
+

Improvements

+ +
+

Bugs

+ +
+

Ghidra 10.0.1 Change History (July 2021)

New Features


diff --git a/Ghidra/Features/Base/src/main/help/help/topics/SymbolTreePlugin/SymbolTree.htm b/Ghidra/Features/Base/src/main/help/help/topics/SymbolTreePlugin/SymbolTree.htm index d09178d123..fd667aad6c 100644 --- a/Ghidra/Features/Base/src/main/help/help/topics/SymbolTreePlugin/SymbolTree.htm +++ b/Ghidra/Features/Base/src/main/help/help/topics/SymbolTreePlugin/SymbolTree.htm @@ -82,9 +82,8 @@ Symbol Tree will show group nodes that represent groups of symbols in order to reduce the "clutter" in the tree. In the sample image above, the Labels category contains a group node (indicated by the icon). The - group node shows the names of the first and last symbols in the group. Long names are - truncated and are displayed with "..." to indicate this. The tool tip for the node shows the - complete names for the first and last symbols in the group.

+ group node shows the common prefix for all the symbols in the group. The tool tip + will display the total number of nodes in the group.

The Symbol Tree does not show @@ -291,10 +290,6 @@

To delete a symbol, make a selection of symbols, right mouse click and choose the Delete option.

-
-

If you select a group - node to delete, all the symbols in that group are deleted.

-

Make a Selection

diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisOptionsDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisOptionsDialog.java index ca30b72063..e5cf8b91f8 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisOptionsDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisOptionsDialog.java @@ -15,9 +15,13 @@ */ package ghidra.app.plugin.core.analysis; +import java.beans.PropertyChangeEvent; +import java.beans.PropertyChangeListener; import java.util.List; import docking.DialogComponentProvider; +import docking.widgets.OptionDialog; +import ghidra.GhidraOptions; import ghidra.framework.options.EditorStateFactory; import ghidra.program.model.listing.Program; import ghidra.util.HelpLocation; @@ -27,10 +31,12 @@ import ghidra.util.Msg; * Dialog to show the panel for the auto analysis options. * */ -public class AnalysisOptionsDialog extends DialogComponentProvider { +public class AnalysisOptionsDialog extends DialogComponentProvider + implements PropertyChangeListener { private boolean doAnalysis; private AnalysisPanel panel; private EditorStateFactory editorStateFactory = new EditorStateFactory(); + private boolean hasChanges; /** * Constructor @@ -49,16 +55,18 @@ public class AnalysisOptionsDialog extends DialogComponentProvider { AnalysisOptionsDialog(List programs) { super("Analysis Options"); setHelpLocation(new HelpLocation("AutoAnalysisPlugin", "AnalysisOptions")); - panel = new AnalysisPanel(programs, editorStateFactory); + panel = new AnalysisPanel(programs, editorStateFactory, this); addWorkPanel(panel); addOKButton(); addCancelButton(); + addApplyButton(); setOkButtonText("Analyze"); okButton.setMnemonic('A'); setOkEnabled(true); setPreferredSize(1000, 600); setRememberSize(true); + setHasChanges(panel.hasChangedValues()); } @Override @@ -73,9 +81,41 @@ public class AnalysisOptionsDialog extends DialogComponentProvider { } } + @Override + protected void cancelCallback() { + if (hasChanges) { + int result = OptionDialog.showYesNoCancelDialog(panel, "Save Changes?", + "These options are different from what is in the program.\n" + + "Do you want to save them to the program?"); + if (result == OptionDialog.CANCEL_OPTION) { + return; + } + if (result == OptionDialog.YES_OPTION) { + panel.applyChanges(); + } + } + close(); + } + + @Override + protected void applyCallback() { + panel.applyChanges(); + setHasChanges(false); + } + boolean wasAnalyzeButtonSelected() { return doAnalysis; } -} + private void setHasChanges(boolean hasChanges) { + this.hasChanges = hasChanges; + setApplyEnabled(hasChanges); + } + @Override + public void propertyChange(PropertyChangeEvent evt) { + if (evt.getPropertyName().equals(GhidraOptions.APPLY_ENABLED)) { + setHasChanges((Boolean) evt.getNewValue()); + } + } +} diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisPanel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisPanel.java index c432354a03..5e04cb76ee 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisPanel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AnalysisPanel.java @@ -101,18 +101,6 @@ class AnalysisPanel extends JPanel implements PropertyChangeListener { this(List.of(program), editorStateFactory, propertyChangeListener); } - /** - * Constructor - * - * @param programs list of programs that will be analyzed - * @param editorStateFactory the editor factory - * @param propertyChangeListener subscriber for property change notifications - */ - AnalysisPanel(List programs, EditorStateFactory editorStateFactory) { - this(programs, editorStateFactory, e -> {}); - } - - /** * Constructor * @@ -357,8 +345,8 @@ class AnalysisPanel extends JPanel implements PropertyChangeListener { return; } File saveFile = getOptionsSaveFile(saveName); - if (saveFile.exists() && OptionDialog.CANCEL_OPTION == - OptionDialog.showOptionDialogWithCancelAsDefaultButton(this, "Overwrite Configuration", + if (saveFile.exists() && OptionDialog.CANCEL_OPTION == OptionDialog + .showOptionDialogWithCancelAsDefaultButton(this, "Overwrite Configuration", "Overwrite existing configuration file: " + saveName + " ?", "Overwrite")) { return; } @@ -493,13 +481,12 @@ class AnalysisPanel extends JPanel implements PropertyChangeListener { @Override public void propertyChange(PropertyChangeEvent evt) { - if (checkForDifferencesWithProgram()) { - propertyChangeListener.propertyChange( - new PropertyChangeEvent(this, GhidraOptions.APPLY_ENABLED, null, Boolean.TRUE)); - } + boolean isDifferent = hasChangedValues(); + propertyChangeListener.propertyChange( + new PropertyChangeEvent(this, GhidraOptions.APPLY_ENABLED, null, isDifferent)); } - private boolean checkForDifferencesWithProgram() { + public boolean hasChangedValues() { List analyzerStates = model.getModelData(); boolean changes = false; for (AnalyzerEnablementState analyzerState : analyzerStates) { @@ -784,11 +771,13 @@ class AnalysisPanel extends JPanel implements PropertyChangeListener { String name = fileOptions.getName(); try { fileOptions.save(getOptionsSaveFile(name)); - } catch (IOException e) { + } + catch (IOException e) { Msg.error(this, "Error copying analysis options from previous Ghidra install", e); } } } + private File getMostRecentApplicationSettingsDirWithSavedOptions() { List ghidraUserDirsByTime = GenericRunInfo.getPreviousApplicationSettingsDirsByTime(); if (ghidraUserDirsByTime.size() == 0) { @@ -805,7 +794,6 @@ class AnalysisPanel extends JPanel implements PropertyChangeListener { return null; } - private boolean isUserConfiguration(Options options) { if (options == STANDARD_DEFAULT_OPTIONS || options == currentProgramOptions) { @@ -833,4 +821,3 @@ class AnalysisPanel extends JPanel implements PropertyChangeListener { deleteButton.setEnabled(isUserConfiguration(selectedOptions)); } } - diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java index a8d85b1a9e..79e622a132 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java @@ -27,6 +27,8 @@ import javax.swing.border.CompoundBorder; import javax.swing.event.*; import javax.swing.table.TableCellEditor; +import org.apache.commons.lang3.StringUtils; + import docking.*; import docking.widgets.OptionDialog; import docking.widgets.checkbox.GCheckBox; @@ -248,7 +250,22 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod signatureTextField.setFont(font.deriveFont(18.0f)); panel.add(signatureTextField); - signatureTextField.setEscapeListener(e -> model.resetSignatureTextField()); + signatureTextField.setEscapeListener(e -> { + + if (!model.hasChanges()) { + // no changes; user wish to close the dialog + cancelCallback(); + return; + } + + // editor has changes, see if they wish to cancel editing + if (!promptToAbortChanges()) { + return; // keep editing + } + + // abort changes confirmed + model.resetSignatureTextField(); + }); signatureTextField.setActionListener(e -> { try { @@ -297,18 +314,31 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod message = details; } - message = HTMLUtilities.wrapAsHTML( - message + "

Do you want to continue editing or " + - "abort your changes?
"); - int result = OptionDialog.showOptionNoCancelDialog(rootPanel, "Invalid Function Signature", - message, "Continue Editing", "Abort Changes", OptionDialog.ERROR_MESSAGE); - if (result == OptionDialog.OPTION_TWO) { + if (doPromptToAbortChanges(message)) { model.resetSignatureTextField(); return true; } return false; } + private boolean promptToAbortChanges() { + return doPromptToAbortChanges(""); + } + + private boolean doPromptToAbortChanges(String message) { + + if (!StringUtils.isBlank(message)) { + message += "

"; + } + + message = HTMLUtilities.wrapAsHTML( + message + "
Do you want to continue editing or " + + "abort your changes?
"); + int result = OptionDialog.showOptionNoCancelDialog(rootPanel, "Invalid Function Signature", + message, "Continue Editing", "Abort Changes", OptionDialog.ERROR_MESSAGE); + return result == OptionDialog.OPTION_TWO; // Option 2 is to abort + } + private Component buildAttributePanel() { JPanel panel = new JPanel(new BorderLayout()); panel.setBorder(BorderFactory.createEmptyBorder(0, 5, 15, 15)); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java index dc46d07bd9..4bb56cd1cd 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java @@ -17,8 +17,6 @@ package ghidra.app.plugin.core.function.editor; import java.util.*; -import javax.swing.SwingUtilities; - import ghidra.app.services.DataTypeManagerService; import ghidra.app.util.cparser.C.ParseException; import ghidra.app.util.parser.FunctionSignatureParser; @@ -30,8 +28,7 @@ import ghidra.program.model.listing.Function.FunctionUpdateType; import ghidra.program.model.pcode.Varnode; import ghidra.program.model.symbol.SourceType; import ghidra.program.model.symbol.SymbolUtilities; -import ghidra.util.Msg; -import ghidra.util.SystemUtilities; +import ghidra.util.*; import ghidra.util.exception.*; public class FunctionEditorModel { @@ -85,14 +82,6 @@ public class FunctionEditorModel { this.listener = listener; } -// public FunctionEditorModel(DataTypeManagerService service, Function function, -// ModelChangeListener listener, FunctionDefinitionDataType functionDef, -// String callingConv) { -// this(service, function, listener); -// callingConventionName = callingConv; -// setFunctionData(functionDef); -// } - // Returns the current calling convention or the default calling convention if current unknown private PrototypeModel getEffectiveCallingConvention() { PrototypeModel effectiveCallingConvention = @@ -193,19 +182,10 @@ public class FunctionEditorModel { this.modelChanged |= functionDataChanged; validate(); if (listener != null) { - SwingUtilities.invokeLater(() -> listener.dataChanged()); + Swing.runLater(() -> listener.dataChanged()); } } -// private void notifyParsingModeChanged() { -// SwingUtilities.invokeLater(new Runnable() { -// @Override -// public void run() { -// listener.parsingModeChanged(); -// } -// }); -// } - private void validate() { statusText = ""; if (signatureTransformed) { @@ -237,7 +217,7 @@ public class FunctionEditorModel { returnType = ((TypeDef) returnType).getBaseDataType(); } if (storageSize > 0 && (returnType instanceof AbstractFloatDataType)) { - return true; // dont constrain float storage size + return true; // don't constrain float storage size } int returnDataTypeSize = returnType.getLength(); @@ -406,7 +386,7 @@ public class FunctionEditorModel { } public String getFunctionSignatureTextFromModel() { - StringBuffer buf = new StringBuffer(); + StringBuilder buf = new StringBuilder(); buf.append(returnInfo.getFormalDataType().getName()).append(" "); buf.append(getNameString()); buf.append(" ("); @@ -495,7 +475,6 @@ public class FunctionEditorModel { * Get the effective function to which changes will be made. This * will be the same as function unless it is a thunk in which case * the returned function will be the ultimate non-thunk function. - * @param function * @return non-thunk function */ private Function getAffectiveFunction() { @@ -663,7 +642,8 @@ public class FunctionEditorModel { try { if (autoParamCount < oldAutoCount) { if (oldParams.get( - autoParamCount).getStorage().getAutoParameterType() != storage.getAutoParameterType()) { + autoParamCount).getStorage().getAutoParameterType() != storage + .getAutoParameterType()) { adjustSelectionForRowRemoved(i); } } @@ -1180,6 +1160,10 @@ public class FunctionEditorModel { setSignatureFieldText(getFunctionSignatureTextFromModel()); } + public boolean hasChanges() { + return !Objects.equals(getFunctionSignatureTextFromModel(), signatureFieldText); + } + public void parseSignatureFieldText() throws ParseException, CancelledException { FunctionSignatureParser parser = new FunctionSignatureParser(program.getDataTypeManager(), dataTypeManagerService); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java index 84e1255168..1643b6eb48 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java @@ -398,8 +398,8 @@ public class SymbolTreeProvider extends ComponentProviderAdapter { tree.refilterLater(); } - private void symbolChanged(Symbol symbol) { - addTask(new SymbolChangedTask(tree, symbol)); + private void symbolChanged(Symbol symbol, String oldName) { + addTask(new SymbolChangedTask(tree, symbol, oldName)); } private void symbolAdded(Symbol symbol) { @@ -577,15 +577,18 @@ public class SymbolTreeProvider extends ComponentProviderAdapter { private class SymbolChangedTask extends AbstactSymbolUpdateTask { - SymbolChangedTask(GTree tree, Symbol symbol) { + private String oldName; + + SymbolChangedTask(GTree tree, Symbol symbol, String oldName) { super(tree, symbol); + this.oldName = oldName; } @Override void doRun(TaskMonitor monitor) throws CancelledException { SymbolTreeRootNode root = (SymbolTreeRootNode) tree.getModelRoot(); - root.symbolRemoved(symbol, monitor); + root.symbolRemoved(symbol, oldName, monitor); // the symbol may have been deleted while we are processing bulk changes if (!symbol.isDeleted()) { @@ -662,7 +665,7 @@ public class SymbolTreeProvider extends ComponentProviderAdapter { if (eventType == ChangeManager.DOCR_SYMBOL_RENAMED) { Symbol symbol = (Symbol) object; - symbolChanged(symbol); + symbolChanged(symbol, (String) rec.getOldValue()); } else if (eventType == ChangeManager.DOCR_SYMBOL_DATA_CHANGED || eventType == ChangeManager.DOCR_SYMBOL_SCOPE_CHANGED || @@ -676,7 +679,7 @@ public class SymbolTreeProvider extends ComponentProviderAdapter { symbol = ((Namespace) object).getSymbol(); } - symbolChanged(symbol); + symbolChanged(symbol, symbol.getName()); } else if (eventType == ChangeManager.DOCR_SYMBOL_ADDED) { Symbol symbol = (Symbol) rec.getNewValue(); @@ -693,7 +696,7 @@ public class SymbolTreeProvider extends ComponentProviderAdapter { SymbolTable symbolTable = program.getSymbolTable(); Symbol[] symbols = symbolTable.getSymbols(address); for (Symbol symbol : symbols) { - symbolChanged(symbol); + symbolChanged(symbol, symbol.getName()); } } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/actions/DeleteAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/actions/DeleteAction.java index 0932c3bb00..15a8cb19ec 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/actions/DeleteAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/actions/DeleteAction.java @@ -1,6 +1,5 @@ /* ### * IP: GHIDRA - * REVIEWED: YES * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,20 +15,19 @@ */ package ghidra.app.plugin.core.symboltree.actions; -import ghidra.app.plugin.core.symboltree.SymbolTreeActionContext; -import ghidra.app.plugin.core.symboltree.SymbolTreePlugin; -import ghidra.app.plugin.core.symboltree.nodes.SymbolNode; -import ghidra.program.model.listing.Program; -import ghidra.program.model.symbol.Symbol; - import java.awt.event.KeyEvent; import javax.swing.Icon; import javax.swing.tree.TreePath; -import resources.ResourceManager; import docking.action.KeyBindingData; import docking.action.MenuData; +import ghidra.app.plugin.core.symboltree.SymbolTreeActionContext; +import ghidra.app.plugin.core.symboltree.SymbolTreePlugin; +import ghidra.app.plugin.core.symboltree.nodes.SymbolNode; +import ghidra.program.model.listing.Program; +import ghidra.program.model.symbol.Symbol; +import resources.ResourceManager; public class DeleteAction extends SymbolTreeContextAction { @@ -60,7 +58,6 @@ public class DeleteAction extends SymbolTreeContextAction { } } - setEnabled(true); return true; } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/MoreNode.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/MoreNode.java new file mode 100644 index 0000000000..1ef7cef511 --- /dev/null +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/MoreNode.java @@ -0,0 +1,118 @@ +/* ### + * IP: GHIDRA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ghidra.app.plugin.core.symboltree.nodes; + +import java.awt.datatransfer.DataFlavor; +import java.util.List; + +import javax.swing.Icon; + +import docking.widgets.tree.GTreeNode; +import ghidra.program.model.symbol.Namespace; +import ghidra.util.exception.CancelledException; +import ghidra.util.task.TaskMonitor; +import resources.Icons; + +/** + * Node to represent nodes that are not shown. After showing a handful of symbol nodes + * with the same name, this node will be used in place of the rest of the nodes and + * will display "xx more..." where xx is the number of nodes that are not being shown. + * + */ +public class MoreNode extends SymbolTreeNode { + private static Icon ICON = Icons.MAKE_SELECTION_ICON; + private int count; + private String name; + + MoreNode(String name, int count) { + this.name = name; + this.count = count; + } + + @Override + public String getName() { + return count + " more..."; + } + + @Override + public boolean canCut() { + return false; + } + + @Override + public boolean canPaste(List pastedNodes) { + return false; + } + + @Override + public void setNodeCut(boolean isCut) { + throw new UnsupportedOperationException("Cannot cut an organization node"); + } + + @Override + public boolean isCut() { + return false; + } + + @Override + public DataFlavor getNodeDataFlavor() { + return null; + } + + @Override + public boolean supportsDataFlavors(DataFlavor[] dataFlavors) { + return false; + } + + @Override + public Namespace getNamespace() { + return null; + } + + @Override + public List generateChildren(TaskMonitor monitor) throws CancelledException { + // not used, children generated in constructor + return null; + } + + @Override + public Icon getIcon(boolean expanded) { + return ICON; + } + + @Override + public String getToolTip() { + return "There are " + count + " nodes named \"" + + name + "\" not being shown"; + } + + @Override + public boolean isLeaf() { + return true; + } + + void incrementCount() { + count++; + } + + void decrementCount() { + count = Math.max(0, --count); + } + + boolean isEmpty() { + return count == 0; + } +} diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java index e705e5c580..e40d212f30 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNode.java @@ -22,16 +22,18 @@ import javax.swing.Icon; import docking.widgets.tree.GTreeNode; import ghidra.program.model.symbol.Namespace; -import ghidra.util.datastruct.IntArray; import ghidra.util.exception.CancelledException; import ghidra.util.task.TaskMonitor; import resources.ResourceManager; /** - * See {@link #computeChildren(List, int, GTreeNode, int, TaskMonitor)} for details on + * These nodes are used to organize large lists of nodes into a hierachical structure based on + * the node names. See {@link #organize(List, int, TaskMonitor)} for details on * how this class works. */ public class OrganizationNode extends SymbolTreeNode { + public static final int MAX_SAME_NAME = 10; + static final Comparator COMPARATOR = new OrganizationNodeComparator(); private static Icon OPEN_FOLDER_GROUP_ICON = @@ -40,45 +42,38 @@ public class OrganizationNode extends SymbolTreeNode { ResourceManager.loadImage("images/closedFolderGroup.png"); private String baseName; + private int totalCount; - /** - * You cannot instantiate this class directly, instead use the factory method below - * {@link #organize(List, int, TaskMonitor)} - * @throws CancelledException if the operation is cancelled - */ - private OrganizationNode(List list, int max, int parentLevel, TaskMonitor monitor) + private MoreNode moreNode; + + private OrganizationNode(List list, int maxGroupSize, TaskMonitor monitor) throws CancelledException { + totalCount = list.size(); + // organize children further if the list is too big + List children = organize(list, maxGroupSize, monitor); - doSetChildren(computeChildren(list, max, this, parentLevel, monitor)); + // if all the entries have the same name and we have more than a handful, show only + // a few and add a special "More" node + if (children.size() > MAX_SAME_NAME && hasSameName(children)) { + // they all have the same name, so just use that as this nodes name + baseName = children.get(0).getName(); - GTreeNode child = getChild(0); - baseName = child.getName().substring(0, getPrefixSizeForGrouping(getChildren(), 1) + 1); + children = new ArrayList<>(children.subList(0, MAX_SAME_NAME)); + moreNode = new MoreNode(baseName, totalCount - MAX_SAME_NAME); + children.add(moreNode); + } + else { + // name this node the prefix that all children nodes have in common + baseName = getCommonPrefix(children); + } + doSetChildren(children); } /** - * A factory method for creating OrganizationNode objects. - * See {@link #computeChildren(List, int, GTreeNode, int, TaskMonitor)} - * - * @param nodes the original list of child nodes to be subdivided. - * @param max The max number of child nodes per parent node at any node level. - * @param monitor the task monitor used to cancel this operation - * @return A list of nodes that is based upon the given list, but subdivided as needed. - * @throws CancelledException if the operation is cancelled - * @see #computeChildren(List, int, GTreeNode, int, TaskMonitor) - */ - public static List organize(List nodes, int max, TaskMonitor monitor) - throws CancelledException { - return organize(nodes, null, max, monitor); - } - - private static List organize(List nodes, GTreeNode parent, int max, - TaskMonitor monitor) throws CancelledException { - return computeChildren(nodes, max, parent, 0, monitor); - } - - /** - * Subdivide the given list of nodes such that the list or no new parent created will have - * more than maxNodes children. + * Subdivide the given list of nodes recursively such that there are generally not more + * than maxGroupSize number of nodes at any level. Also, if there are ever many + * nodes of the same name, a group for them will be created and only a few will be shown with + * an "xx more..." node to indicate there are additional nodes that are not shown. *

* This algorithm uses the node names to group nodes based upon common prefixes. For example, * if a parent node contained more than maxNodes children then a possible grouping @@ -98,103 +93,47 @@ public class OrganizationNode extends SymbolTreeNode { * g * *

- * The algorithm prefers to group nodes that have the longest common prefixes. - *

- * Note: the given data must be sorted for this method to work properly. - * - * @param list list of child nodes of parent to breakup into smaller groups. - * @param maxNodes The max number of child nodes per parent node at any node level. - * @param parent The parent of the given children - * @param parentLevel node depth in the tree of Organization nodes. - * @return the given list sub-grouped as outlined above. + * @param list list of child nodes of to breakup into smaller groups + * @param maxGroupSize the max number of nodes to allow before trying to organize into + * smaller groups + * @param monitor the TaskMonitor to be checked for canceling this operation + * @return the given list sub-grouped as outlined above * @throws CancelledException if the operation is cancelled */ - private static List computeChildren(List list, int maxNodes, - GTreeNode parent, int parentLevel, TaskMonitor monitor) throws CancelledException { - List children; - if (list.size() <= maxNodes) { - children = new ArrayList<>(list); + public static List organize(List list, int maxGroupSize, + TaskMonitor monitor) throws CancelledException { + + Map> prefixMap = partition(list, maxGroupSize, monitor); + + // if they didn't partition, just add all given nodes as children + if (prefixMap == null) { + return new ArrayList<>(list); } - else { - int characterOffset = getPrefixSizeForGrouping(list, maxNodes); - characterOffset = Math.max(characterOffset, parentLevel + 1); + // otherwise, the nodes have been partitioned into groups with a common prefix + // loop through and create organization nodes for groups larger than one element + List children = new ArrayList<>(); + for (String prefix : prefixMap.keySet()) { + monitor.checkCanceled(); - children = new ArrayList<>(); - String prevStr = list.get(0).getName(); - int start = 0; - int end = list.size(); - for (int i = 1; i < end; i++) { - monitor.checkCanceled(); - String str = list.get(i).getName(); - if (stringsDiffer(prevStr, str, characterOffset)) { - addNode(children, list, start, i - 1, maxNodes, characterOffset, monitor); - start = i; - } - prevStr = str; + List nodesSamePrefix = prefixMap.get(prefix); + + // all the nodes that don't have a common prefix get added directly + if (prefix.isEmpty()) { + children.addAll(nodesSamePrefix); + } + // groups with one entry, just add in the element directly + else if (nodesSamePrefix.size() == 1) { + children.addAll(nodesSamePrefix); + } + else { + // add an organization node for each unique prefix + children.add(new OrganizationNode(nodesSamePrefix, maxGroupSize, monitor)); } - addNode(children, list, start, end - 1, maxNodes, characterOffset, monitor); } return children; } - private static boolean stringsDiffer(String s1, String s2, int diffLevel) { - if (s1.length() <= diffLevel || s2.length() <= diffLevel) { - return true; - } - return s1.substring(0, diffLevel + 1) - .compareToIgnoreCase(s2.substring(0, diffLevel + 1)) != 0; - } - - private static void addNode(List children, List list, int start, int end, - int max, int diffLevel, TaskMonitor monitor) throws CancelledException { - if (end - start > 0) { - children.add( - new OrganizationNode(list.subList(start, end + 1), max, diffLevel, monitor)); - } - else { - GTreeNode node = list.get(start); - children.add(node); - } - } - - /** - * Returns the longest prefix size such that the list of nodes can be grouped by - * those prefixes while not exceeding maxNodes number of children. - */ - private static int getPrefixSizeForGrouping(List list, int maxNodes) { - IntArray prefixSizeCountBins = new IntArray(); - Iterator it = list.iterator(); - String previousNodeName = it.next().getName(); - prefixSizeCountBins.put(0, 1); - while (it.hasNext()) { - String currentNodeName = it.next().getName(); - int prefixSize = getCommonPrefixSize(previousNodeName, currentNodeName); - prefixSizeCountBins.put(prefixSize, prefixSizeCountBins.get(prefixSize) + 1); - previousNodeName = currentNodeName; - } - - int binContentsTotal = 0; - for (int i = 0; i <= prefixSizeCountBins.getLastNonEmptyIndex(); i++) { - binContentsTotal += prefixSizeCountBins.get(i); - if (binContentsTotal > maxNodes) { - return Math.max(0, i - 1); // we've crossed the max; take a step back - } - } - - return prefixSizeCountBins.getLastNonEmptyIndex(); // all are allowed; use max prefix size - } - - private static int getCommonPrefixSize(String s1, String s2) { - int maxCompareLength = Math.min(s1.length(), s2.length()); - for (int i = 0; i < maxCompareLength; i++) { - if (Character.toUpperCase(s1.charAt(i)) != Character.toUpperCase(s2.charAt(i))) { - return i; - } - } - return maxCompareLength; // one string is a subset of the other (or the same) - } - @Override public boolean equals(Object o) { if (this == o) { @@ -226,10 +165,6 @@ public class OrganizationNode extends SymbolTreeNode { return false; } - public boolean isModifiable() { - return false; - } - @Override public void setNodeCut(boolean isCut) { throw new UnsupportedOperationException("Cannot cut an organization node"); @@ -245,12 +180,12 @@ public class OrganizationNode extends SymbolTreeNode { @Override public String getName() { - return baseName + "..."; + return baseName; } @Override public String getToolTip() { - return getName(); + return "Contains labels that start with \"" + getName() + "\" (" + totalCount + ")"; } @Override @@ -282,6 +217,11 @@ public class OrganizationNode extends SymbolTreeNode { * @param newNode the node to insert. */ public void insertNode(GTreeNode newNode) { + if (moreNode != null) { + moreNode.incrementCount(); + return; + } + int index = Collections.binarySearch(getChildren(), newNode, getChildrenComparator()); if (index >= 0) { // found a match @@ -309,6 +249,23 @@ public class OrganizationNode extends SymbolTreeNode { return null; } + // special case: all symbols in this group have the same name. + if (moreNode != null) { + if (!symbolName.equals(baseName)) { + return null; + } + // The node either belongs to this node's children or it is represented by the + // 'more' node + for (GTreeNode child : children()) { + SymbolTreeNode symbolTreeNode = (SymbolTreeNode) child; + if (symbolTreeNode.getSymbol() == key.getSymbol()) { + return child; + } + } + + return moreNode; + } + // // Note: The 'key' node used for searching will find us the parent node of the symbol // that has changed if it is an org node (this is because the org node searches @@ -339,6 +296,112 @@ public class OrganizationNode extends SymbolTreeNode { return COMPARATOR; } + // We are being tricky here. The findSymbolTreeNode above returns the 'more' node + // if the searched node is one of the nodes not being shown, so then the removeNode gets + // called with the 'more' node, which just means to decrement the count. + @Override + public void removeNode(GTreeNode node) { + if (node == moreNode) { + moreNode.decrementCount(); + if (!moreNode.isEmpty()) { + return; + } + // The 'more' node is empty, just let it be removed + moreNode = null; + } + super.removeNode(node); + } + + @Override + public List generateChildren(TaskMonitor monitor) throws CancelledException { + // not used, children generated in constructor + return null; + } + + private String getCommonPrefix(List children) { + int commonPrefixSize = getCommonPrefixSize(children); + return children.get(0).getName().substring(0, commonPrefixSize); + } + + /** + * This is the algorithm for partitioning a list of nodes into a hierarchical structure based + * on common prefixes + * @param nodeList the list of nodes to be partitioned + * @param maxGroupSize the maximum number of nodes in a group before an organization is attempted + * @param monitor {@link TaskMonitor} so the operation can be cancelled + * @return a map of common prefixes to lists of nodes that have that common prefix. Returns null + * if the size is less than maxGroupSize or the partition didn't reduce the number of nodes + * @throws CancelledException if the operation was cancelled + */ + private static Map> partition(List nodeList, + int maxGroupSize, TaskMonitor monitor) throws CancelledException { + + // no need to partition of the number of nodes is small enough + if (nodeList.size() <= maxGroupSize) { + return null; + } + int commonPrefixSize = getCommonPrefixSize(nodeList); + int uniquePrefixSize = commonPrefixSize + 1; + Map> map = new LinkedHashMap<>(); + for (GTreeNode node : nodeList) { + monitor.checkCanceled(); + String prefix = getPrefix(node, uniquePrefixSize); + List list = map.computeIfAbsent(prefix, k -> new ArrayList()); + list.add(node); + } + if (map.size() == 1) { + return null; + } + if (map.size() >= nodeList.size()) { + return null; // no reduction + } + + return map; + } + + private static String getPrefix(GTreeNode gTreeNode, int uniquePrefixSize) { + String name = gTreeNode.getName(); + if (name.length() <= uniquePrefixSize) { + return name; + } + return name.substring(0, uniquePrefixSize); + } + + private static int getCommonPrefixSize(List list) { + GTreeNode node = list.get(0); + String first = node.getName(); + int inCommonSize = first.length(); + for (int i = 1; i < list.size(); i++) { + String next = list.get(i).getName(); + inCommonSize = Math.min(inCommonSize, getCommonPrefixSize(first, next, inCommonSize)); + } + return inCommonSize; + } + + private static int getCommonPrefixSize(String base, String candidate, int max) { + int maxCompareLength = Math.min(max, candidate.length()); + for (int i = 0; i < maxCompareLength; i++) { + if (base.charAt(i) != candidate.charAt(i)) { + return i; + } + } + return maxCompareLength; // one string is a subset of the other (or the same) + } + + private static boolean hasSameName(List list) { + if (list.size() < 2) { + return false; + } + String name = list.get(0).getName(); + for (GTreeNode node : list) { + if (!name.equals(node.getName())) { + return false; + } + } + return true; + + } + static class OrganizationNodeComparator implements Comparator { @Override public int compare(GTreeNode g1, GTreeNode g2) { @@ -355,9 +418,4 @@ public class OrganizationNode extends SymbolTreeNode { } } - @Override - public List generateChildren(TaskMonitor monitor) throws CancelledException { - // not used, children generated in constructor - return null; - } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java index 098444f696..53e4cbef6e 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolCategoryNode.java @@ -28,6 +28,7 @@ import ghidra.util.task.TaskMonitor; import ghidra.util.task.TaskMonitorAdapter; public abstract class SymbolCategoryNode extends SymbolTreeNode { + private static final int MAX_NODES = 40; protected SymbolCategory symbolCategory; protected SymbolTable symbolTable; protected GlobalNamespace globalNamespace; @@ -53,10 +54,7 @@ public abstract class SymbolCategoryNode extends SymbolTreeNode { SymbolType symbolType = symbolCategory.getSymbolType(); List list = getSymbols(symbolType, monitor); monitor.checkCanceled(); - if (list.size() > MAX_CHILD_NODES) { - list = OrganizationNode.organize(list, MAX_CHILD_NODES, monitor); - } - return list; + return OrganizationNode.organize(list, MAX_NODES, monitor); } public Program getProgram() { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolTreeNode.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolTreeNode.java index 1915b28426..d92c3b5695 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolTreeNode.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/SymbolTreeNode.java @@ -41,8 +41,6 @@ import ghidra.util.task.TaskMonitor; */ public abstract class SymbolTreeNode extends GTreeSlowLoadingNode { - public static final int MAX_CHILD_NODES = 40; - public static final Comparator SYMBOL_COMPARATOR = (s1, s2) -> { // note: not really sure if we care about the cases where 'symbol' is null, as that // implies the symbol was deleted and the node will go away. Just be consistent. diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bean/SetEquateDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bean/SetEquateDialog.java index b42a1e404d..02b1a0e326 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bean/SetEquateDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bean/SetEquateDialog.java @@ -16,8 +16,7 @@ package ghidra.app.util.bean; import java.awt.*; -import java.awt.event.MouseAdapter; -import java.awt.event.MouseEvent; +import java.awt.event.*; import java.util.*; import java.util.List; import java.util.stream.Collectors; @@ -28,6 +27,7 @@ import javax.swing.table.TableColumnModel; import org.apache.commons.lang3.StringUtils; import docking.DialogComponentProvider; +import docking.actions.KeyBindingUtils; import docking.widgets.button.GRadioButton; import docking.widgets.checkbox.GCheckBox; import docking.widgets.filter.FilterListener; @@ -220,7 +220,7 @@ public class SetEquateDialog extends DialogComponentProvider { return entries; } - /** + /* * Builds the main panel of the dialog and returns it. */ protected JPanel buildMainPanel() { @@ -274,10 +274,18 @@ public class SetEquateDialog extends DialogComponentProvider { suggestedEquatesTable = new GhidraTable(model); suggestedEquatesTable.setSelectionMode(ListSelectionModel.SINGLE_SELECTION); - JPanel tablePanel = new JPanel(new BorderLayout()); - JScrollPane scrollPane = new JScrollPane(suggestedEquatesTable); - tablePanel.add(scrollPane); + // allows users to press enter in the table to accept the current selection + KeyStroke enter = KeyStroke.getKeyStroke(KeyEvent.VK_ENTER, 0); + AbstractAction action = new AbstractAction() { + @Override + public void actionPerformed(ActionEvent e) { + okCallback(); + } + }; + KeyBindingUtils.registerAction(suggestedEquatesTable, enter, action, + JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT); + // allows users to double-click a row to accept the item suggestedEquatesTable.addMouseListener(new MouseAdapter() { @Override public void mouseReleased(MouseEvent e) { @@ -289,6 +297,10 @@ public class SetEquateDialog extends DialogComponentProvider { } } }); + + JPanel tablePanel = new JPanel(new BorderLayout()); + JScrollPane scrollPane = new JScrollPane(suggestedEquatesTable); + tablePanel.add(scrollPane); tablePanel.setBorder(BorderFactory.createEmptyBorder(2, 5, 5, 5)); filterPanel = @@ -378,7 +390,7 @@ public class SetEquateDialog extends DialogComponentProvider { return result; } - /** + /* * Get the Equate Name entered or chosen by the user. */ public String getEquateName() { @@ -441,7 +453,7 @@ public class SetEquateDialog extends DialogComponentProvider { /** * Returns the type of selection the user has chosen. * - * @return + * @return the selection type */ public SelectionType getSelectionType() { if (applyToAll.isSelected()) { @@ -458,7 +470,7 @@ public class SetEquateDialog extends DialogComponentProvider { /** * Returns true if the user has chosen to overwrite any existing equate rules. * - * @return + * @return true if the user has chosen to overwrite any existing equate rules. */ public boolean getOverwriteExisting() { return overwriteExistingEquates.isSelected(); @@ -492,6 +504,7 @@ public class SetEquateDialog extends DialogComponentProvider { /** * Sets the dialogs status display to the given message. + * @param text the text */ void setStatus(String text) { this.setStatusText(text); diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedActions2Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedActions2Test.java index f9398846bc..6d19c55eb8 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedActions2Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedActions2Test.java @@ -307,10 +307,10 @@ public class StructureEditorLockedActions2Test extends AbstractStructureEditorLo assertTrue(internalDt0.isEquivalent(originalDt1)); assertTrue(internalDt1.isEquivalent(originalDt2)); assertTrue(internalDt2.isEquivalent(originalDt3)); - assertEquals(14, getDataType(1).getLength()); - assertEquals(14, getModel().getComponent(1).getLength()); + assertEquals(16, getDataType(1).getLength()); + assertEquals(16, getModel().getComponent(1).getLength()); assertEquals(originalDt4, getDataType(2)); - assertEquals(32, getModel().getLength()); + assertEquals(36, getModel().getLength()); } /** diff --git a/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNodeTest.java b/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNodeTest.java new file mode 100644 index 0000000000..91f04cbc18 --- /dev/null +++ b/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNodeTest.java @@ -0,0 +1,193 @@ +/* ### + * IP: GHIDRA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ghidra.app.plugin.core.symboltree.nodes; + +import static org.junit.Assert.*; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; + +import docking.test.AbstractDockingTest; +import docking.widgets.tree.GTreeNode; +import ghidra.program.model.symbol.Symbol; +import ghidra.program.model.symbol.StubSymbol; +import ghidra.util.Swing; +import ghidra.util.exception.AssertException; +import ghidra.util.exception.CancelledException; +import ghidra.util.task.TaskMonitor; + +public class OrganizationNodeTest extends AbstractDockingTest { + + @Test + public void testOrganizeDoesNothingIfBelowMaxGroupSize() { + List nodeList = + nodes("AAA", "AAB", "AAB", "AABA", "BBA", "BBB", "BBC", "CCC", "DDD"); + List result = organize(nodeList, 10); + assertEquals(nodeList, result); + + result = organize(nodeList, 5); + assertNotEquals(nodeList, result); + } + + @Test + public void testBasicPartitioning() { + List nodeList = nodes("AAA", "AAB", "AAC", "BBA", "BBB", "BBC", "CCC", "DDD"); + List result = organize(nodeList, 5); + assertEquals(4, result.size()); + assertEquals("AA", result.get(0).getName()); + assertEquals("BB", result.get(1).getName()); + assertEquals("CCC", result.get(2).getName()); + assertEquals("DDD", result.get(3).getName()); + + GTreeNode aaGroup = result.get(0); + assertEquals(3, aaGroup.getChildCount()); + assertEquals("AAA", aaGroup.getChild(0).getName()); + assertEquals("AAB", aaGroup.getChild(1).getName()); + assertEquals("AAC", aaGroup.getChild(2).getName()); + + GTreeNode bbGroup = result.get(1); + assertEquals(3, bbGroup.getChildCount()); + } + + @Test + public void testMultiLevel() { + List nodeList = nodes("A", "B", "CAA", "CAB", "CAC", "CAD", "CAE", "CAF", "CBA"); + List result = organize(nodeList, 5); + assertEquals(3, result.size()); + assertEquals("A", result.get(0).getName()); + assertEquals("B", result.get(1).getName()); + assertEquals("C", result.get(2).getName()); + + GTreeNode cGroup = result.get(2); + assertEquals(2, cGroup.getChildCount()); + assertEquals("CA", cGroup.getChild(0).getName()); + assertEquals("CBA", cGroup.getChild(1).getName()); + + GTreeNode caGroup = cGroup.getChild(0); + assertEquals(6, caGroup.getChildCount()); + assertEquals("CAA", caGroup.getChild(0).getName()); + assertEquals("CAF", caGroup.getChild(5).getName()); + } + + @Test + public void testManySameLabels() { + List nodeList = + nodes("A", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", + "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP"); + + List result = organize(nodeList, 5); + assertEquals(2, result.size()); + assertEquals("A", result.get(0).getName()); + assertEquals("DUP", result.get(1).getName()); + + GTreeNode dupNode = result.get(1); + assertEquals(OrganizationNode.MAX_SAME_NAME + 1, dupNode.getChildCount()); + assertEquals("11 more...", dupNode.getChild(OrganizationNode.MAX_SAME_NAME).getName()); + + } + + @Test + public void testRemoveNotShownNode() { + List nodeList = + nodes("A", "D1", "D2", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", + "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP"); + + List result = organize(nodeList, 5); + + SymbolTreeNode dNode = (SymbolTreeNode) result.get(1); + GTreeNode dupNode = dNode.getChild(2); + + assertEquals(OrganizationNode.MAX_SAME_NAME + 1, dupNode.getChildCount()); + assertEquals("11 more...", dupNode.getChild(OrganizationNode.MAX_SAME_NAME).getName()); + + SymbolTreeNode node = (SymbolTreeNode) nodeList.get(nodeList.size() - 1); + simulateSmbolDeleted(dNode, node.getSymbol()); + + assertEquals("10 more...", dupNode.getChild(dupNode.getChildCount() - 1).getName()); + } + + @Test + public void testRemoveShownNode() { + List nodeList = + nodes("A", "D1", "D2", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", + "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP"); + + List result = organize(nodeList, 5); + + SymbolTreeNode dNode = (SymbolTreeNode) result.get(1); + GTreeNode dupNode = dNode.getChild(2); + + assertEquals(OrganizationNode.MAX_SAME_NAME + 1, dupNode.getChildCount()); + assertEquals("11 more...", dupNode.getChild(OrganizationNode.MAX_SAME_NAME).getName()); + + SymbolTreeNode node = (SymbolTreeNode) nodeList.get(4); + simulateSmbolDeleted(dNode, node.getSymbol()); + + assertEquals(OrganizationNode.MAX_SAME_NAME, dupNode.getChildCount()); + assertEquals("11 more...", dupNode.getChild(dupNode.getChildCount() - 1).getName()); + } + + @Test + public void testAddDupNodeJustIncrementsCount() { + List nodeList = + nodes("A", "D1", "D2", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", + "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP", "DUP"); + + List result = organize(nodeList, 5); + + SymbolTreeNode dNode = (SymbolTreeNode) result.get(1); + GTreeNode dupNode = dNode.getChild(2); + + assertEquals(OrganizationNode.MAX_SAME_NAME + 1, dupNode.getChildCount()); + assertEquals("11 more...", dupNode.getChild(OrganizationNode.MAX_SAME_NAME).getName()); + + ((OrganizationNode) dNode).insertNode(node("DUP")); + + assertEquals(OrganizationNode.MAX_SAME_NAME + 1, dupNode.getChildCount()); + assertEquals("12 more...", dupNode.getChild(OrganizationNode.MAX_SAME_NAME).getName()); + + } + + private void simulateSmbolDeleted(SymbolTreeNode root, Symbol symbolToDelete) { + SymbolNode key = SymbolNode.createKeyNode(symbolToDelete, symbolToDelete.getName(), null); + GTreeNode found = root.findSymbolTreeNode(key, false, TaskMonitor.DUMMY); + Swing.runNow(() -> found.getParent().removeNode(found)); + } + + private List organize(List list, int size) { + try { + return OrganizationNode.organize(list, size, TaskMonitor.DUMMY); + } + catch (CancelledException e) { + throw new AssertException("Can't happen"); + } + } + + private List nodes(String... names) { + List list = new ArrayList<>(); + for (String name : names) { + list.add(node(name)); + } + return list; + } + + private GTreeNode node(String name) { + return new CodeSymbolNode(null, new StubSymbol(name, null)); + } + +} diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/cast.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/cast.cc index f11695524c..97c4a14322 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/cast.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/cast.cc @@ -231,7 +231,7 @@ Datatype *CastStrategyC::castStandard(Datatype *reqtype,Datatype *curtype, care_uint_int = true; isptr = true; } - if (curtype == reqtype) return (Datatype *)0; // Different typedefs could point to the same type + if (curbase == reqbase) return (Datatype *)0; // Different typedefs could point to the same type if ((reqbase->getMetatype()==TYPE_VOID)||(curtype->getMetatype()==TYPE_VOID)) return (Datatype *)0; // Don't cast from or to VOID if (reqbase->getSize() != curbase->getSize()) { diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc index 4d7277572c..cd799f631f 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc @@ -76,7 +76,7 @@ void StackSolver::propagate(int4 varnum,int4 val) while(!workstack.empty()) { varnum = workstack.back(); workstack.pop_back(); - + eqn.var1 = varnum; top = lower_bound(eqs.begin(),eqs.end(),eqn,StackEqn::compare); while((top!=eqs.end())&&((*top).var1 == varnum)) { @@ -212,7 +212,7 @@ void StackSolver::build(const Funcdata &data,AddrSpace *id,int4 spcbase) } } } - + eqn.rhs = 4; // Otherwise make a guess guess.push_back(eqn); } @@ -274,7 +274,7 @@ void ActionStackPtrFlow::analyzeExtraPop(Funcdata &data,AddrSpace *stackspace,in } if (solver.getNumVariables() == 0) return; solver.solve(); // Solve the equations - + Varnode *invn = solver.getVariable(0); bool warningprinted = false; @@ -622,7 +622,7 @@ int4 ActionSegmentize::apply(Funcdata &data) vector bindlist; bindlist.push_back((Varnode *)0); bindlist.push_back((Varnode *)0); - + for(int4 i=0;iuserops.getSegmentOp(i); if (segdef == (SegmentOp *)0) continue; @@ -703,7 +703,7 @@ int4 ActionConstbase::apply(Funcdata &data) // PcodeOp *op; // list::const_iterator iter; // uintm hash; - + // for(iter=data.op_alive_begin();iter!=data.op_alive_end();++iter) { // op = *iter; // hash = op->getCseHash(); @@ -1384,7 +1384,7 @@ int4 ActionExtraPopSetup::apply(Funcdata &data) const VarnodeData &point(stackspace->getSpacebase(0)); Address sb_addr(point.space,point.offset); int4 sb_size = point.size; - + for(int4 i=0;igetExtraPop() == 0) continue; // Stack pointer is undisturbed @@ -1552,7 +1552,7 @@ int4 ActionParamDouble::apply(Funcdata &data) data.opSetInput(op,mostvn,slot+1); } count += 1; // Indicate that a change was made - + j -= 1; // Note we decrement j here, so that we can check nested CONCATs } } @@ -1573,7 +1573,7 @@ int4 ActionParamDouble::apply(Funcdata &data) else if (whole.inHandLo(vn1)) { if (whole.getHi() != vn2) continue; isslothi = false; - } + } else continue; if (fc->checkInputJoin(j,isslothi,vn1,vn2)) { @@ -1838,7 +1838,7 @@ int4 ActionReturnRecovery::apply(Funcdata &data) Varnode *vn; list::const_iterator iter,iterend; int4 i; - + int4 maxancestor = data.getArch()->trim_recurse_max; iterend = data.endOp(CPUI_RETURN); AncestorRealistic ancestorReal; @@ -1861,7 +1861,7 @@ int4 ActionReturnRecovery::apply(Funcdata &data) active->finishPass(); if (active->getNumPasses() > active->getMaxPass()) active->markFullyChecked(); - + if (active->isFullyChecked()) { data.getFuncProto().deriveOutputMap(active); iterend = data.endOp(CPUI_RETURN); @@ -1887,7 +1887,7 @@ int4 ActionRestrictLocal::apply(Funcdata &data) Varnode *vn; int4 i; vector::const_iterator eiter,endeiter; - + for(i=0;igetOp(); @@ -2126,7 +2126,7 @@ int4 ActionRestructureHigh::apply(Funcdata &data) l1->restructureHigh(); if (data.syncVarnodesWithSymbols(l1,true)) count += 1; - + #ifdef OPACTION_DEBUG if ((flags&rule_debug)==0) return 0; l1->turnOffDebug(); @@ -2151,7 +2151,7 @@ int4 ActionDefaultParams::apply(Funcdata &data) fc = data.getCallSpecs(i); if (!fc->hasModel()) { Funcdata *otherfunc = fc->getFuncdata(); - + if (otherfunc != (Funcdata *)0) { fc->copy(otherfunc->getFuncProto()); if ((!fc->isModelLocked())&&(!fc->hasMatchingModel(evalfp))) @@ -2165,6 +2165,36 @@ int4 ActionDefaultParams::apply(Funcdata &data) return 0; // Indicate success } +/// \brief Test if the given cast conflict can be resolved by passing to the first structure field +/// +/// Test if the given Varnode data-type is a pointer to a structure and if interpreting +/// the data-type as a pointer to the structure's first field will get it to match the +/// desired data-type. +/// \param vn is the given Varnode +/// \param ct is the desired data-type +/// \param castStrategy is used to determine if the data-types are compatible +/// \return \b true if a pointer to the first field makes sense +bool ActionSetCasts::testStructOffset0(Varnode *vn,Datatype *ct,CastStrategy *castStrategy) + +{ + if (ct->getMetatype() != TYPE_PTR) return false; + Datatype *highType = vn->getHigh()->getType(); + if (highType->getMetatype() != TYPE_PTR) return false; + Datatype *highPtrTo = ((TypePointer *)highType)->getPtrTo(); + if (highPtrTo->getMetatype() != TYPE_STRUCT) return false; + TypeStruct *highStruct = (TypeStruct *)highPtrTo; + if (highStruct->numDepend() == 0) return false; + vector::const_iterator iter = highStruct->beginField(); + if ((*iter).offset != 0) return false; + Datatype *reqtype = ((TypePointer *)ct)->getPtrTo(); + Datatype *curtype = (*iter).type; + if (reqtype->getMetatype() == TYPE_ARRAY) + reqtype = ((TypeArray *)reqtype)->getBase(); + if (curtype->getMetatype() == TYPE_ARRAY) + curtype = ((TypeArray *)curtype)->getBase(); + return (castStrategy->castStandard(reqtype, curtype, true, true) == (Datatype *)0); +} + /// \brief Insert cast to output Varnode type after given PcodeOp if it is necessary /// /// \param op is the given PcodeOp @@ -2229,7 +2259,7 @@ int4 ActionSetCasts::castInput(PcodeOp *op,int4 slot,Funcdata &data,CastStrategy { Datatype *ct; - Varnode *vn; + Varnode *vn,*vnout; PcodeOp *newop; ct = op->getOpcode()->getInputCast(op,slot,castStrategy); // Input type expected by this operation @@ -2253,16 +2283,29 @@ int4 ActionSetCasts::castInput(PcodeOp *op,int4 slot,Funcdata &data,CastStrategy if (vn->getType() == ct) return 1; } + else if (testStructOffset0(vn, ct, castStrategy)) { + // Insert a PTRSUB(vn,#0) instead of a CAST + newop = data.newOp(2,op->getAddr()); + vnout = data.newUniqueOut(vn->getSize(), newop); + vnout->updateType(ct,false,false); + vnout->setImplied(); + data.opSetOpcode(newop, CPUI_PTRSUB); + data.opSetInput(newop,vn,0); + data.opSetInput(newop,data.newConstant(4, 0),1); + data.opSetInput(op,vnout,slot); + data.opInsertBefore(newop,op); + return 1; + } newop = data.newOp(1,op->getAddr()); - vn = data.newUniqueOut(op->getIn(slot)->getSize(),newop); - vn->updateType(ct,false,false); - vn->setImplied(); + vnout = data.newUniqueOut(vn->getSize(),newop); + vnout->updateType(ct,false,false); + vnout->setImplied(); #ifdef CPUI_STATISTICS data.getArch()->stats->countCast(); #endif data.opSetOpcode(newop,CPUI_CAST); - data.opSetInput(newop,op->getIn(slot),0); - data.opSetInput(op,vn,slot); + data.opSetInput(newop,vn,0); + data.opSetInput(op,vnout,slot); data.opInsertBefore(newop,op); // Cast comes AFTER operation return 1; } @@ -2614,7 +2657,7 @@ int4 ActionMarkExplicit::baseExplicit(Varnode *vn,int4 maxref) desccount += 1; if (desccount > maxref) return -1; // Must not exceed max descendants } - + return desccount; } @@ -3010,7 +3053,7 @@ int4 ActionDoNothing::apply(Funcdata &data) int4 i; const BlockGraph &graph(data.getBasicBlocks()); BlockBasic *bb; - + for(i=0;iisDoNothing()) { @@ -3825,7 +3868,7 @@ int4 ActionPrototypeTypes::apply(Funcdata &data) data.opSetInput(op,vn,0); } } - + if (data.getFuncProto().isOutputLocked()) { ProtoParameter *outparam = data.getFuncProto().getOutput(); if (outparam->getType()->getMetatype() != TYPE_VOID) { @@ -3877,7 +3920,7 @@ int4 ActionPrototypeTypes::apply(Funcdata &data) BlockBasic *topbl = (BlockBasic *)0; if (data.getBasicBlocks().getSize() > 0) topbl = (BlockBasic *)data.getBasicBlocks().getBlock(0); - + int4 numparams = data.getFuncProto().numParams(); for(i=0;icode() == CPUI_PTRADD)&&(slot==0)) - return op->getIn(2)->getOffset(); - if ((op->code() == CPUI_PTRSUB)&&(slot==0)) - return op->getIn(1)->getOffset(); + if (op->code() == CPUI_PTRADD) { + if (slot != 0) return 1; + Varnode *constvn = op->getIn(1); + uintb mult = op->getIn(2)->getOffset(); + if (constvn->isConstant()) { + off = (constvn->getOffset() * mult) & calc_mask(constvn->getSize()) ; + return 0; + } + if (sz != 0 && (mult % sz) != 0) + return 1; + return 2; + } + if (op->code() == CPUI_PTRSUB) { + if (slot != 0) return 1; + off = op->getIn(1)->getOffset(); + return 0; + } if (op->code() == CPUI_INT_ADD) { Varnode *othervn = op->getIn(1-slot); // Check if othervn is an offset if (!othervn->isConstant()) { - if ((!othervn->isWritten())||(othervn->getDef()->code() != CPUI_INT_MULT)) - return -1; + if (othervn->isWritten()) { + PcodeOp *multop = othervn->getDef(); + if (multop->code() == CPUI_INT_MULT) { + Varnode *constvn = multop->getIn(1); + if (constvn->isConstant()) { + uintb mult = constvn->getOffset(); + if (mult == calc_mask(constvn->getSize())) // If multiplying by -1 + return 1; // Assume this is a pointer difference and don't propagate + if (sz != 0 && (mult % sz) !=0) + return 1; + } + return 2; + } + } + if (sz == 1) + return 2; + return 1; } if (othervn->getTempType()->getMetatype() == TYPE_PTR) // Check if othervn marked as ptr - return -1; - if (othervn->isConstant()) - return othervn->getOffset(); + return 1; + off = othervn->getOffset(); return 0; } - return -1; + return 1; } /// \brief Propagate a pointer data-type through an ADD operation. @@ -4240,30 +4316,24 @@ int4 ActionInferTypes::propagateAddPointer(PcodeOp *op,int4 slot) /// \param inslot is the edge to propagate along /// \return the transformed Datatype or the original output Datatype Datatype *ActionInferTypes::propagateAddIn2Out(TypeFactory *typegrp,PcodeOp *op,int4 inslot) - + { - Datatype *rettype = op->getIn(inslot)->getTempType(); // We know this is a pointer type - Datatype *tstruct = ((TypePointer *)rettype)->getPtrTo(); - int4 offset = propagateAddPointer(op,inslot); - if (offset==-1) return op->getOut()->getTempType(); // Doesn't look like a good pointer add - uintb uoffset = AddrSpace::addressToByte(offset,((TypePointer *)rettype)->getWordSize()); - if (tstruct->getSize() > 0 && !tstruct->isVariableLength()) - uoffset = uoffset % tstruct->getSize(); - if (uoffset==0) { - if (op->code() == CPUI_PTRSUB) // Go down at least one level - rettype = typegrp->downChain(rettype,uoffset); - if (rettype == (Datatype *)0) - rettype = op->getOut()->getTempType(); - } - else { - while(uoffset != 0) { - rettype = typegrp->downChain(rettype,uoffset); - if (rettype == (Datatype *)0) { - rettype = op->getOut()->getTempType(); // Don't propagate anything - break; - } - } + TypePointer *pointer = (TypePointer *)op->getIn(inslot)->getTempType(); // We know this is a pointer type + uintb uoffset; + int4 command = propagateAddPointer(uoffset,op,inslot,pointer->getPtrTo()->getSize()); + if (command == 1) return op->getOut()->getTempType(); // Doesn't look like a good pointer add + if (command != 2) { + uoffset = AddrSpace::addressToByte(uoffset,pointer->getWordSize()); + bool allowWrap = (op->code() != CPUI_PTRSUB); + do { + pointer = pointer->downChain(uoffset,allowWrap,*typegrp); + if (pointer == (TypePointer *)0) + return op->getOut()->getTempType(); + } while(uoffset != 0); } + Datatype *rettype = pointer; + if (rettype == (Datatype *)0) + rettype = op->getOut()->getTempType(); if (op->getIn(inslot)->isSpacebase()) { if (rettype->getMetatype() == TYPE_PTR) { TypePointer *ptype = (TypePointer *)rettype; @@ -4367,7 +4437,7 @@ bool ActionInferTypes::propagateGoodEdge(PcodeOp *op,int4 inslot,int4 outslot,Va /// \param outslot indicates the edge's output Varnode /// \return \b true if the data-type propagates bool ActionInferTypes::propagateTypeEdge(TypeFactory *typegrp,PcodeOp *op,int4 inslot,int4 outslot) - + { Varnode *invn,*outvn; Datatype *newtype; @@ -4608,7 +4678,7 @@ void ActionInferTypes::propagateRef(Funcdata &data,Varnode *vn,const Address &ad } while(cur != (Datatype *)0); } if (lastct->getSize() != cursize) continue; - + // Try to propagate the reference type into a varnode that is pointed to by that reference if (0>lastct->typeOrder(*curvn->getTempType())) { #ifdef TYPEPROP_DEBUG @@ -4850,7 +4920,7 @@ void ActionDatabase::buildDefaultGroups(void) "cleanup", "merge", "dynamic", "casts", "analysis", "fixateglobals", "fixateproto", "segment", "returnsplit", "nodejoin", "doubleload", "doubleprecis", - "unreachable", "subvar", "floatprecision", + "unreachable", "subvar", "floatprecision", "conditionalexe", "" }; setGroup("decompile",members); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh index 7e0668a6aa..3e1390ab88 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh @@ -310,6 +310,7 @@ public: /// input. In this case, it casts to the necessary pointer type /// immediately. class ActionSetCasts : public Action { + static bool testStructOffset0(Varnode *vn,Datatype *ct,CastStrategy *castStrategy); static int4 castOutput(PcodeOp *op,Funcdata &data,CastStrategy *castStrategy); static int4 castInput(PcodeOp *op,int4 slot,Funcdata &data,CastStrategy *castStrategy); public: @@ -924,7 +925,7 @@ class ActionInferTypes : public Action { int4 localcount; ///< Number of passes performed for this function static void buildLocaltypes(Funcdata &data); ///< Assign initial data-type based on local info static bool writeBack(Funcdata &data); ///< Commit the final propagated data-types to Varnodes - static int4 propagateAddPointer(PcodeOp *op,int4 slot); ///< Test if edge is pointer plus a constant + static int4 propagateAddPointer(uintb &off,PcodeOp *op,int4 slot,int4 sz); ///< Test if edge is pointer plus a constant static Datatype *propagateAddIn2Out(TypeFactory *typegrp,PcodeOp *op,int4 inslot); static bool propagateGoodEdge(PcodeOp *op,int4 inslot,int4 outslot,Varnode *invn); static bool propagateTypeEdge(TypeFactory *typegrp,PcodeOp *op,int4 inslot,int4 outslot); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.cc index a3b4612e87..f645fc7295 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.cc @@ -361,7 +361,7 @@ void Funcdata::spacebaseConstant(PcodeOp *op,int4 slot,SymbolEntry *entry,const bool typelock = sym->isTypeLocked(); if (typelock && (entrytype->getMetatype() == TYPE_UNKNOWN)) typelock = false; - outvn->updateType(ptrentrytype,typelock,true); + outvn->updateType(ptrentrytype,typelock,false); if (extra != 0) { if (extraOp == (PcodeOp *)0) { extraOp = newOp(2,op->getAddr()); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc index a9c28db6c2..bb8a3201e2 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc @@ -6141,6 +6141,33 @@ bool RulePtrArith::verifyAddTreeBottom(PcodeOp *op,int4 slot) return true; } +/// \brief Test for other pointers in the ADD tree above the given op that might be a preferred base +/// +/// This tests the condition of RulePushPtr, making sure that the given op isn't the lone descendant +/// of a pointer constructed by INT_ADD on another pointer (which would then be preferred). +/// \param op is the given op +/// \param slot is the input slot of the pointer +/// \return \b true if the indicated slot holds the preferred pointer +bool RulePtrArith::verifyPreferredPointer(PcodeOp *op,int4 slot) + +{ + Varnode *vn = op->getIn(slot); + // Check if RulePushPtr would apply here + if (op->getIn(1-slot)->getType()->getMetatype() != TYPE_PTR && vn->isWritten()) { + PcodeOp *preOp = vn->getDef(); + if (preOp->code() == CPUI_INT_ADD) { + if (vn->loneDescend() == op) { + int ptrCount = 0; + if (preOp->getIn(0)->getType()->getMetatype() == TYPE_PTR) ptrCount += 1; + if (preOp->getIn(1)->getType()->getMetatype() == TYPE_PTR) ptrCount += 1; + if (ptrCount == 1) + return false; // RulePushPtr would apply, so we are not preferred + } + } + } + return true; +} + /// \class RulePtrArith /// \brief Transform pointer arithmetic /// @@ -6180,11 +6207,14 @@ int4 RulePtrArith::applyOp(PcodeOp *op,Funcdata &data) } if (slot == op->numInput()) return 0; if (!verifyAddTreeBottom(op, slot)) return 0; + if (!verifyPreferredPointer(op, slot)) return 0; const TypePointer *tp = (const TypePointer *) ct; ct = tp->getPtrTo(); // Type being pointed to int4 unitsize = AddrSpace::addressToByteInt(1,tp->getWordSize()); if (ct->getSize() == unitsize) { // Degenerate case + if (op->getOut()->getType()->getMetatype() != TYPE_PTR) // Make sure pointer propagates thru INT_ADD + return 0; vector newparams; newparams.push_back( op->getIn(slot) ); newparams.push_back( op->getIn(1-slot) ); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.hh index 8a0d0c4382..732689751a 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.hh @@ -1028,6 +1028,7 @@ public: }; class RulePtrArith : public Rule { static bool verifyAddTreeBottom(PcodeOp *op,int4 slot); + static bool verifyPreferredPointer(PcodeOp *op,int4 slot); public: RulePtrArith(const string &g) : Rule(g, 0, "ptrarith") {} ///< Constructor virtual Rule *clone(const ActionGroupList &grouplist) const { diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc index 9739980821..10c02eddbc 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/type.cc @@ -556,6 +556,45 @@ void TypePointer::restoreXml(const Element *el,TypeFactory &typegrp) flags = ptrto->getInheritable(); } +/// \brief Find a sub-type pointer given an offset into \b this +/// +/// Add a constant offset to \b this pointer. +/// If there is a valid component at that offset, return a pointer +/// to the data-type of the component or NULL otherwise. +/// This routine only goes down one level at most. Pass back the +/// renormalized offset relative to the new data-type +/// \param off is a reference to the offset to add +/// \param allowArrayWrap is \b true if the pointer should be treated as a pointer to an array +/// \return a pointer datatype for the component or NULL +TypePointer *TypePointer::downChain(uintb &off,bool allowArrayWrap,TypeFactory &typegrp) + +{ + int4 ptrtoSize = ptrto->getSize(); + if (off >= ptrtoSize) { // Check if we are wrapping + if (ptrtoSize != 0 && !ptrto->isVariableLength()) { // Check if pointed-to is wrappable + if (!allowArrayWrap) + return (TypePointer *)0; + intb signOff = (intb)off; + sign_extend(signOff,size*8-1); + signOff = signOff % ptrtoSize; + if (signOff < 0) + signOff = signOff + ptrtoSize; + off = signOff; + if (off == 0) // If we've wrapped and are now at zero + return this; // consider this going down one level + } + } + + // If we know we have exactly one of an array, strip the array to get pointer to element + bool doStrip = (ptrto->getMetatype() != TYPE_ARRAY); + Datatype *pt = ptrto->getSubType(off,&off); + if (pt == (Datatype *)0) + return (TypePointer *)0; + if (doStrip) + return typegrp.getTypePointerStripArray(size, pt, wordsize); + return typegrp.getTypePointer(size,pt,wordsize); +} + void TypeArray::printRaw(ostream &s) const { @@ -2159,31 +2198,6 @@ void TypeFactory::destroyType(Datatype *ct) delete ct; } -/// Add a constant offset to a pointer with known data-type. -/// If there is a valid component at that offset, return a pointer -/// to the data-type of the component or NULL otherwise. -/// This routine only goes down one level at most. Pass back the -/// renormalized offset relative to the new data-type -/// \param ptrtype is the pointer data-type being added to -/// \param off is a reference to the offset to add -/// \return a pointer datatype for the component or NULL -Datatype *TypeFactory::downChain(Datatype *ptrtype,uintb &off) - -{ // Change ptr->struct => ptr->substruct - // where substruct starts at offset off - if (ptrtype->metatype != TYPE_PTR) return (Datatype *)0; - TypePointer *ptype = (TypePointer *)ptrtype; - Datatype *pt = ptype->ptrto; - // If we know we have exactly one of an array, strip the array to get pointer to element - bool doStrip = (pt->getMetatype() != TYPE_ARRAY); - pt = pt->getSubType(off,&off); - if (pt == (Datatype *)0) - return (Datatype *)0; - if (doStrip) - return getTypePointerStripArray(ptype->size, pt, ptype->getWordSize()); - return getTypePointer(ptype->size,pt,ptype->getWordSize()); -} - /// The data-type propagation system can push around data-types that are \e partial or are /// otherwise unrepresentable in the source language. This method substitutes those data-types /// with a concrete data-type that is representable, or returns the same data-type if is already concrete. diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/type.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/type.hh index aa79ae2dda..e318fe7308 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/type.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/type.hh @@ -248,6 +248,7 @@ public: virtual int4 compareDependency(const Datatype &op) const; // For tree structure virtual Datatype *clone(void) const { return new TypePointer(*this); } virtual void saveXml(ostream &s) const; + virtual TypePointer *downChain(uintb &off,bool allowArrayWrap,TypeFactory &typegrp); }; /// \brief Datatype object representing an array of elements @@ -451,7 +452,6 @@ public: const vector &intypes, bool dotdotdot); ///< Create a "function" datatype void destroyType(Datatype *ct); ///< Remove a data-type from \b this - Datatype *downChain(Datatype *ptrtype,uintb &off); ///< Find a sub-type matching a pointer and offset Datatype *concretize(Datatype *ct); ///< Convert given data-type to concrete form void dependentOrder(vector &deporder) const; ///< Place all data-types in dependency order void saveXml(ostream &s) const; ///< Save \b this container to stream diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/typeop.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/typeop.cc index 56ce320bc4..133d993103 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/typeop.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/typeop.cc @@ -1711,7 +1711,7 @@ Datatype *TypeOpPtrsub::getOutputToken(const PcodeOp *op,CastStrategy *castStrat TypePointer *ptype = (TypePointer *)op->getIn(0)->getHigh()->getType(); if (ptype->getMetatype() == TYPE_PTR) { uintb offset = AddrSpace::addressToByte(op->getIn(1)->getOffset(),ptype->getWordSize()); - Datatype *rettype = tlst->downChain(ptype,offset); + Datatype *rettype = ptype->downChain(offset,false,*tlst); if ((offset==0)&&(rettype != (Datatype *)0)) return rettype; } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc index 7a651026cf..ba12e77f39 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc @@ -270,6 +270,7 @@ ScopeLocal::ScopeLocal(uint8 id,AddrSpace *spc,Funcdata *fd,Architecture *g) : S { space = spc; + deepestParamOffset = ~((uintb)0); rangeLocked = false; stackGrowsNegative = true; restrictScope(fd); @@ -310,12 +311,14 @@ void ScopeLocal::collectNameRecs(void) void ScopeLocal::resetLocalWindow(void) { + stackGrowsNegative = fd->getFuncProto().isStackGrowsNegative(); + deepestParamOffset = stackGrowsNegative ? ~((uintb)0) : 0; + if (rangeLocked) return; - localRange = fd->getFuncProto().getLocalRange(); + const RangeList &localRange( fd->getFuncProto().getLocalRange() ); const RangeList ¶mrange( fd->getFuncProto().getParamRange() ); - stackGrowsNegative = fd->getFuncProto().isStackGrowsNegative(); RangeList newrange; set::const_iterator iter; @@ -375,16 +378,13 @@ void ScopeLocal::markNotMapped(AddrSpace *spc,uintb first,int4 sz,bool parameter last = spc->getHighest(); if (parameter) { // Everything above parameter if (stackGrowsNegative) { - const Range *rng = localRange.getRange(spc,first); - if (rng != (const Range *)0) - first = rng->getFirst(); // Everything less is not mapped + if (first < deepestParamOffset) + deepestParamOffset = first; } else { - const Range *rng = localRange.getRange(spc,last); - if (rng != (const Range *)0) - last = rng->getLast(); // Everything greater is not mapped + if (first > deepestParamOffset) + deepestParamOffset = first; } - sz = (last-first)+1; } Address addr(space,first); // Remove any symbols under range @@ -427,6 +427,11 @@ string ScopeLocal::buildVariableName(const Address &addr, s << 'X'; // Indicate local stack space allocated by caller start = -start; } + else { + if (deepestParamOffset + 1 > 1 && stackGrowsNegative == (addr.getOffset() < deepestParamOffset)) { + s << 'Y'; // Indicate unusual region of stack + } + } s << dec << start; return makeNameUnique(s.str()); } @@ -1095,6 +1100,9 @@ void ScopeLocal::markUnaliased(const vector &alias) EntryMap *rangemap = maptable[space->getIndex()]; if (rangemap == (EntryMap *)0) return; list::iterator iter,enditer; + set::const_iterator rangeIter, rangeEndIter; + rangeIter = getRangeTree().begin(); + rangeEndIter = getRangeTree().end(); int4 alias_block_level = glb->alias_block_level; bool aliason = false; @@ -1105,31 +1113,39 @@ void ScopeLocal::markUnaliased(const vector &alias) enditer = rangemap->end_list(); while(iter!=enditer) { - if ((i 0xffff)) - aliason = false; - if (!aliason) - symbol->getScope()->setAttribute(symbol,Varnode::nolocalalias); - if (symbol->isTypeLocked() && alias_block_level != 0) { - if (alias_block_level == 3) - aliason = false; // For this level, all locked data-types block aliases - else { - type_metatype meta = symbol->getType()->getMetatype(); - if (meta == TYPE_STRUCT) - aliason = false; // Only structures block aliases - else if (meta == TYPE_ARRAY && alias_block_level > 1) - aliason = false; // Only arrays (and structures) block aliases - } + // Aliases shouldn't go thru unmapped regions of the local variables + while(rangeIter != rangeEndIter) { + const Range &rng(*rangeIter); + if (rng.getSpace() == space) { + if (rng.getFirst() > curalias && curoff >= rng.getFirst()) + aliason = false; + if (rng.getLast() >= curoff) break; // Check if symbol past end of mapped range + if (rng.getLast() > curalias) // If past end of range AND past last alias offset + aliason = false; // turn aliases off + } + ++rangeIter; + } + Symbol *symbol = entry.getSymbol(); + // Test if there is enough distance between symbol + // and last alias to warrant ignoring the alias + // NOTE: this is primarily to reset aliasing between + // stack parameters and stack locals + if (aliason && (curoff - curalias > 0xffff)) aliason = false; + if (!aliason) symbol->getScope()->setAttribute(symbol,Varnode::nolocalalias); + if (symbol->isTypeLocked() && alias_block_level != 0) { + if (alias_block_level == 3) + aliason = false; // For this level, all locked data-types block aliases + else { + type_metatype meta = symbol->getType()->getMetatype(); + if (meta == TYPE_STRUCT) + aliason = false; // Only structures block aliases + else if (meta == TYPE_ARRAY && alias_block_level > 1) aliason = false;// Only arrays (and structures) block aliases } } } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh index 8a4f9a0fdb..0944a3c975 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh @@ -195,10 +195,10 @@ public: /// portions are used for temporary storage (not mapped), and what portion is for parameters. class ScopeLocal : public ScopeInternal { AddrSpace *space; ///< Address space containing the local stack - RangeList localRange; ///< The set of addresses that might hold mapped locals (not parameters) list nameRecommend; ///< Symbol name recommendations for specific addresses list dynRecommend; ///< Symbol name recommendations for dynamic locations list typeRecommend; ///< Data-types for specific storage locations + uintb deepestParamOffset; ///< Deepest position of a parameter passed (to a called function) on the stack bool stackGrowsNegative; ///< Marked \b true if the stack is considered to \e grow towards smaller offsets bool rangeLocked; ///< True if the subset of addresses \e mapped to \b this scope has been locked bool adjustFit(RangeHint &a) const; ///< Make the given RangeHint fit in the current Symbol map diff --git a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java index 61c07b36d2..0d07f4211f 100644 --- a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java +++ b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java @@ -297,7 +297,16 @@ public class DefaultGraphDisplay implements GraphDisplay { Lens lens = Lens.builder().lensShape(Lens.Shape.RECTANGLE).magnification(3.f).build(); lens.setMagnification(2.f); LensMagnificationGraphMousePlugin magnificationPlugin = - new LensMagnificationGraphMousePlugin(1.f, 60.f, .2f); + new LensMagnificationGraphMousePlugin(1.f, 60.f, .2f) { + // Override to address a bug when using a high resolution mouse wheel. + // May be removed when jungrapht-visualization version is updated + @Override + public void mouseWheelMoved(MouseWheelEvent e) { + if (e.getWheelRotation() != 0) { + super.mouseWheelMoved(e); + } + } + }; MutableTransformer transformer = viewer.getRenderContext() .getMultiLayerTransformer() diff --git a/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsDialog.java b/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsDialog.java index 6206c3b4f0..0a89d0330a 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsDialog.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsDialog.java @@ -22,6 +22,7 @@ import java.beans.PropertyChangeListener; import javax.swing.tree.TreePath; import docking.DialogComponentProvider; +import docking.widgets.OptionDialog; import ghidra.framework.options.Options; /** @@ -85,9 +86,20 @@ public class OptionsDialog extends DialogComponentProvider { @Override protected void cancelCallback() { - if (panel.cancel()) { - close(); + // stop any active editors + panel.cancel(); + + if (hasChanges) { + int result = OptionDialog.showYesNoCancelDialog(panel, "Save Changes?", + "These options have changed. Do you want to save them?"); + if (result == OptionDialog.CANCEL_OPTION) { + return; + } + if (result == OptionDialog.YES_OPTION) { + applyChanges(); + } } + close(); } @Override diff --git a/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsPanel.java b/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsPanel.java index 38ed5ff74e..f35b5bc41d 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsPanel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/options/editor/OptionsPanel.java @@ -183,7 +183,7 @@ public class OptionsPanel extends JPanel { processSelection(node); } - public boolean cancel() { + public void cancel() { Set> entrySet = editorMap.entrySet(); for (Map.Entry entry : entrySet) { OptionsEditor editor = entry.getValue(); @@ -199,8 +199,6 @@ public class OptionsPanel extends JPanel { Msg.showError(this, this, title, title + "\nError Message: " + msg, e); } } - - return true; } public boolean apply() { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighGlobal.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighGlobal.java index 54a236ae44..2177eddb6f 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighGlobal.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighGlobal.java @@ -17,6 +17,7 @@ package ghidra.program.model.pcode; import ghidra.program.model.address.Address; import ghidra.program.model.data.DataType; +import ghidra.util.Msg; import ghidra.util.xml.SpecXmlUtils; import ghidra.xml.XmlElement; import ghidra.xml.XmlPullParser; @@ -58,10 +59,12 @@ public class HighGlobal extends HighVariable { offset = SpecXmlUtils.decodeInt(attrString); } restoreInstances(parser, el); - if (symref == 0) { - throw new PcodeXMLException("Missing symref attribute in tag"); + if (symref != 0) { + symbol = function.getGlobalSymbolMap().getSymbol(symref); + } + else { + Msg.warn(this, "Missing symref attribute in tag"); } - symbol = function.getGlobalSymbolMap().getSymbol(symref); if (symbol == null) { // If we don't already have symbol, synthesize it DataType symbolType; int symbolSize; diff --git a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/symbol/StubSymbol.java b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/symbol/StubSymbol.java new file mode 100644 index 0000000000..0c8d0b253c --- /dev/null +++ b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/symbol/StubSymbol.java @@ -0,0 +1,228 @@ +/* ### + * IP: GHIDRA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ghidra.program.model.symbol; + +import ghidra.program.model.address.Address; +import ghidra.program.model.listing.CircularDependencyException; +import ghidra.program.model.listing.Program; +import ghidra.program.util.ProgramLocation; +import ghidra.util.exception.DuplicateNameException; +import ghidra.util.exception.InvalidInputException; +import ghidra.util.task.TaskMonitor; + +// Simple symbol test implementation +public class StubSymbol implements Symbol { + private static long nextId = 0; + + private long id; + private String name; + private Address address; + + public StubSymbol(String name, Address address) { + this.name = name; + this.address = address; + id = nextId++; + } + + @Override + public Address getAddress() { + return address; + } + + @Override + public String getName() { + return name; + } + + @Override + public String[] getPath() { + return new String[] { name }; + } + + @Override + public Program getProgram() { + return null; + } + + @Override + public String getName(boolean includeNamespace) { + return name; + } + + @Override + public Namespace getParentNamespace() { + return null; + } + + @Override + public Symbol getParentSymbol() { + return null; + } + + @Override + public boolean isDescendant(Namespace namespace) { + return false; + } + + @Override + public boolean isValidParent(Namespace parent) { + return false; + } + + @Override + public SymbolType getSymbolType() { + return SymbolType.LABEL; + } + + @Override + public int getReferenceCount() { + return 0; + } + + @Override + public boolean hasMultipleReferences() { + return false; + } + + @Override + public boolean hasReferences() { + return false; + } + + @Override + public Reference[] getReferences(TaskMonitor monitor) { + return null; + } + + @Override + public Reference[] getReferences() { + return null; + } + + @Override + public ProgramLocation getProgramLocation() { + return null; + } + + @Override + public void setName(String newName, SourceType source) + throws DuplicateNameException, InvalidInputException { + this.name = newName; + } + + @Override + public void setNamespace(Namespace newNamespace) + throws DuplicateNameException, InvalidInputException, CircularDependencyException { + // do nothing + } + + @Override + public void setNameAndNamespace(String newName, Namespace newNamespace, SourceType source) + throws DuplicateNameException, InvalidInputException, CircularDependencyException { + this.name = newName; + } + + @Override + public boolean delete() { + return false; + } + + @Override + public boolean isPinned() { + return false; + } + + @Override + public void setPinned(boolean pinned) { + // nothing + } + + @Override + public boolean isDynamic() { + return false; + } + + @Override + public boolean isExternal() { + return false; + } + + @Override + public boolean isPrimary() { + return true; + } + + @Override + public boolean setPrimary() { + return false; + } + + @Override + public boolean isExternalEntryPoint() { + return false; + } + + @Override + public long getID() { + return name.hashCode(); + } + + @Override + public Object getObject() { + return null; + } + + @Override + public boolean isGlobal() { + return true; + } + + @Override + public void setSource(SourceType source) { + // nothing + } + + @Override + public SourceType getSource() { + return SourceType.USER_DEFINED; + } + + @Override + public boolean isDeleted() { + return false; + } + + @Override + public int hashCode() { + return (int) id; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + StubSymbol other = (StubSymbol) obj; + return id == other.id; + } + +} diff --git a/Ghidra/Processors/SuperH4/data/languages/SuperH4.sinc b/Ghidra/Processors/SuperH4/data/languages/SuperH4.sinc index 724c889b7f..3fa465b6a2 100644 --- a/Ghidra/Processors/SuperH4/data/languages/SuperH4.sinc +++ b/Ghidra/Processors/SuperH4/data/languages/SuperH4.sinc @@ -1421,12 +1421,12 @@ N_0txx: r0",@"^N_0 is r0 & N_0 { tmp:4 = N_0; export tmp; } OP_0=0xf & N_0t_at_neg & XDRM & FRM_0 & OP_1=0xb { if (!( $(FPSCR_SZ) == 0 )) goto ; - *:4 N_0t_at_neg = FRM_0; N_0t_at_neg = N_0t_at_neg - 4; + *:4 N_0t_at_neg = FRM_0; goto ; - *:8 N_0t_at_neg = XDRM; N_0t_at_neg = N_0t_at_neg - 8; + *:8 N_0t_at_neg = XDRM; }