GP-1711: Fix CodeUnitInsertionException during emulation.

2026-05-24 03:09:36 +08:00 · 2023-01-31 16:31:29 -05:00
parent d8a144eeee
commit b7c8b8bc72
17 changed files with 559 additions and 141 deletions
@@ -20,12 +20,14 @@ import java.util.Map.Entry;

 import ghidra.program.model.address.*;
 import ghidra.program.model.util.CodeUnitInsertionException;
-import ghidra.trace.database.*;
+import ghidra.trace.database.DBTraceCacheForContainingQueries;
 import ghidra.trace.database.DBTraceCacheForContainingQueries.GetKey;
+import ghidra.trace.database.DBTraceCacheForSequenceQueries;
 import ghidra.trace.database.context.DBTraceRegisterContextSpace;
 import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapAddressSetView;
 import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapSpace;
 import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapTree.TraceAddressSnapRangeQuery;
+import ghidra.trace.database.memory.DBTraceMemorySpace;
 import ghidra.trace.model.*;
 import ghidra.trace.model.Trace.TraceCodeChangeType;
 import ghidra.trace.model.listing.TraceBaseDefinedUnitsView;
@@ -359,20 +361,28 @@ public abstract class AbstractBaseDBTraceDefinedUnitsView<T extends AbstractDBTr
 	 * Select a sub-lifespan from that given so that the box does not overlap an existing unit
 	 * 
 	 * <p>
-	 * The selected lifespan will have the same start snap at that given. The box is the bounding
+	 * The selected lifespan will have the same start snap as that given. The box is the bounding
 	 * box of a unit the client is trying to create.
 	 * 
 	 * @param span the lifespan of the box
+	 * @param extending if applicable, the unit whose lifespan is being extended
 	 * @param range the address range of the box
 	 * @return the selected sub-lifespan
 	 * @throws CodeUnitInsertionException if the start snap is contained in an existing unit
 	 */
