From 90c831483748651896d58fcab0c2f286d95143a6 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Tue, 27 Jul 2021 18:43:28 -0400 Subject: [PATCH 1/9] GP-1162 - Fixed 'Enter' key in Set Equates dialog to choose the selected table row; Updated the Function Signature Editor dialog to allow the 'Cancel' key to close the dialog when the focus is in the top text editor. Closes #3235 --- .../function/editor/FunctionEditorDialog.java | 44 ++++++++++++++++--- .../function/editor/FunctionEditorModel.java | 36 +++++---------- .../ghidra/app/util/bean/SetEquateDialog.java | 31 +++++++++---- 3 files changed, 69 insertions(+), 42 deletions(-) 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/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); From f72d679efff65de85c0a3f9a62a09a4597959a39 Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Tue, 3 Aug 2021 11:29:16 -0400 Subject: [PATCH 2/9] GP-1111 corrected affected test and help docs --- .../help/topics/DataTypeEditors/StructureEditor.htm | 13 ++++++++++++- .../StructureEditorLockedActions2Test.java | 6 +++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Ghidra/Features/Base/src/main/help/help/topics/DataTypeEditors/StructureEditor.htm b/Ghidra/Features/Base/src/main/help/help/topics/DataTypeEditors/StructureEditor.htm index fa8119e379..f3be8ce6b7 100644 --- a/Ghidra/Features/Base/src/main/help/help/topics/DataTypeEditors/StructureEditor.htm +++ b/Ghidra/Features/Base/src/main/help/help/topics/DataTypeEditors/StructureEditor.htm @@ -631,7 +631,18 @@ does this by selecting the components in the editor and clicking the Create Structure From Selection button. The user is then prompted for the name of the new structure. Once a unique name is specified the new structure - is created and a component containing it replaces the selected components.

+ is created and a component containing it replaces the selected components. The newly created + structure will adopt the same pack setting as the current structure.

+ +

It is important to note that when using this action with a packed structure the placement of components + within the new stucture may change as can offsets of trailing components within the current structure. + The size of the current structure may also be affected as a result of this change.

+ +

If you do not like the result of the change you can restore the previous state of the + current structure by using the action on + the new structure component, although the newly created structure will still exist within + the Data Type Manager.

