Merge remote-tracking branch 'origin/GP-1620_Dan_staleFutureInEmu--SQUASHED' into patch

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

@@ -75,6 +75,10 @@ public enum DBTraceUtils {
 		public int hashCode() {
 			return Objects.hash(offset, snap);
 		}
+
+		public boolean isScratch() {
+			return DBTraceUtils.isScratch(snap);
+		}
 	}
 
 	// TODO: Should this be in by default?
@@ -311,6 +315,10 @@ public enum DBTraceUtils {
 		return a.isConnected(b) && !a.intersection(b).isEmpty();
 	}
 
+	public static boolean isScratch(long snap) {
+		return snap < 0;
+	}
+
 	public static <C extends Comparable<C>> int compareRanges(Range<C> a, Range<C> b) {
 		int result;
 		if (!a.hasLowerBound() && b.hasLowerBound()) {
@@ -367,6 +375,7 @@ public enum DBTraceUtils {
 	/**
 	 * TODO: Document me
 	 * 
+	 * <p>
 	 * Only call this method for entries which definitely intersect the given span
 	 * 
 	 * @param data

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

@@ -45,7 +45,10 @@ class DBTraceMemoryBlockEntry extends DBAnnotatedObject {
 		return DBTraceUtils.tableName(TABLE_NAME, space, threadKey, frameLevel);
 	}
 
-	@DBAnnotatedField(column = LOCATION_COLUMN_NAME, indexed = true, codec = OffsetThenSnapDBFieldCodec.class)
+	@DBAnnotatedField(
+		column = LOCATION_COLUMN_NAME,
+		indexed = true,
+		codec = OffsetThenSnapDBFieldCodec.class)
 	private OffsetSnap location;
 	@DBAnnotatedField(column = BUFFER_COLUMN_NAME)
 	private long bufferKey = -1;
@@ -73,6 +76,10 @@ class DBTraceMemoryBlockEntry extends DBAnnotatedObject {
 		return location.snap;
 	}
 
+	public boolean isScratch() {
+		return DBTraceUtils.isScratch(location.snap);
+	}
+
 	private int getBlockNumber() {
 		return Byte.toUnsignedInt(blockNum);
 	}
@@ -82,6 +89,8 @@ class DBTraceMemoryBlockEntry extends DBAnnotatedObject {
 		assert loc.snap > location.snap;
 		DBTraceMemoryBlockEntry cp = space.blockStore.create();
 		cp.setLoc(loc);
+		space.blockCacheMostRecent.clear();
+		space.blockCacheMostRecent.put(loc, cp);
 		DBTraceMemoryBufferEntry myBuf = findAssignedBuffer();
 		if (myBuf == null) {
 			return cp;
@@ -134,11 +143,11 @@ class DBTraceMemoryBlockEntry extends DBAnnotatedObject {
 	}
 
 	protected DBTraceMemoryBufferEntry findFreeBufferInFuture() throws IOException {
-		DBTraceMemoryBlockEntry prev = space.findSoonestBlockEntry(location, false);
+		DBTraceMemoryBlockEntry next = space.findSoonestBlockEntry(location, false);
-		if (prev == null) {
+		if (next == null) {
 			return null;
 		}
-		return findFreeBuffer(prev);
+		return findFreeBuffer(next);
 	}
 
 	protected DBTraceMemoryBufferEntry findFreeBuffer() throws IOException {

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

@@ -85,7 +85,7 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 	protected final DBCachedObjectStore<DBTraceMemoryBufferEntry> bufferStore;
 	protected final DBCachedObjectStore<DBTraceMemoryBlockEntry> blockStore;
 	protected final DBCachedObjectIndex<OffsetSnap, DBTraceMemoryBlockEntry> blocksByOffset;
-	protected final Map<OffsetSnap, DBTraceMemoryBlockEntry> blockCache = CacheBuilder
+	protected final Map<OffsetSnap, DBTraceMemoryBlockEntry> blockCacheMostRecent = CacheBuilder
 			.newBuilder()
 			.removalListener(this::blockCacheEntryRemoved)
 			.maximumSize(10)
@@ -495,7 +495,7 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 		if (!inclusive) {
 			loc = new OffsetSnap(loc.offset, loc.snap - 1);
 		}
-		ent = blockCache.get(loc);
+		ent = blockCacheMostRecent.get(loc);
 		if (ent != null) {
 			return ent;
 		}
@@ -504,13 +504,26 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 			return null;
 		}
 		ent = it.next();
-		if (ent.getOffset() != loc.offset) {
+		if (ent.getOffset() != loc.offset || ent.isScratch() != loc.isScratch()) {
 			return null;
 		}
-		blockCache.put(loc, ent);
+		blockCacheMostRecent.put(loc, ent);
 		return ent;
 	}
 
+	/**
+	 * Locate the soonest block entry for the given offset-snap pair
+	 * 
+	 * <p>
+	 * To qualify, the entry must have a snap greater than (or optionally equal to) that given and
+	 * an offset exactly equal to that given. That is, it is the earliest in time, but most follow
+	 * the given snap. Additionally, if the given snap is in scratch space, the found entry must
+	 * also be in scratch space.
+	 * 
+	 * @param loc the offset-snap pair
+	 * @param inclusive true to allow equal snap
+	 * @return the found entry, or null
+	 */
 	protected DBTraceMemoryBlockEntry findSoonestBlockEntry(OffsetSnap loc, boolean inclusive) {
 		Iterator<DBTraceMemoryBlockEntry> it;
 		if (inclusive) {
@@ -524,7 +537,7 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 			return null;
 		}
 		DBTraceMemoryBlockEntry next = it.next();
-		if (next.getOffset() != loc.offset) {
+		if (next.getOffset() != loc.offset || next.isScratch() != loc.isScratch()) {
 			return null;
 		}
 		return next;
@@ -553,19 +566,19 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 			}
 			ent = ent.copy(loc);
 			ent.setBytes(buf, dstOffset, maxLen);
-			blockCache.put(loc, ent);
 			return;
 		}
-		// (1) or (2a)
+		// (1) or (2b)
 		ent = blockStore.create();
 		ent.setLoc(loc);
+		blockCacheMostRecent.clear();
+		blockCacheMostRecent.put(loc, ent);
 		if (ent.cmpBytes(buf, dstOffset, maxLen) == 0) {
 			// Keep the entry, but don't allocate storage in a buffer
 			buf.position(buf.position() + maxLen);
 			return;
 		}
 		ent.setBytes(buf, dstOffset, maxLen);
-		blockCache.put(loc, ent);
 	}
 
 	protected static class OutSnap {
@@ -580,13 +593,14 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 			OutSnap lastSnap, Set<TraceAddressSnapRange> changed) throws IOException {
 		// NOTE: Do not leave the buffer advanced from here
 		int pos = buf.position();
+		// exclusive?
 		Iterator<DBTraceMemoryBlockEntry> it =
-			blocksByOffset.tail(new OffsetSnap(loc.offset, loc.snap + 1), true).values().iterator(); // exclusive
+			blocksByOffset.tail(new OffsetSnap(loc.offset, loc.snap + 1), true).values().iterator();
 		AddressSet remaining = new AddressSet(space.getAddress(loc.offset + dstOffset),
 			space.getAddress(loc.offset + dstOffset + maxLen - 1));
 		while (it.hasNext()) {
 			DBTraceMemoryBlockEntry next = it.next();
-			if (next.getOffset() != loc.offset) {
+			if (next.getOffset() != loc.offset || next.isScratch() != loc.isScratch()) {
 				break;
 			}
 			AddressSetView withState =
@@ -904,6 +918,26 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 		return BLOCK_SIZE;
 	}
 
+	protected boolean isCross(long lower, long upper) {
+		return lower < 0 && upper >= 0;
+	}
+
+	/**
+	 * Determine the truncation snap if the given span and range include byte changes
+	 * 
+	 * <p>
+	 * Code units do not understand or accommodate changes in time, so the underlying bytes of the
+	 * unit must be the same throughout its lifespan. Typically, units are placed with a desired
+	 * creation snap, and then its life is extended into the future opportunistically. Thus, when
+	 * truncating, we desire to keep the start snap, then search for the soonest byte change within
+	 * the desired lifespan. Furthermore, we generally don't permit a unit to exist in both record
+	 * and scratch spaces, i.e., it cannot span both the -1 and 0 snaps.
+	 * 
+	 * @param span the desired lifespan
+	 * @param range the address range covered
+	 * @return the first snap that should be excluded, or {@link Long#MIN_VALUE} to indicate no
+	 *         change.
+	 */
 	public long getFirstChange(Range<Long> span, AddressRange range) {
 		assertInSpace(range);
 		long lower = DBTraceUtils.lowerEndpoint(span);
@@ -911,21 +945,24 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 		if (lower == upper) {
 			return Long.MIN_VALUE;
 		}
-		Range<Long> fwdOne = DBTraceUtils.toRange(lower + 1, upper);
+		boolean cross = isCross(lower, upper);
+		if (cross && lower == -1) {
+			return 0; // Avoid reversal of range end points. 
+		}
+		Range<Long> fwdOne = DBTraceUtils.toRange(lower + 1, cross ? -1 : upper);
 		ByteBuffer buf1 = ByteBuffer.allocate(BLOCK_SIZE);
 		ByteBuffer buf2 = ByteBuffer.allocate(BLOCK_SIZE);
 		try (LockHold hold = LockHold.lock(lock.readLock())) {
 			for (TraceAddressSnapRange tasr : stateMapSpace.reduce(
 				TraceAddressSnapRangeQuery.intersecting(range, fwdOne)
-						.starting(
+						.starting(Rectangle2DDirection.BOTTOMMOST))
-							Rectangle2DDirection.BOTTOMMOST))
 					.orderedKeys()) {
 				AddressRange toExamine = range.intersect(tasr.getRange());
 				if (doCheckBytesChanged(tasr.getY1(), toExamine, buf1, buf2)) {
 					return tasr.getY1();
 				}
 			}
-			return Long.MIN_VALUE;
+			return cross ? 0 : Long.MIN_VALUE;
 		}
 		catch (IOException e) {
 			blockStore.dbError(e);
@@ -948,7 +985,8 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 			getBytes(snap, start, oldBytes);
 			// New in the sense that they're about to replace the old bytes
 			ByteBuffer newBytes = ByteBuffer.allocate(len);
-			if (snap != 0) {
+			// NB. Don't want to wrap to Long.MAX_VALUE, but also don't want to read from scratch
+			if (snap != 0 && snap != Long.MIN_VALUE) {
 				getBytes(snap - 1, start, newBytes);
 				newBytes.flip();
 			}
@@ -1003,7 +1041,7 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace
 			stateMapSpace.invalidateCache();
 			bufferStore.invalidateCache();
 			blockStore.invalidateCache();
-			blockCache.clear();
+			blockCacheMostRecent.clear();
 		}
 	}
 }

Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/listing/DBTraceCodeManagerTest.java

@@ -201,6 +201,24 @@ public class DBTraceCodeManagerTest extends AbstractGhidraHeadlessIntegrationTes
 		}
 	}
 
+	@Test
+	public void testAddInstructionInScratch() throws CodeUnitInsertionException {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			b.trace.getMemoryManager().putBytes(-5L, b.addr(0x4001), b.buf(0xaa));
+			TraceInstruction i4000 =
+				b.addInstruction(-10, b.addr(0x4000), b.language, b.buf(0xf4, 0));
+			assertEquals(Range.closed(-10L, -6L), i4000.getLifespan());
+
+			TraceInstruction i4004 =
+				b.addInstruction(-1, b.addr(0x4004), b.language, b.buf(0xf4, 0));
+			assertEquals(Range.closed(-1L, -1L), i4004.getLifespan());
+
+			TraceInstruction i4008 =
+				b.addInstruction(-10, b.addr(0x4008), b.language, b.buf(0xf4, 0));
+			assertEquals(Range.closed(-10L, -1L), i4008.getLifespan());
+		}
+	}
+
 	@Test
 	public void testPutBytesTruncatesInstruction() throws CodeUnitInsertionException {
 		try (UndoableTransaction tid = b.startTransaction()) {

Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/memory/AbstractDBTraceMemoryManagerTest.java

@@ -708,6 +708,21 @@ public abstract class AbstractDBTraceMemoryManagerTest
 		assertArrayEquals(arr(0, 0, 9, 10, 5, 6, 7, 8, 3, 4, 17, 18, 0, 0), read.array());
 	}
 
+	@Test
+	public void testGetBytesCrossScratch() {
+		try (UndoableTransaction tid = b.startTransaction()) {
+			assertEquals(4, memory.putBytes(Long.MIN_VALUE, b.addr(0x4000), buf(1, 2, 3, 4)));
+		}
+
+		ByteBuffer read = buf(-1, -1, -1, -1);
+		assertEquals(4, memory.getBytes(1, b.addr(0x4000), read));
+		assertArrayEquals(arr(-1, -1, -1, -1), read.array());
+
+		read.position(0);
+		assertEquals(4, memory.getBytes(-1, b.addr(0x4000), read));
+		assertArrayEquals(arr(1, 2, 3, 4), read.array());
+	}
+
 	@Test
 	public void testPutBytesPackGetBytes() {
 		try (UndoableTransaction tid = b.startTransaction()) {