-	protected Lifespan truncateSoonestDefined(Lifespan span, AddressRange range)
-			throws CodeUnitInsertionException {
+	protected Lifespan truncateSoonestDefined(Lifespan span, AbstractDBTraceCodeUnit<?> extending,
+			AddressRange range) throws CodeUnitInsertionException {
+		final Lifespan toScan;
+		if (extending == null) {
+			toScan = span;
+		}
+		else {
+			assert span.lmax() > extending.getEndSnap();
+			toScan = span.withMin(extending.getEndSnap() + 1);
+		}
 		T truncateBy =
-			mapSpace.reduce(TraceAddressSnapRangeQuery.intersecting(range, span)
-					.starting(
-						Rectangle2DDirection.BOTTOMMOST))
+			mapSpace.reduce(TraceAddressSnapRangeQuery.intersecting(range, toScan)
+					.starting(Rectangle2DDirection.BOTTOMMOST))
 					.firstValue();
 		if (truncateBy == null) {
 			return span;
@@ -380,7 +390,25 @@ public abstract class AbstractBaseDBTraceDefinedUnitsView<T extends AbstractDBTr
 		if (truncateBy.getStartSnap() <= span.lmin()) {
 			throw new CodeUnitInsertionException("Code units cannot overlap");
 		}
-		return Lifespan.span(span.lmin(), truncateBy.getStartSnap() - 1);
+		return span.withMax(truncateBy.getStartSnap() - 1);
+	}
+
+	protected long computeTruncatedMax(Lifespan lifespan, T extending, AddressRange range)
+			throws CodeUnitInsertionException {
+		// First, truncate lifespan to the next code unit when upper bound is max
+		if (!lifespan.maxIsFinite()) {
+			lifespan = space.instructions.truncateSoonestDefined(lifespan, extending, range);
+			lifespan = space.definedData.truncateSoonestDefined(lifespan, extending, range);
+		}
+		// Second, truncate lifespan to the next change of bytes in the range
+		DBTraceMemorySpace memSpace =
+			space.trace.getMemoryManager().getMemorySpace(space.space, true);
+		Lifespan fullSpan = extending == null ? lifespan : lifespan.bound(extending.getLifespan());
+		long endSnap = memSpace.getFirstChange(fullSpan, range);
+		if (endSnap == Long.MIN_VALUE) {
+			return lifespan.lmax();
+		}
+		return endSnap - 1;
 	}

 	/**
@@ -20,7 +20,6 @@ import java.nio.ByteBuffer;
 import db.DBRecord;
 import ghidra.program.model.address.Address;
 import ghidra.trace.database.DBTrace;
-import ghidra.trace.database.DBTraceUtils;
 import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapTree;
 import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapTree.AbstractDBTraceAddressSnapRangePropertyMapData;
 import ghidra.trace.database.memory.DBTraceMemorySpace;
@@ -123,8 +122,13 @@ public abstract class AbstractDBTraceCodeUnit<T extends AbstractDBTraceCodeUnit<
 			}
 			// Copy from the cache
 			int toCopyFromCache =
-				Math.max(0, Math.min(byteCache.position() - addressOffset, buffer.remaining()));
-			buffer.put(byteCache.array(), addressOffset, toCopyFromCache);
+				Math.min(byteCache.position() - addressOffset, buffer.remaining());
+			if (toCopyFromCache > 0) {
+				buffer.put(byteCache.array(), addressOffset, toCopyFromCache);
+			}
+			else {
+				toCopyFromCache = 0;
+			}
 			if (byteCache.position() >= end) {
 				return toCopyFromCache;
 			}
@@ -20,7 +20,8 @@ import java.io.IOException;
 import db.DBRecord;
 import ghidra.docking.settings.Settings;
 import ghidra.program.model.address.AddressSpace;
-import ghidra.program.model.data.*;
+import ghidra.program.model.data.DataType;
+import ghidra.program.model.data.TypeDef;
 import ghidra.program.model.lang.Language;
 import ghidra.trace.database.DBTrace;
 import ghidra.trace.database.DBTraceUtils;
@@ -131,21 +131,8 @@ public class DBTraceDefinedDataView extends AbstractBaseDBTraceDefinedUnitsView<
 			Address endAddress = address.addNoWrap(length - 1);
 			AddressRangeImpl createdRange = new AddressRangeImpl(address, endAddress);

-			// First, truncate lifespan to the next code unit when upper bound is max
-			if (!lifespan.maxIsFinite()) {
-				lifespan = space.instructions.truncateSoonestDefined(lifespan, createdRange);
-				lifespan = space.definedData.truncateSoonestDefined(lifespan, createdRange);
-			}
-
-			// Second, extend to the next change of bytes in the range within lifespan
-			// Then, check that against existing code units.
-			long endSnap = memSpace.getFirstChange(lifespan, createdRange);
-			if (endSnap == Long.MIN_VALUE) {
-				endSnap = lifespan.lmax();
-			}
-			else {
-				endSnap--;
-			}
+			// Truncate, then check that against existing code units.
+			long endSnap = computeTruncatedMax(lifespan, null, createdRange);
 			TraceAddressSnapRange tasr = new ImmutableTraceAddressSnapRange(createdRange,
 				Lifespan.span(startSnap, endSnap));
 			if (!space.undefinedData.coversRange(tasr)) {
@@ -158,13 +145,13 @@ public class DBTraceDefinedDataView extends AbstractBaseDBTraceDefinedUnitsView<
 			}

 			long dataTypeID = space.dataTypeManager.getResolvedID(dataType);
-			DBTraceData created = space.dataMapSpace.put(tasr, null);
+			DBTraceData created = mapSpace.put(tasr, null);
 			// TODO: data units with a guest platform
 			created.set(space.trace.getPlatformManager().getHostPlatform(), dataTypeID);
 			// TODO: Explicitly remove undefined from cache, or let weak refs take care of it?

-			cacheForContaining.notifyNewEntry(lifespan, createdRange, created);
-			cacheForSequence.notifyNewEntry(lifespan, createdRange, created);
+			cacheForContaining.notifyNewEntry(tasr.getLifespan(), createdRange, created);
+			cacheForSequence.notifyNewEntry(tasr.getLifespan(), createdRange, created);
 			space.undefinedData.invalidateCache();

 			if (dataType instanceof Composite || dataType instanceof Array ||
@@ -23,11 +23,11 @@ import ghidra.program.model.address.*;
 import ghidra.program.model.lang.*;
 import ghidra.program.model.lang.InstructionError.InstructionErrorType;
 import ghidra.program.model.listing.*;
+import ghidra.program.model.mem.MemoryAccessException;
 import ghidra.program.model.util.CodeUnitInsertionException;
 import ghidra.trace.database.context.DBTraceRegisterContextManager;
 import ghidra.trace.database.context.DBTraceRegisterContextSpace;
 import ghidra.trace.database.guest.InternalTracePlatform;
-import ghidra.trace.database.memory.DBTraceMemorySpace;
 import ghidra.trace.model.*;
 import ghidra.trace.model.Trace.TraceCodeChangeType;
 import ghidra.trace.model.guest.TracePlatform;
@@ -59,6 +59,7 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 		private final Address errorAddress;
 		private final InstructionError conflict;
 		private final CodeUnit conflictCodeUnit;
+		private final boolean overwrite;

 		protected int count = 0;

@@ -75,10 +76,11 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 		 * @param errorAddress the address of the first error in the block, if any
 		 * @param conflict a description of the error, if any
 		 * @param conflictCodeUnit if a conflict, the code unit that already exists
+		 * @param overwrite true to overwrite existing defined units
 		 */
 		private InstructionBlockAdder(Set<Address> skipDelaySlots, Lifespan lifespan,
 				InternalTracePlatform platform, InstructionBlock block, Address errorAddress,
-				InstructionError conflict, CodeUnit conflictCodeUnit) {
+				InstructionError conflict, CodeUnit conflictCodeUnit, boolean overwrite) {
 			this.skipDelaySlots = skipDelaySlots;
 			this.lifespan = lifespan;
 			this.platform = platform;
@@ -86,20 +88,113 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 			this.errorAddress = errorAddress;
 			this.conflict = conflict;
 			this.conflictCodeUnit = conflictCodeUnit;
+			this.overwrite = overwrite;
+		}
+
+		protected void truncateOrDelete(TraceInstruction exists) {
+			if (exists.getStartSnap() < lifespan.lmin()) {
+				exists.setEndSnap(lifespan.lmin());
+			}
+			else {
+				exists.delete();
+			}
+		}
+
+		protected boolean isSuitable(Instruction candidate, Instruction protoInstr) {
+			try {
+				return candidate.getPrototype().equals(protoInstr.getPrototype()) &&
+					Arrays.equals(candidate.getBytes(), protoInstr.getBytes()) &&
+					candidate.isFallThroughOverridden() == protoInstr.isFallThroughOverridden() &&
+					Objects.equals(candidate.getFallThrough(), protoInstr.getFallThrough()) &&
+					candidate.getFlowOverride() == protoInstr.getFlowOverride();
+			}
+			catch (MemoryAccessException e) {
+				throw new AssertionError(e);
+			}
+		}
+
+		protected Instruction doAdjustExisting(Address address, Instruction protoInstr)
+				throws AddressOverflowException, CancelledException, CodeUnitInsertionException {
+			DBTraceInstruction exists = getAt(lifespan.lmin(), address);
+			if (exists == null || exists.getLength() != protoInstr.getLength()) {
+				AddressRange range =
+					new AddressRangeImpl(protoInstr.getAddress(), protoInstr.getLength());
+				space.definedUnits.clear(lifespan, range, false, TaskMonitor.DUMMY);
+				return null;
+			}
+			if (!isSuitable(exists, protoInstr)) {
+				truncateOrDelete(exists);
+				return null;
+			}
+			long curEnd = exists.getEndSnap();
+			if (curEnd < lifespan.lmax()) {
+				AddressRange range =
+					new AddressRangeImpl(protoInstr.getAddress(), protoInstr.getLength());
+				long endSnap = computeTruncatedMax(lifespan, exists, range);
+				exists.setEndSnap(endSnap);
+			}
+			return exists;
+		}
+
+		/**
+		 * Check the preceding unit and see if it can be extended to "create" the desired one
+		 * 
+		 * <p>
+		 * For overwrite, the caller should first use
+		 * {@link #doAdjustExisting(Address, InstructionPrototype, Instruction)}.
+		 * 
+		 * @param address the starting address of the instruction
+		 * @param protoInstr the prototype instruction
+		 * @return the extended instruction, if it was a match
+		 * @throws AddressOverflowException should never
+		 * @throws CodeUnitInsertionException if there's no room to extend the preceding unit
+		 */
+		protected Instruction doExtendPreceding(Address address, Instruction protoInstr)
+				throws AddressOverflowException, CodeUnitInsertionException {
+			if (!lifespan.minIsFinite()) {
+				return null;
+			}
+			DBTraceInstruction prec = getAt(lifespan.lmin() - 1, address);
+			if (prec == null || prec.getLength() != protoInstr.getLength()) {
+				return null;
+			}
+			if (!isSuitable(prec, protoInstr)) {
+				return null;
+			}
+			AddressRange range =
+				new AddressRangeImpl(protoInstr.getAddress(), protoInstr.getLength());
+			long endSnap = computeTruncatedMax(lifespan, prec, range);
+			if (!lifespan.contains(endSnap)) {
+				// We can't extend it far enough
+				return null;
+			}
+			prec.setEndSnap(endSnap);
+			return prec;
 		}

 		/**
 		 * Store the given instruction in the database
 		 * 
 		 * @param address the address of the instruction
-		 * @param prototype the instruction prototype
 		 * @param protoInstr the parsed (usually pseudo) instruction
 		 * @return the created instruction
 		 */
-		protected Instruction doCreateInstruction(Address address,
-				InstructionPrototype prototype, Instruction protoInstr) {
+		protected Instruction doCreateInstruction(Address address, Instruction protoInstr) {
 			try {
-				Instruction created = doCreate(lifespan, address, platform, prototype, protoInstr);
+				if (overwrite) {
+					Instruction exists = doAdjustExisting(address, protoInstr);
+					if (exists != null) {
+						return exists;
+					}
+				}
+
+				Instruction prec = doExtendPreceding(address, protoInstr);
+				if (prec != null) {
+					return prec;
+				}
+
+				Instruction created =
+					doCreate(lifespan, address, platform, protoInstr.getPrototype(), protoInstr);
 				// copy override settings to replacement instruction
 				if (protoInstr.isFallThroughOverridden()) {
 					created.setFallThrough(protoInstr.getFallThrough());
@@ -110,9 +205,9 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 				}
 				return created;
 			}
-			catch (CodeUnitInsertionException | AddressOverflowException e) {
-				// End address already computed when protoInstr created.
-				// We've also already checked for conflicts
+			catch (AddressOverflowException | // End address already checked via protoInstr
+					CodeUnitInsertionException | // We've already checked for conflicts
+					CancelledException e) { // There's no monitor to cancel
 				throw new AssertionError(e);
 			}
 		}
@@ -160,8 +255,7 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 				}

 				if (!skipDelaySlots.contains(startAddress)) {
-					InstructionPrototype prototype = protoInstr.getPrototype();
-					if (!areDelaySlots && prototype.hasDelaySlots()) {
+					if (!areDelaySlots && protoInstr.getPrototype().hasDelaySlots()) {
 						// Reverse their order then add them. This ensures pcode can be generated
 						// for the delay-slotted instruction upon its creation.
 						Deque<Instruction> delayed =
@@ -172,8 +266,7 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 						lastInstruction = replaceIfNotNull(lastInstruction,
 							doAddInstructions(delayed.iterator(), true));
 					}
-					lastInstruction =
-						doCreateInstruction(startAddress, prototype, protoInstr);
+					lastInstruction = doCreateInstruction(startAddress, protoInstr);
 				}
 				if (errorAddress != null && conflictCodeUnit == null &&
 					errorAddress.compareTo(startAddress) <= 0) {
@@ -259,23 +352,8 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 		Address endAddress = address.addNoWrap(prototype.getLength() - 1);
 		AddressRangeImpl createdRange = new AddressRangeImpl(address, endAddress);

-		// First, truncate lifespan to the next code unit when upper bound is max
-		if (!lifespan.maxIsFinite()) {
-			lifespan = space.instructions.truncateSoonestDefined(lifespan, createdRange);
-			lifespan = space.definedData.truncateSoonestDefined(lifespan, createdRange);
-		}
-
-		// Second, truncate lifespan to the next change of bytes in the range
-		// Then, check that against existing code units.
-		DBTraceMemorySpace memSpace =
-			space.trace.getMemoryManager().getMemorySpace(space.space, true);
-		long endSnap = memSpace.getFirstChange(lifespan, createdRange);
-		if (endSnap == Long.MIN_VALUE) {
-			endSnap = lifespan.lmax();
-		}
-		else {
-			endSnap--;
-		}
+		// Truncate, then check that against existing code units.
+		long endSnap = computeTruncatedMax(lifespan, null, createdRange);
 		TraceAddressSnapRange tasr =
 			new ImmutableTraceAddressSnapRange(createdRange, lifespan.withMax(endSnap));

@@ -289,8 +367,8 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 		DBTraceInstruction created = space.instructionMapSpace.put(tasr, null);
 		created.set(platform, prototype, context);

-		cacheForContaining.notifyNewEntry(lifespan, createdRange, created);
-		cacheForSequence.notifyNewEntry(lifespan, createdRange, created);
+		cacheForContaining.notifyNewEntry(tasr.getLifespan(), createdRange, created);
+		cacheForSequence.notifyNewEntry(tasr.getLifespan(), createdRange, created);
 		space.undefinedData.invalidateCache();

 		// TODO: Save the context register into the context manager? Flow it?
@@ -348,14 +426,19 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 	 * @param skipDelaySlots the addresses of delay-slotted instructions to skip
 	 * @param platform the instructions' platform (language, compiler)
 	 * @param block the block of instructions to add
+	 * @param overwrite true to overwrite existing defined units
 	 * @return the adder, or null
 	 */
-	protected InstructionBlockAdder startAddingBlock(Lifespan lifespan,
-			Set<Address> skipDelaySlots, InternalTracePlatform platform, InstructionBlock block) {
+	protected InstructionBlockAdder startAddingBlock(Lifespan lifespan, Set<Address> skipDelaySlots,
+			InternalTracePlatform platform, InstructionBlock block, boolean overwrite) {
+		if (overwrite) {
+			return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block, null, null,
+				null, overwrite);
+		}
 		InstructionError conflict = block.getInstructionConflict();
 		if (conflict == null) {
 			return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block, null, null,
-				null);
+				null, overwrite);
 		}
 		Address errorAddress = conflict.getInstructionAddress();
 		if (errorAddress == null) {
@@ -363,13 +446,13 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 		}
 		if (!conflict.getInstructionErrorType().isConflict) {
 			return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block,
-				errorAddress, conflict, null);
+				errorAddress, conflict, null, overwrite);
 		}
 		long startSnap = lifespan.lmin();
 		CodeUnit conflictCodeUnit =
 			space.definedUnits.getAt(startSnap, conflict.getConflictAddress());
 		return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block, errorAddress,
-			conflict, conflictCodeUnit);
+			conflict, conflictCodeUnit, overwrite);
 	}

 	/**
@@ -468,19 +551,14 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 		try (LockHold hold = LockHold.lock(space.lock.writeLock())) {
 			long startSnap = lifespan.lmin();
 			Set<Address> skipDelaySlots = new HashSet<>();
-			if (overwrite) {
-				for (AddressRange range : instructionSet.getAddressSet()) {
-					space.definedUnits.clear(lifespan, range, false, TaskMonitor.DUMMY);
-				}
-			}
-			else {
+			if (!overwrite) {
 				checkInstructionSet(startSnap, instructionSet, skipDelaySlots);
 			}

 			// Add blocks
 			for (InstructionBlock block : instructionSet) {
 				InstructionBlockAdder adder =
-					startAddingBlock(lifespan, skipDelaySlots, dbPlatform, block);
+					startAddingBlock(lifespan, skipDelaySlots, dbPlatform, block, overwrite);
 				if (adder == null) {
 					continue;
 				}
@@ -496,9 +574,6 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView
 			}
 			return result;
 		}
-		catch (CancelledException e) {
-			throw new AssertionError(e); // No actual monitor
-		}
 		catch (AddressOverflowException e) {
 			// Better have skipped any delay-slotted instructions whose delays overflowed
 			throw new AssertionError(e);
@@ -299,21 +299,28 @@ public class DBTraceAddressSnapRangePropertyMapTree<T, DR extends AbstractDBTrac
 			return range;
 		}

-		@SuppressWarnings({ "unchecked", "hiding" })
+		@SuppressWarnings("unchecked")
 		protected void doSetRange(AddressRange range) {
+			long newMinOffset = tree.mapSpace.assertInSpace(range.getMinAddress());
+			long newMaxOffset = range.getMaxAddress().getOffset();
+			if (minOffset == newMinOffset && maxOffset == newMaxOffset) {
+				return;
+			}
 			@SuppressWarnings("rawtypes")
 			DBTraceAddressSnapRangePropertyMapTree tree = this.tree;
-			long newMinOffset = tree.mapSpace.assertInSpace(range.getMinAddress());
 			tree.doUnparentEntry(this);
 			minOffset = newMinOffset;
-			maxOffset = range.getMaxAddress().getOffset();
+			maxOffset = newMaxOffset;
 			update(MIN_ADDRESS_COLUMN, MAX_ADDRESS_COLUMN);
 			this.range = range;
 			tree.doInsertDataEntry(this);
 		}

-		@SuppressWarnings({ "unchecked", "hiding" })
+		@SuppressWarnings("unchecked")
 		protected void doSetLifespan(Lifespan lifespan) {
+			if (minSnap == lifespan.lmin() && maxSnap == lifespan.lmax()) {
+				return;
+			}
 			@SuppressWarnings("rawtypes")
 			DBTraceAddressSnapRangePropertyMapTree tree = this.tree;
 			tree.doUnparentEntry(this);
@@ -634,7 +634,8 @@ public class DBTraceMemorySpace
 		if (!remaining.isEmpty()) {
 			lastSnap.snap = Long.MAX_VALUE;
 			for (AddressRange rng : remaining) {
-				changed.add(new ImmutableTraceAddressSnapRange(rng, Lifespan.nowOn(loc.snap)));
+				changed.add(
+					new ImmutableTraceAddressSnapRange(rng, Lifespan.nowOnMaybeScratch(loc.snap)));
 			}
 		}
 		buf.position(pos);
@@ -17,11 +17,9 @@ package ghidra.trace.database.space;

 import java.io.IOException;
 import java.util.*;
+import java.util.Map.Entry;
 import java.util.concurrent.locks.ReadWriteLock;

-import org.apache.commons.lang3.tuple.ImmutablePair;
-import org.apache.commons.lang3.tuple.Pair;
-
 import db.DBHandle;
 import db.DBRecord;
 import generic.CatenatedCollection;
@@ -89,6 +87,19 @@ public abstract class AbstractDBTraceSpaceBasedManager<M extends DBTraceSpaceBas
 		}
 	}

+	private record Frame(TraceThread thread, int level) {
+	}
+
+	private record TabledSpace(DBTraceSpaceEntry entry, AddressSpace space, TraceThread thread) {
+		private boolean isRegisterSpace() {
+			return space.isRegisterSpace();
+		}
+
+		private Frame frame() {
+			return new Frame(thread, entry.frameLevel);
+		}
+	}
+
 	protected final String name;
 	protected final DBHandle dbh;
 	protected final ReadWriteLock lock;
@@ -100,7 +111,7 @@ public abstract class AbstractDBTraceSpaceBasedManager<M extends DBTraceSpaceBas
 	// Note: use tree map so traversal is ordered by address space
 	protected final Map<AddressSpace, M> memSpaces = new TreeMap<>();
 	// Note: can use hash map here. I see no need to order these spaces
-	protected final Map<Pair<TraceThread, Integer>, M> regSpaces = new HashMap<>();
+	protected final Map<Frame, M> regSpaces = new HashMap<>();
 	protected final Map<TraceObject, M> regSpacesByObject = new HashMap<>();

 	protected final Collection<M> memSpacesView =
@@ -130,47 +141,61 @@ public abstract class AbstractDBTraceSpaceBasedManager<M extends DBTraceSpaceBas
 		return DBTraceUtils.tableName(name, space, threadKey, frameLevel);
 	}

-	@SuppressWarnings("unchecked")
 	protected void loadSpaces() throws VersionException, IOException {
-		for (DBTraceSpaceEntry ent : spaceStore.asMap().values()) {
-			AddressFactory addressFactory = trace.getBaseAddressFactory();
-			AddressSpace space;
-			if (NO_ADDRESS_SPACE.getName().equals(ent.spaceName)) {
-				space = NO_ADDRESS_SPACE;
+		Map<Frame, TabledSpace> newRegSpaces = new HashMap<>();
+		Map<AddressSpace, TabledSpace> newMemSpaces = new HashMap<>();
+		for (TabledSpace ts : getTabledSpaces()) {
+			if (ts.isRegisterSpace()) {
+				newRegSpaces.put(ts.frame(), ts);
 			}
 			else {
-				space = addressFactory.getAddressSpace(ent.spaceName);
+				newMemSpaces.put(ts.space(), ts);
 			}
+		}
+		regSpaces.keySet().retainAll(newRegSpaces.keySet());
+		memSpaces.keySet().retainAll(newMemSpaces.keySet());
+		for (Entry<Frame, TabledSpace> ent : newRegSpaces.entrySet()) {
+			if (!regSpaces.containsKey(ent.getKey())) {
+				regSpaces.put(ent.getKey(), createRegisterSpace(ent.getValue()));
+			}
+		}
+		for (Entry<AddressSpace, TabledSpace> ent : newMemSpaces.entrySet()) {
+			if (!memSpaces.containsKey(ent.getKey())) {
+				memSpaces.put(ent.getKey(), createSpace(ent.getValue()));
+			}
+		}
+	}
+
+	protected AddressSpace getSpaceByName(AddressFactory factory, String name) {
+		if (NO_ADDRESS_SPACE.getName().equals(name)) {
+			return NO_ADDRESS_SPACE;
+		}
+		return factory.getAddressSpace(name);
+	}
+
+	protected List<TabledSpace> getTabledSpaces() {
+		AddressFactory factory = trace.getBaseAddressFactory();
+		List<TabledSpace> result = new ArrayList<>();
+		for (DBTraceSpaceEntry ent : spaceStore.asMap().values()) {
+			AddressSpace space = getSpaceByName(factory, ent.spaceName);
 			if (space == null) {
 				Msg.error(this, "Space " + ent.spaceName + " does not exist in trace (language=" +
 					baseLanguage + ").");
+				continue;
 			}
-			else if (space.isRegisterSpace()) {
+			if (space.isRegisterSpace()) {
 				if (threadManager == null) {
 					Msg.error(this, "Register spaces are not allowed without a thread manager.");
 					continue;
 				}
 				TraceThread thread = threadManager.getThread(ent.threadKey);
-				M regSpace;
-				if (ent.space == null) {
-					regSpace = createRegisterSpace(space, thread, ent);
-				}
-				else {
-					regSpace = (M) ent.space;
-				}
-				regSpaces.put(ImmutablePair.of(thread, ent.getFrameLevel()), regSpace);
+				result.add(new TabledSpace(ent, space, thread));
 			}
 			else {
-				M memSpace;
-				if (ent.space == null) {
-					memSpace = createSpace(space, ent);
-				}
-				else {
-					memSpace = (M) ent.space;
-				}
-				memSpaces.put(space, memSpace);
+				result.add(new TabledSpace(ent, space, null));
 			}
 		}
+		return result;
 	}

 	protected M getForSpace(AddressSpace space, boolean createIfAbsent) {
@@ -210,7 +235,7 @@ public abstract class AbstractDBTraceSpaceBasedManager<M extends DBTraceSpaceBas
 			return getForRegisterSpaceObjectThread((TraceObjectThread) thread, frameLevel,
 				createIfAbsent);
 		}
-		Pair<TraceThread, Integer> frame = ImmutablePair.of(thread, frameLevel);
+		Frame frame = new Frame(thread, frameLevel);
 		if (!createIfAbsent) {
 			try (LockHold hold = LockHold.lock(lock.readLock())) {
 				return regSpaces.get(frame);
@@ -340,6 +365,26 @@ public abstract class AbstractDBTraceSpaceBasedManager<M extends DBTraceSpaceBas
 	protected abstract M createRegisterSpace(AddressSpace space, TraceThread thread,
 			DBTraceSpaceEntry ent) throws VersionException, IOException;

+	@SuppressWarnings("unchecked")
+	private M createSpace(TabledSpace ts) throws VersionException, IOException {
+		if (ts.entry.space != null) {
+			return (M) ts.entry.space;
+		}
+		M space = createSpace(ts.space, ts.entry);
+		ts.entry.space = space;
+		return space;
+	}
+
+	@SuppressWarnings("unchecked")
+	private M createRegisterSpace(TabledSpace ts) throws VersionException, IOException {
+		if (ts.entry.space != null) {
+			return (M) ts.entry.space;
+		}
+		M space = createRegisterSpace(ts.space, ts.thread, ts.entry);
+		ts.entry.space = space;
+		return space;
+	}
+
 	@Override
 	public void dbError(IOException e) {
 		trace.dbError(e);
@@ -349,10 +394,6 @@ public abstract class AbstractDBTraceSpaceBasedManager<M extends DBTraceSpaceBas
 	public void invalidateCache(boolean all) {
 		try (LockHold hold = LockHold.lock(lock.writeLock())) {
 			spaceStore.invalidateCache();
-			// TODO: Need to do a real delta here, not blow away and remake
-			// Currently, object identities are not preserved by this operation
-			memSpaces.clear();
-			regSpaces.clear();
 			loadSpaces();
 			for (M m : memSpaces.values()) {
 				m.invalidateCache();
@@ -20,17 +20,8 @@ import ghidra.trace.model.thread.TraceThread;
 import ghidra.trace.util.TraceAddressSpace;

 public interface DBTraceSpaceKey extends TraceAddressSpace {
-	static class DefaultDBTraceSpaceKey implements DBTraceSpaceKey {
-		private final TraceThread thread;
-		private final AddressSpace space;
-		private final int frameLevel;
-
-		private DefaultDBTraceSpaceKey(TraceThread thread, AddressSpace space, int frameLevel) {
-			this.thread = thread;
-			this.space = space;
-			this.frameLevel = frameLevel;
-		}
-
+	record DefaultDBTraceSpaceKey(TraceThread thread, AddressSpace space, int frameLevel)
+			implements DBTraceSpaceKey {
 		@Override
 		public AddressSpace getAddressSpace() {
 			return space;
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 import java.util.function.Function;

 import org.junit.*;
@@ -171,6 +172,23 @@ public class DBTraceCodeManagerTest extends AbstractGhidraHeadlessIntegrationTes
 		}
 	}

+	@Test
+	public void testPutBytesInScratchLeavesStaticDataUntouched() throws CodeUnitInsertionException {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			TraceData d0at4000 =
+				b.addData(0, b.addr(0x4000), IntegerDataType.dataType, b.buf(1, 2, 3, 4));
+			assertEquals(Lifespan.nowOn(0), d0at4000.getLifespan());
+			assertEquals(new Scalar(32, 0x01020304), d0at4000.getValue());
+
+			b.trace.getMemoryManager().putBytes(-10, b.addr(0x4000), b.buf(5, 6, 7, 8));
+			TraceData d10at4000 =
+				b.trace.getCodeManager().definedData().getContaining(10, b.addr(0x4000));
+			assertSame(d0at4000, d10at4000);
+			assertEquals(Lifespan.nowOn(0), d10at4000.getLifespan());
+			assertEquals(new Scalar(32, 0x01020304), d10at4000.getValue());
+		}
+	}
+
 	@Test
 	public void testPutBytesDeletesDynamicData() throws CodeUnitInsertionException {
 		try (UndoableTransaction tid = b.startTransaction()) {
@@ -292,6 +310,110 @@ public class DBTraceCodeManagerTest extends AbstractGhidraHeadlessIntegrationTes
 		}
 	}

+	@Test
+	public void testOverlapErrorsMultithreaded() throws Throwable {
+		ArrayList<CompletableFuture<Integer>> creators = new ArrayList<>();
+		for (int i = 0; i < 10; i++) {
+			creators.add(CompletableFuture.supplyAsync(() -> {
+				try (UndoableTransaction tid = b.startTransaction()) {
+					b.trace.getCodeManager()
+							.definedData()
+							.create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType);
+					return 0;
+				}
+				catch (CodeUnitInsertionException e) {
+					return 1;
+				}
+			}));
+		}
+		CompletableFuture.allOf(creators.toArray(CompletableFuture[]::new)).get();
+		assertEquals(9, creators.stream()
+				.mapToInt(c -> c.getNow(null))
+				.reduce(Integer::sum)
+				.orElse(-1));
+	}
+
+	@Test
+	public void testOverlapAllowedAfterAbort() throws Throwable {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType);
+			tid.abort();
+		}
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType);
+		}
+	}
+
+	public void testOverlapErrAfterInvalidate() throws Throwable {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType);
+		}
+		b.trace.undo();
+		b.trace.redo();
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType);
+			fail();
+		}
+		catch (CodeUnitInsertionException e) {
+			// pass
+		}
+	}
+
+	/**
+	 * This test is interesting because the pointer type def causes an update to the data type
+	 * settings <em>while the unit is still being created</em>. This will invalidate the trace's
+	 * caches. All of them, including the defined data units, which can become the cause of many
+	 * timing issues.
+	 */
+	@Test
+	public void testOverlapErrWithDataTypeSettings() throws Throwable {
+		AddressSpace space = b.trace.getBaseAddressFactory().getDefaultAddressSpace();
+		PointerTypedef type = new PointerTypedef(null, VoidDataType.dataType, 8, null, space);
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), type);
+		}
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), type);
+			fail();
+		}
+		catch (CodeUnitInsertionException e) {
+			// pass
+		}
+	}
+
+	@Test
+	public void testOverlapErrAfterSetEndSnap() throws Throwable {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			DBTraceDataAdapter data = b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType);
+			assertEquals(Lifespan.before(0), data.getLifespan());
+			data.setEndSnap(-10);
+			assertEquals(Lifespan.before(-9), data.getLifespan());
+		}
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getCodeManager()
+					.definedData()
+					.create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType);
+			fail();
+		}
+		catch (CodeUnitInsertionException e) {
+			// pass
+		}
+	}
+
 	protected void assertAllNullFunc(Function<TraceBaseCodeUnitsView<?>, TraceCodeUnit> func) {
 		assertNull(func.apply(manager.codeUnits()));
 		assertNull(func.apply(manager.data()));
@@ -1538,7 +1660,8 @@ public class DBTraceCodeManagerTest extends AbstractGhidraHeadlessIntegrationTes
 			coversTwoWays(manager.definedData(), Lifespan.span(0, 5), b.range(0x4000, 0x4007)));
 		assertTrue(
 			coversTwoWays(manager.definedData(), Lifespan.span(0, 9), b.range(0x4001, 0x4003)));
