GP-3921 Improved Diff data instance settings support

Ghidra/Features/Base/src/main/java/ghidra/program/util/ProgramDiff.java

@@ -17,8 +17,12 @@ package ghidra.program.util;
 
 import java.util.*;
 
+import javax.help.UnsupportedOperationException;
+
+import ghidra.program.database.data.ProgramBasedDataTypeManagerDB;
 import ghidra.program.database.properties.UnsupportedMapDB;
 import ghidra.program.model.address.*;
+import ghidra.program.model.data.ProgramBasedDataTypeManager;
 import ghidra.program.model.lang.Register;
 import ghidra.program.model.lang.RegisterValue;
 import ghidra.program.model.listing.*;
@@ -2966,6 +2970,30 @@ public class ProgramDiff {
 				return false;
 			}
 
+			// assume only top-level data code units are compared
+			// we should not be a DataComponent (i.e., no parent)
+			if (d1.getParent() != null || d2.getParent() != null) {
+				throw new UnsupportedOperationException("Expecting top-level Data only");
+			}
+
+			// Only top-level Data instance Settings are supported 
+
+			String[] settingNames1 = d1.getNames();
+			Arrays.sort(settingNames1);
+			String[] settingNames2 = d2.getNames();
+			Arrays.sort(settingNames2);
+			if (!Arrays.equals(settingNames1, settingNames2)) {
+				return false;
+			}
+
+			for (int i = 0; i < settingNames1.length; i++) {
+				Object v1 = d1.getValue(settingNames1[i]);
+				Object v2 = d2.getValue(settingNames2[i]);
+				if (!Objects.equals(v1, v2)) {
+					return false;
+				}
+			}
+
 			return true;
 		}
 

Ghidra/Features/Base/src/main/java/ghidra/program/util/ProgramDiffDetails.java

@@ -22,6 +22,8 @@ import java.util.*;
 import javax.swing.text.*;
 
 import generic.theme.GColor;
+import ghidra.docking.settings.EnumSettingsDefinition;
+import ghidra.docking.settings.SettingsDefinition;
 import ghidra.program.database.properties.UnsupportedMapDB;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
