GP-7220 - Version Tracking - Added an option to use the existing data

type during an apply

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/api/markuptype/DataTypeMarkupType.java

@@ -266,8 +266,17 @@ public class DataTypeMarkupType extends VTMarkupType {
 			// data that would be overwritten following the first data in the destination.
 			return false;
 		}
-		
-		DataType resolvedDataType = program.getDataTypeManager().resolve(dataType, conflictHandler);
+
+		/*
+		 	Note: the resolve may add a .conflict type.  That will not be removed if an exception is
+		 	thrown.  Also if an unapply is executed, .conflict types will not be removed.  For now,
+		 	we leave this up to the user to fix, should they care.  Trying to remove .conflict types
+		 	during an unapply may have unintended side-effects if the user added new uses of that 
+		 	conflict type.
+		 */
+
+		ProgramBasedDataTypeManager dtm = program.getDataTypeManager();
+		DataType resolvedDataType = dtm.resolve(dataType, conflictHandler);
 
 		listing.clearCodeUnits(startAddress, endAddress, false);
 

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/util/VTMatchApplyChoices.java

@@ -4,9 +4,9 @@
  * 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.

Ghidra/Features/VersionTracking/src/test.slow/java/ghidra/feature/vt/api/markupitem/DataTypeMarkupItemTest.java

@@ -29,6 +29,7 @@ import ghidra.feature.vt.gui.util.VTMatchApplyChoices.DataTypeConflictChoices;
 import ghidra.feature.vt.gui.util.VTMatchApplyChoices.ReplaceDataChoices;
 import ghidra.feature.vt.gui.util.VTOptionDefines;
 import ghidra.framework.options.ToolOptions;
+import ghidra.program.database.data.ProgramDataTypeManager;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.*;
 import ghidra.program.model.lang.*;
@@ -86,6 +87,57 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 		doTestFindAndApplyMarkupItem(validator);
 	}
 