-		assertFalse(intersectsTwoWays(manager.definedData(), Lifespan.ALL, b.range(0x0000, 0x3fff)));
+		assertFalse(
+			intersectsTwoWays(manager.definedData(), Lifespan.ALL, b.range(0x0000, 0x3fff)));
 		assertFalse(
 			intersectsTwoWays(manager.definedData(), Lifespan.ALL, b.range(0x4008, -0x0001)));
 		assertFalse(intersectsTwoWays(manager.definedData(), Lifespan.toNow(-1), all));
@@ -761,6 +761,18 @@ public class DBTraceCodeUnitTest extends AbstractGhidraHeadlessIntegrationTest
 		assertEquals(4, data.getBytes(buf, 0));
 		assertArrayEquals(b.arr(1, 2, 3, 4), buf.array());

+		buf = ByteBuffer.allocate(1);
+		assertEquals(1, data.getBytes(buf, 4));
+		assertArrayEquals(b.arr(0xeb), buf.array());
+
+		buf = ByteBuffer.allocate(1);
+		assertEquals(1, data.getBytes(buf, 5));
+		assertArrayEquals(b.arr(0xfe), buf.array());
+
+		buf = ByteBuffer.allocate(4);
+		assertEquals(4, data.getBytes(buf, 2));
+		assertArrayEquals(b.arr(3, 4, 0xeb, 0xfe), buf.array());
+
 		assertArrayEquals(b.arr(1, 2, 3, 4), data.getBytes());

 		byte[] arr = new byte[6];
