GP-4508 Corrected program context mutli-user merge issue for overlays

Ghidra/Features/Base/src/main/java/ghidra/app/merge/listing/RegisterMergeManager.java

@@ -15,6 +15,14 @@
  */
 package ghidra.app.merge.listing;
 
+import java.lang.reflect.InvocationTargetException;
+import java.math.BigInteger;
+import java.util.ArrayList;
+
+import javax.swing.SwingUtilities;
+import javax.swing.event.ChangeEvent;
+import javax.swing.event.ChangeListener;
+
 import ghidra.app.merge.MergeConstants;
 import ghidra.app.merge.ProgramMultiUserMergeManager;
 import ghidra.app.merge.tool.ListingMergePanel;
@@ -27,14 +35,6 @@ import ghidra.program.util.*;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
-import java.lang.reflect.InvocationTargetException;
-import java.math.BigInteger;
-import java.util.ArrayList;
-
-import javax.swing.SwingUtilities;
-import javax.swing.event.ChangeEvent;
-import javax.swing.event.ChangeListener;
-
 /**
  * <code>RegisterMergeManager</code> handles the merge for a single named register.
  */
@@ -167,9 +167,8 @@ class RegisterMergeManager implements ListingMergeConstants {
 		if (conflictSet != null) {
 			return; //This method only needs to be called once.
 		}
-		RegisterConflicts rc =
+		RegisterConflicts rc = new RegisterConflicts(registerName, originalContext, latestContext,
-			new RegisterConflicts(registerName, originalContext, latestContext, myContext,
+			myContext, resultContext);
-				resultContext);
 		Memory resultMem = resultPgm.getMemory();
 		AddressSetView myDiffs =
 			rc.getRegisterDifferences(registerName, originalContext, myContext, mySet, monitor);
@@ -177,48 +176,89 @@ class RegisterMergeManager implements ListingMergeConstants {
 		conflictSet = new AddressSet();
 		rvrs = rc.getConflicts(setToCheck, monitor);
 		if (rvrs.length > 0) {
-			for (int j = 0; j < rvrs.length; j++) {
+			for (AddressRange rvr : rvrs) {
-				conflictSet.add(rvrs[j]);
+				conflictSet.add(rvr);
 			}
 		}
 		autoSet = setToCheck.subtract(conflictSet);
 	}
 
+	private Address getCompatibleAddress(AddressFactory addrFactory, Address otherAddr) {
+		AddressSpace space = otherAddr.getAddressSpace();
+		AddressSpace s = addrFactory.getAddressSpace(space.getName());
+		if (!space.equals(s)) {
+			throw new IllegalArgumentException("Invalid address for target program: " + otherAddr);
+		}
+		return s.getAddress(otherAddr.getOffset());
+	}
+
+	private void setValue(Program p, Register register, Address start, Address end,
+			BigInteger value) {
+
+		// Since all program must have the same set of AddressSpaces, we can assume that addresses
+		// from one program will be valid within another.  However, in order to comply with 
+		// address space instance restrictions we must ensure that addresses used for applying
+		// register context correspond to the target program.
+
+		ProgramContext ctx = p.getProgramContext();
+		AddressFactory addrFactory = p.getAddressFactory();
+
+		try {
+			ctx.setValue(register, getCompatibleAddress(addrFactory, start),
+				getCompatibleAddress(addrFactory, end), value);
+		}
+		catch (ContextChangeException e) {
+			// ignore since processor context-register is not handled by this merge manager
+		}
+	}
+
+	private void removeValue(Program p, Register register, Address start, Address end) {
+
+		// Since all program must have the same set of AddressSpaces, we can assume that addresses
+		// from one program will be valid within another.  However, in order to comply with 
+		// address space instance restrictions we must ensure that addresses used for applying
+		// register context correspond to the target program.
+
+		ProgramContext ctx = p.getProgramContext();
+		AddressFactory addrFactory = p.getAddressFactory();
+
+		try {
+			ctx.remove(getCompatibleAddress(addrFactory, start),
+				getCompatibleAddress(addrFactory, end), register);
+		}
+		catch (ContextChangeException e) {
+			// ignore since processor context-register is not handled by this merge manager
+		}
+	}
+
 	/**
 	 * Merges all the register values for the named register being managed by this merge manager.
 	 * @param monitor the monitor that provides feedback to the user.
-	 * @throws ProgramConflictException
 	 * @throws CancelledException if the user cancels
 	 */
 	public void merge(TaskMonitor monitor) throws CancelledException {
-		monitor.setMessage("Auto-merging " + registerName +
+		monitor.setMessage(
-			" Register Values and determining conflicts.");
+			"Auto-merging " + registerName + " Register Values and determining conflicts.");
 
 		determineConflicts(monitor);
 
 		// Auto merge any program context changes from my program where the
 		// resulting program has the mem addresses but the latest doesn't.
 		AddressRangeIterator arIter = autoSet.getAddressRanges();
-		try {
 		while (arIter.hasNext() && !monitor.isCancelled()) {
 			AddressRange range = arIter.next();
 			Address rangeMin = range.getMinAddress();
 			Address rangeMax = range.getMaxAddress();
-				resultContext.remove(rangeMin, rangeMax, resultReg);
+			removeValue(resultPgm, resultReg, rangeMin, rangeMax);
 			AddressRangeIterator it =
 				myContext.getRegisterValueAddressRanges(resultReg, rangeMin, rangeMax);
 			while (it.hasNext()) {
 				AddressRange valueRange = it.next();
-					BigInteger value =
+				BigInteger value = myContext.getValue(resultReg, valueRange.getMinAddress(), false);
-						myContext.getValue(resultReg, valueRange.getMinAddress(), false);
+				setValue(resultPgm, resultReg, valueRange.getMinAddress(),
-					resultContext.setValue(resultReg, valueRange.getMinAddress(),
 					valueRange.getMaxAddress(), value);
 			}
 		}
-		}
-		catch (ContextChangeException e) {
-			// ignore since processor context-register is not handled by this merge manager
-		}
 
 		int totalConflicts = rvrs.length;
 		if (totalConflicts == 0) {
@@ -294,12 +334,7 @@ class RegisterMergeManager implements ListingMergeConstants {
 	 */
 	private void merge(Address minAddress, Address maxAddress, Register resultRegister,
 			BigInteger myValue) {
-		try {
+		setValue(resultPgm, resultRegister, minAddress, maxAddress, myValue);
-			resultContext.setValue(resultRegister, minAddress, maxAddress, myValue);
-		}
-		catch (ContextChangeException e) {
-			// ignore since this merge manager does not handle the processor context register
-		}
 	}
 
 	/**
@@ -316,7 +351,8 @@ class RegisterMergeManager implements ListingMergeConstants {
 	}
 
 	private void showMergePanel(final Address minAddress, final Address maxAddress,
-			final BigInteger latestValue, final BigInteger myValue, final BigInteger originalValue) {
+			final BigInteger latestValue, final BigInteger myValue,
+			final BigInteger originalValue) {
 		this.min = minAddress;
 		this.max = maxAddress;
 		try {
@@ -339,9 +375,8 @@ class RegisterMergeManager implements ListingMergeConstants {
 			SwingUtilities.invokeAndWait(new Runnable() {
 				@Override
 				public void run() {
-					VerticalChoicesPanel panel =
+					VerticalChoicesPanel panel = getConflictsPanel(minAddress, maxAddress,
-						getConflictsPanel(minAddress, maxAddress, latestValue, myValue,
+						latestValue, myValue, originalValue, changeListener);
-							originalValue, changeListener);
 					listingMergePanel.setBottomComponent(panel);
 				}
 			});
@@ -377,8 +412,7 @@ class RegisterMergeManager implements ListingMergeConstants {
 			conflictPanel.clear();
 		}
 		conflictPanel.setTitle("\"" + registerName + "\" Register Value");
-		String text =
+		String text = "Register: " + ConflictUtility.getEmphasizeString(registerName) +
-			"Register: " + ConflictUtility.getEmphasizeString(registerName) +
 			ConflictUtility.spaces(4) + "Address Range: " +
 			ConflictUtility.getAddressString(minAddress) + " - " +
 			ConflictUtility.getAddressString(maxAddress) +
@@ -431,7 +465,8 @@ class RegisterMergeManager implements ListingMergeConstants {
 		Register conflictResultReg;
 
 		RegisterConflicts(String registerName, ProgramContext originalContext,
-				ProgramContext latestContext, ProgramContext myContext, ProgramContext resultContext) {
+				ProgramContext latestContext, ProgramContext myContext,
+				ProgramContext resultContext) {
 			this.conflictRegisterName = registerName;
 			this.conflictOriginalContext = originalContext;
 			this.conflictLatestContext = latestContext;
@@ -496,13 +531,11 @@ class RegisterMergeManager implements ListingMergeConstants {
 
 			ArrayList<AddressRange> conflicts = new ArrayList<AddressRange>();
 
-			AddressSet tempLatestChanges =
+			AddressSet tempLatestChanges = getRegisterDifferences(conflictRegisterName,
-				getRegisterDifferences(conflictRegisterName, conflictOriginalContext,
+				conflictOriginalContext, conflictLatestContext, addressSet, monitor);
-					conflictLatestContext, addressSet, monitor);
 
-			AddressSet tempMyChanges =
+			AddressSet tempMyChanges = getRegisterDifferences(conflictRegisterName,
-				getRegisterDifferences(conflictRegisterName, conflictOriginalContext,
+				conflictOriginalContext, conflictMyContext, addressSet, monitor);
-					conflictMyContext, addressSet, monitor);
 
 			AddressSet bothChanged = tempMyChanges.intersect(tempLatestChanges);
 
@@ -513,18 +546,15 @@ class RegisterMergeManager implements ListingMergeConstants {
 				Address rangeMin = range.getMinAddress();
 				Address rangeMax = range.getMaxAddress();
 
-				AddressRangeIterator it1 =
+				AddressRangeIterator it1 = conflictLatestContext
-					conflictLatestContext.getRegisterValueAddressRanges(conflictLatestReg,
+						.getRegisterValueAddressRanges(conflictLatestReg, rangeMin, rangeMax);
-						rangeMin, rangeMax);
+				AddressRangeIterator it2 = conflictMyContext
-				AddressRangeIterator it2 =
+						.getRegisterValueAddressRanges(conflictMyReg, rangeMin, rangeMax);
-					conflictMyContext.getRegisterValueAddressRanges(conflictMyReg, rangeMin,
-						rangeMax);
 				AddressRangeIterator it = new CombinedAddressRangeIterator(it1, it2);
 
 				while (it.hasNext()) {
 					AddressRange addrRange = it.next();
-					BigInteger lastestValue =
+					BigInteger lastestValue = conflictLatestContext.getValue(conflictLatestReg,
-						conflictLatestContext.getValue(conflictLatestReg,
 						addrRange.getMinAddress(), false);
 					BigInteger myValue =
 						conflictMyContext.getValue(conflictMyReg, addrRange.getMinAddress(), false);

Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/listing/ProgramContextMergeManagerTest.java

@@ -53,8 +53,10 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 				ProgramContext pc = program.getProgramContext();
 				Register regDR0 = pc.getRegister(regNameDR0);
 
-				// Initially Direction was 0x1e240
 				setRegValue(pc, addr("1002085"), addr("1002100"), regDR0, 0x5L);
+
+				setRegValue(pc, addr("TextOverlay:1001700"), addr("TextOverlay:1001780"), regDR0,
+					0x5L);
 			}
 
 			@Override
@@ -63,6 +65,9 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 				Register regDR0 = pc.getRegister(regNameDR0);
 
 				setRegValue(pc, addr("1002000"), addr("1002074"), regDR0, 0x22L);
+
+				setRegValue(pc, addr("TextOverlay:1001630"), addr("TextOverlay:1001680"), regDR0,
+					0x22L);
 			}
 		});
 
@@ -89,6 +94,34 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 		for (Address a = addr("1002101"); a.compareTo(addr("1002150")) <= 0; a = a.add(0x1L)) {
 			assertUndefinedRegValue("DR0", a);
 		}
+
+		// Check Overlay
+
+		// Neither set it
+		for (Address a = addr("TextOverlay:1001629"); a
+				.compareTo(addr("TextOverlay:1001629")) <= 0; a = a.add(0x1L)) {
+			assertUndefinedRegValue("DR0", a);
+		}
+		// From MY
+		for (Address a = addr("TextOverlay:1001630"); a
+				.compareTo(addr("TextOverlay:1001680")) <= 0; a = a.add(0x1L)) {
+			assertRegValue("DR0", a, 0x22L);
+		}
+		// Neither set it
+		for (Address a = addr("TextOverlay:1001681"); a
+				.compareTo(addr("TextOverlay:10016ff")) <= 0; a = a.add(0x1L)) {
+			assertUndefinedRegValue("DR0", a);
+		}
+		// From LATEST
+		for (Address a = addr("TextOverlay:1001700"); a
+				.compareTo(addr("TextOverlay:1001780")) <= 0; a = a.add(0x1L)) {
+			assertRegValue("DR0", a, 0x5L);
+		}
+		// Neither set it
+		for (Address a = addr("TextOverlay:1001781"); a
+				.compareTo(addr("TextOverlay:100182f")) <= 0; a = a.add(0x1L)) {
+			assertUndefinedRegValue("DR0", a);
+		}
 	}
 
 	@Test
@@ -100,7 +133,6 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 				ProgramContext pc = program.getProgramContext();
 				Register regDR0 = pc.getRegister(regNameDR0);
 
-				// Initially Direction was 0x1e240
 				setRegValue(pc, addr("10022d4"), addr("10022d9"), regDR0, 0x66L);
 			}
 
@@ -807,6 +839,9 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 
 				setRegValue(pc, addr("10022d4"), addr("10022e5"), reg1, 0x66L);
 				setRegValue(pc, addr("10022ee"), addr("10022fc"), reg1, 0x44L);
+
+				setRegValue(pc, addr("TextOverlay:1001700"), addr("TextOverlay:1001700"), reg1,
+					0x44L);
 			}
 
 			@Override
@@ -816,6 +851,9 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 
 				setRegValue(pc, addr("10022d4"), addr("10022e5"), reg1, 0x7L);
 				setRegValue(pc, addr("10022ee"), addr("10022fc"), reg1, 0x5L);
+
+				setRegValue(pc, addr("TextOverlay:1001700"), addr("TextOverlay:1001700"), reg1,
+					0x5L);
 			}
 		});
 
@@ -824,6 +862,8 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 		chooseRadioButton(CHECKED_OUT_BUTTON_NAME);
 		checkDisplayValues(Long.valueOf(0x44L), Long.valueOf(0x5L), (Long) null);
 		chooseRadioButton(CHECKED_OUT_BUTTON_NAME);
+		checkDisplayValues(Long.valueOf(0x44L), Long.valueOf(0x5L), (Long) null);
+		chooseRadioButton(CHECKED_OUT_BUTTON_NAME);
 		waitForMergeCompletion();
 
 		for (Address a = addr("10022d4"); a.compareTo(addr("10022e5")) <= 0; a = a.add(0x1L)) {
@@ -832,6 +872,7 @@ public class ProgramContextMergeManagerTest extends AbstractListingMergeManagerT
 		for (Address a = addr("10022ee"); a.compareTo(addr("10022fc")) <= 0; a = a.add(0x1L)) {
 			assertRegValue("DR0", a, 0x5L);
 		}
+		assertRegValue("DR0", addr("TextOverlay:1001700"), 0x5L);
 	}
 
 	@Test

Ghidra/Features/Base/src/test/java/ghidra/program/database/MergeProgramGenerator_DiffTestPrograms.java

@@ -202,6 +202,7 @@ class MergeProgramGenerator_DiffTestPrograms implements MergeProgramGenerator {
 		builder.createMemory(".data", "0x1008000", 0x600);
 		builder.createMemory(".datau", "0x1008600", 0x1344);
 		builder.createMemory(".rsrc", "0x100a000", 0x5400);
+		builder.createOverlayMemory("TextOverlay", "0x01001630", 0x200);
 
 		// for FunctionMergeManager2Test
 		//

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/util/AbstractStoredProgramContext.java

@@ -22,7 +22,6 @@ import ghidra.program.database.register.InMemoryRangeMapAdapter;
 import ghidra.program.model.address.*;
 import ghidra.program.model.lang.*;
 import ghidra.program.model.listing.ContextChangeException;
-import ghidra.util.exception.AssertException;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
@@ -249,7 +248,8 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	public void remove(Address start, Address end, Register register)
 			throws ContextChangeException {
 		if (start.getAddressSpace() != end.getAddressSpace()) {
-			throw new AssertException("start and end address must be in the same address space");
+			throw new IllegalArgumentException(
+				"start and end address must refer to the same address space instance");
 		}
 		RegisterValueStore values = registerValueMap.get(register.getBaseRegister());
 		if (values != null) {
@@ -260,7 +260,7 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 
 //	public void removeDefault(Address start, Address end, Register register) {
 //		if (start.getAddressSpace() != end.getAddressSpace()) {
-//			throw new AssertException("start and end address must be in the same address space");
+//			throw new IllegalArgumentException("start and end address must refer to the same address space instance");
 //		}
 //		invalidateCache();
 //		RegisterValueStore values = defaultRegisterValueMap.get(register.getBaseRegister());
@@ -273,7 +273,8 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	public void setValue(Register register, Address start, Address end, BigInteger value)
 			throws ContextChangeException {
 		if (start.getAddressSpace() != end.getAddressSpace()) {
-			throw new AssertException("start and end address must be in the same address space");
+			throw new IllegalArgumentException(
+				"start and end address must refer to the same address space instance");
 		}
 		if (value == null) {
 			remove(start, end, register);
@@ -285,7 +286,8 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	@Override
 	public void setDefaultValue(RegisterValue registerValue, Address start, Address end) {
 		if (start.getAddressSpace() != end.getAddressSpace()) {
-			throw new AssertException("start and end address must be in the same address space");
+			throw new IllegalArgumentException(
+				"start and end address must refer to the same address space instance");
 		}
 		Register baseRegister = registerValue.getRegister().getBaseRegister();
 		RegisterValueStore store = defaultRegisterValueMap.get(baseRegister);