GP-1493: Validating read/write ranges during emulation

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/AbstractReadsTargetPcodeExecutorState.java

@@ -57,7 +57,7 @@ public abstract class AbstractReadsTargetPcodeExecutorState
 		public byte[] read(long offset, int size) {
 			if (source != null) {
 				AddressSet uninitialized = new AddressSet();
-				for (Range<UnsignedLong> rng : cache.getUninitialized(offset, offset + size)
+				for (Range<UnsignedLong> rng : cache.getUninitialized(offset, offset + size - 1)
 						.asRanges()) {
 					uninitialized.add(space.getAddress(lower(rng)),
 						space.getAddress(upper(rng)));

Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/memory/CachedMemory.java

@@ -58,11 +58,29 @@ public class CachedMemory implements MemoryReader, MemoryWriter {
 	private final MemoryWriter writer;
 
 	protected static class PendingRead {
+		protected static Range<UnsignedLong> normalize(Range<UnsignedLong> range) {
+			if (range.lowerBoundType() == BoundType.CLOSED) {
+				if (range.upperBoundType() == BoundType.OPEN ||
+					range.upperEndpoint().longValue() == -1) {
+					return range;
+				}
+				return Range.closedOpen(range.lowerEndpoint(),
+					range.upperEndpoint().plus(UnsignedLong.ONE));
+			}
+			assert range.lowerEndpoint().longValue() != -1;
+			UnsignedLong lower = range.lowerEndpoint().plus(UnsignedLong.ONE);
+			if (range.upperBoundType() == BoundType.OPEN ||
+				range.upperEndpoint().longValue() == -1) {
+				return Range.closed(lower, range.upperEndpoint());
+			}
+			return Range.closedOpen(lower, range.upperEndpoint().plus(UnsignedLong.ONE));
+		}
+
 		final Range<UnsignedLong> range;
 		final CompletableFuture<Void> future;
 
 		protected PendingRead(Range<UnsignedLong> range, CompletableFuture<Void> future) {
-			this.range = range;
+			this.range = normalize(range);
 			this.future = future;
 		}
 	}
@@ -90,7 +108,7 @@ public class CachedMemory implements MemoryReader, MemoryWriter {
 	}
 
 	protected synchronized CompletableFuture<Void> waitForReads(long addr, int len) {
-		RangeSet<UnsignedLong> undefined = memory.getUninitialized(addr, addr + len);
+		RangeSet<UnsignedLong> undefined = memory.getUninitialized(addr, addr + len - 1);
 		// Do the reads in parallel
 		AsyncFence fence = new AsyncFence();
 		for (Range<UnsignedLong> rng : undefined.asRanges()) {
@@ -115,7 +133,8 @@ public class CachedMemory implements MemoryReader, MemoryWriter {
 			}
 		}
 		NavigableMap<UnsignedLong, PendingRead> applicablePending =
-			pendingByLoc.subMap(rng.lowerEndpoint(), true, rng.upperEndpoint(), false);
+			pendingByLoc.subMap(rng.lowerEndpoint(), true, rng.upperEndpoint(),
+				rng.upperBoundType() == BoundType.CLOSED);
 		for (Map.Entry<UnsignedLong, PendingRead> ent : applicablePending.entrySet()) {
 			PendingRead pending = ent.getValue();
 			if (pending.future.isCompletedExceptionally()) {
@@ -128,7 +147,10 @@ public class CachedMemory implements MemoryReader, MemoryWriter {
 		// Now we're left with a set of needed ranges. Make a request for each
 		for (Range<UnsignedLong> needed : needRequests.asRanges()) {
 			final UnsignedLong lower = needed.lowerEndpoint();
-			final UnsignedLong upper = needed.upperEndpoint();
+			// NB. upper is only used in size computation, so overflow to 0 is no big deal
+			final UnsignedLong upper = needed.upperBoundType() == BoundType.CLOSED
+					? needed.upperEndpoint().plus(UnsignedLong.ONE)
+					: needed.upperEndpoint();
 			/*Msg.debug(this,
 				"Need to read: [" + lower.toString(16) + ":" + upper.toString(16) + ")");*/
 			CompletableFuture<byte[]> futureRead =

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/memory/CachedMemoryTest.java

@@ -17,57 +17,82 @@ package ghidra.dbg.memory;
 
 import static org.junit.Assert.*;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Deque;
+import java.util.LinkedList;
 import java.util.concurrent.*;
 
 import org.junit.Test;
 
-import mockit.Expectations;
-import mockit.Mocked;
-
 public class CachedMemoryTest {
 
-	class ReadRecord extends CompletableFuture<byte[]> {
+	static class RequestRecord<T> {
+		final CompletableFuture<T> future = new CompletableFuture<>();
 		final long address;
+
+		public RequestRecord(long address) {
+			this.address = address;
+		}
+	}
+
+	static class ReadRequestRecord extends RequestRecord<byte[]> {
 		final int length;
 
-		public ReadRecord(long address, int length) {
-			this.address = address;
+		public ReadRequestRecord(long address, int length) {
+			super(address);
 			this.length = length;
 		}
 	}
 
-	class WriteRecord extends CompletableFuture<Void> {
-		final long address;
+	static class WriteRequestRecord extends RequestRecord<Void> {
 		final byte[] data;
 
-		public WriteRecord(long address, byte[] data) {
-			this.address = address;
+		public WriteRequestRecord(long address, byte[] data) {
+			super(address);
 			this.data = data;
 		}
 	}
 
-	class DummyMemory implements MemoryReader, MemoryWriter {
-		final List<CompletableFuture<?>> record = new ArrayList<>();
-
-		@Override
-		public CompletableFuture<Void> writeMemory(long address, byte[] data) {
-			return null;
-		}
+	static class TestMemoryReaderWriter implements MemoryReader, MemoryWriter {
+		Deque<RequestRecord<?>> earlies = new LinkedList<>();
+		Deque<RequestRecord<?>> requests = new LinkedList<>();
 
 		@Override
 		public CompletableFuture<byte[]> readMemory(long address, int length) {
-			return new ReadRecord(address, length);
+			RequestRecord<?> early = earlies.poll();
+			if (early != null) {
+				ReadRequestRecord req = (ReadRequestRecord) early;
+				assertEquals(req.address, address);
+				assertEquals(req.length, length);
+				return req.future;
+			}
+			ReadRequestRecord req = new ReadRequestRecord(address, length);
+			requests.add(req);
+			return req.future;
+		}
+
+		@Override
+		public CompletableFuture<Void> writeMemory(long address, byte[] data) {
+			WriteRequestRecord req = new WriteRequestRecord(address, data);
+			requests.add(req);
+			return req.future;
+		}
+
+		public void expectEarlyRead(long address, byte[] data) {
+			ReadRequestRecord req = new ReadRequestRecord(address, data.length);
+			req.future.complete(data);
+			earlies.add(req);
+		}
+
+		public ReadRequestRecord assertPollRead() {
+			return (ReadRequestRecord) requests.remove();
+		}
+
+		public WriteRequestRecord assertPollWrite() {
+			return (WriteRequestRecord) requests.remove();
 		}
 	}
 
-	interface MemoryReaderWriter extends MemoryReader, MemoryWriter {
-		// Nothing new, just combined interfaces
-	}
-
-	@Mocked
-	protected MemoryReaderWriter memory;
+	protected TestMemoryReaderWriter memory = new TestMemoryReaderWriter();
 
 	byte[] inc(int len) {
 		byte[] result = new byte[len];
@@ -83,30 +108,37 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testSingleRead() throws Exception {
-		final CompletableFuture<byte[]> raw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(1234, 90);
-				result = raw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 		CompletableFuture<byte[]> future = cache.readMemory(1234, 90);
-		raw.complete(inc(90));
+
+		ReadRequestRecord rec = memory.assertPollRead();
+		assertEquals(1234, rec.address);
+		assertEquals(90, rec.length);
+
+		rec.future.complete(inc(90));
 		byte[] arr = future.get(1000, TimeUnit.MILLISECONDS);
 
 		assertArrayEquals(inc(90), arr);
 	}
 
+	@Test
+	public void testSingleReadIncludesMax() throws Exception {
+		CachedMemory cache = new CachedMemory(memory, memory);
+		CompletableFuture<byte[]> future = cache.readMemory(-4, 4);
+
+		ReadRequestRecord rec = memory.assertPollRead();
+		assertEquals(-4, rec.address);
+		assertEquals(4, rec.length);
+
+		rec.future.complete(new byte[] { 1, 2, 3, 4 });
+		byte[] arr = future.get(1000, TimeUnit.MILLISECONDS);
+
+		assertArrayEquals(new byte[] { 1, 2, 3, 4 }, arr);
+	}
+
 	@Test
 	public void testSingleReadCompletedEarly() throws Exception {
-		new Expectations() {
-			{
-				memory.readMemory(1234, 90);
-				result = CompletableFuture.completedFuture(inc(90));
-			}
-		};
+		memory.expectEarlyRead(1234, inc(90));
 
 		CachedMemory cache = new CachedMemory(memory, memory);
 		CompletableFuture<byte[]> future = cache.readMemory(1234, 90);
@@ -117,25 +149,20 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testOverlappingSequentialReads() throws Exception {
-		final CompletableFuture<byte[]> firstRaw = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(1234, 100);
-				result = firstRaw;
-				memory.readMemory(1334, 50);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
 		CompletableFuture<byte[]> first = cache.readMemory(1234, 100);
-		firstRaw.complete(inc(100));
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(1234, req1.address);
+		assertEquals(100, req1.length);
+		req1.future.complete(inc(100));
 		byte[] firstArr = first.get(1000, TimeUnit.MILLISECONDS);
 
 		CompletableFuture<byte[]> second = cache.readMemory(1284, 100);
-		secondRaw.complete(inc(50));
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(1334, req2.address);
+		assertEquals(50, req2.length);
+		req2.future.complete(inc(50));
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 
 		assertArrayEquals(inc(100), firstArr);
@@ -148,23 +175,20 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testOverlappingParallelReads() throws Exception {
-		final CompletableFuture<byte[]> firstRaw = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(1234, 100);
-				result = firstRaw;
-				memory.readMemory(1334, 50);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
 		CompletableFuture<byte[]> first = cache.readMemory(1234, 100);
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(1234, req1.address);
+		assertEquals(100, req1.length);
+
 		CompletableFuture<byte[]> second = cache.readMemory(1284, 100);
-		firstRaw.complete(inc(100));
-		secondRaw.complete(inc(50));
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(1334, req2.address);
+		assertEquals(50, req2.length);
+
+		req1.future.complete(inc(100));
+		req2.future.complete(inc(50));
 		byte[] firstArr = first.get(1000, TimeUnit.MILLISECONDS);
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 
@@ -178,28 +202,24 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testSameStartsGrowingParallelReads() throws Exception {
-		final CompletableFuture<byte[]> firstRaw = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(1234, 50);
-				result = firstRaw;
-				memory.readMemory(1284, 50);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
 		CompletableFuture<byte[]> first = cache.readMemory(1234, 50);
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(1234, req1.address);
+		assertEquals(50, req1.length);
+
 		CompletableFuture<byte[]> second = cache.readMemory(1234, 100);
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(1284, req2.address);
+		assertEquals(50, req2.length);
 
 		assertFalse(first.isDone());
-		firstRaw.complete(inc(50));
+		req1.future.complete(inc(50));
 		byte[] firstArr = first.get(1000, TimeUnit.MILLISECONDS);
 
 		assertFalse(second.isDone());
-		secondRaw.complete(inc(50));
+		req2.future.complete(inc(50));
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 
 		assertArrayEquals(inc(50), firstArr);
@@ -212,23 +232,20 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testLargeOffsetsParallelReads() throws Exception {
-		final CompletableFuture<byte[]> firstRaw = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(0x8000000000000000L, 100);
-				result = firstRaw;
-				memory.readMemory(0x8000000000000000L + 100, 50);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
-		CompletableFuture<byte[]> first = cache.readMemory(0x8000000000000000L, 100);
-		CompletableFuture<byte[]> second = cache.readMemory(0x8000000000000000L + 50, 100);
-		firstRaw.complete(inc(100));
-		secondRaw.complete(inc(50));
+		CompletableFuture<byte[]> first = cache.readMemory(0x8000_0000_0000_0000L, 100);
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(0x8000_0000_0000_0000L, req1.address);
+		assertEquals(100, req1.length);
+
+		CompletableFuture<byte[]> second = cache.readMemory(0x8000_0000_0000_0000L + 50, 100);
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(0x8000_0000_0000_0000L + 100, req2.address);
+		assertEquals(50, req2.length);
+
+		req1.future.complete(inc(100));
+		req2.future.complete(inc(50));
 		byte[] firstArr = first.get(1000, TimeUnit.MILLISECONDS);
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 
@@ -242,22 +259,15 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testErroneousRead() throws Exception {
-		final CompletableFuture<byte[]> firstErr = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(0, 100);
-				result = firstErr;
-				memory.readMemory(50, 100);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
 		CompletableFuture<byte[]> first = cache.readMemory(0, 100);
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(0, req1.address);
+		assertEquals(100, req1.length);
+
 		Throwable sentinel = new AssertionError("Sentinel");
-		firstErr.completeExceptionally(sentinel);
+		req1.future.completeExceptionally(sentinel);
 		try {
 			first.get(1000, TimeUnit.MILLISECONDS);
 			fail();
@@ -267,7 +277,10 @@ public class CachedMemoryTest {
 		}
 
 		CompletableFuture<byte[]> second = cache.readMemory(50, 100);
-		secondRaw.complete(inc(100));
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(50, req2.address);
+		assertEquals(100, req2.length);
+		req2.future.complete(inc(100));
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 
 		assertArrayEquals(inc(100), secondArr);
@@ -275,26 +288,23 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testPartialResult() throws Exception {
-		final CompletableFuture<byte[]> firstPartial = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(0, 100);
-				result = firstPartial;
-				memory.readMemory(50, 50);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
 		CompletableFuture<byte[]> first = cache.readMemory(0, 100);
-		firstPartial.complete(inc(50)); // request was for 100!
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(0, req1.address);
+		assertEquals(100, req1.length);
+
+		req1.future.complete(inc(50)); // request was for 100!
 		byte[] firstArr = first.get(1000, TimeUnit.MILLISECONDS);
 		assertArrayEquals(inc(50), firstArr);
 
 		CompletableFuture<byte[]> second = cache.readMemory(25, 75);
-		secondRaw.complete(inc(50));
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(50, req2.address);
+		assertEquals(50, req2.length);
+
+		req2.future.complete(inc(50));
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 
 		byte[] dinc = new byte[75];
@@ -305,24 +315,20 @@ public class CachedMemoryTest {
 
 	@Test
 	public void testDisjointParallellFirstErrs() throws Exception {
-		final CompletableFuture<byte[]> firstErr = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(0, 25);
-				result = firstErr;
-				memory.readMemory(50, 25);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
 		CompletableFuture<byte[]> first = cache.readMemory(0, 25);
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(0, req1.address);
+		assertEquals(25, req1.length);
+
 		CompletableFuture<byte[]> second = cache.readMemory(50, 25);
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(50, req2.address);
+		assertEquals(25, req2.length);
 
 		Throwable sentinel = new AssertionError("Sentinel");
-		firstErr.completeExceptionally(sentinel);
+		req1.future.completeExceptionally(sentinel);
 		try {
 			first.get(0, TimeUnit.MILLISECONDS);
 			fail();
@@ -331,31 +337,27 @@ public class CachedMemoryTest {
 			assertEquals(sentinel, e.getCause());
 		}
 
-		secondRaw.complete(inc(25));
+		req2.future.complete(inc(25));
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 		assertArrayEquals(inc(25), secondArr);
 	}
 
 	@Test
 	public void testPartialFromErr() throws Exception {
-		final CompletableFuture<byte[]> firstErr = new CompletableFuture<>();
-		final CompletableFuture<byte[]> secondRaw = new CompletableFuture<>();
-		new Expectations() {
-			{
-				memory.readMemory(50, 50);
-				result = firstErr;
-				memory.readMemory(0, 50);
-				result = secondRaw;
-			}
-		};
-
 		CachedMemory cache = new CachedMemory(memory, memory);
 
 		CompletableFuture<byte[]> first = cache.readMemory(50, 50);
+		ReadRequestRecord req1 = memory.assertPollRead();
+		assertEquals(50, req1.address);
+		assertEquals(50, req1.length);
+
 		CompletableFuture<byte[]> second = cache.readMemory(0, 100);
+		ReadRequestRecord req2 = memory.assertPollRead();
+		assertEquals(0, req2.address);
+		assertEquals(50, req2.length);
 
 		Throwable sentinel = new AssertionError("Sentinel");
-		firstErr.completeExceptionally(sentinel);
+		req1.future.completeExceptionally(sentinel);
 		try {
 			first.get(0, TimeUnit.MILLISECONDS);
 			fail();
@@ -364,7 +366,7 @@ public class CachedMemoryTest {
 			assertEquals(sentinel, e.getCause());
 		}
 		// First should still succeed partially
-		secondRaw.complete(inc(50));
+		req2.future.complete(inc(50));
 		byte[] secondArr = second.get(1000, TimeUnit.MILLISECONDS);
 		assertArrayEquals(inc(50), secondArr);
 	}

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/pcode/exec/trace/AbstractCheckedTraceCachedWriteBytesPcodeExecutorState.java

@@ -48,7 +48,8 @@ public abstract class AbstractCheckedTraceCachedWriteBytesPcodeExecutorState
 
 		@Override
 		public byte[] read(long offset, int size) {
-			RangeSet<UnsignedLong> uninitialized = cache.getUninitialized(offset, offset + size);
+			RangeSet<UnsignedLong> uninitialized =
+				cache.getUninitialized(offset, offset + size - 1);
 
 			if (!uninitialized.isEmpty()) {
 				size = checkUninitialized(source, space.getAddress(offset), size,

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/pcode/exec/trace/TraceCachedWriteBytesPcodeExecutorState.java

@@ -155,7 +155,7 @@ public class TraceCachedWriteBytesPcodeExecutorState
 				// TODO: Warn or bail when reading UNKNOWN bytes
 				// NOTE: Read without regard to gaps
 				// NOTE: Cannot write those gaps, though!!!
-				readUninitializedFromSource(cache.getUninitialized(offset, offset + size));
+				readUninitializedFromSource(cache.getUninitialized(offset, offset + size - 1));
 			}
 			return readCached(offset, size);
 		}

Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/pcode/exec/trace/TracePcodeEmulatorTest.java

@@ -900,4 +900,87 @@ public class TracePcodeEmulatorTest extends AbstractGhidraHeadlessIntegrationTes
 				TraceSleighUtils.evaluate("RCX", tb.trace, 1, thread, 0));
 		}
 	}
+
+	/**
+	 * Test the read max boundary case
+	 * 
+	 * <p>
+	 * This happens very easily when RBP is uninitialized, as code commonly uses negative offsets
+	 * from RBP. The range will have upper endpoint {@code ULONG_MAX+1}, non-inclusive, which would
+	 * crash, instead requiring some special logic.
+	 */
+	@Test
+	public void testMOV_EAX_dword_RBPm4() throws Throwable {
+		try (ToyDBTraceBuilder tb = new ToyDBTraceBuilder("Test", "x86:LE:64:default")) {
+			TraceThread thread = initTrace(tb,
+				List.of(
+					"RIP = 0x00400000;",
+					"RSP = 0x00110000;",
+					"*:4 (0:8-4) = 0x12345678;"),
+				List.of(
+					"MOV EAX, dword ptr [RBP + -0x4]"));
+
+			TracePcodeEmulator emu = new TracePcodeEmulator(tb.trace, 0);
+			PcodeThread<byte[]> emuThread = emu.newThread(thread.getPath());
+			emuThread.overrideContextWithDefault();
+			emuThread.stepInstruction();
+
+			try (UndoableTransaction tid = tb.startTransaction()) {
+				emu.writeDown(tb.trace, 1, 1, false);
+			}
+
+			assertEquals(BigInteger.valueOf(0x12345678),
+				TraceSleighUtils.evaluate("EAX", tb.trace, 1, thread, 0));
+		}
+	}
+
+	/**
+	 * Test the read wrap-around case for x86_64
+	 * 
+	 * <p>
+	 * This tests a rare (I hope) case where a read would wrap around the address space: 2 bytes
+	 * including the max address, and 2 bytes at the min address. I imagine the behavior here varies
+	 * by architecture? TODO: For now, I think it's acceptable just to throw an exception, but in
+	 * reality, we should probably handle it and allow some mechanism for architectures to forbid
+	 * it, if that's in fact what they do.
+	 */
+	@Test(expected = PcodeExecutionException.class)
+	public void testMOV_EAX_dword_RBPm2_x64() throws Throwable {
+		try (ToyDBTraceBuilder tb = new ToyDBTraceBuilder("Test", "x86:LE:64:default")) {
+			TraceThread thread = initTrace(tb,
+				List.of(
+					"RIP = 0x00400000;",
+					"RSP = 0x00110000;"),
+				List.of(
+					"MOV EAX, dword ptr [RBP + -0x2]"));
+
+			TracePcodeEmulator emu = new TracePcodeEmulator(tb.trace, 0);
+			PcodeThread<byte[]> emuThread = emu.newThread(thread.getPath());
+			emuThread.overrideContextWithDefault();
+			emuThread.stepInstruction();
+		}
+	}
+
+	/**
+	 * Test the read wrap-around case for x86 (32)
+	 * 
+	 * <p>
+	 * This test ensures the rule applies for spaces smaller than 64 bits
+	 */
+	@Test(expected = PcodeExecutionException.class)
+	public void testMOV_EAX_dword_EBPm2_x86() throws Throwable {
+		try (ToyDBTraceBuilder tb = new ToyDBTraceBuilder("Test", "x86:LE:32:default")) {
+			TraceThread thread = initTrace(tb,
+				List.of(
+					"EIP = 0x00400000;",
+					"ESP = 0x00110000;"),
+				List.of(
+					"MOV EAX, dword ptr [EBP + -0x2]"));
+
+			TracePcodeEmulator emu = new TracePcodeEmulator(tb.trace, 0);
+			PcodeThread<byte[]> emuThread = emu.newThread(thread.getPath());
+			emuThread.overrideContextWithDefault();
+			emuThread.stepInstruction();
+		}
+	}
 }

Ghidra/Debug/ProposedUtils/src/main/java/ghidra/generic/util/datastruct/SemisparseByteArray.java

@@ -137,32 +137,32 @@ public class SemisparseByteArray {
 	 * Enumerate the initialized ranges within the given range
 	 * 
 	 * <p>
-	 * The given range is interpreted as closed-open, i.e., [a, b).
+	 * The given range is interpreted as closed, i.e., [a, b].
 	 * 
 	 * @param a the lower-bound, inclusive, of the range
-	 * @param b the upper-bound, exclusive, of the range
+	 * @param b the upper-bound, inclusive, of the range
 	 * @return the set of initialized ranges
 	 */
 	public synchronized RangeSet<UnsignedLong> getInitialized(long a, long b) {
 		UnsignedLong ua = UnsignedLong.fromLongBits(a);
 		UnsignedLong ub = UnsignedLong.fromLongBits(b);
-		return ImmutableRangeSet.copyOf(defined.subRangeSet(Range.closedOpen(ua, ub)));
+		return ImmutableRangeSet.copyOf(defined.subRangeSet(Range.closed(ua, ub)));
 	}
 
 	/**
 	 * Check if a range is completely initialized
 	 * 
 	 * <p>
-	 * The given range is interpreted as closed-open, i.e., [a,b).
+	 * The given range is interpreted as closed, i.e., [a, b].
 	 * 
 	 * @param a the lower-bound, inclusive, of the range
-	 * @param b the upper-bound, exclusive, of the range
+	 * @param b the upper-bound, inclusive, of the range
 	 * @return true if all indices in the range are initialized, false otherwise
 	 */
 	public synchronized boolean isInitialized(long a, long b) {
 		UnsignedLong ua = UnsignedLong.fromLongBits(a);
 		UnsignedLong ub = UnsignedLong.fromLongBits(b);
-		return defined.encloses(Range.closedOpen(ua, ub));
+		return defined.encloses(Range.closed(ua, ub));
 	}
 
 	/**
@@ -179,16 +179,16 @@ public class SemisparseByteArray {
 	 * Enumerate the uninitialized ranges within the given range
 	 * 
 	 * <p>
-	 * The given range is interpreted as closed-open, i.e., [a, b).
+	 * The given range is interpreted as closed, i.e., [a, b].
 	 * 
 	 * @param a the lower-bound, inclusive, of the range
-	 * @param b the upper-bound, exclusive, of the range
+	 * @param b the upper-bound, inclusive, of the range
 	 * @return the set of uninitialized ranges
 	 */
 	public synchronized RangeSet<UnsignedLong> getUninitialized(long a, long b) {
 		UnsignedLong ua = UnsignedLong.fromLongBits(a);
 		UnsignedLong ub = UnsignedLong.fromLongBits(b);
-		return ImmutableRangeSet.copyOf(defined.complement().subRangeSet(Range.closedOpen(ua, ub)));
+		return ImmutableRangeSet.copyOf(defined.complement().subRangeSet(Range.closed(ua, ub)));
 	}
 
 	/**
@@ -215,9 +215,17 @@ public class SemisparseByteArray {
 		if (length < 0) {
 			throw new IllegalArgumentException("length: " + length);
 		}
+		if (loc + length < loc) {
+			throw new IndexOutOfBoundsException("given offset and length would exceed ULONG_MAX");
+		}
 		UnsignedLong uLoc = UnsignedLong.fromLongBits(loc);
 		UnsignedLong uEnd = UnsignedLong.fromLongBits(loc + length);
-		defined.add(Range.closedOpen(uLoc, uEnd));
+		if (uEnd.longValue() == 0) {
+			defined.add(Range.closed(uLoc, UnsignedLong.MAX_VALUE));
+		}
+		else {
+			defined.add(Range.closedOpen(uLoc, uEnd));
+		}
 
 		// Write out portion of first block (could be full block)
 		long blockNum = Long.divideUnsigned(loc, BLOCK_SIZE);
@@ -253,6 +261,12 @@ public class SemisparseByteArray {
 			return 0;
 		}
 		UnsignedLong diff = rng.upperEndpoint().minus(uLoc);
+		if (diff.longValue() == -1) {
+			return Integer.MAX_VALUE;
+		}
+		if (rng.upperBoundType() == BoundType.CLOSED) {
+			diff = diff.plus(UnsignedLong.ONE);
+		}
 		if (diff.compareTo(UnsignedLong.valueOf(Integer.MAX_VALUE)) >= 0) {
 			return Integer.MAX_VALUE;
 		}

Ghidra/Debug/ProposedUtils/src/main/java/ghidra/pcode/exec/AbstractLongOffsetPcodeExecutorStatePiece.java

@@ -63,6 +63,7 @@ public abstract class AbstractLongOffsetPcodeExecutorStatePiece<A, T, S>
 	@Override
 	public void setVar(AddressSpace space, long offset, int size, boolean truncateAddressableUnit,
 			T val) {
+		checkRange(space, offset, size);
 		if (space.isConstantSpace()) {
 			throw new IllegalArgumentException("Cannot write to constant space");
 		}
@@ -82,6 +83,7 @@ public abstract class AbstractLongOffsetPcodeExecutorStatePiece<A, T, S>
 
 	@Override
 	public T getVar(AddressSpace space, long offset, int size, boolean truncateAddressableUnit) {
+		checkRange(space, offset, size);
 		if (space.isConstantSpace()) {
 			return arithmetic.fromConst(offset, size);
 		}

Ghidra/Debug/ProposedUtils/src/main/java/ghidra/pcode/exec/PcodeExecutorStatePiece.java

@@ -15,14 +15,23 @@
  */
 package ghidra.pcode.exec;
 
-import ghidra.program.model.address.Address;
-import ghidra.program.model.address.AddressSpace;
+import ghidra.program.model.address.*;
 import ghidra.program.model.lang.Register;
 import ghidra.program.model.mem.MemBuffer;
 import ghidra.program.model.pcode.Varnode;
 
 public interface PcodeExecutorStatePiece<A, T> {
 
+	default void checkRange(AddressSpace space, long offset, int size) {
+		// TODO: Perhaps get/setVar should just take an AddressRange?
+		try {
+			new AddressRangeImpl(space.getAddress(offset), size);
+		}
+		catch (AddressOverflowException | AddressOutOfBoundsException e) {
+			throw new IllegalArgumentException("Given offset and length exceeds address space");
+		}
+	}
+
 	A longToOffset(AddressSpace space, long l);
 
 	default void setVar(Register reg, T val) {
@@ -39,6 +48,7 @@ public interface PcodeExecutorStatePiece<A, T> {
 
 	default void setVar(AddressSpace space, long offset, int size, boolean truncateAddressableUnit,
 			T val) {
+		checkRange(space, offset, size);
 		setVar(space, longToOffset(space, offset), size, truncateAddressableUnit, val);
 	}
 
@@ -56,6 +66,7 @@ public interface PcodeExecutorStatePiece<A, T> {
 	T getVar(AddressSpace space, A offset, int size, boolean truncateAddressableUnit);
 
 	default T getVar(AddressSpace space, long offset, int size, boolean truncateAddressableUnit) {
+		checkRange(space, offset, size);
 		return getVar(space, longToOffset(space, offset), size, truncateAddressableUnit);
 	}
 

Ghidra/Debug/ProposedUtils/src/test/java/ghidra/generic/util/datastruct/SemisparseByteArrayTest.java

@@ -44,16 +44,16 @@ public class SemisparseByteArrayTest {
 		cache.putData(0, HW, 0, 1);
 		exp.clear();
 		exp.add(makeRange(0, 1));
-		assertEquals(exp, cache.getInitialized(0, HW.length + 8));
+		assertEquals(exp, cache.getInitialized(0, HW.length + 7));
 		exp.clear();
 		exp.add(makeRange(1, HW.length));
-		assertEquals(exp, cache.getUninitialized(0, HW.length));
+		assertEquals(exp, cache.getUninitialized(0, HW.length - 1));
 
 		cache.putData(2, HW, 2, 1);
 		exp.clear();
 		exp.add(makeRange(1, 2));
 		exp.add(makeRange(3, HW.length));
-		assertEquals(exp, cache.getUninitialized(0, HW.length));
+		assertEquals(exp, cache.getUninitialized(0, HW.length - 1));
 
 		cache.putData(11, HW, 11, 2);
 		byte[] read = new byte[HW.length + 5]; // 5 extra