@@ -15,9 +15,7 @@
 */
 package ghidra.trace.database.module;

-import static ghidra.lifecycle.Unfinished.TODO;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;

 import java.io.File;
 import java.util.*;
@@ -334,7 +332,6 @@ public class DBTraceModuleManagerTest extends AbstractGhidraHeadlessIntegrationT
 	}

 	@Test
-	@Ignore("GP-479")
 	public void testUndoIdentitiesPreserved() throws Exception {
 		TraceModule mod1;
 		try (UndoableTransaction tid = b.startTransaction()) {
@@ -351,8 +348,7 @@ public class DBTraceModuleManagerTest extends AbstractGhidraHeadlessIntegrationT

 		b.trace.undo();

-		assertEquals(mod1, assertOne(moduleManager.getModulesByPath("Modules[first]")));
-		TODO(); // TODO: mod1 should still be identical to that in database
+		assertSame(mod1, assertOne(moduleManager.getModulesByPath("Modules[first]")));
 		assertTrue(moduleManager.getModulesByPath("Modules[second]").isEmpty());
 	}

@@ -19,15 +19,19 @@ import static org.junit.Assert.assertEquals;

 import java.io.File;
 import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;

 import org.junit.*;

 import ghidra.app.cmd.disassemble.*;
+import ghidra.app.plugin.assembler.*;
 import ghidra.program.database.ProgramBuilder;
 import ghidra.program.disassemble.Disassembler;
-import ghidra.program.model.address.AddressOverflowException;
-import ghidra.program.model.address.AddressSet;
+import ghidra.program.model.address.*;
 import ghidra.program.model.lang.*;
 import ghidra.program.model.listing.CodeUnit;
 import ghidra.program.model.mem.MemoryBlock;
@@ -37,7 +41,8 @@ import ghidra.trace.database.guest.DBTraceGuestPlatform;
 import ghidra.trace.database.listing.*;
 import ghidra.trace.database.memory.DBTraceMemoryManager;
 import ghidra.trace.database.memory.DBTraceMemorySpace;
-import ghidra.trace.model.Lifespan;
+import ghidra.trace.model.*;
+import ghidra.trace.model.listing.TraceCodeUnit;
 import ghidra.trace.model.memory.TraceMemoryFlag;
 import ghidra.trace.model.memory.TraceOverlappedRegionException;
 import ghidra.trace.util.LanguageTestWatcher;
@@ -306,4 +311,136 @@ public class DBTraceDisassemblerIntegrationTest extends AbstractGhidraHeadlessIn
 			assertEquals("MOV ECX,EAX", cu2.toString());
 		}
 	}
