diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java b/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java index 7cafd59bb6..c79b631489 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java @@ -36,7 +36,6 @@ import ghidra.util.*; import ghidra.util.datastruct.LongObjectHashtable; import ghidra.util.exception.*; import ghidra.util.task.TaskMonitor; -import ghidra.util.task.TaskMonitorAdapter; /** * Manager for merging category and data type changes @@ -93,11 +92,12 @@ public class DataTypeMergeManager implements MergeResolver { /** * Manager for merging the data types using the four programs. - * @param resultPgm the program to be updated with the result of the merge. + * @param mergeManager overall merge manager for domain object + * @param resultDomainObject the program to be updated with the result of the merge. * This is the program that will actually get checked in. - * @param myPgm the program requesting to be checked in. - * @param originalPgm the program that was checked out. - * @param latestPgm the latest checked-in version of the program. + * @param myDomainObject the program requesting to be checked in. + * @param originalDomainObject the program that was checked out. + * @param latestDomainObject the latest checked-in version of the program. * @param latestChanges the address set of changes between original and latest versioned program. * @param myChanges the address set of changes between original and my modified program. */ @@ -185,8 +185,7 @@ public class DataTypeMergeManager implements MergeResolver { /** * Merge the data types using the four programs. - * @param monitor - * @return true if the merge completed + * @param monitor merge task monitor * @see MergeConstants */ @Override @@ -262,7 +261,7 @@ public class DataTypeMergeManager implements MergeResolver { /** * For JUnit testing only, set the option for resolving a conflict. - * @param option + * @param option forced conflict resolution option */ void setConflictResolution(int option) { conflictOption = option; @@ -272,9 +271,7 @@ public class DataTypeMergeManager implements MergeResolver { private void processSourceArchiveChanges() throws CancelledException { conflictOption = OPTION_MY; for (int i = 0; i < myArchiveChangeList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long id = myArchiveChangeList.get(i).longValue(); @@ -324,9 +321,7 @@ public class DataTypeMergeManager implements MergeResolver { private void processSourceArchiveAdditions() throws CancelledException { for (int i = 0; i < myArchiveAddedList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long id = myArchiveAddedList.get(i).longValue(); @@ -389,9 +384,7 @@ public class DataTypeMergeManager implements MergeResolver { private void processSourceArchiveConflicts() throws CancelledException { for (int i = 0; i < archiveConflictList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long sourceArchiveID = archiveConflictList.get(i).longValue(); @@ -418,13 +411,11 @@ public class DataTypeMergeManager implements MergeResolver { /** * Add new categories. - * @throws CancelledException + * @throws CancelledException if task cancelled */ private void processCategoriesAdded() throws CancelledException { for (int i = 0; i < myCatAddedList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long id = myCatAddedList.get(i).longValue(); @@ -439,14 +430,12 @@ public class DataTypeMergeManager implements MergeResolver { /** * Process conflicts for categories. - * @throws CancelledException + * @throws CancelledException task was cancelled */ private void processCategoryConflicts() throws CancelledException { for (int i = 0; i < catConflictList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long id = catConflictList.get(i).longValue(); @@ -474,9 +463,7 @@ public class DataTypeMergeManager implements MergeResolver { private void processCategoryChanges() throws CancelledException { for (int i = 0; i < myCatChangeList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long id = myCatChangeList.get(i).longValue(); @@ -489,9 +476,7 @@ public class DataTypeMergeManager implements MergeResolver { private void processCategoriesDeleted() throws CancelledException { for (int i = 0; i < myCatChangeList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); long id = myCatChangeList.get(i).longValue(); processCategoryDeleted(id); @@ -500,15 +485,13 @@ public class DataTypeMergeManager implements MergeResolver { private void processDataTypeConflicts() throws CancelledException { while (dtConflictList.size() > 0) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long id = dtConflictList.get(0).longValue(); ++currentConflictIndex; handleDataTypeConflict(id, currentConflictIndex); - dtConflictList.remove(new Long(id)); + dtConflictList.remove(Long.valueOf(id)); } fixUpDataTypes(); @@ -530,10 +513,6 @@ public class DataTypeMergeManager implements MergeResolver { } } - /** - * @param id - * @param conflictIndex - */ private void handleDataTypeConflict(long id, int conflictIndex) throws CancelledException { DataType myDt = dtms[MY].getDataType(id); @@ -585,9 +564,6 @@ public class DataTypeMergeManager implements MergeResolver { } } - /** - * @param id - */ private void setSourceDataType(long myID) { DataType myDt = dtms[MY].getDataType(myID); @@ -635,9 +611,6 @@ public class DataTypeMergeManager implements MergeResolver { } } - /** - * @param id - */ private void changeSourceArchive(long dtID) { int optionToUse = (dataTypeChoice == ASK_USER) ? conflictOption : dataTypeChoice; @@ -656,9 +629,6 @@ public class DataTypeMergeManager implements MergeResolver { } } - /** - * @param id - */ private void dataTypeChanged(long id) { int optionToUse = (dataTypeChoice == ASK_USER) ? conflictOption : dataTypeChoice; @@ -750,7 +720,7 @@ public class DataTypeMergeManager implements MergeResolver { switch (conflictOption) { case OPTION_LATEST: if (latestDt == null) { - if (!myDtAddedList.contains(new Long(id))) { + if (!myDtAddedList.contains(Long.valueOf(id))) { // remove the data type if it was already added DataType dt = myResolvedDts.get(id); if (dt != null) { @@ -781,6 +751,12 @@ public class DataTypeMergeManager implements MergeResolver { } } + /** + * Set category path. If name conflict occurs within new category + * the specified dt will remain within its' current category + * @param dt datatype whoose category is to changed + * @param newPath new category path + */ private void setCategoryPath(DataType dt, CategoryPath newPath) { if (dt.getCategoryPath().equals(newPath)) { return; @@ -789,6 +765,7 @@ public class DataTypeMergeManager implements MergeResolver { dt.setCategoryPath(newPath); } catch (DuplicateNameException e) { + // ignore - no change made } } @@ -831,11 +808,7 @@ public class DataTypeMergeManager implements MergeResolver { } updateHashTables(id, resultDt, resolvedDataTypes); if (updatePath && !resultDt.getCategoryPath().equals(myDt.getCategoryPath())) { - try { - resultDt.setCategoryPath(myDt.getCategoryPath()); - } - catch (DuplicateNameException e) { - } + setCategoryPath(resultDt, myDt.getCategoryPath()); } return resultDt; @@ -883,7 +856,7 @@ public class DataTypeMergeManager implements MergeResolver { return existingDt; // Data type is already resolved and mapped in the table. } - if (!myDtAddedList.contains(new Long(dataTypeID))) { + if (!myDtAddedList.contains(Long.valueOf(dataTypeID))) { existingDt = dtms[RESULT].getDataType(dataTypeID); if (existingDt != null) { Msg.warn(this, " ** WARNING ** : Unexpectedly found data type \"" + @@ -936,10 +909,10 @@ public class DataTypeMergeManager implements MergeResolver { if (resolvedDt == null) { // Haven't resolved this yet. // use dt from results - if (!myDtAddedList.contains(new Long(baseID))) { + if (!myDtAddedList.contains(Long.valueOf(baseID))) { resolvedDt = dtms[RESULT].getDataType(baseID); if (resolvedDt == null) { - if (origDtConflictList.contains(new Long(baseID))) { + if (origDtConflictList.contains(Long.valueOf(baseID))) { // was deleted, but add it back so we can create // data types depending on it; will get resolved later resolvedDt = addDataType(baseID, baseDt, resolvedDataTypes); @@ -1054,11 +1027,7 @@ public class DataTypeMergeManager implements MergeResolver { private DataType addFunctionDef(long id, FunctionDefinition myDt, LongObjectHashtable resolvedDataTypes) { FunctionDefinition newDt = (FunctionDefinition) myDt.clone(dtms[RESULT]); - try { - newDt.setCategoryPath(myDt.getCategoryPath()); - } - catch (DuplicateNameException e) { - } + setCategoryPath(newDt, myDt.getCategoryPath()); updateFunctionDef(id, myDt, newDt, resolvedDataTypes); return newDt; } @@ -1066,7 +1035,7 @@ public class DataTypeMergeManager implements MergeResolver { private void updateHashTables(long id, DataType newDt, LongObjectHashtable resolvedDataTypes) { resolvedDataTypes.put(id, newDt); - if (!myDtAddedList.contains(new Long(id))) { + if (!myDtAddedList.contains(Long.valueOf(id))) { if (resolvedDataTypes == myResolvedDts) { origResolvedDts.put(id, newDt); latestResolvedDts.put(id, newDt); @@ -1098,7 +1067,7 @@ public class DataTypeMergeManager implements MergeResolver { if (baseDt != DataType.DEFAULT) { DataTypeManager dtm = baseDt.getDataTypeManager(); long baseID = dtm.getID(baseDt); - if (!myDtAddedList.contains(new Long(baseID))) { + if (!myDtAddedList.contains(Long.valueOf(baseID))) { if (dtms[RESULT].getDataType(baseID) != null) { return resolvedDt; } @@ -1118,15 +1087,7 @@ public class DataTypeMergeManager implements MergeResolver { destStruct.deleteAll(); // Set to correct alignment and packing. - try { - updateAlignment(sourceDt, destStruct); - } - catch (InvalidInputException e) { - String msg = "Some of your changes to " + destStruct.getName() + - " cannot be merged.\nProblem: " + e.getMessage(); - Msg.showError(this, null, "Structure Update Failed", msg); - return; - } + updateAlignment(sourceDt, destStruct); DataTypeManager sourceDTM = sourceDt.getDataTypeManager(); boolean aligned = sourceDt.isInternallyAligned(); @@ -1148,7 +1109,7 @@ public class DataTypeMergeManager implements MergeResolver { if (resultCompDt == null) { // We didn't have a map entry for the data type. - if (!myDtAddedList.contains(new Long(sourceComponentID))) { + if (!myDtAddedList.contains(Long.valueOf(sourceComponentID))) { // Not added so should be in result if it wasn't deleted there. DataType rDt = dtms[RESULT].getDataType(sourceComponentID); @@ -1242,15 +1203,7 @@ public class DataTypeMergeManager implements MergeResolver { } // Set to correct alignment and packing. - try { - updateAlignment(sourceDt, destUnion); - } - catch (InvalidInputException e) { - String msg = "Some of your changes to " + destUnion.getName() + - " cannot be merged.\nProblem: " + e.getMessage(); - Msg.showError(this, null, "Union Update Failed", msg); - return; - } + updateAlignment(sourceDt, destUnion); DataTypeManager sourceDTM = sourceDt.getDataTypeManager(); @@ -1263,7 +1216,7 @@ public class DataTypeMergeManager implements MergeResolver { DataType resultCompDt = getResolvedComponent(sourceCompID, resolvedDataTypes); if (resultCompDt == null) { - if (!myDtAddedList.contains(new Long(sourceCompID))) { + if (!myDtAddedList.contains(Long.valueOf(sourceCompID))) { // Not added so should be in result if it wasn't deleted there. DataType resultsDt = dtms[RESULT].getDataType(sourceCompID); @@ -1324,8 +1277,7 @@ public class DataTypeMergeManager implements MergeResolver { } } - private void updateAlignment(Composite sourceDt, Composite destinationDt) - throws InvalidInputException { + private void updateAlignment(Composite sourceDt, Composite destinationDt) { if (sourceDt.isDefaultAligned()) { destinationDt.setToDefaultAlignment(); } @@ -1363,30 +1315,28 @@ public class DataTypeMergeManager implements MergeResolver { DataTypeManager sourceDTM = sourceFunctionDefDt.getDataTypeManager(); DataType sourceReturnType = sourceFunctionDefDt.getReturnType(); ParameterDefinition[] sourceVars = sourceFunctionDefDt.getArguments(); - ParameterDefinition[] destVars = destDt.getArguments(); + ParameterDefinition[] destVars = new ParameterDefinition[sourceVars.length]; boolean sourceHasVarArgs = sourceFunctionDefDt.hasVarArgs(); + DataType resolvedRDT = DataType.DEFAULT; if (sourceReturnType != null) { long returnTypeID = sourceDTM.getID(sourceReturnType); - DataType resolvedRDT = + resolvedRDT = getResolvedParam(sourceFunctionDefDtID, returnTypeID, -1, resolvedDataTypes); - destDt.setReturnType(resolvedRDT); } + destDt.setReturnType(resolvedRDT); + for (int i = 0; i < sourceVars.length; i++) { DataType varDt = sourceVars[i].getDataType(); long varID = sourceDTM.getID(varDt); DataType resolvedDt = getResolvedParam(sourceFunctionDefDtID, varID, i, resolvedDataTypes); - destVars[i].setComment(sourceVars[i].getComment()); - try { - destVars[i].setDataType(resolvedDt); - } - catch (InvalidInputException e) { - } - } - if (sourceHasVarArgs != destDt.hasVarArgs()) { - destDt.setVarArgs(sourceHasVarArgs); + destVars[i] = new ParameterDefinitionImpl(sourceVars[i].getName(), resolvedDt, + sourceVars[i].getComment()); } + destDt.setArguments(destVars); + destDt.setVarArgs(sourceHasVarArgs); + destDt.setLastChangeTime(oldLastChangeTime); destDt.setLastChangeTimeInSourceArchive(oldLastChangeTimeInSourceArchive); } @@ -1405,7 +1355,7 @@ public class DataTypeMergeManager implements MergeResolver { LongObjectHashtable resolvedDataTypes) { DataType resolvedDt = getResolvedComponent(paramID, resolvedDataTypes); if (resolvedDt == null) { - if (!myDtAddedList.contains(new Long(paramID))) { + if (!myDtAddedList.contains(Long.valueOf(paramID))) { // Not added so should be in result if it wasn't deleted there. DataType resultsDt = dtms[RESULT].getDataType(paramID); @@ -1435,14 +1385,12 @@ public class DataTypeMergeManager implements MergeResolver { * Process data types that were changed (renamed, moved, or edited) in * MY program, but are not conflicts, i.e., not renamed, moved or edited * in LATEST. The corresponding data type in RESULT program is updated. - * @throws CancelledException + * @throws CancelledException if task is cancelled */ private void processDataTypeChanges() throws CancelledException { for (int i = 0; i < myDtChangeList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long id = myDtChangeList.get(i).longValue(); @@ -1777,8 +1725,8 @@ public class DataTypeMergeManager implements MergeResolver { * was moved; if so, move the category according to the conflictOption * selected. Moves are handled here because a rename and a move is * considered to be a single conflict. - * @param id - * @throws CancelledException + * @param id category ID + * @throws CancelledException if task is cancelled */ private void categoryRenamedOrMoved(long id) throws CancelledException { @@ -1842,7 +1790,7 @@ public class DataTypeMergeManager implements MergeResolver { * Handle conflicts on a category that was deleted in one program, and * renamed or moved in another program. * @param id category ID - * @throws CancelledException + * @throws CancelledException if operation is cancelled */ private void categoryDeleted(long id) throws CancelledException { @@ -1933,7 +1881,7 @@ public class DataTypeMergeManager implements MergeResolver { if (doDelete) { Category parentCat = dtms[RESULT].getCategory(latestCat.getParent().getCategoryPath()); if (parentCat != null) { - parentCat.removeCategory(latestCat.getName(), TaskMonitorAdapter.DUMMY_MONITOR); + parentCat.removeCategory(latestCat.getName(), TaskMonitor.DUMMY); } } } @@ -1955,8 +1903,10 @@ public class DataTypeMergeManager implements MergeResolver { }); } catch (InterruptedException e) { + // ignore } catch (InvocationTargetException e) { + e.printStackTrace(); } mergeManager.setApplyEnabled(false); mergeManager.showComponent(archiveMergePanel, "SourceArchiveMerge", @@ -1987,8 +1937,10 @@ public class DataTypeMergeManager implements MergeResolver { }); } catch (InterruptedException e) { + // ignore } catch (InvocationTargetException e) { + e.printStackTrace(); } mergeManager.setApplyEnabled(false); mergeManager.showComponent(catMergePanel, "CategoryMerge", @@ -2010,8 +1962,10 @@ public class DataTypeMergeManager implements MergeResolver { }); } catch (InterruptedException e) { + // ignore } catch (InvocationTargetException e) { + e.printStackTrace(); } mergeManager.showComponent(dtMergePanel, "DataTypeMerge", new HelpLocation(HelpTopics.REPOSITORY, "DataTypeConflicts")); @@ -2024,9 +1978,7 @@ public class DataTypeMergeManager implements MergeResolver { private void processDataTypesDeleted() throws CancelledException { for (int i = 0; i < myDtChangeList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); long id = myDtChangeList.get(i).longValue(); processDataTypeDeleted(id); @@ -2038,9 +1990,7 @@ public class DataTypeMergeManager implements MergeResolver { */ private void processDataTypesAdded() throws CancelledException { for (int i = 0; i < myDtAddedList.size(); i++) { - if (currentMonitor.isCancelled()) { - throw new CancelledException(); - } + currentMonitor.checkCanceled(); currentMonitor.setProgress(++progressIndex); long myDtKey = myDtAddedList.get(i).longValue(); @@ -2160,11 +2110,7 @@ public class DataTypeMergeManager implements MergeResolver { } else { ParameterDefinition[] vars = fd.getArguments(); - try { - vars[info.index].setDataType(dt); - } - catch (InvalidInputException e) { - } + vars[info.index].setDataType(dt); } } fd.setLastChangeTime(lastChangeTime); // Reset the last change time to the merged data type's. @@ -2393,7 +2339,7 @@ public class DataTypeMergeManager implements MergeResolver { } return null; } - if (!myDtAddedList.contains(new Long(id))) { + if (!myDtAddedList.contains(Long.valueOf(id))) { // use data type from RESULT return dtms[RESULT].getDataType(id); } @@ -2507,7 +2453,7 @@ public class DataTypeMergeManager implements MergeResolver { long[] myArchiveChanges) { for (long myChangeID : myArchiveChanges) { UniversalID sourceID = new UniversalID(myChangeID); - Long myChangeIDObject = new Long(myChangeID); + Long myChangeIDObject = Long.valueOf(myChangeID); if (myArchiveAddedList.contains(myChangeIDObject) || archiveConflictList.contains(myChangeIDObject)) { continue; @@ -2550,7 +2496,7 @@ public class DataTypeMergeManager implements MergeResolver { continue; } } - myArchiveChangeList.add(new Long(myChangeID)); + myArchiveChangeList.add(Long.valueOf(myChangeID)); } } @@ -2572,14 +2518,14 @@ public class DataTypeMergeManager implements MergeResolver { dtms[LATEST].getSourceArchive(new UniversalID(latestAddID)); if (!StringUtils.equals(mySourceArchive.getName(), latestSourceArchive.getName())) { - archiveConflictList.add(new Long(myAddID)); + archiveConflictList.add(Long.valueOf(myAddID)); foundConflict = true; break; } } } if (!foundConflict) { - myArchiveAddedList.add(new Long(myAddID)); + myArchiveAddedList.add(Long.valueOf(myAddID)); } } } @@ -2593,7 +2539,7 @@ public class DataTypeMergeManager implements MergeResolver { boolean latestDirty = (latestSourceArchive != null) ? latestSourceArchive.isDirty() : false; boolean myDirty = mySourceArchive.isDirty(); - dirtyMap.put(sourceID, new Boolean(myDirty || latestDirty)); + dirtyMap.put(sourceID, Boolean.valueOf(myDirty || latestDirty)); } } @@ -2618,18 +2564,18 @@ public class DataTypeMergeManager implements MergeResolver { dtSourceConflictList = new ArrayList<>(); myDtChangeList = new ArrayList<>(); for (long myDtChange : myDtChanges) { - myDtChangeList.add(new Long(myDtChange)); + myDtChangeList.add(Long.valueOf(myDtChange)); } processAddIDs(myDtAdds); ArrayList resultDtChangeList = new ArrayList<>(); for (long latestDtChange : latestDtChanges) { - resultDtChangeList.add(new Long(latestDtChange)); + resultDtChangeList.add(Long.valueOf(latestDtChange)); } ArrayList resultDtAddList = new ArrayList<>(); for (long latestDtAdd : latestDtAdds) { - resultDtAddList.add(new Long(latestDtAdd)); + resultDtAddList.add(Long.valueOf(latestDtAdd)); } // remove my Added dt's from my changed dt's // Added and then changed data types should only be in added. @@ -2683,15 +2629,15 @@ public class DataTypeMergeManager implements MergeResolver { !DataTypeUtilities.equalsIgnoreConflict(resultDt.getName(), myDt.getName()) || !resultDt.isEquivalent(myDt)) { - dtConflictList.add(new Long(myDtAdd)); - dtSourceConflictList.add(new Long(myDtAdd)); + dtConflictList.add(Long.valueOf(myDtAdd)); + dtSourceConflictList.add(Long.valueOf(myDtAdd)); continue; } } - myDtAddedList.add(new Long(myDtAdd)); + myDtAddedList.add(Long.valueOf(myDtAdd)); } else { // Added and then removed data types go on the change list. - Long l = new Long(myDtAdd); + Long l = Long.valueOf(myDtAdd); if (!myDtChangeList.contains(l)) { myDtChangeList.add(l); } @@ -2762,7 +2708,7 @@ public class DataTypeMergeManager implements MergeResolver { // Just need the change from My. dtConflictList.remove(i); if (myChanged || renamedMy || mySourceChanged) { - myDtChangeList.add(new Long(id)); + myDtChangeList.add(Long.valueOf(id)); } --i; } @@ -2786,7 +2732,7 @@ public class DataTypeMergeManager implements MergeResolver { long[] myCatChanges = myChanges.getCategoryChanges(); Arrays.sort(myCatChanges); for (long myCatChange : myCatChanges) { - myCatChangeList.add(new Long(myCatChange)); + myCatChangeList.add(Long.valueOf(myCatChange)); } myCatAddedList = new ArrayList<>(); @@ -2794,10 +2740,10 @@ public class DataTypeMergeManager implements MergeResolver { Arrays.sort(myCatAdds); for (long myCatAdd : myCatAdds) { if (dtms[MY].getCategory(myCatAdd) != null) { - myCatAddedList.add(new Long(myCatAdd)); + myCatAddedList.add(Long.valueOf(myCatAdd)); } else { // Added and then removed categories go on the change list. - Long l = new Long(myCatAdd); + Long l = Long.valueOf(myCatAdd); if (!myCatChangeList.contains(l)) { myCatChangeList.add(l); } @@ -2809,7 +2755,7 @@ public class DataTypeMergeManager implements MergeResolver { Arrays.sort(latestCatChanges); ArrayList resultCatChangeList = new ArrayList<>(); for (long latestCatChange : latestCatChanges) { - resultCatChangeList.add(new Long(latestCatChange)); + resultCatChangeList.add(Long.valueOf(latestCatChange)); } // remove my Added categories from my changed categories // Added and then changed categories should only be in added. @@ -2853,7 +2799,7 @@ public class DataTypeMergeManager implements MergeResolver { continue; } if (renamedMy) { // Renamed My category without conflict. - myCatChangeList.add(new Long(id)); + myCatChangeList.add(Long.valueOf(id)); } catConflictList.remove(i); // remove ID from conflicts since not actually in conflict. --i; @@ -2894,8 +2840,10 @@ public class DataTypeMergeManager implements MergeResolver { SwingUtilities.invokeAndWait(() -> Msg.showInfo(getClass(), null, title, msg)); } catch (InterruptedException e) { + // ignore } catch (InvocationTargetException e) { + e.printStackTrace(); } } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/AbstractDataTypeMergeTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/AbstractDataTypeMergeTest.java index cf02166fe2..88599e308d 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/AbstractDataTypeMergeTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/AbstractDataTypeMergeTest.java @@ -104,7 +104,7 @@ public abstract class AbstractDataTypeMergeTest extends AbstractMergeTest { if (option >= 0) { dataTypeMergeManager.setConflictResolution(option); } - dataTypeMergeManager.merge(TaskMonitorAdapter.DUMMY_MONITOR); + dataTypeMergeManager.merge(TaskMonitor.DUMMY); } private Window getMergeWindow(CountDownLatch doneLatch) { diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge2Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge2Test.java index 46d1d6ec80..ea3f654c3d 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge2Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge2Test.java @@ -32,7 +32,7 @@ import ghidra.program.model.symbol.SourceType; import ghidra.program.model.symbol.Symbol; import ghidra.util.InvalidNameException; import ghidra.util.exception.DuplicateNameException; -import ghidra.util.task.TaskMonitorAdapter; +import ghidra.util.task.TaskMonitor; /** * Tests for merging data types. @@ -1198,7 +1198,7 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest { Structure bar = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Bar"); try { // remove Bar from the data type manager - dtm.remove(bar, TaskMonitorAdapter.DUMMY_MONITOR); + dtm.remove(bar, TaskMonitor.DUMMY); commit = true; } finally { @@ -1290,7 +1290,7 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest { Structure bar = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Bar"); try { // remove Bar from the data type manager - dtm.remove(bar, TaskMonitorAdapter.DUMMY_MONITOR); + dtm.remove(bar, TaskMonitor.DUMMY); commit = true; } finally { @@ -1430,7 +1430,7 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest { try { fd.setReturnType(bar); Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo"); - dtm.remove(foo, TaskMonitorAdapter.DUMMY_MONITOR); + dtm.remove(foo, TaskMonitor.DUMMY); commit = true; } finally { @@ -1498,7 +1498,7 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest { try { fd.setVarArgs(true); Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo"); - dtm.remove(foo, TaskMonitorAdapter.DUMMY_MONITOR); + dtm.remove(foo, TaskMonitor.DUMMY); commit = true; } finally { @@ -1567,7 +1567,7 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest { try { fd.setVarArgs(true); Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo"); - dtm.remove(foo, TaskMonitorAdapter.DUMMY_MONITOR); + dtm.remove(foo, TaskMonitor.DUMMY); commit = true; } finally { @@ -1620,6 +1620,79 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest { assertEquals(true, fd.hasVarArgs()); } + @Test + public void testEditFuncSig5() throws Exception { + + mtf.initialize("notepad3", new ProgramModifierListener() { + /* (non-Javadoc) + * @see ghidra.framework.data.ProgramModifierListener#modifyLatest(ghidra.program.database.ProgramDB) + */ + @Override + public void modifyLatest(ProgramDB program) throws Exception { + boolean commit = false; + DataTypeManager dtm = program.getDataTypeManager(); + int transactionID = program.startTransaction("test"); + + FunctionDefinition fd = + (FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"), + "MyFunctionDef"); + + try { + fd.setReturnType(VoidDataType.dataType); + + commit = true; + } + finally { + program.endTransaction(transactionID, commit); + } + } + + /* (non-Javadoc) + * @see ghidra.framework.data.ProgramModifierListener#modifyPrivate(ghidra.program.database.ProgramDB) + */ + @Override + public void modifyPrivate(ProgramDB program) throws Exception { + boolean commit = false; + DataTypeManager dtm = program.getDataTypeManager(); + + FunctionDefinition fd = + (FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"), + "MyFunctionDef"); + ParameterDefinition[] vars = fd.getArguments(); + + int transactionID = program.startTransaction("test"); + try { + ParameterDefinition[] newVars = new ParameterDefinition[vars.length + 1]; + System.arraycopy(vars, 0, newVars, 0, vars.length); + newVars[vars.length] = new ParameterDefinitionImpl("Bar", WordDataType.dataType, + "this is another comment"); + fd.setArguments(newVars); + fd.setVarArgs(true); + commit = true; + } + finally { + program.endTransaction(transactionID, commit); + } + } + }); + executeMerge(DataTypeMergeManager.OPTION_MY); + DataTypeManager dtm = resultProgram.getDataTypeManager(); + FunctionDefinition fd = + (FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"), "MyFunctionDef"); + assertNotNull(fd); + Structure dll = (Structure) dtm.getDataType(CategoryPath.ROOT, "DLL_Table"); + assertEquals(dll, fd.getReturnType()); + ParameterDefinition[] vars = fd.getArguments(); + assertTrue(vars[0].getDataType() instanceof WordDataType); + assertTrue(vars[1].getDataType() instanceof CharDataType); + assertTrue(vars[2].getDataType() instanceof Undefined4DataType); + assertTrue(vars[3].getDataType() instanceof Undefined4DataType); + assertTrue(vars[4].getDataType() instanceof WordDataType); + assertEquals("Bar", vars[4].getName()); + assertEquals("this is another comment", vars[4].getComment()); + assertTrue(fd.hasVarArgs()); + } + private void checkDataType(DataType expectedDataType, DataType actualDataType) { assertTrue("Expected " + expectedDataType + ", but is " + actualDataType + ".", actualDataType.isEquivalent(expectedDataType)); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java index 0790804622..e3825ad114 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java @@ -85,9 +85,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { return false; } - /** - * @see ghidra.program.model.data.DataType#isDynamicallySized() - */ @Override public boolean isDynamicallySized() { return false; @@ -142,9 +139,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#getArguments() - */ @Override public ParameterDefinition[] getArguments() { lock.acquire(); @@ -157,9 +151,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#getReturnType() - */ @Override public DataType getReturnType() { lock.acquire(); @@ -168,7 +159,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { long dtId = record.getLongValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_RETURN_ID_COL); DataType dt = dataMgr.getDataType(dtId); if (dt == null) { - dt = DataType.VOID; + dt = DataType.DEFAULT; } return dt; } @@ -187,12 +178,16 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { private void doReplaceWith(FunctionDefinition functionDefinition) { setArguments(functionDefinition.getArguments()); - setReturnType(functionDefinition.getReturnType()); + try { + setReturnType(functionDefinition.getReturnType()); + } + catch (IllegalArgumentException e) { + setReturnType(DEFAULT); + } + setVarArgs(functionDefinition.hasVarArgs()); + setGenericCallingConvention(functionDefinition.getGenericCallingConvention()); } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#getComment() - */ @Override public String getComment() { lock.acquire(); @@ -216,49 +211,31 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { getSourceArchive(), getLastChangeTime(), getLastChangeTimeInSourceArchive(), dtm); } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getMnemonic(ghidra.program.model.data.Settings) - */ @Override public String getMnemonic(Settings settings) { return getPrototypeString(); } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getLength() - */ @Override public int getLength() { return -1; } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getDescription() - */ @Override public String getDescription() { return "Function Signature Data Type"; } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getValue(ghidra.program.model.mem.MemBuffer, ghidra.program.model.lang.ProcessorContext, ghidra.program.model.data.Settings, int) - */ @Override public Object getValue(MemBuffer buf, Settings settings, int length) { return null; } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getRepresentation(ghidra.program.model.mem.MemBuffer, ghidra.program.model.lang.ProcessorContext, ghidra.program.model.data.Settings, int) - */ @Override public String getRepresentation(MemBuffer buf, Settings settings, int length) { return getPrototypeString(); } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setArguments(ghidra.program.model.listing.Variable[]) - */ @Override public void setArguments(ParameterDefinition[] args) { lock.acquire(); @@ -272,17 +249,15 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } parameters.clear(); for (int i = 0; i < args.length; i++) { - DataType resolvedDt = resolve(args[i].getDataType()); + DataType type = + ParameterDefinitionImpl.checkDataType(args[i].getDataType(), dataMgr, false); + DataType resolvedDt = resolve(type); paramAdapter.createRecord(dataMgr.getID(resolvedDt), key, i, args[i].getName(), args[i].getComment(), args[i].getLength()); + resolvedDt.addParent(this); } getParameters(); - it = parameters.iterator(); - while (it.hasNext()) { - ParameterDefinitionDB param = it.next(); - param.getDataType().addParent(this); - } - funDefAdapter.updateRecord(record, true); + funDefAdapter.updateRecord(record, true); // update last change time dataMgr.dataTypeChanged(this); } catch (IOException e) { @@ -293,24 +268,22 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setReturnType(ghidra.program.model.data.DataType) - */ @Override public void setReturnType(DataType type) { + type = ParameterDefinitionImpl.checkDataType(type, dataMgr, true); lock.acquire(); try { checkDeleted(); getReturnType().removeParent(this); if (type == null) { - type = DataType.VOID; + type = DataType.DEFAULT; } DataType resolvedDt = resolve(type); record.setLongValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_RETURN_ID_COL, dataMgr.getID(resolvedDt)); funDefAdapter.updateRecord(record, true); - dataMgr.dataTypeChanged(this); resolvedDt.addParent(this); + dataMgr.dataTypeChanged(this); } catch (IOException e) { dataMgr.dbError(e); @@ -320,9 +293,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setComment(java.lang.String) - */ @Override public void setComment(String comment) { lock.acquire(); @@ -340,9 +310,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /** - * @see ghidra.program.model.data.DataType#dataTypeDeleted(ghidra.program.model.data.DataType) - */ @Override public void dataTypeDeleted(DataType dt) { lock.acquire(); @@ -352,13 +319,11 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { for (int i = 0; i < n; i++) { ParameterDefinitionDB param = parameters.get(i); if (param.getDataType() == dt) { - dt.removeParent(this); param.setDataType(DataType.DEFAULT); } } if (dt == getReturnType()) { - dt.removeParent(this); - setReturnType(DataType.VOID); + setReturnType(DataType.DEFAULT); } } finally { @@ -366,9 +331,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#isEquivalent(ghidra.program.model.data.DataType) - */ @Override public boolean isEquivalent(DataType dt) { if (dt == this) { @@ -416,46 +378,50 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { return false; } - /** - * @see ghidra.program.model.data.DataType#setCategoryPath(ghidra.program.model.data.CategoryPath) - */ @Override protected void doSetCategoryPathRecord(long categoryID) throws IOException { record.setLongValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_CAT_ID_COL, categoryID); funDefAdapter.updateRecord(record, false); } - /** - * @see ghidra.program.model.data.DataType#setName(java.lang.String) - */ @Override protected void doSetNameRecord(String name) throws IOException { record.setString(FunctionDefinitionDBAdapter.FUNCTION_DEF_NAME_COL, name); funDefAdapter.updateRecord(record, true); } - /** - * @see ghidra.program.model.data.DataType#dataTypeReplaced(ghidra.program.model.data.DataType, ghidra.program.model.data.DataType) - */ @Override public void dataTypeReplaced(DataType oldDt, DataType newDt) { lock.acquire(); try { checkDeleted(); if (newDt == this) { + // TODO: document why this is neccessary newDt = DataType.DEFAULT; } DataType retType = getReturnType(); if (oldDt == retType) { - setReturnType(newDt); + try { + setReturnType(newDt); + } + catch (IllegalArgumentException e) { + // oldDt replaced with incompatible type - treat as removal + dataTypeDeleted(oldDt); + return; + } } int n = parameters.size(); for (int i = 0; i < n; i++) { ParameterDefinitionDB param = parameters.get(i); if (param.getDataType() == oldDt) { - oldDt.removeParent(this); - param.setDataType(newDt); - newDt.addParent(this); + try { + param.setDataType(newDt); + } + catch (IllegalArgumentException e) { + // oldDt replaced with incompatible type - treat as removal + dataTypeDeleted(oldDt); + return; + } } } } @@ -467,7 +433,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { @Override public void replaceArgument(int ordinal, String name, DataType dt, String comment, SourceType source) { -//TODO: if (dt.getLength() <= 0) { throw new IllegalArgumentException("Fixed length data type expected"); } @@ -499,17 +464,11 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /** - * @see ghidra.program.model.data.DataType#dataTypeNameChanged(ghidra.program.model.data.DataType, java.lang.String) - */ @Override public void dataTypeNameChanged(DataType dt, String oldName) { // don't care } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#hasVarArgs() - */ @Override public boolean hasVarArgs() { lock.acquire(); @@ -526,9 +485,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setVarArgs(boolean) - */ @Override public void setVarArgs(boolean hasVarArgs) { lock.acquire(); @@ -555,9 +511,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /** - * @see ghidra.program.model.data.FunctionDefinition#setGenericCallingConvention(ghidra.program.model.data.GenericCallingConvention) - */ @Override public void setGenericCallingConvention(GenericCallingConvention genericCallingConvention) { lock.acquire(); @@ -587,9 +540,6 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /** - * @see ghidra.program.model.listing.FunctionSignature#getGenericCallingConvention() - */ @Override public GenericCallingConvention getGenericCallingConvention() { lock.acquire(); @@ -609,17 +559,11 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getLastChangeTime() - */ @Override public long getLastChangeTime() { return record.getLongValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_LAST_CHANGE_TIME_COL); } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getSourceSyncTime() - */ @Override public long getLastChangeTimeInSourceArchive() { return record.getLongValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_SOURCE_SYNC_TIME_COL); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java index 041d15f230..530c719da9 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java @@ -64,7 +64,13 @@ final class ParameterDefinitionDB implements ParameterDefinition { @Override public void setDataType(DataType type) { - type = ParameterDefinitionImpl.checkDataType(type, dataMgr); + type = ParameterDefinitionImpl.checkDataType(type, dataMgr, false); + + getDataType().removeParent(parent); + + type = dataMgr.resolve(type, null); + type.addParent(parent); + record.setLongValue(FunctionParameterAdapter.PARAMETER_DT_ID_COL, dataMgr.getResolvedID(type)); record.setIntValue(FunctionParameterAdapter.PARAMETER_DT_LENGTH_COL, type.getLength()); @@ -136,27 +142,6 @@ final class ParameterDefinitionDB implements ParameterDefinition { return record.getIntValue(FunctionParameterAdapter.PARAMETER_ORDINAL_COL); } - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (obj == this) { - return true; - } - if (!(obj instanceof ParameterDefinition)) { - return false; - } - ParameterDefinition p = (ParameterDefinition) obj; - if ((getOrdinal() == p.getOrdinal()) && (getName().equals(p.getName())) && - (getDataType().isEquivalent(p.getDataType()))) { - String myCmt = getComment(); - String otherCmt = p.getComment(); - return (myCmt == null) ? (otherCmt == null) : (myCmt.equals(otherCmt)); - } - return false; - } - @Override public boolean isEquivalent(Variable otherVar) { if (otherVar == null) { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java index 7cae55c80c..5a5b05795a 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java @@ -32,6 +32,7 @@ public interface FunctionDefinition extends DataType, FunctionSignature { /** * Set the return data type for this function * @param type the return datatype to be set. + * @throws IllegalArgumentException if data type is not a fixed length type */ public void setReturnType(DataType type); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java index 58eb3d08e1..73f88b7968 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java @@ -25,15 +25,12 @@ import ghidra.program.model.listing.*; import ghidra.program.model.mem.MemBuffer; import ghidra.program.model.symbol.SourceType; import ghidra.util.UniversalID; -import ghidra.util.exception.AssertException; -import ghidra.util.exception.InvalidInputException; /** * Definition of a function for things like function pointers. */ public class FunctionDefinitionDataType extends GenericDataType implements FunctionDefinition { - private static final long serialVersionUID = 1L; private DataType returnType = DataType.DEFAULT; private ParameterDefinition[] params; private String comment; @@ -77,8 +74,8 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct public FunctionDefinitionDataType(CategoryPath path, String name, FunctionSignature sig, UniversalID universalID, SourceArchive sourceArchive, long lastChangeTime, long lastChangeTimeInSourceArchive, DataTypeManager dtm) { - super(path, name, universalID, sourceArchive, lastChangeTime, - lastChangeTimeInSourceArchive, dtm); + super(path, name, universalID, sourceArchive, lastChangeTime, lastChangeTimeInSourceArchive, + dtm); init(sig); } @@ -135,82 +132,40 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct } } - /** - * @param sig - */ private void copySignature(FunctionSignature sig) { comment = sig.getComment(); DataType rtnType = sig.getReturnType(); - if (rtnType == null) { - rtnType = DataType.DEFAULT; - } setReturnType(rtnType.clone(getDataTypeManager())); setArguments(sig.getArguments()); hasVarArgs = sig.hasVarArgs(); genericCallingConvention = sig.getGenericCallingConvention(); } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setArguments(ghidra.program.model.listing.ParameterDefinition[]) - */ @Override public void setArguments(ParameterDefinition[] args) { - for (int i = 0; i < params.length; i++) { - params[i].getDataType().removeParent(this); - } - - if (args.length == 1) { - if (args[0].getDataType() instanceof VoidDataType) { - args = new ParameterDefinition[0]; - } - } - params = new ParameterDefinition[args.length]; for (int i = 0; i < args.length; i++) { DataType dt = args[i].getDataType(); - dt.addParent(this); - params[i] = - new ParameterDefinitionImpl(args[i].getName(), dt.clone(getDataTypeManager()), - args[i].getComment(), i); + params[i] = new ParameterDefinitionImpl(args[i].getName(), + dt.clone(getDataTypeManager()), args[i].getComment(), i); } } - /** - * @see ghidra.program.model.data.DataType#isDynamicallySized() - */ @Override public boolean isDynamicallySized() { return false; } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setReturnType(ghidra.program.model.data.DataType) - */ @Override public void setReturnType(DataType type) { - if (type == null || type.getLength() < 0) { - returnType = DataType.DEFAULT; - } - else { - returnType.removeParent(this); - } - returnType = type; - if (returnType != null) { - returnType.addParent(this); - } + returnType = ParameterDefinitionImpl.checkDataType(type, dataMgr, true); } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setComment(java.lang.String) - */ @Override public void setComment(String comment) { this.comment = comment; } - /* (non-Javadoc) - * @see ghidra.program.model.data.FunctionDefinition#setVarArgs(boolean) - */ @Override public void setVarArgs(boolean hasVarArgs) { this.hasVarArgs = hasVarArgs; @@ -241,41 +196,26 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct getSourceArchive(), getLastChangeTime(), getLastChangeTimeInSourceArchive(), dtm); } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getMnemonic(ghidra.program.model.data.Settings) - */ @Override public String getMnemonic(Settings settings) { return getPrototypeString(); } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getLength() - */ @Override public int getLength() { return -1; } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getDescription() - */ @Override public String getDescription() { return "Function: " + getMnemonic(null); } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getValue(ghidra.program.model.mem.MemBuffer, ghidra.program.model.lang.ProcessorContext, ghidra.program.model.data.Settings, int) - */ @Override public Object getValue(MemBuffer buf, Settings settings, int length) { return null; } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#getRepresentation(ghidra.program.model.mem.MemBuffer, ghidra.program.model.lang.ProcessorContext, ghidra.program.model.data.Settings, int) - */ @Override public String getRepresentation(MemBuffer buf, Settings settings, int length) { @@ -320,9 +260,6 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct return buf.toString(); } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#getArguments() - */ @Override public ParameterDefinition[] getArguments() { ParameterDefinition[] args = new ParameterDefinition[params.length]; @@ -330,25 +267,16 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct return args; } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#getReturnType() - */ @Override public DataType getReturnType() { return returnType; } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#getComment() - */ @Override public String getComment() { return comment; } - /* (non-Javadoc) - * @see ghidra.program.model.listing.FunctionSignature#hasVarArgs() - */ @Override public boolean hasVarArgs() { return hasVarArgs; @@ -405,30 +333,37 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct return false; } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#dataTypeReplaced(ghidra.program.model.data.DataType, ghidra.program.model.data.DataType) - */ @Override public void dataTypeReplaced(DataType oldDt, DataType newDt) { - if (returnType == oldDt) { - oldDt.removeParent(this); - returnType = newDt; - newDt.addParent(this); + + if (newDt == this) { + // TODO: document why this is neccessary + newDt = DataType.DEFAULT; + } + DataType retType = getReturnType(); + if (oldDt == retType) { + try { + setReturnType(newDt); + } + catch (IllegalArgumentException e) { + // oldDt replaced with incompatible type - treat as removal + dataTypeDeleted(oldDt); + return; + } } for (int i = 0; i < params.length; i++) { ParameterDefinition param = params[i]; if (param.getDataType() == oldDt) { - oldDt.removeParent(this); try { param.setDataType(newDt); } - catch (InvalidInputException e) { - throw new IllegalArgumentException(e.getMessage()); + catch (IllegalArgumentException e) { + // oldDt replaced with incompatible type - treat as removal + dataTypeDeleted(oldDt); + return; } - newDt.addParent(this); } } - } @Override @@ -439,57 +374,36 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct ParameterDefinition[] newParams = new ParameterDefinition[ordinal + 1]; System.arraycopy(params, 0, newParams, 0, params.length); for (int i = params.length; i < ordinal + 1; i++) { - newParams[i] = - new ParameterDefinitionImpl(Function.DEFAULT_PARAM_PREFIX + (i + 1), - DataType.DEFAULT, newComment, i); + newParams[i] = new ParameterDefinitionImpl(Function.DEFAULT_PARAM_PREFIX + (i + 1), + DataType.DEFAULT, newComment, i); } params = newParams; } - params[ordinal].getDataType().removeParent(this); params[ordinal] = new ParameterDefinitionImpl(newName, dt, newComment, ordinal); - dt.addParent(this); - } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#dataTypeSizeChanged(ghidra.program.model.data.DataType) - */ @Override public void dataTypeSizeChanged(DataType dt) { + // ignore - no affect } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#dataTypeDeleted(ghidra.program.model.data.DataType) - */ @Override public void dataTypeDeleted(DataType dt) { if (returnType == dt) { - dt.removeParent(this); - returnType = new VoidDataType(); + returnType = DataType.DEFAULT; } for (int i = 0; i < params.length; i++) { if (params[i].getDataType() == dt) { - try { - params[i].setDataType(DataType.DEFAULT); - } - catch (InvalidInputException e) { - throw new AssertException( - "Setting DataType to Default should not throw this exception"); - } + params[i].setDataType(DataType.DEFAULT); } } } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#dataTypeNameChanged(ghidra.program.model.data.DataType, java.lang.String) - */ @Override public void dataTypeNameChanged(DataType dt, String oldName) { + // ignore - no affect } - /* (non-Javadoc) - * @see ghidra.program.model.data.DataType#dependsOn(ghidra.program.model.data.DataType) - */ @Override public boolean dependsOn(DataType dt) { return false; diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java index 3ae89bbd32..e99cda139f 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java @@ -15,8 +15,8 @@ */ package ghidra.program.model.data; +import ghidra.program.model.listing.Parameter; import ghidra.program.model.listing.Variable; -import ghidra.util.exception.InvalidInputException; /** * ParameterDefinition specifies a parameter which can be @@ -25,7 +25,9 @@ import ghidra.util.exception.InvalidInputException; public interface ParameterDefinition extends Comparable { /** - * Returns the ordinal (index) of this parameter within the function signature. + * Get the parameter ordinal + * + * @return the ordinal (index) of this parameter within the function signature. */ int getOrdinal(); @@ -39,9 +41,9 @@ public interface ParameterDefinition extends Comparable { /** * Set the Data Type of this variable. The given dataType must have a fixed length. * @param type the data type - * @throws InvalidInputException if data type is not a fixed length or will not fit. + * @throws IllegalArgumentException if data type is not a fixed length type */ - public void setDataType(DataType type) throws InvalidInputException; + public void setDataType(DataType type); /** * Get the Name of this variable. @@ -77,16 +79,22 @@ public interface ParameterDefinition extends Comparable { public void setComment(String comment); /** - * Returns true if the specified variable - * represents the same parameter by ordinal - * and dataType + * Determine if a variable corresponds to a parameter which is equivelent to + * this parameter definition by both ordinal and datatype. Name is not considered + * relavent. + * @param variable variable to be compared with this parameter definition. + * @return true if the specified variable represents the same parameter by ordinal + * and dataType. False will always be returned if specified variable is + * not a {@link Parameter}. */ public boolean isEquivalent(Variable variable); /** - * Returns true if the specified parameter definition - * represents the same parameter by ordinal - * and dataType + * Determine if parm is equivelent to this parameter definition by both ordinal + * and datatype. Name is not considered relavent. + * @param parm parameter definition to be compared with this parameter definition. + * @return true if the specified parameter definition represents the same parameter + * by ordinal and dataType. */ public boolean isEquivalent(ParameterDefinition parm); } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java index 48d695c290..ce1497bce6 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java @@ -15,13 +15,10 @@ */ package ghidra.program.model.data; -import org.apache.commons.lang3.StringUtils; - import ghidra.program.database.data.DataTypeUtilities; import ghidra.program.model.listing.Parameter; import ghidra.program.model.listing.Variable; import ghidra.program.model.symbol.SymbolUtilities; -import ghidra.util.exception.InvalidInputException; public class ParameterDefinitionImpl implements ParameterDefinition { @@ -49,13 +46,15 @@ public class ParameterDefinitionImpl implements ParameterDefinition { * @param ordinal the index of this parameter within the function signature. */ protected ParameterDefinitionImpl(String name, DataType dataType, String comment, int ordinal) { - this.dataType = checkDataType(dataType, null); + this.dataType = checkDataType(dataType, null, false); this.name = name; this.comment = comment; this.ordinal = ordinal; } - public static DataType checkDataType(DataType dataType, DataTypeManager dtMgr) { + public static DataType checkDataType(DataType dataType, DataTypeManager dtMgr, boolean isReturn) + throws IllegalArgumentException { + String kind = isReturn ? "Return" : "Parameter"; if (dataType == null) { dataType = DataType.DEFAULT; } @@ -65,21 +64,23 @@ public class ParameterDefinitionImpl implements ParameterDefinition { } else if (dataType instanceof Dynamic || dataType instanceof FactoryDataType) { throw new IllegalArgumentException( - "Parameter may not be defined with Dynamic or Factory data-type: " + + kind + " type may not be defined with Dynamic or Factory data-type: " + dataType.getName()); } dataType = dataType.clone(dtMgr != null ? dtMgr : dataType.getDataTypeManager()); if (!dataType.isDynamicallySized() && dataType.getLength() < 0) { - throw new IllegalArgumentException( - "Parameter must be specified with fixed-length data type: " + dataType.getName()); + throw new IllegalArgumentException(kind + + " type must be specified with fixed-length data type: " + dataType.getName()); } if (dataType instanceof VoidDataType) { - throw new IllegalArgumentException( - "Parameter may not specify the void datatype - empty parameter list should be used"); + if (!isReturn) { + throw new IllegalArgumentException( + "Parameter type may not specify the void datatype - empty parameter list should be used"); + } } - if (!(dataType instanceof Composite) && dataType.getLength() == 0) { - throw new IllegalArgumentException( - "Parameter must be specified with fixed-length data type: " + dataType.getName()); + else if (!(dataType instanceof Composite) && dataType.getLength() == 0) { + throw new IllegalArgumentException(kind + + " type must be specified with fixed-length data type: " + dataType.getName()); } return dataType; } @@ -129,8 +130,8 @@ public class ParameterDefinitionImpl implements ParameterDefinition { } @Override - public void setDataType(DataType type) throws InvalidInputException { - this.dataType = checkDataType(type, dataType.getDataTypeManager()); + public void setDataType(DataType type) { + this.dataType = checkDataType(type, dataType.getDataTypeManager(), false); } @Override @@ -141,31 +142,6 @@ public class ParameterDefinitionImpl implements ParameterDefinition { this.name = name; } - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (obj == this) { - return true; - } - if (!(obj instanceof ParameterDefinition)) { - return false; - } - - ParameterDefinition otherVar = (ParameterDefinition) obj; - if (ordinal != otherVar.getOrdinal()) { - return false; - } - if (!DataTypeUtilities.isSameOrEquivalentDataType(dataType, otherVar.getDataType())) { - return false; - } - if (!StringUtils.equals(getName(), otherVar.getName())) { - return false; - } - return StringUtils.equals(getComment(), otherVar.getComment()); - } - @Override public boolean isEquivalent(Variable variable) { if (variable == null) {