+
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()); } /** From 58558981d5988100d89e9c9f08d159fe9fdbaf13 Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Tue, 3 Aug 2021 13:12:15 -0400 Subject: [PATCH 3/9] Adjustments to data-type propagation through INT_ADD --- .../Decompiler/src/decompile/cpp/cast.cc | 2 +- .../src/decompile/cpp/coreaction.cc | 196 ++++++++++++------ .../src/decompile/cpp/coreaction.hh | 3 +- .../Decompiler/src/decompile/cpp/funcdata.cc | 2 +- .../src/decompile/cpp/ruleaction.cc | 30 +++ .../src/decompile/cpp/ruleaction.hh | 1 + .../Decompiler/src/decompile/cpp/type.cc | 64 +++--- .../Decompiler/src/decompile/cpp/type.hh | 2 +- .../Decompiler/src/decompile/cpp/typeop.cc | 2 +- .../Decompiler/src/decompile/cpp/varmap.cc | 78 ++++--- .../Decompiler/src/decompile/cpp/varmap.hh | 2 +- 11 files changed, 257 insertions(+), 125 deletions(-) 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 From 0ccdd45f250b5da66400e067e10bd8dab6bbdf31 Mon Sep 17 00:00:00 2001 From: ghidravore Date: Tue, 3 Aug 2021 13:16:14 -0400 Subject: [PATCH 4/9] GP-1159 fixes #3264 where symbol tree becomes unstable when grouping duplicate symbols --- .../topics/SymbolTreePlugin/SymbolTree.htm | 9 +- .../core/symboltree/SymbolTreeProvider.java | 17 +- .../core/symboltree/actions/DeleteAction.java | 15 +- .../core/symboltree/nodes/MoreNode.java | 118 +++++++ .../symboltree/nodes/OrganizationNode.java | 322 +++++++++++------- .../symboltree/nodes/SymbolCategoryNode.java | 6 +- .../core/symboltree/nodes/SymbolTreeNode.java | 2 - .../nodes/OrganizationNodeTest.java | 193 +++++++++++ .../program/model/symbol/StubSymbol.java | 228 +++++++++++++ 9 files changed, 749 insertions(+), 161 deletions(-) create mode 100644 Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/nodes/MoreNode.java create mode 100644 Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/symboltree/nodes/OrganizationNodeTest.java create mode 100644 Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/symbol/StubSymbol.java 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/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/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/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; + } + +} From 59adf883430b3882ec77e1075ad1b3aa049c2422 Mon Sep 17 00:00:00 2001 From: ghidravore Date: Tue, 3 Aug 2021 13:21:56 -0400 Subject: [PATCH 5/9] GP-116 added ability to save changes in options when cancelling --- .../core/analysis/AnalysisOptionsDialog.java | 46 +++++++++++++++++-- .../plugin/core/analysis/AnalysisPanel.java | 31 ++++--------- .../docking/options/editor/OptionsDialog.java | 16 ++++++- .../docking/options/editor/OptionsPanel.java | 4 +- 4 files changed, 67 insertions(+), 30 deletions(-) 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/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() { From 8ec719d4498043fb2525512b520a748ce114f2da Mon Sep 17 00:00:00 2001 From: tom Date: Fri, 30 Jul 2021 09:37:45 -0400 Subject: [PATCH 6/9] GP-1181 override to address bug with high resolution mouse wheel (Closes #3281, Closes #3284) --- .../graph/visualization/DefaultGraphDisplay.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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() From bfd83ee1f64ca967e76755520fe779be8028600d Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Tue, 3 Aug 2021 22:13:01 +0000 Subject: [PATCH 7/9] GP-1184_emteere Allowing decompiler to produce results if HighGlobal does not have a symref tag --- .../main/java/ghidra/program/model/pcode/HighGlobal.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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; From 86a85afd1b6734aae561c71ec92d60dc126abbef Mon Sep 17 00:00:00 2001 From: ghidorahrex Date: Mon, 2 Aug 2021 14:06:00 -0400 Subject: [PATCH 8/9] GP-1152 Fixed issue with superh fmov/fmov.s decrement/read ordering --- Ghidra/Processors/SuperH4/data/languages/SuperH4.sinc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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; } From 17f9cf0bc7311e73d9e013a7786a215533fbc6e5 Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Tue, 3 Aug 2021 19:14:39 -0400 Subject: [PATCH 9/9] Updated Change History for 10.0.2 --- .../src/global/docs/ChangeHistory.html | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) 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

+
    +
  • Scripting. Created an example script which demonstrates how to use the FileBytes class to do a binary export of the current program. (GP-1157)
  • +
+
+

Improvements

+
    +
  • Data Types. When creating a substructure from existing components, the new structure will adopt the pack setting of the parent structure from which it was created. Note that a packed structure may still move based upon component alignment rules. (GP-1111, Issue #3193)
  • +
  • Decompiler. Added E key binding to the Decompiler's Equate action. (GP-1146, Issue #3195)
  • +
  • GUI. Added Apply button to analysis options dialog. Also added a last chance save/cancel dialog that is shown when a user cancels an options dialog that has unsaved changes. (GP-1169, Issue #3274)
  • +
  • Scripting. For stripped gcc binaries, improved prototype RecoverClassesFromRTTIScript identification of vtables and simple class data, constructors, and destructors. (GP-1055, Issue #3266)
  • +
+
+

Bugs

+
    +
  • Basic Infrastructure. Fixed regression that prevented Ghidra from launching on Windows when its path contained spaces. (GP-1113, Issue #3201, #3205)
  • +
  • Data Types. Fixed IllegalArgumentException error message when adding a duplicate enumerate name for EnumDataType. (GP-1173, Issue #3246)
  • +
  • Debugger. Changed diagnostics to write GDB.log to user directory, not installation. Clarified an error message. (GP-1133, Issue #3218)
  • +
  • Debugger. Improved error reporting when failing to start a Debugger GADP agent. (GP-1136, Issue #3175)
  • +
  • Debugger. Added system property to toggle alternative icons/colors for breakpoints. (GP-1139, Issue #3204)
  • +
  • Debugger. Applying a default everything memory map for GDB targets if info proc mappings fails or produces an empty list. (GP-1142, Issue #3071, #3074, #3161, #3169)
  • +
  • Debugger. Fixed issue with Debugger ignoring JAVA_HOME when launching child JVM. (GP-1143, Issue #3231)
  • +
  • Debugger. Fixed command-reply matching issue when using GDB via SSH. (GP-1153, Issue #3238)
  • +
  • Debugger:Emulator. Fixed bug in Trace Emulation causing ArrayIndexOutOfBoundsExceptions. (GP-1058)
  • +
  • Decompiler. The decompiler now shows results when a HighGlobal has no associated symbol reference in the program. (GP-1184)
  • +
  • DWARF. Changed processing to ignore incomplete DWARF parameter lists in Rust binaries. (GP-1121, Issue #3060)
  • +
  • Exporter. The C/C++ Exporter now emits semicolons after function prototypes when using the Create Header File option. (GP-1145, Issue #1644)
  • +
  • Framework. Corrected address comparison for 64-bit signed address spaces (e.g., stack space, constant space) which could produce non-transitive comparison results. (GP-1178, Issue #3302)
  • +
  • Graphing. Corrected graph magnification behavior when using a high resolution mouse wheel. (GP-1181, Issue #3281, #3284)
  • +
  • GUI. Fixed NullPointerException when Hovering in Decompiler over a function that is not in memory. (GP-1131)
  • +
  • GUI. Fixed bug in Find References to search results that prevented '<' characters from being rendered. (GP-1137, Issue #3217)
  • +
  • GUI. Fixed issue where duplicate label names could cause the symbol tree to become unstable, evidenced by broken display and scrolling actions. Also, improved grouping algorithm. (GP-1159, Issue #3264)
  • +
  • GUI. Fixed Enter key in Set Equates dialog to choose the selected table row. Updated the Function Signature Editor dialog to allow the Cancel key to close the dialog when the focus is in the top text editor. (GP-1162, Issue #3235)
  • +
  • Headless. Fixed a regression in analyzeHeadless.bat that prevented the headless analyzer from running on Windows in some cases. (GP-1156, Issue #3261)
  • +
  • Importer. The MzLoader now populates the relocation table when relocations are performed. (GP-1160)
  • +
  • Importer:ELF. Corrected dynamic GOT/PLT markup problem for images which do not contain section headers. In cases where image does not define symbols within the PLT, analysis may be relied upon for its disassembly. ELF Importer's goal is to migrate symbols which may be defined within the PLT to the External symbol space. (GP-1110, Issue #3198)
  • +
  • Importer:Mach-O. The Mach-O importer now correctly interprets indirect symbols as references to symbols within another .dylib. (GP-1120)
  • +
  • Processors. Fixed bug in SuperH4 fmov.s pcode. (GP-1152)
  • +
  • Processors. The ARM instruction semantics for the mulitple-single-element forms of the vld1/vst1 vector instructions have been corrected. (GP-1167)
  • +
  • Sleigh. Fixed a string formatting error in the sleigh compiler. (GP-1124, Issue #3168)
  • +
+
+

Ghidra 10.0.1 Change History (July 2021)

New Features