+
+	record Repetition(Lifespan lifespan, boolean overwrite) {
+	}
+
+	protected <T> List<T> toList(Iterable<? extends T> it) {
+		return StreamSupport.stream(it.spliterator(), false).collect(Collectors.toList());
+	}
+
+	protected void runTestCoalesceInstructions(List<Repetition> repetitions) throws Exception {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			DBTraceMemoryManager memory = b.trace.getMemoryManager();
+			DBTraceCodeManager code = b.trace.getCodeManager();
+
+			memory.createRegion(".text", 0, b.range(0x00400000, 0x00400fff));
+			Assembler asm = Assemblers.getAssembler(b.language);
+			Address entry = b.addr(0x00400000);
+			AssemblyBuffer buf = new AssemblyBuffer(asm, entry);
+			buf.assemble("imm r0, #123");
+			buf.assemble("mov r1, r0");
+			buf.assemble("ret");
+
+			long snap = Lifespan.isScratch(repetitions.get(0).lifespan.lmin()) ? Long.MIN_VALUE : 0;
+			memory.putBytes(snap, entry, ByteBuffer.wrap(buf.getBytes()));
+
+			AddressFactory factory = b.trace.getBaseAddressFactory();
+			Disassembler dis =
+				Disassembler.getDisassembler(b.language, factory, TaskMonitor.DUMMY, null);
+			InstructionSet set = new InstructionSet(factory);
+			set.addBlock(dis.pseudoDisassembleBlock(memory.getBufferAt(snap, entry), null, 10));
+
+			List<TraceCodeUnit> units = null;
+			TraceAddressSnapRange all =
+				new ImmutableTraceAddressSnapRange(b.range(0, -1), Lifespan.ALL);
+			for (Repetition rep : repetitions) {
+				code.instructions().addInstructionSet(rep.lifespan, set, rep.overwrite);
+				if (units == null) {
+					units = toList(code.definedUnits().getIntersecting(all));
+				}
+				else {
+					/**
+					 * Technically, getIntersecting makes no guarantee regarding order.
+					 * Nevertheless, the structure shouldn't be perturbed, so I think it's fair to
+					 * expect the same order.
+					 */
+					assertEquals(units, toList(code.definedUnits().getIntersecting(all)));
+				}
+			}
+		}
+	}
+
+	@Test
+	@TestLanguage(ProgramBuilder._TOY64_BE)
+	public void testCoalesceInstructionsMinTwiceNoOverwrite() throws Exception {
+		runTestCoalesceInstructions(List.of(
+			new Repetition(Lifespan.nowOn(Long.MIN_VALUE), false),
+			new Repetition(Lifespan.nowOn(Long.MIN_VALUE), false)));
+	}
+
+	@Test
+	@TestLanguage(ProgramBuilder._TOY64_BE)
+	public void testCoalesceInstructionsMinTwiceYesOverwrite() throws Exception {
+		runTestCoalesceInstructions(List.of(
+			new Repetition(Lifespan.nowOn(Long.MIN_VALUE), true),
+			new Repetition(Lifespan.nowOn(Long.MIN_VALUE), true)));
+	}
+
+	@Test
+	@TestLanguage(ProgramBuilder._TOY64_BE)
+	public void testCoalesceInstructionsZeroTwiceYesOverwrite() throws Exception {
+		runTestCoalesceInstructions(List.of(
+			new Repetition(Lifespan.nowOn(0), true),
+			new Repetition(Lifespan.nowOn(0), true)));
+	}
+
+	@Test
+	@TestLanguage(ProgramBuilder._TOY64_BE)
+	public void testCoalesceInstructionsZeroThenOneYesOverwrite() throws Exception {
+		runTestCoalesceInstructions(List.of(
+			new Repetition(Lifespan.nowOn(0), true),
+			new Repetition(Lifespan.nowOn(1), true)));
+	}
+
+	@Test
+	@TestLanguage(ProgramBuilder._TOY64_BE)
+	public void testCoalesceInstructionsZeroOnlyThenOneNoOverwrite() throws Exception {
+		runTestCoalesceInstructions(List.of(
+			new Repetition(Lifespan.at(0), false),
+			new Repetition(Lifespan.nowOn(1), false)));
+	}
+
+	@Test
+	@TestLanguage(ProgramBuilder._TOY64_BE)
+	public void testCoalesceInstructionsZeroOnlyThenOneYesOverwrite() throws Exception {
+		runTestCoalesceInstructions(List.of(
+			new Repetition(Lifespan.at(0), true),
+			new Repetition(Lifespan.nowOn(1), true)));
+	}
+
+	@Test
+	public void testNoCoalesceAcrossByteChanges() throws Exception {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			DBTraceMemoryManager memory = b.trace.getMemoryManager();
+			DBTraceCodeManager code = b.trace.getCodeManager();
+
+			memory.createRegion(".text", 0, b.range(0x00400000, 0x00400fff));
+			Assembler asm = Assemblers.getAssembler(b.language);
+			Address entry = b.addr(0x00400000);
+			AssemblyBuffer buf = new AssemblyBuffer(asm, entry);
+			buf.assemble("imm r0, #123");
+			buf.assemble("mov r1, r0");
+			buf.assemble("ret");
+
+			memory.putBytes(-1, entry, ByteBuffer.wrap(buf.getBytes()));
+
+			AddressFactory factory = b.trace.getBaseAddressFactory();
+			Disassembler dis =
+				Disassembler.getDisassembler(b.language, factory, TaskMonitor.DUMMY, null);
+			InstructionSet set = new InstructionSet(factory);
+			set.addBlock(dis.pseudoDisassembleBlock(memory.getBufferAt(-1, entry), null, 10));
+
+			TraceAddressSnapRange all =
+				new ImmutableTraceAddressSnapRange(b.range(0, -1), Lifespan.ALL);
+			code.instructions().addInstructionSet(Lifespan.nowOn(-1), set, true);
+			/**
+			 * This is already a bogus sort of operation: The prototypes may not match the bytes. In
+			 * any case, we should not expect coalescing.
+			 */
+			code.instructions().addInstructionSet(Lifespan.nowOn(0), set, true);
+			List<TraceCodeUnit> units = toList(code.definedUnits().getIntersecting(all));
+			assertEquals(6, units.size());
+		}
+	}
 }
