Merge remote-tracking branch 'origin/GP-6933_Dan_improvePutBytesWhenEventsDisabled'

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

@@ -16,6 +16,7 @@
 package ghidra.trace.database.listing;
 
 import java.io.IOException;
+import java.nio.ByteBuffer;
 import java.util.*;
 import java.util.concurrent.locks.ReadWriteLock;
 
@@ -261,7 +262,7 @@ public class DBTraceCodeSpace implements TraceCodeSpace, DBTraceSpaceBased {
 
 	@Override
 	public void invalidateCache() {
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			instructionMapSpace.invalidateCache();
 			instructions.invalidateCache();
 
@@ -287,7 +288,7 @@ public class DBTraceCodeSpace implements TraceCodeSpace, DBTraceSpaceBased {
 	 * @param newBytes the new bytes
 	 */
 	public void bytesChanged(Set<TraceAddressSnapRange> changed, long snap, Address start,
-			byte[] oldBytes, byte[] newBytes) {
+			ByteBuffer oldBytes, ByteBuffer newBytes) {
 		AddressSet diffs = ByteArrayUtils.computeDiffsAddressSet(start, oldBytes, newBytes);
 		Set<AbstractDBTraceCodeUnit<?>> affectedUnits = new HashSet<>();
 		for (TraceAddressSnapRange box : changed) {
@@ -301,8 +302,8 @@ public class DBTraceCodeSpace implements TraceCodeSpace, DBTraceSpaceBased {
 			}
 		}
 
-		MemBuffer newBuf =
-			new ByteMemBufferImpl(start, newBytes, trace.getBaseLanguage().isBigEndian());
+		MemBuffer newBuf = new ByteMemBufferImpl(start, ByteArrayUtils.arrayOrGet(newBytes),
+			trace.getBaseLanguage().isBigEndian());
 		for (AbstractDBTraceCodeUnit<?> unit : affectedUnits) {
 			// Rule:
 			//     Break unit down into time portions before affected range, and at/within range
@@ -321,12 +322,10 @@ public class DBTraceCodeSpace implements TraceCodeSpace, DBTraceSpaceBased {
 				unitStartSnap = unit.getStartSnap();
 				unit.delete();
 			}
-			if (unit instanceof DBTraceData) {
-				DBTraceData dataUnit = (DBTraceData) unit;
-				boolean reApply = false;
+			if (unit instanceof DBTraceData dataUnit) {
+				boolean reApply;
 				DataType dataType = dataUnit.getDataType();
-				if (dataType instanceof Dynamic) {
-					Dynamic ddt = (Dynamic) dataType;
+				if (dataType instanceof Dynamic ddt) {
 					WrappedMemBuffer newWrapped =
 						new WrappedMemBuffer(newBuf, (int) dataUnit.getAddress().subtract(start));
 					int newLength = ddt.getLength(newWrapped, dataUnit.getLength());

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

@@ -40,8 +40,7 @@ import ghidra.trace.database.space.DBTraceSpaceBased;
 import ghidra.trace.model.*;
 import ghidra.trace.model.memory.TraceMemorySpace;
 import ghidra.trace.model.memory.TraceMemoryState;
-import ghidra.trace.util.TraceChangeRecord;
-import ghidra.trace.util.TraceEvents;
+import ghidra.trace.util.*;
 import ghidra.util.*;
 import ghidra.util.AddressIteratorAdapter;
 import ghidra.util.database.*;
@@ -340,7 +339,7 @@ public class DBTraceMemorySpace
 	// TODO: Ensure a code unit is not having rug taken out from under it?
 	public void setState(long snap, Address start, Address end, TraceMemoryState state) {
 		checkState(state);
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			doSetState(snap, start, end, state);
 		}
 	}
@@ -348,7 +347,7 @@ public class DBTraceMemorySpace
 	@Override
 	public void setState(long snap, AddressRange range, TraceMemoryState state) {
 		checkState(state);
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			doSetState(snap, range.getMinAddress(), range.getMaxAddress(), state);
 		}
 	}
@@ -356,7 +355,7 @@ public class DBTraceMemorySpace
 	@Override
 	public void setState(long snap, Address address, TraceMemoryState state) {
 		checkState(state);
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			doSetState(snap, address, address, state);
 		}
 	}
@@ -364,7 +363,7 @@ public class DBTraceMemorySpace
 	@Override
 	public void setState(long snap, AddressSetView set, TraceMemoryState state) {
 		checkState(state);
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			for (AddressRange range : set) {
 				doSetState(snap, range.getMinAddress(), range.getMaxAddress(), state);
 			}
@@ -413,7 +412,7 @@ public class DBTraceMemorySpace
 			Address address) {
 		// LATER: Cache here or on the delegate?
 		return mostRecentStateEntryCache.computeIfAbsent(new MostRecentStateCacheKey(snap, address),
-			k -> getViewMostRecentStateEntry(snap, new AddressRangeImpl(address, address),
+			_ -> getViewMostRecentStateEntry(snap, new AddressRangeImpl(address, address),
 				StatePredicate.IS_KNOWN_OR_ERROR));
 	}
 
@@ -454,7 +453,7 @@ public class DBTraceMemorySpace
 	@Override
 	public AddressSetView getAddressesWithState(Lifespan span, AddressSetView set,
 			Predicate<TraceMemoryState> predicate) {
-		try (LockHold hold = LockHold.lock(lock.readLock())) {
+		try (LockHold _ = LockHold.lock(lock.readLock())) {
 			if (!(predicate instanceof StatePredicate stock)) {
 				return doGetAddressesWithState(span, set, predicate);
 			}
@@ -469,7 +468,7 @@ public class DBTraceMemorySpace
 
 				result.add(addressSetStateCache.computeIfAbsent(
 					new AddressSetStateCacheKey(span, blockMinOffset, stock),
-					k -> doGetAddressesWithState(span, new AddressSet(blockMin, blockMax), stock)));
+					_ -> doGetAddressesWithState(span, new AddressSet(blockMin, blockMax), stock)));
 			}
 			return result.intersect(set);
 		}
@@ -708,11 +707,12 @@ public class DBTraceMemorySpace
 	@Override
 	public int putBytes(long snap, Address start, ByteBuffer buf) {
 		assertInSpace(start);
-		int pos = buf.position();
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
 
+		int pos = buf.position();
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			ByteBuffer oldBuf = ByteBuffer.allocate(buf.remaining());
 			getBytes(snap, start, oldBuf);
+			oldBuf.flip();
 
 			OutSnap lastSnap = new OutSnap(snap);
 			Set<TraceAddressSnapRange> changed = new HashSet<>();
@@ -721,20 +721,31 @@ public class DBTraceMemorySpace
 				Address end = start.add(result - 1);
 				doSetState(snap, start, end, TraceMemoryState.KNOWN);
 
-				// Read back the written bytes and fire event
-				byte[] bytes = new byte[result];
-				byte[] oldBytes = new byte[result];
-				buf.get(pos, bytes);
-				oldBuf.get(0, oldBytes);
 				ImmutableTraceAddressSnapRange tasr = new ImmutableTraceAddressSnapRange(start,
 					start.add(result - 1), snap, lastSnap.snap);
-				trace.setChanged(new TraceChangeRecord<>(TraceEvents.BYTES_CHANGED, space, tasr,
-					oldBytes, bytes));
+				// Fire event
+				if (trace.isSendingEvents()) {
+					byte[] bytes = ByteArrayUtils.arrayOrGet(buf);
+					byte[] oldBytes = ByteArrayUtils.arrayOrGet(oldBuf);
+					trace.setChanged(new TraceChangeRecord<>(TraceEvents.BYTES_CHANGED, space, tasr,
+						oldBytes, bytes));
+				}
 
 				// Fixup affected code units
 				DBTraceCodeSpace codeSpace = trace.getCodeManager().get(space, false);
 				if (codeSpace != null) {
-					codeSpace.bytesChanged(changed, snap, start, oldBytes, bytes);
+					int savePos = buf.position();
+					int saveLimit = buf.limit();
+					try {
+						buf.limit(pos + result);
+						buf.position(pos);
+						oldBuf.limit(oldBuf.position() + result); // No need to save & restore
+						codeSpace.bytesChanged(changed, snap, start, oldBuf, buf);
+					}
+					finally {
+						buf.limit(saveLimit);
+						buf.position(savePos);
+					}
 				}
 				// Clear program view caches
 				trace.updateViewsBytesChanged(tasr.getRange());
@@ -763,7 +774,7 @@ public class DBTraceMemorySpace
 	public int getBytes(long snap, Address start, ByteBuffer buf) {
 		assertInSpace(start);
 		int result = 0;
-		try (LockHold hold = LockHold.lock(lock.readLock())) {
+		try (LockHold _ = LockHold.lock(lock.readLock())) {
 			int maxLen;
 			for (Address cur = start; buf.hasRemaining(); cur = cur.addNoWrap(maxLen)) {
 				long offset = cur.getOffset();
@@ -961,7 +972,7 @@ public class DBTraceMemorySpace
 	@Override
 	public Long getSnapOfMostRecentChangeToBlock(long snap, Address address) {
 		assertInSpace(address);
-		try (LockHold hold = LockHold.lock(lock.readLock())) {
+		try (LockHold _ = LockHold.lock(lock.readLock())) {
 			long offset = address.getOffset();
 			long roundOffset = offset & BLOCK_MASK;
 			OffsetSnap loc = new OffsetSnap(roundOffset, snap);
@@ -1012,7 +1023,7 @@ public class DBTraceMemorySpace
 		Lifespan fwdOne = Lifespan.span(lower + 1, cross ? -1 : upper);
 		ByteBuffer buf1 = ByteBuffer.allocate(BLOCK_SIZE);
 		ByteBuffer buf2 = ByteBuffer.allocate(BLOCK_SIZE);
-		try (LockHold hold = LockHold.lock(lock.readLock())) {
+		try (LockHold _ = LockHold.lock(lock.readLock())) {
 			for (TraceAddressSnapRange tasr : stateMapSpace
 					.reduce(TraceAddressSnapRangeQuery.leastRecent(range, fwdOne))
 					.orderedKeys()) {
@@ -1039,7 +1050,7 @@ public class DBTraceMemorySpace
 		if (len <= 0) {
 			return;
 		}
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			ByteBuffer oldBytes = ByteBuffer.allocate(len);
 			getBytes(snap, start, oldBytes);
 			// New in the sense that they're about to replace the old bytes
@@ -1064,7 +1075,7 @@ public class DBTraceMemorySpace
 			// Fixup affected code units
 			DBTraceCodeSpace codeSpace = trace.getCodeManager().get(space, false);
 			if (codeSpace != null) {
-				codeSpace.bytesChanged(changed, snap, start, oldBytes.array(), newBytes.array());
+				codeSpace.bytesChanged(changed, snap, start, oldBytes, newBytes);
 			}
 		}
 		catch (IOException e) {
@@ -1079,7 +1090,7 @@ public class DBTraceMemorySpace
 
 	@Override
 	public void pack() {
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			// TODO: Check and rearrange blocks chronologically
 			// TODO: Remove identical, adjacent future blocks
 			for (DBTraceMemoryBufferEntry bufEnt : bufferStore.asMap().values()) {
@@ -1093,7 +1104,7 @@ public class DBTraceMemorySpace
 
 	@Override
 	public void invalidateCache() {
-		try (LockHold hold = LockHold.lock(lock.writeLock())) {
+		try (LockHold _ = LockHold.lock(lock.writeLock())) {
 			trace.updateViewsRefreshBlocks();
 			trace.updateViewsBytesChanged(null);
 			stateMapSpace.invalidateCache();
@@ -1107,6 +1118,9 @@ public class DBTraceMemorySpace
 
 	/**
 	 * For developers and testers.
+	 * 
+	 * @param painter painter
+	 * @param depth depth
 	 */
 	@Internal
 	public void paint(Painter painter, int depth) {
@@ -1115,6 +1129,8 @@ public class DBTraceMemorySpace
 
 	/**
 	 * For developers and testers.
+	 * 
+	 * @return depth
 	 */
 	@Internal
 	public int getDepth() {
@@ -1123,6 +1139,8 @@ public class DBTraceMemorySpace
 
 	/**
 	 * For developers and testers.
+	 * 
+	 * @return root bounds
 	 */
 	@Internal
 	public TraceAddressSnapRange getRootBounds() {
@@ -1131,6 +1149,9 @@ public class DBTraceMemorySpace
 
 	/**
 	 * For developers and testers.
+	 * 
+	 * @param rec record
+	 * @return children
 	 */
 	@Internal
 	public Collection<? extends DBTreeRecord<?, ? extends TraceAddressSnapRange>>

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/ByteArrayUtils.java

@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -15,6 +15,8 @@
  */
 package ghidra.trace.util;
 
+import java.nio.ByteBuffer;
+
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSet;
 
@@ -22,25 +24,29 @@ public enum ByteArrayUtils {
 	;
 
 	/**
-	 * Compute the address set where two byte arrays differ, given a start address
+	 * Compute the address set where two byte buffers differ, given a start address
 	 * 
-	 * @param start the address of the first byte in each array
-	 * @param a the first array
-	 * @param b the second array
-	 * @return the address set where the arrays differ
+	 * @param start the address of the byte at each buffer's current position
+	 * @param a the first buffer
+	 * @param b the second buffer
+	 * @return the address set where the buffers differ
+	 * @throws IllegalArgumentException if the two buffers have different amounts remaining
 	 */
-	public static AddressSet computeDiffsAddressSet(Address start, byte[] a, byte[] b) {
-		if (a.length != b.length) {
-			throw new IllegalArgumentException("Arrays must be the same length");
+	public static AddressSet computeDiffsAddressSet(Address start, ByteBuffer a, ByteBuffer b) {
+		int length = a.remaining();
+		if (length != b.remaining()) {
+			throw new IllegalArgumentException("Buffers must have the same remaining count");
 		}
 		// A means of early parameter checking, and I'll need it later
-		Address end = start.add(a.length - 1);
+		Address end = start.add(length - 1);
 
 		AddressSet result = new AddressSet();
 
 		Address diffStart = null;
-		for (int i = 0; i < a.length; i++) {
-			if (a[i] == b[i]) {
+		int aPos = a.position();
+		int bPos = b.position();
+		for (int i = 0; i < length; i++) {
+			if (a.get(aPos + i) == b.get(bPos + i)) {
 				if (diffStart != null) {
 					result.add(diffStart, start.add(i - 1));
 				}
@@ -56,4 +62,23 @@ public enum ByteArrayUtils {
 		}
 		return result;
 	}
+
+	/**
+	 * Get or copy a byte buffer's backing array.
+	 * <p>
+	 * If the buffer's position and array offset is 0, then this returns the backing array.
+	 * Otherwise, this copies the remaining contents of the buffer into a new array, without
+	 * affecting the buffer's position.
+	 * 
+	 * @param buf the buffer
+	 * @return the backing array or a new array with the buffer's remaining contents
+	 */
+	public static byte[] arrayOrGet(ByteBuffer buf) {
+		if (buf.hasArray() && buf.arrayOffset() == 0 && buf.position() == 0) {
+			return buf.array();
+		}
+		byte[] arr = new byte[buf.remaining()];
+		buf.get(buf.position(), arr);
+		return arr;
+	}
 }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/TraceEvents.java

@@ -194,9 +194,10 @@ public interface TraceEvents {
 
 	/**
 	 * The {@link Trace}'s memory or register values were changed.
-	 * 
 	 * <p>
-	 * Note the given byte arrays may be larger than the actual change.
+	 * Note the given byte arrays may be larger than the actual change. Do not modify the byte
+	 * arrays. They may be references to internal working copies. Changing them could cause
+	 * undefined behavior.
 	 */
 	TraceBytesEvent BYTES_CHANGED = TraceBytesEvent.BYTES_CHANGED;