GP-3903 Refactor of OverlayAddressSpaces to allow multiple blocks within

the same overlay space

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTrace.java

@@ -161,8 +161,8 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 		// TODO: Should there be a TraceCompilerSpec?
 		this.baseCompilerSpec =
 			baseLanguage.getCompilerSpecByID(baseCompilerSpec.getCompilerSpecID());
-		this.baseAddressFactory =
-			new TraceAddressFactory(this.baseLanguage, this.baseCompilerSpec);
+		this.baseAddressFactory = new TraceAddressFactory(this.baseLanguage, this.baseCompilerSpec,
+			space -> getAddressSet(space));
 
 		try (Transaction tx = this.openTransaction("Create")) {
 			initOptions(DBOpenMode.CREATE);
@@ -182,6 +182,11 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 		programView = createProgramView(0);
 	}
 
+	private AddressSetView getAddressSet(OverlayAddressSpace space) {
+		// use entire space
+		return new AddressSet(space.getMinAddress(), space.getMaxAddress());
+	}
+
 	public DBTrace(DBHandle dbh, DBOpenMode openMode, TaskMonitor monitor, Object consumer)
 			throws CancelledException, VersionException, IOException, LanguageNotFoundException {
 		super(dbh, openMode, monitor, "Untitled", DB_TIME_INTERVAL, DB_BUFFER_SIZE, consumer);
@@ -214,11 +219,11 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 		else {
 			name = traceInfo.getString(NAME, "Unnamed?");
 			baseLanguage = DefaultLanguageService.getLanguageService()
-					.getLanguage(
-						new LanguageID(traceInfo.getString(BASE_LANGUAGE, null)));
+					.getLanguage(new LanguageID(traceInfo.getString(BASE_LANGUAGE, null)));
 			baseCompilerSpec = baseLanguage.getCompilerSpecByID(
 				new CompilerSpecID(traceInfo.getString(BASE_COMPILER, null)));
-			baseAddressFactory = new TraceAddressFactory(baseLanguage, baseCompilerSpec);
+			baseAddressFactory = new TraceAddressFactory(baseLanguage, baseCompilerSpec,
+				space -> getAddressSet(space));
 		}
 	}
 
@@ -247,9 +252,8 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 			return;
 		}
 		if (baseAddressFactory.getAddressSpace(as.getSpaceID()) != as) {
-			throw new IllegalArgumentException(
-				"AddressSpace '" + as + "' is not in this trace (language=" + getBaseLanguage() +
-					")");
+			throw new IllegalArgumentException("AddressSpace '" + as +
+				"' is not in this trace (language=" + getBaseLanguage() + ")");
 		}
 	}
 
@@ -329,9 +333,8 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 	@DependentService
 	protected DBTraceDataTypeManager createDataTypeManager()
 			throws CancelledException, IOException {
-		return createTraceManager("Data Type Manager",
-			(openMode, monitor) -> new DBTraceDataTypeManager(dbh, openMode, rwLock, monitor,
-				this));
+		return createTraceManager("Data Type Manager", (openMode,
+				monitor) -> new DBTraceDataTypeManager(dbh, openMode, rwLock, monitor, this));
 	}
 
 	@DependentService
@@ -366,8 +369,7 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 	}
 
 	@DependentService