@@ -272,7 +272,7 @@ public class PcodeExecutor<T> {
 			throw e;
 		}
 		catch (Exception e) {
-			throw new PcodeExecutionException("Exception during pcode execution", frame, e);
+			throw new PcodeExecutionException(e.getMessage(), frame, e);
 		}
 	}

@@ -326,4 +326,8 @@ public class DBAnnotatedObject extends DatabaseObject {
 	public boolean isDeleted() {
 		return super.isDeleted(adapter.getLock());
 	}
+
+	public String getTableName() {
+		return store.getTableName();
+	}
 }
@@ -1275,6 +1275,15 @@ public class DBCachedObjectStore<T extends DBAnnotatedObject> implements ErrorHa
 		return builder.toString();
 	}

+	/**
+	 * Get the name of the table backing this store
+	 * 
+	 * @return the name
+	 */
+	public String getTableName() {
+		return tableName;
+	}
+
 	/**
 	 * Invalidate this store's cache
 	 * 
@@ -82,7 +82,7 @@ public abstract class AbstractConstraintsTree< //
 	protected abstract NR createNodeEntry(DBCachedObjectStore<NR> store, DBRecord record);

 	protected void init() {
-		assert root == null;
+		// assert root == null;
 		root = getOrCreateRoot();
 		leafLevel = computeLeafLevel();
 	}
@@ -843,6 +843,7 @@ public abstract class AbstractConstraintsTree< //
 					throw new AssertionError(
 						"Leaf node " + n + " cannot contain nodes " + nodeChildren);
 				}
+				break;
 		}

 		// Check that child count matches by counting over iterator
@@ -911,9 +912,6 @@ public abstract class AbstractConstraintsTree< //
 					" out of sync: cache=" + cachedChildren + " db=" + databasedChildren);
 			}
 		}
-		if (leafLevel != computeLeafLevel()) {
-			throw new AssertionError("Leaf level is incorrect");
-		}
 		visit(null, new TreeRecordVisitor() {
 			@Override
 			protected VisitResult beginNode(NR parent, NR n, QueryInclusion inclusion) {
@@ -927,6 +925,9 @@ public abstract class AbstractConstraintsTree< //
 				return VisitResult.NEXT;
 			}
 		}, false);
+		if (leafLevel != computeLeafLevel()) {
+			throw new AssertionError("Leaf level is incorrect");
+		}
 	}

 	public abstract AbstractConstraintsTreeSpatialMap<DS, DR, NS, T, Q> asSpatialMap();
@@ -945,6 +946,7 @@ public abstract class AbstractConstraintsTree< //
 			cachedNodeChildren.clear();
 			dataStore.invalidateCache();
 			nodeStore.invalidateCache();
+			init();
 		}
 	}
 }