@@ -972,8 +974,8 @@ public class ProgramDiffDetails {
 		Address max = cu.getMaxAddress();
 		String addrRangeStr = min + ((min.equals(max)) ? "" : " - " + max);
 		String cuRep;
-		if (cu instanceof Data) {
+		if (cu instanceof Data data) {
-			cuRep = ((Data) cu).getDataType().getPathName();
+			cuRep = data.getDataType().getPathName();
 		}
 		else if (cu instanceof Instruction) {
 			Instruction inst = (Instruction) cu;
@@ -1018,6 +1020,35 @@ public class ProgramDiffDetails {
 			cuRep = cu.toString();
 		}
 		buf.append(indent + addrRangeStr + "    " + cuRep + newLine);
+
+		if (cu instanceof Data data) {
+			// NOTE: Diff operates on the outmost code-unit only and not data components
+			String[] settingNames = data.getNames();
+			if (settingNames.length != 0) {
+				Map<String, SettingsDefinition> defMap = new HashMap<>();
+				for (SettingsDefinition settingsDef : data.getDataType().getSettingsDefinitions()) {
+					defMap.put(settingsDef.getStorageKey(), settingsDef);
+				}
+				buf.append(indent + indent + "Data Settings: ");
+				int count = 0;
+				Arrays.sort(settingNames);
+				for (String settingName : settingNames) {
+					Object value = data.getValue(settingName);
+					SettingsDefinition def = defMap.get(settingName);
+					if (def != null) {
+						settingName = def.getName();
+					}
+					if (value instanceof Long && def instanceof EnumSettingsDefinition eDef) {
+						value = eDef.getValueString(data);
+					}
+					if (count++ != 0) {
+						buf.append(", ");
+					}
+					buf.append(settingName + "=" + value);
+				}
+				buf.append(newLine);
+			}
+		}
 		return min;
 	}
 
@@ -2364,6 +2395,30 @@ public class ProgramDiffDetails {
 			return false;
 		}
 
+		// assume only top-level data code units are compared
+		// we should not be a DataComponent (i.e., no parent)
+		if (d1.getParent() != null || d2.getParent() != null) {
+			throw new UnsupportedOperationException("Expecting top-level Data only");
+		}
+
+		// Only top-level Data instance Settings are supported 
+
+		String[] settingNames1 = d1.getNames();
+		Arrays.sort(settingNames1);
+		String[] settingNames2 = d2.getNames();
+		Arrays.sort(settingNames2);
+		if (!Arrays.equals(settingNames1, settingNames2)) {
+			return false;
+		}
+
+		for (int i = 0; i < settingNames1.length; i++) {
+			Object v1 = d1.getValue(settingNames1[i]);
+			Object v2 = d2.getValue(settingNames2[i]);
+			if (!Objects.equals(v1, v2)) {
+				return false;
+			}
+		}
+
 		return true;
 	}
 

Ghidra/Features/Base/src/test.slow/java/ghidra/program/util/ProgramDiff1Test.java

@@ -24,10 +24,14 @@ package ghidra.program.util;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
+import java.util.Arrays;
+import java.util.Objects;
+
 import org.junit.*;
 
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
+import ghidra.program.model.data.DataUtilities.ClearDataMode;
 import ghidra.program.model.lang.Register;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.symbol.Namespace;
@@ -315,6 +319,51 @@ public class ProgramDiff1Test extends AbstractGhidraHeadedIntegrationTest {
 		assertEquals(addrSet, diffAs);
 	}
 
+	/**
+	 * Test that ProgramDiff can determine the code unit differences between
+	 * Program1 and Program2 where Data Settings differ.
+	 */
+	@Test
+	public void testGetCodeUnitSettingsDifferences() throws Exception {
+
+		p1.withTransaction("Apply p1 Data", () -> {
+
+			// NOTE: unknown settings will be dropped
+
+			Data d1 = DataUtilities.createData(p1, addr(p1, 0x1008014), StringDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+			CharsetSettingsDefinition.CHARSET.setChoice(d1, 2); // differs from p2
+
+			d1 = DataUtilities.createData(p1, addr(p1, 0x1008028), StringDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+			CharsetSettingsDefinition.CHARSET.setChoice(d1, 3); // same as p2
+
+		});
+
+		p2.withTransaction("Apply p2 Data", () -> {
+
+			DataUtilities.createData(p2, addr(p2, 0x1008014), StringDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+
+			Data d2 = DataUtilities.createData(p2, addr(p2, 0x1008028), StringDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+			CharsetSettingsDefinition.CHARSET.setChoice(d2, 3); // same as p1
+
+		});
+
+		AddressSet as = new AddressSet();
+		as.addRange(addr(p1, 0x1008000), addr(p1, 0x10085ff));
+		programDiff = new ProgramDiff(p1, p2, as);
+		programDiff.setFilter(new ProgramDiffFilter(ProgramDiffFilter.CODE_UNIT_DIFFS));
+
+		AddressSetView diffAs = programDiff.getDifferences(programDiff.getFilter(), null);
+
+		AddressSet addrSet = new AddressSet();
+		addrSet.addRange(addr(p1, 0x1008014), addr(p1, 0x1008015));
+
+		assertEquals(addrSet, diffAs);
+	}
+
 	@Test
 	public void testGetCommentDifference1() throws Exception {
 		// 0x1002040: p1 has Plate, Pre, EOL, Post, & Repeatable comment.

Ghidra/Features/Base/src/test.slow/java/ghidra/program/util/ProgramMerge3Test.java

@@ -19,8 +19,10 @@ import static org.junit.Assert.*;
 
 import org.junit.*;
 
+import ghidra.docking.settings.FormatSettingsDefinition;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
+import ghidra.program.model.data.DataUtilities.ClearDataMode;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.mem.MemoryAccessException;
 import ghidra.program.model.symbol.*;
@@ -493,6 +495,48 @@ public class ProgramMerge3Test extends AbstractGhidraHeadedIntegrationTest {
 		assertEquals(new AddressSet(), programMerge.getFilteredDifferences());
 	}
 
+	@Test
+	public void testApplyDataDifference15() throws Exception {
+
+		p1.withTransaction("Apply p1 Data", () -> {
+
+			Data d1 = DataUtilities.createData(p1, addr(p1, 0x1008014), StringDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+			CharsetSettingsDefinition.CHARSET.setChoice(d1, 2);
+
+			d1 = DataUtilities.createData(p1, addr(p1, 0x1008028), StringDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+			CharsetSettingsDefinition.CHARSET.setChoice(d1, 2);
+
+		});
+
+		p2.withTransaction("Apply p2 Data", () -> {
+
+			Data d2 = DataUtilities.createData(p2, addr(p2, 0x1008014), ByteDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+			FormatSettingsDefinition.DEF.setChoice(d2, FormatSettingsDefinition.DECIMAL);
+
+			d2 = DataUtilities.createData(p2, addr(p2, 0x1008028), StringDataType.dataType, -1,
+				ClearDataMode.CLEAR_ALL_UNDEFINED_CONFLICT_DATA);
+			CharsetSettingsDefinition.CHARSET.setChoice(d2, 3);
+
+		});
+
+		// NOTE: unknown settings will be dropped during merge
+
+		programMerge = new ProgramMergeManager(p1, p2, null, TaskMonitor.DUMMY);
+
+		AddressSet addrSet = new AddressSet();
+		addrSet.addRange(addr(p1, 0x1008014), addr(p1, 0x1008015));
+		addrSet.add(addr(p1, 0x1008028));
+
+		assertEquals(addrSet, programMerge.getFilteredDifferences());
+
+		mergeCodeUnitDifference(addrSet);
+
+		assertEquals(new AddressSet(), programMerge.getFilteredDifferences());
+	}
+
 	@Test
 	public void testApplyDataDifference2() throws Exception {
 
@@ -1134,8 +1178,7 @@ public class ProgramMerge3Test extends AbstractGhidraHeadedIntegrationTest {
 		symtab2.createLabel(addr(0x1002973), "TWO", SourceType.USER_DEFINED);
 		AddressSet as;
 		AddressSet limitedAddrSet = new AddressSet(addr(0x1002950), addr(0x100299b));
-		programMerge =
+		programMerge = new ProgramMergeManager(p1, p2, limitedAddrSet, TaskMonitor.DUMMY);
-			new ProgramMergeManager(p1, p2, limitedAddrSet, TaskMonitor.DUMMY);
 
 		as = new AddressSet();
 		as.addRange(addr(0x100295d), addr(0x100295d));
@@ -1353,7 +1396,7 @@ public class ProgramMerge3Test extends AbstractGhidraHeadedIntegrationTest {
 		assertEquals(unMergableByteAddresses, programMerge.getFilteredDifferences());
 	}
 
-	private Address addr(Program program, int offset) {
+	private Address addr(Program program, long offset) {
 		return program.getAddressFactory().getDefaultAddressSpace().getAddress(offset);
 	}
 
@@ -1373,10 +1416,15 @@ public class ProgramMerge3Test extends AbstractGhidraHeadedIntegrationTest {
 		return pathName;
 	}
 
-	private void mergeCodeUnitDifference(int diffStart, int diffEnd)
+	private void mergeCodeUnitDifference(long diffStart, long diffEnd)
 			throws CancelledException, MemoryAccessException {
 		AddressSet addrSet = new AddressSet();
 		addrSet.addRange(addr(p1, diffStart), addr(p1, diffEnd));
+		mergeCodeUnitDifference(addrSet);
+	}
+
+	private void mergeCodeUnitDifference(AddressSet addrSet)
+			throws CancelledException, MemoryAccessException {
 
 		programMerge.setDiffFilter(new ProgramDiffFilter(ProgramDiffFilter.CODE_UNIT_DIFFS));
 		programMerge.setMergeFilter(

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

@@ -310,6 +310,7 @@ public abstract class ProgramBasedDataTypeManagerDB extends DataTypeManagerDB
 		if (instanceSettingsAdapter == null) {
 			throw new UnsupportedOperationException();
 		}
+		lock.acquire();
 		try {
 			Address dataAddr = getDataSettingsAddress(data);
 			return instanceSettingsAdapter
@@ -318,6 +319,9 @@ public abstract class ProgramBasedDataTypeManagerDB extends DataTypeManagerDB
 		catch (IOException e) {
 			errHandler.dbError(e);
 		}
+		finally {
+			lock.release();
+		}
 		return true;
 	}
 
@@ -427,14 +431,15 @@ public abstract class ProgramBasedDataTypeManagerDB extends DataTypeManagerDB
 		}
 	}
 
-	private static Address getDataSettingsAddress(Data data) {
+	public static Address getDataSettingsAddress(Data data) {
-		Data parent = data.getParent();
+		Address addr = data.getAddress();
-		if (parent != null) {
+		for (Data parent = data.getParent(); parent != null; parent = parent.getParent()) {
 			DataType dataType = parent.getDataType();
-			if (dataType instanceof Array) {
+			if (!(dataType instanceof Array a)) {
-				return getDataSettingsAddress(parent);
+				break;
-			}
+			}
-		}
+			addr = parent.getAddress();
-		return data.getAddress();
+		}
+		return addr;
 	}
 }