+	@Test
+	public void testReplace_ConflictInDtm_ChooseExistingType() throws Exception {
+
+		Address srcAddr = addr("0x010074e6", sourceProgram);
+		StructureDataType coolStruct1 = createCoolStruct1();
+		Data sourceData = setDataType(sourceProgram, srcAddr, coolStruct1, coolStruct1.getLength());
+
+		// Add a type to the destination that has the same data type path, but is not equivalent
+		StructureDataType coolStruct2 = createCoolStruct2();
+		addToDestinationDtm(coolStruct2);
+
+		// make room for the type to be applied
+		Address destAddr = addr("0x010074e6", destinationProgram);
+		clear(destinationProgram, destAddr, coolStruct2.getLength());
+
+		StringDataType destDt = new StringDataType();
+		Data destData = setDataType(destinationProgram, destAddr, destDt, 4);
+
+		DataTypeValidator validator = new DataTypeValidator(sourceData, destData,
+			ReplaceDataChoices.REPLACE_FIRST_DATA_ONLY);
+		validator.setConflictChoice(DataTypeConflictChoices.USE_EXISTING);
+		validator.setKeepExistingType(true);
+		doTestFindAndApplyMarkupItem(validator);
+
+		assertConflictTypeInDestinationDtm(coolStruct1, false);
+	}
+
+	@Test
+	public void testReplace_ConflictInDtm_ChooseRenameAndAdd() throws Exception {
+
+		Address srcAddr = addr("0x010074e6", sourceProgram);
+		StructureDataType coolStruct1 = createCoolStruct1();
+		Data sourceData = setDataType(sourceProgram, srcAddr, coolStruct1, coolStruct1.getLength());
+
+		// Add a type to the destination that has the same data type path, but is not equivalent
+		StructureDataType coolStruct2 = createCoolStruct2();
+		addToDestinationDtm(coolStruct2);
+
+		Address destAddr = addr("0x010074e6", destinationProgram);
+		StringDataType destDt = new StringDataType();
+		Data destData = setDataType(destinationProgram, destAddr, destDt, 4);
+		setDataType(destinationProgram, destAddr.add(4), destDt, 6);
+
+		DataTypeValidator validator = new DataTypeValidator(sourceData, destData,
+			ReplaceDataChoices.REPLACE_FIRST_DATA_ONLY);
+		validator.setConflictChoice(DataTypeConflictChoices.RENAME_AND_ADD);
+		doTestFindAndApplyMarkupItem(validator);
+
+		assertConflictTypeInDestinationDtm(coolStruct1, true);
+	}
+
 	@Test
 	public void testReplaceLargerWithSmaller() throws Exception {
 
@@ -308,7 +360,7 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 
 	@Test
 	public void testRejectedApplyDoesNotMutateDestinationDataTypeManager() throws Exception {
-		
+
 		Address sourceAddress = addr("0x010074e6", sourceProgram); // LoadCursorW
 		StructureDataType sourceDataType = new StructureDataType("RejectedApplyStruct", 0);
 		sourceDataType.add(new DWordDataType());
@@ -446,6 +498,48 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 // Private Methods
 //==================================================================================================
 
+	private void addToDestinationDtm(StructureDataType struct) {
+		ProgramDataTypeManager destDtm = destinationProgram.getDataTypeManager();
+		tx(destDtm, () -> {
+			destDtm.resolve(struct, null);
+		});
+
+		DataTypePath dtp = struct.getDataTypePath();
+		DataType resolvedDtm = destDtm.getDataType(dtp);
+		assertNotNull(resolvedDtm);
+	}
+
+	private StructureDataType createCoolStruct1() {
+		StructureDataType struct = new StructureDataType("CoolStructure", 0);
+		struct.add(new DWordDataType());
+		return struct;
+	}
+
+	// 'CoolStructure' that is slightly different than that made in createCoolStruct1()
+	private StructureDataType createCoolStruct2() {
+		StructureDataType struct = new StructureDataType("CoolStructure", 0);
+		struct.add(new DWordDataType());
+		struct.add(new DWordDataType());
+		return struct;
+	}
+
+	private void assertConflictTypeInDestinationDtm(DataType dt, boolean expectConflict) {
+
+		DataTypePath dtp = dt.getDataTypePath();
+		String name = dt.getName() + ".conflict";
+		CategoryPath cp = dtp.getCategoryPath();
+		DataTypePath conflictPath = new DataTypePath(cp, name);
+
+		ProgramDataTypeManager destDtm = destinationProgram.getDataTypeManager();
+		DataType conflictType = destDtm.getDataType(conflictPath);
+		if (expectConflict) {
+			assertNotNull(conflictType);
+		}
+		else {
+			assertNull(conflictType);
+		}
+	}
+
 	private Structure createGadgetStruct() {
 
 		Structure gadgetStruct = new StructureDataType("Gadget", 0);
@@ -461,14 +555,13 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 
 	private Data setDataType(Program program, Address address, DataType dataType, int length) {
 
-		int txID = program.startTransaction("Change Data Type");
-		boolean commit = false;
-		try {
+		return tx(program, () -> {
 			Listing listing = program.getListing();
 			Data sourceData = listing.getDataAt(address);
 			if (sourceData == null) {
 				return null;
 			}
+
 			listing.clearCodeUnits(address, sourceData.getMaxAddress(), false);
 			Data data;
 			if (length > 0) {
@@ -477,23 +570,20 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 			else {
 				data = listing.createData(address, dataType);
 			}
-			commit = true;
 			return data;
-		}
-		catch (Exception e) {
-			// Commit is false by default so nothing else to do.
-			return null;
-		}
-		finally {
-			program.endTransaction(txID, commit);
-		}
+		});
+	}
+
+	private void clear(Program p, Address a, int length) {
+		tx(p, () -> {
+			Listing listing = p.getListing();
+			listing.clearCodeUnits(a, a.add(length), false);
+		});
 	}
 
 	private Instruction createInstruction(Program program, Address atAddress) {
 
-		int txID = program.startTransaction("Create Instruction");
-		boolean commit = false;
-		try {
+		return tx(program, () -> {
 			Listing listing = program.getListing();
 			Memory memory = program.getMemory();
 			MemBuffer buf = new DumbMemBufferImpl(memory, atAddress);
@@ -502,16 +592,8 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 			InstructionPrototype proto = program.getLanguage().parse(buf, context, false);
 			Instruction createdInstruction =
 				listing.createInstruction(atAddress, proto, buf, context, 0);
-			commit = true;
 			return createdInstruction;
-		}
-		catch (Exception e) {
-			// Commit is false by default so nothing else to do.
-			return null;
-		}
-		finally {
-			program.endTransaction(txID, commit);
-		}
+		});
 	}
 
 //==================================================================================================
@@ -528,6 +610,7 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 		private int originalDestinationLength;
 		private ReplaceDataChoices dataTypeChoice;
 		private DataTypeConflictChoices conflictChoice;
+		private boolean keepExistingType;
 
 		DataTypeValidator(Data sourceData, Data destinationData,
 				ReplaceDataChoices dataTypeChoice) {
@@ -548,6 +631,10 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 			this.conflictChoice = conflictChoice;
 		}
 
+		void setKeepExistingType(boolean keep) {
+			this.keepExistingType = keep;
+		}
+
 		@Override
 		protected Address getDestinationApplyAddress() {
 			return getDestinationMatchAddress();
@@ -587,10 +674,19 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
 			Data currentDestinationData = listing.getDataAt(getDestinationApplyAddress());
 			DataType currentDestinationDataType = currentDestinationData.getDataType();
 			int currentDestinationLength = currentDestinationData.getLength();
-			assertTrue("Data type was not applied",
-				sourceDataType.isEquivalent(currentDestinationDataType));
-			assertTrue("Data type was not set to the source data type's size",
-				sourceLength == currentDestinationLength);
+
+			if (keepExistingType) {
+				// guilty knowledge: keeping the existing type is used when the types have the same
+				// data type path, but are not equivalent
+				assertEquals(sourceDataType.getDataTypePath(),
+					currentDestinationDataType.getDataTypePath());
+			}
+			else {
+				assertTrue("Data type was not applied",
+					sourceDataType.isEquivalent(currentDestinationDataType));
+				assertTrue("Data type was not set to the source data type's size",
+					sourceLength == currentDestinationLength);
+			}
 		}
 
 		@Override