Merge remote-tracking branch

'origin/GP-1307_ghidra1_DTResolveWithSource--SQUASHED' (Closes #4634)

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeUtilitiesTest.java

@@ -63,6 +63,52 @@ public class DataTypeUtilitiesTest extends AbstractGenericTest {
 		}
 	}
 
+	@Test
+	public void testIsSameKindDataType() {
+
+		assertTrue(
+			DataTypeUtilities.isSameKindDataType(IntegerDataType.dataType, ShortDataType.dataType));
+		assertFalse(
+			DataTypeUtilities.isSameKindDataType(FloatDataType.dataType, ShortDataType.dataType));
+
+		assertTrue(
+			DataTypeUtilities.isSameKindDataType(new PointerDataType(IntegerDataType.dataType),
+				new PointerDataType(ShortDataType.dataType)));
+		assertFalse(
+			DataTypeUtilities.isSameKindDataType(new PointerDataType(FloatDataType.dataType),
+				new PointerDataType(ShortDataType.dataType)));
+
+		assertTrue(
+			DataTypeUtilities.isSameKindDataType(new StructureDataType("X", 10),
+				new StructureDataType("Y", 5)));
+		assertTrue(
+			DataTypeUtilities.isSameKindDataType(new UnionDataType("X"), new UnionDataType("Y")));
+		assertFalse(
+			DataTypeUtilities.isSameKindDataType(new StructureDataType("X", 10),
+				new UnionDataType("Y")));
+
+		assertTrue(
+			DataTypeUtilities.isSameKindDataType(
+				new PointerDataType(new StructureDataType("X", 10)),
+				new PointerDataType(new StructureDataType("Y", 5))));
+		assertTrue(
+			DataTypeUtilities.isSameKindDataType(new PointerDataType(new UnionDataType("X")),
+				new PointerDataType(new UnionDataType("Y"))));
+		assertFalse(
+			DataTypeUtilities.isSameKindDataType(
+				new PointerDataType(new StructureDataType("X", 10)),
+				new PointerDataType(new UnionDataType("Y"))));
+
+		assertTrue(
+			DataTypeUtilities.isSameKindDataType(
+				new TypedefDataType("Foo", new PointerDataType(new StructureDataType("X", 10))),
+				new PointerDataType(new StructureDataType("Y", 5))));
+		assertFalse(
+			DataTypeUtilities.isSameKindDataType(
+				new TypedefDataType("Foo", new PointerDataType(new StructureDataType("X", 10))),
+				new PointerDataType(new UnionDataType("Y"))));
+	}
+
 	@Test
 	public void testEqualsIgnoreConflictviaManagedDataTypes() throws Exception {
 

Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/data/ConflictHandlerTest.java

@@ -24,6 +24,8 @@ import ghidra.program.database.ProgramDB;
 import ghidra.program.model.data.*;
 import ghidra.program.model.data.DataTypeConflictHandler.ConflictResult;
 import ghidra.test.AbstractGhidraHeadedIntegrationTest;
+import ghidra.util.UniversalID;
+import ghidra.util.UniversalIdGenerator;
 
 /**
  * Tests for the {@link DataTypeConflictHandler conflict handler} stuff.
@@ -153,7 +155,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	/**
-	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to ensure that adding a empty conflicting structure resolves to a previous
 	 * populated structure.
 	 */
@@ -168,7 +170,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	/**
-	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to ensure that adding a populated structure replaces an existing
 	 * 'empty' structure.  'Empty' means either 0 byte length or 1 byte length structs
 	 * as previous versions of Ghidra did not allow truly empty structs.
@@ -227,7 +229,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	/**
-	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to ensure that adding a conflicting typedef to a conflicting stub structure 
 	 * (when there is already a typedef to a populated structure) correctly uses the 
 	 * existing populated structure and existing typedef to the populated structure.
@@ -253,7 +255,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	/**
-	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to ensure that adding truly conflicting structures and typedefs
 	 * are treated as new data types and are renamed to a different name when added.
 	 */
@@ -283,7 +285,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	/**
-	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler when adding a conflicting typedef impl that is referred to multiple 
 	 * times during a single addDataType() call.
 	 * <p>
@@ -328,7 +330,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	/**
-	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler  when adding a conflicting typedef impl (but equiv) that is referred to multiple 
 	 * times during a single addDataType() call.
 	 * <p>
@@ -368,7 +370,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 	/**
-	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * Tests the {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler when adding a typedef to a populated when there is already a typedef
 	 * to a stub structure.  
 	 */
@@ -397,7 +399,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 
 	/**
 	 * Tests the
-	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to be sure that, if all else is the same, the packed version is chosen
 	 * over the non-packed version.
 	 * <p>
@@ -422,7 +424,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 
 	/**
 	 * Tests the
-	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to be sure that, if all else is the same, the packed version is chosen
 	 * over the non-packed version.
 	 * <p>
@@ -447,7 +449,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 
 	/**
 	 * Tests the
-	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to be sure that, if all else is the same, the new non-packed version is
 	 * chosen over the existing non-packed version.
 	 * <p>
@@ -476,7 +478,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 
 	/**
 	 * Tests the
-	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to be sure that, if all else is the same, the new non-packed version is
 	 * chosen over the existing packed version.
 	 * <p>
@@ -505,7 +507,7 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 
 	/**
 	 * Tests the
-	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER RESORAAH}
+	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
 	 * conflict handler to be sure that, if all else is the same, the packed version is chosen
 	 * over the non-packed version.
 	 * <p>
@@ -515,24 +517,147 @@ public class ConflictHandlerTest extends AbstractGhidraHeadedIntegrationTest {
 	public void testResolveDataTypeNonStructConflict() throws Exception {
 		DataTypeManager dtm = new StandAloneDataTypeManager("Test");
 		int id = dtm.startTransaction("");
-		Category otherRoot = dataMgr.getRootCategory();
+		try {
-		Category subc = otherRoot.createCategory("subc");
+			Category otherRoot = dataMgr.getRootCategory();
+			Category subc = otherRoot.createCategory("subc");
 
-		EnumDataType e = new EnumDataType(subc.getCategoryPath(), "Enum", 2);
+			EnumDataType e = new EnumDataType(subc.getCategoryPath(), "Enum", 2);
 
-		DataType resolvedEnum =
+			DataType resolvedEnum =
-			dtm.resolve(e, DataTypeConflictHandler.REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER);
+				dtm.resolve(e,
-		assertTrue(e.isEquivalent(resolvedEnum));
+					DataTypeConflictHandler.REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER);
-		assertEquals("/subc/Enum", resolvedEnum.getPathName());
+			assertTrue(e.isEquivalent(resolvedEnum));
+			assertEquals("/subc/Enum", resolvedEnum.getPathName());
 
-		e.add("xyz", 1);
+			e.add("xyz", 1);
 
-		resolvedEnum =
+			resolvedEnum =
-			dtm.resolve(e, DataTypeConflictHandler.REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER);
+				dtm.resolve(e,
-		assertTrue(e.isEquivalent(resolvedEnum));
+					DataTypeConflictHandler.REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER);
-		assertEquals("/subc/Enum.conflict", resolvedEnum.getPathName());
+			assertTrue(e.isEquivalent(resolvedEnum));
+			assertEquals("/subc/Enum.conflict", resolvedEnum.getPathName());
+		}
+		finally {
+			dtm.endTransaction(id, true);
+			dtm.close();
+		}
+	}
+
+	/**
+	 * Tests the
+	 * {@link DataTypeConflictHandler#REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER}
+	 * conflict handler to be sure that and empty local structure will be replaced by
+	 * a structure with source.
+	 * <p>
+	 * Success is the source version is chosen over the empty local version.
+	 */
+	@Test
+	public void testChooseStructWithSourceOverExistingEmptyStructures() throws Exception {
+
+		Structure struct = new StructureDataType(root, "TestStruct", 0, dataMgr);
+		struct = (Structure) dataMgr.resolve(struct, null);
+
+		SourceArchive source = new DummySourceArchive("Test");
+
+		Structure structWithSource = new StructureDataType(root, "TestStruct", 0, dataMgr);
+		structWithSource.setSourceArchive(source);
+		structWithSource.add(ByteDataType.dataType);
+
+		structWithSource = (Structure) dataMgr.resolve(structWithSource,
+			DataTypeConflictHandler.REPLACE_EMPTY_STRUCTS_OR_RENAME_AND_ADD_HANDLER);
+
+		assertEquals("TestStruct", structWithSource.getName());
+		assertTrue(struct == structWithSource);
+		SourceArchive sourceArchive = struct.getSourceArchive();
+		assertEquals(source.getSourceArchiveID(), sourceArchive.getSourceArchiveID());
+		assertEquals(source.getName(), sourceArchive.getName());
+	}
+
+	@Test
+	public void testResolvePointerConflict() {
+
+		DataType ptr1 =
+			new PointerDataType(new TypedefDataType("size_t", UnsignedIntegerDataType.dataType));
+		DataType ptr2 =
+			new PointerDataType(new TypedefDataType("size_t", IntegerDataType.dataType));
+
+		DataType ptr1resolved = dataMgr.resolve(ptr1, DataTypeConflictHandler.DEFAULT_HANDLER);
+		assertEquals("size_t *", ptr1resolved.getName());
+
+		DataType ptr2resolvedA = dataMgr.resolve(ptr2, DataTypeConflictHandler.KEEP_HANDLER);
+		assertTrue(ptr2resolvedA == ptr1resolved);
+
+		DataType ptr2resolvedB = dataMgr.resolve(ptr2, DataTypeConflictHandler.DEFAULT_HANDLER);
+		assertEquals("size_t.conflict *", ptr2resolvedB.getName());
+
+		DataType ptr2resolvedC = dataMgr.resolve(ptr2, DataTypeConflictHandler.REPLACE_HANDLER);
+		assertTrue(ptr2resolvedC == ptr2resolvedB);
+	}
+
+	@Test
+	public void testResolveArrayConflict() {
+
+		DataType array1 =
+			new ArrayDataType(new TypedefDataType("size_t", UnsignedIntegerDataType.dataType), 2,
+				-1);
+		DataType array2 =
+			new ArrayDataType(new TypedefDataType("size_t", IntegerDataType.dataType), 2, -1);
+
+		DataType array1resolved = dataMgr.resolve(array1, DataTypeConflictHandler.DEFAULT_HANDLER);
+		assertEquals("size_t[2]", array1resolved.getName());
+
+		DataType array2resolvedA = dataMgr.resolve(array2, DataTypeConflictHandler.KEEP_HANDLER);
+		assertTrue(array2resolvedA == array1resolved);
+
+		DataType array2resolvedB = dataMgr.resolve(array2, DataTypeConflictHandler.DEFAULT_HANDLER);
+		assertEquals("size_t.conflict[2]", array2resolvedB.getName());
+
+		DataType array2resolvedC = dataMgr.resolve(array2, DataTypeConflictHandler.REPLACE_HANDLER);
+		assertTrue(array2resolvedC == array2resolvedB);
+	}
+
+	private static class DummySourceArchive implements SourceArchive {
+
+		private final UniversalID id;
+		private final String archiveName;
+
+		public DummySourceArchive(String archiveName) {
+			this.id = UniversalIdGenerator.nextID();
+			this.archiveName = archiveName;
+		}
+
+		public ArchiveType getArchiveType() {
+			return ArchiveType.FILE;
+		}
+
+		public String getDomainFileID() {
+			return null;
+		}
+
+		public long getLastSyncTime() {
+			return 0;
+		}
+
+		public String getName() {
+			return archiveName;
+		}
+
+		public UniversalID getSourceArchiveID() {
+			return id;
+		}
+
+		public boolean isDirty() {
+			return false;
+		}
+
+		public void setDirtyFlag(boolean dirty) {
+		}
+
+		public void setLastSyncTime(long time) {
+		}
+
+		public void setName(String name) {
+		}
 
-		dtm.endTransaction(id, true);
-		dtm.close();
 	}
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CategoryDB.java

@@ -540,7 +540,7 @@ class CategoryDB extends DatabaseObject implements Category {
 				return;
 			}
 			if (existing != null) {
-				ConflictResult result = mgr.resolveConflict(handler, movedDataType, existing);
+				ConflictResult result = handler.resolveConflict(movedDataType, existing);
 				if (result == ConflictResult.REPLACE_EXISTING) { // replace existing dt with new dt.
 					mgr.replaceDataType(existing, movedDataType, true);
 				}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeUtilities.java

@@ -205,7 +205,7 @@ public class DataTypeUtilities {
 	}
 
 	/**
-	 * Returns true if the two dataTypes have the same sourceArchive and the same UniversalID OR are
+	 * Returns true if two dataTypes have the same sourceArchive and the same UniversalID OR are
 	 * equivalent
 	 * 
 	 * @param dataType1 first data type (if invoked by DB object or manager, this argument must
@@ -223,6 +223,79 @@ public class DataTypeUtilities {
 		return dataType1.isEquivalent(dataType2);
 	}
 
+	/**
+	 * Determine if two dataTypes are the same kind of datatype without considering naming or
+	 * component makeup.  The use of Typedefs is ignored and stripped away for comparison.
+	 * This method also ignores details about most built-in types, pointers and arrays 
+	 * (e.g., number of elements or size).  Implementations of the following abstract classes
+	 * will be treated as the same kind as another datatype which extends the same abstract
+	 * class:
+	 * <ul>
+	 * <li>{@link AbstractIntegerDataType}</li> 
+	 * <li>{@link AbstractFloatDataType}</li>
+	 * <li>{@link AbstractStringDataType}</li>
+	 * </ul>
+	 *  Other uses of {@link BuiltInDataType} must match the specific implementation class. 
+	 * @param dataType1 first data type
+	 * @param dataType2 second data type
+	 * @return true if the two dataTypes are the same basic kind else false
+	 */
+	public static boolean isSameKindDataType(DataType dataType1, DataType dataType2) {
+
+		while (true) {
+			if (dataType1 == dataType2) {
+				return true;
+			}
+
+			// Ignore the use of typedefs - strip away
+			if (dataType1 instanceof TypeDef td1) {
+				dataType1 = td1.getBaseDataType();
+			}
+			if (dataType2 instanceof TypeDef td2) {
+				dataType2 = td2.getBaseDataType();
+			}
+
+			if (dataType1 instanceof Pointer p1 && dataType2 instanceof Pointer p2) {
+				dataType1 = p1.getDataType();
+				dataType2 = p2.getDataType();
+			}
+			else if (dataType2 instanceof Array a1 && dataType2 instanceof Array a2) {
+				dataType1 = a1.getDataType();
+				dataType2 = a2.getDataType();
+			}
+			else if (dataType1 instanceof Enum) {
+				return dataType2 instanceof Enum;
+			}
+			else if (dataType1 instanceof Structure) {
+				return dataType2 instanceof Structure;
+			}
+			else if (dataType1 instanceof Union) {
+				return dataType2 instanceof Union;
+			}
+			else if (dataType1 instanceof BuiltInDataType dt1) {
+				return isSameKindBuiltInDataType(dt1, dataType2);
+			}
+			else {
+				return false;
+			}
+		}
+	}
+
+	private static boolean isSameKindBuiltInDataType(BuiltInDataType dataType1,
+			DataType dataType2) {
+		if (dataType1 instanceof BuiltIn) {
+			// Same kind if both types share a common BuiltIn implementation
+			Class<?> baseClass = dataType1.getClass().getSuperclass();
+			Class<?> superClass;
+			while ((superClass = baseClass.getSuperclass()) != BuiltIn.class) {
+				baseClass = superClass;
+			}
+			return baseClass.isAssignableFrom(dataType2.getClass());
+		}
+		// Ensure built-in implementation class is the same
+		return dataType1.getClass().equals(dataType2.getClass());
+	}
+
 	/**
 	 * Get the name of a data type with all conflict naming patterns removed.
 	 * 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/SourceArchiveUpgradeMap.java

@@ -90,54 +90,54 @@ public class SourceArchiveUpgradeMap {
 		return new String[] { "short", "int", "long", "longlong", "wchar_t", "bool" };
 	}
 
-}
+	private static class SourceArchiveImpl implements SourceArchive {
 
-class SourceArchiveImpl implements SourceArchive {
+		private final UniversalID id;
+		private final String archiveName;
 
-	private final UniversalID id;
+		public SourceArchiveImpl(UniversalID id, String archiveName) {
-	private final String archiveName;
+			this.id = id;
+			this.archiveName = archiveName;
+		}
 
-	public SourceArchiveImpl(UniversalID id, String archiveName) {
+		public SourceArchiveImpl() {
-		this.id = id;
+			id = DataTypeManager.LOCAL_ARCHIVE_UNIVERSAL_ID;
-		this.archiveName = archiveName;
+			archiveName = "";
-	}
+		}
 
-	public SourceArchiveImpl() {
+		public ArchiveType getArchiveType() {
-		id = DataTypeManager.LOCAL_ARCHIVE_UNIVERSAL_ID;
+			return ArchiveType.FILE;
-		archiveName = "";
+		}
-	}
 
-	public ArchiveType getArchiveType() {
+		public String getDomainFileID() {
-		return ArchiveType.FILE;
+			return null;
-	}
+		}
 
-	public String getDomainFileID() {
+		public long getLastSyncTime() {
-		return null;
+			return 0;
-	}
+		}
 
-	public long getLastSyncTime() {
+		public String getName() {
-		return 0;
+			return archiveName;
-	}
+		}
 
-	public String getName() {
+		public UniversalID getSourceArchiveID() {
-		return archiveName;
+			return id;
-	}
+		}
 
-	public UniversalID getSourceArchiveID() {
+		public boolean isDirty() {
-		return id;
+			return false;
-	}
+		}
 
-	public boolean isDirty() {
+		public void setDirtyFlag(boolean dirty) {
-		return false;
+		}
-	}
 
-	public void setDirtyFlag(boolean dirty) {
+		public void setLastSyncTime(long time) {
-	}
+		}
 
-	public void setLastSyncTime(long time) {
+		public void setName(String name) {
-	}
+		}
 
-	public void setName(String name) {
 	}
 
 }