-	protected DBTraceObjectManager createObjectManager()
-			throws CancelledException, IOException {
+	protected DBTraceObjectManager createObjectManager() throws CancelledException, IOException {
 		return createTraceManager("Object Manager",
 			(openMode, monitor) -> new DBTraceObjectManager(dbh, openMode, rwLock, monitor,
 				baseLanguage, this));
@@ -376,9 +378,8 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 	@DependentService
 	protected DBTraceOverlaySpaceAdapter createOverlaySpaceAdapter()
 			throws CancelledException, IOException {
-		return createTraceManager("Overlay Space Adapter",
-			(openMode, monitor) -> new DBTraceOverlaySpaceAdapter(dbh, openMode, rwLock, monitor,
-				this));
+		return createTraceManager("Overlay Space Adapter", (openMode,
+				monitor) -> new DBTraceOverlaySpaceAdapter(dbh, openMode, rwLock, monitor, this));
 	}
 
 	@DependentService
@@ -409,9 +410,9 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 	@DependentService
 	protected DBTraceStaticMappingManager createStaticMappingManager(
 			DBTraceOverlaySpaceAdapter overlayAdapter) throws CancelledException, IOException {
-		return createTraceManager("Static Mapping Manager", (openMode,
-				monitor) -> new DBTraceStaticMappingManager(dbh, openMode, rwLock, monitor, this,
-					overlayAdapter));
+		return createTraceManager("Static Mapping Manager",
+			(openMode, monitor) -> new DBTraceStaticMappingManager(dbh, openMode, rwLock, monitor,
+				this, overlayAdapter));
 	}
 
 	@DependentService
@@ -434,9 +435,8 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 	@DependentService
 	protected DBTraceTimeManager createTimeManager(DBTraceThreadManager threadManager)
 			throws IOException, CancelledException {
-		return createTraceManager("Time Manager",
-			(openMode, monitor) -> new DBTraceTimeManager(dbh, openMode, rwLock, monitor, this,
-				threadManager));
+		return createTraceManager("Time Manager", (openMode, monitor) -> new DBTraceTimeManager(dbh,
+			openMode, rwLock, monitor, this, threadManager));
 	}
 
 	@Override
@@ -830,13 +830,13 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 		allViews(v -> v.updateMemoryChangeRegionBlockFlags(region, lifespan));
 	}
 
-	public void updateViewsChangeRegionBlockRange(TraceMemoryRegion region,
-			AddressRange oldRange, AddressRange newRange) {
+	public void updateViewsChangeRegionBlockRange(TraceMemoryRegion region, AddressRange oldRange,
+			AddressRange newRange) {
 		allViews(v -> v.updateMemoryChangeRegionBlockRange(region, oldRange, newRange));
 	}
 
-	public void updateViewsChangeRegionBlockLifespan(TraceMemoryRegion region,
-			Lifespan oldLifespan, Lifespan newLifespan) {
+	public void updateViewsChangeRegionBlockLifespan(TraceMemoryRegion region, Lifespan oldLifespan,
+			Lifespan newLifespan) {
 		allViews(v -> v.updateMemoryChangeRegionBlockLifespan(region, oldLifespan, newLifespan));
 	}
 

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/address/DBTraceOverlaySpaceAdapter.java

@@ -23,7 +23,10 @@ import java.util.*;
 import java.util.concurrent.locks.ReadWriteLock;
 
 import db.*;
-import ghidra.program.model.address.*;
+import ghidra.program.database.ProgramAddressFactory;
+import ghidra.program.database.ProgramOverlayAddressSpace;
+import ghidra.program.model.address.Address;
+import ghidra.program.model.address.AddressSpace;
 import ghidra.trace.database.DBTrace;
 import ghidra.trace.database.DBTraceManager;
 import ghidra.trace.model.Trace.TraceOverlaySpaceChangeType;
@@ -118,8 +121,6 @@ public class DBTraceOverlaySpaceAdapter implements DBTraceManager {
 		static final String NAME_COLUMN_NAME = "Name";
 		static final String BASE_COLUMN_NAME = "Base";
 
-		// NOTE: I don't care to record min/max limit
-
 		@DBAnnotatedColumn(NAME_COLUMN_NAME)
 		static DBObjectColumn NAME_COLUMN;
 		@DBAnnotatedColumn(BASE_COLUMN_NAME)
@@ -148,8 +149,6 @@ public class DBTraceOverlaySpaceAdapter implements DBTraceManager {
 	protected final DBCachedObjectStore<DBTraceOverlaySpaceEntry> overlayStore;
 	protected final DBCachedObjectIndex<String, DBTraceOverlaySpaceEntry> overlaysByName;
 
-	private final Map<Long, AddressSpace> spacesByKey = new HashMap<>();
-
 	public DBTraceOverlaySpaceAdapter(DBHandle dbh, DBOpenMode openMode, ReadWriteLock lock,
 			TaskMonitor monitor, DBTrace trace) throws VersionException, IOException {
 		this.dbh = dbh;
@@ -183,61 +182,92 @@ public class DBTraceOverlaySpaceAdapter implements DBTraceManager {
 	}
 
 	protected void resyncAddressFactory(TraceAddressFactory factory) {
-		// Clean and rename existing overlays, first
+
+		// Perform reconciliation of overlay address spaces while attempting to preserve 
+		// address space instances associated with a given key
+
+		// Put all overlay records into key-based map
+		Map<Long, DBTraceOverlaySpaceEntry> keyToRecordMap = new HashMap<>(overlayStore.asMap());
+
+		// Examine existing overlay spaces for removals and renames
+		List<ProgramOverlayAddressSpace> renameList = new ArrayList<>();
 		for (AddressSpace space : factory.getAllAddressSpaces()) {
-			if (!(space instanceof OverlayAddressSpace)) {
-				continue;
-			}
-			OverlayAddressSpace os = (OverlayAddressSpace) space;
-			DBTraceOverlaySpaceEntry ent = overlayStore.getObjectAt(os.getDatabaseKey());
-			if (ent == null) {
-				spacesByKey.remove(os.getDatabaseKey());
-				factory.removeOverlaySpace(os.getName());
-			}
-			else if (!os.getName().equals(ent.name)) {
-				factory.removeOverlaySpace(os.getName());
-				os.setName(ent.name);
-				try {
-					factory.addOverlayAddressSpace(os);
+			if (space instanceof ProgramOverlayAddressSpace os) {
+				String name = os.getName();
+				DBTraceOverlaySpaceEntry ent = keyToRecordMap.get(os.getKey());
+				if (ent == null || !isCompatibleOverlay(os, ent, factory)) {
+					// Remove overlay if entry does not exist or base space differs
+					factory.removeOverlaySpace(name);
 				}
-				catch (DuplicateNameException e) {
-					throw new AssertionError(); // I just removed it
+				else if (name.equals(ent.name)) {
+					keyToRecordMap.remove(os.getKey());
+					continue; // no change to space
+				}
+				else {
+					// Add space to map of those that need to be renamed
+					renameList.add(os);
+					factory.removeOverlaySpace(name);
 				}
 			}
-			// else it's already in sync
 		}
-		// Add missing overlays
-		for (DBTraceOverlaySpaceEntry ent : overlayStore.asMap().values()) {
-			AddressSpace exists = factory.getAddressSpace(ent.name);
-			if (exists != null) {
-				// it's already in sync and/or its a physical space
-				continue;
+
+		try {
+			// Handle all renamed overlays which had been temporarily removed from factory
+			for (ProgramOverlayAddressSpace existingSpace : renameList) {
+				long key = existingSpace.getKey();
+				DBTraceOverlaySpaceEntry ent = keyToRecordMap.get(key);
+				existingSpace.setName(ent.name);
+				factory.addOverlaySpace(existingSpace); // re-add renamed space
+				keyToRecordMap.remove(key);
 			}
-			AddressSpace baseSpace = factory.getAddressSpace(ent.baseSpace);
-			try {
-				OverlayAddressSpace space = factory.addOverlayAddressSpace(ent.name, true,
-					baseSpace, baseSpace.getMinAddress().getOffset(),
-					baseSpace.getMaxAddress().getOffset());
-				space.setDatabaseKey(ent.getKey());
-				spacesByKey.put(space.getDatabaseKey(), space);
-			}
-			catch (IllegalArgumentException e) {
-				throw new AssertionError(); // Name should be validated already, no?
+
+			// Add any remaing overlay which are missing from factory
+			for (long key : keyToRecordMap.keySet()) {
+				DBTraceOverlaySpaceEntry ent = keyToRecordMap.get(key);
+				String spaceName = ent.name;
+				AddressSpace baseSpace = factory.getAddressSpace(ent.baseSpace);
+				factory.addOverlaySpace(key, spaceName, baseSpace);
 			}
 		}
+		catch (IllegalArgumentException | DuplicateNameException e) {
+			throw new AssertionError("Unexpected error updating overlay address spaces", e);
+		}
+
+		factory.refreshStaleOverlayStatus();
 	}
 
-	protected AddressSpace doCreateOverlaySpace(String name, AddressSpace base) {
+	private boolean isCompatibleOverlay(ProgramOverlayAddressSpace os, DBTraceOverlaySpaceEntry ent,
+			ProgramAddressFactory factory) {
+		AddressSpace baseSpace = factory.getAddressSpace(ent.baseSpace);
+		if (baseSpace == null) {
+			// Error condition should be handled better - language may have dropped original base space
+			throw new RuntimeException("Base space for overlay not found: " + ent.baseSpace);
+		}
+		return baseSpace == os.getOverlayedSpace();
+	}
+
+	protected AddressSpace doCreateOverlaySpace(String name, AddressSpace base)
+			throws DuplicateNameException {
 		TraceAddressFactory factory = trace.getInternalAddressFactory();
-		OverlayAddressSpace space =
-			factory.addOverlayAddressSpace(name, true, base, base.getMinAddress().getOffset(),
-				base.getMaxAddress().getOffset());
-		// Only if it succeeds do we store the record
+
+		if (!factory.isValidOverlayBaseSpace(base)) {
+			throw new IllegalArgumentException(
+				"Invalid address space for overlay: " + base.getName());
+		}
+
+		if (factory.getAddressSpace(name) != null) {
+			throw new DuplicateNameException(
+				"Overlay space '" + name + "' duplicates name of another address space");
+		}
+
 		DBTraceOverlaySpaceEntry ent = overlayStore.create();
+		ProgramOverlayAddressSpace space = factory.addOverlaySpace(ent.getKey(), name, base);
+
+		// Only if it succeeds do we store the record
 		ent.set(space.getName(), base.getName());
 		trace.updateViewsAddSpaceBlock(space);
-		trace.setChanged(new TraceChangeRecord<>(TraceOverlaySpaceChangeType.ADDED, null,
-			trace, null, space));
+		trace.setChanged(
+			new TraceChangeRecord<>(TraceOverlaySpaceChangeType.ADDED, null, trace, null, space));
 		return space;
 	}
 
@@ -261,7 +291,12 @@ public class DBTraceOverlaySpaceAdapter implements DBTraceManager {
 			if (space != null) {
 				return space.getPhysicalSpace() == base ? space : null;
 			}
-			return doCreateOverlaySpace(name, base);
+			try {
+				return doCreateOverlaySpace(name, base);
+			}
+			catch (DuplicateNameException e) {
+				throw new AssertionError(e);
+			}
 		}
 	}
 
@@ -280,6 +315,8 @@ public class DBTraceOverlaySpaceAdapter implements DBTraceManager {
 			trace.updateViewsDeleteSpaceBlock(space);
 			trace.setChanged(new TraceChangeRecord<>(TraceOverlaySpaceChangeType.DELETED, null,
 				trace, space, null));
+			invalidateCache(true);
 		}
 	}
+
 }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/address/TraceAddressFactory.java

@@ -15,46 +15,51 @@
  */
 package ghidra.trace.database.address;
 
-import ghidra.program.database.ProgramAddressFactory;
+import ghidra.program.database.*;
 import ghidra.program.model.address.AddressSpace;
-import ghidra.program.model.address.OverlayAddressSpace;
 import ghidra.program.model.lang.CompilerSpec;
 import ghidra.program.model.lang.Language;
 import ghidra.util.exception.DuplicateNameException;
 
 public class TraceAddressFactory extends ProgramAddressFactory {
 
-	public TraceAddressFactory(Language language, CompilerSpec compilerSpec) {
-		super(language, compilerSpec);
+	public TraceAddressFactory(Language language, CompilerSpec compilerSpec,
+			OverlayRegionSupplier overlayRegionSupplier) {
+		super(language, compilerSpec, overlayRegionSupplier);
 	}
 
 	@Override
-	protected boolean validateOriginalSpace(AddressSpace originalSpace) {
-		return (originalSpace.isMemorySpace() || originalSpace.isRegisterSpace()) &&
-			!originalSpace.isOverlaySpace();
-	}
-
-	@Override
-	protected boolean assignUniqueID(AddressSpace originalSpace) {
-		return super.assignUniqueID(originalSpace) ||
-			originalSpace.getType() == AddressSpace.TYPE_REGISTER;
+	protected boolean isValidOverlayBaseSpace(AddressSpace baseSpace) {
+		return super.isValidOverlayBaseSpace(baseSpace) ||
+			baseSpace.getType() == AddressSpace.TYPE_REGISTER;
 	}
 
 	@Override // for peer access
-	protected OverlayAddressSpace addOverlayAddressSpace(String name, boolean preserveName,
-			AddressSpace originalSpace, long minOffset, long maxOffset) {
-		return super.addOverlayAddressSpace(name, preserveName, originalSpace, minOffset,
-			maxOffset);
+	protected ProgramOverlayAddressSpace addOverlaySpace(long key, String overlayName,
+			AddressSpace baseSpace) throws DuplicateNameException {
+		return super.addOverlaySpace(key, overlayName, baseSpace);
 	}
 
 	@Override // for peer access
-	protected void addOverlayAddressSpace(OverlayAddressSpace ovSpace)
+	protected void addOverlaySpace(ProgramOverlayAddressSpace ovSpace)
 			throws DuplicateNameException {
-		super.addOverlayAddressSpace(ovSpace);
+		super.addOverlaySpace(ovSpace);
 	}
 
 	@Override // for peer access
 	protected void removeOverlaySpace(String name) {
 		super.removeOverlaySpace(name);
 	}
+
+	@Override // for peer access
+	protected void overlaySpaceRenamed(String oldOverlaySpaceName, String newName,
+			boolean refreshStatusIfNeeded) {
+		super.overlaySpaceRenamed(oldOverlaySpaceName, newName, refreshStatusIfNeeded);
+	}
+
+	@Override // for peer access
+	protected void refreshStaleOverlayStatus() {
+		super.refreshStaleOverlayStatus();
+	}
+
 }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramView.java

@@ -30,6 +30,7 @@ import ghidra.framework.model.*;
 import ghidra.framework.options.Options;
 import ghidra.framework.store.LockException;
 import ghidra.program.database.IntRangeMap;
+import ghidra.program.database.ProgramOverlayAddressSpace;
 import ghidra.program.database.map.AddressMap;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
@@ -63,8 +64,7 @@ import ghidra.trace.model.thread.TraceThread;
 import ghidra.trace.util.TraceAddressSpace;
 import ghidra.util.*;
 import ghidra.util.datastruct.WeakValueHashMap;
-import ghidra.util.exception.CancelledException;
-import ghidra.util.exception.DuplicateNameException;
+import ghidra.util.exception.*;
 import ghidra.util.task.TaskMonitor;
 
 /**
@@ -480,7 +480,7 @@ public class DBTraceProgramView implements TraceProgramView {
 			queues.fireEvent(new ProgramChangeRecord(ChangeManager.DOCR_LENGTH_OVERRIDE_CHANGED,
 				instruction.getMinAddress(), instruction.getMinAddress(), null, null, null));
 		}
-		
+
 		private void memoryBytesChanged(TraceAddressSpace space, TraceAddressSnapRange range,
 				byte[] oldIsNull, byte[] bytes) {
 			DomainObjectEventQueues queues = isBytesVisible(space, range);
@@ -1021,6 +1021,25 @@ public class DBTraceProgramView implements TraceProgramView {
 		return changes;
 	}
 
+	@Override
+	public ProgramOverlayAddressSpace createOverlaySpace(String overlaySpaceName,
+			AddressSpace baseSpace) throws IllegalStateException, DuplicateNameException,
+			InvalidNameException, LockException {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public void renameOverlaySpace(String oldOverlaySpaceName, String newName)
+			throws NotFoundException, InvalidNameException, DuplicateNameException, LockException {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public boolean removeOverlaySpace(String overlaySpaceName)
+			throws LockException, NotFoundException {
+		throw new UnsupportedOperationException();
+	}
+
 	@Override
 	public AddressFactory getAddressFactory() {
 		return trace.getBaseAddressFactory();

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisters.java

@@ -25,6 +25,7 @@ import ghidra.framework.model.*;
 import ghidra.framework.options.Options;
 import ghidra.framework.store.LockException;
 import ghidra.program.database.IntRangeMap;
+import ghidra.program.database.ProgramOverlayAddressSpace;
 import ghidra.program.database.map.AddressMap;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.CategoryPath;
@@ -43,8 +44,8 @@ import ghidra.trace.model.data.TraceBasedDataTypeManager;
 import ghidra.trace.model.program.TraceProgramView;
 import ghidra.trace.model.program.TraceProgramViewMemory;
 import ghidra.trace.model.thread.TraceThread;
-import ghidra.util.exception.CancelledException;
-import ghidra.util.exception.DuplicateNameException;
+import ghidra.util.InvalidNameException;
+import ghidra.util.exception.*;
 import ghidra.util.task.TaskMonitor;
 
 public class DBTraceProgramViewRegisters implements TraceProgramView {
@@ -293,6 +294,25 @@ public class DBTraceProgramViewRegisters implements TraceProgramView {
 		return view.getImageBase();
 	}
 
+	@Override
+	public ProgramOverlayAddressSpace createOverlaySpace(String overlaySpaceName,
+			AddressSpace baseSpace) throws IllegalStateException, DuplicateNameException,
+			InvalidNameException, LockException {
+		return view.createOverlaySpace(overlaySpaceName, baseSpace);
+	}
+
+	@Override
+	public void renameOverlaySpace(String overlaySpaceName, String newName)
+			throws NotFoundException, InvalidNameException, DuplicateNameException, LockException {
+		view.renameOverlaySpace(overlaySpaceName, newName);
+	}
+
+	@Override
+	public boolean removeOverlaySpace(String overlaySpaceName)
+			throws LockException, NotFoundException {
+		return view.removeOverlaySpace(overlaySpaceName);
+	}
+
 	@Override
 	public void setImageBase(Address base, boolean commit)
 			throws AddressOverflowException, LockException, IllegalStateException {