Merge remote-tracking branch 'origin/GP-1256_Dan_bitRegisters--SQUASHED' into patch

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/listing/TraceDefinedDataRegisterView.java

@@ -26,7 +26,8 @@ public interface TraceDefinedDataRegisterView
 		extends TraceDefinedDataView, TraceBaseDefinedRegisterView<TraceData> {
 	default TraceData create(Range<Long> lifespan, Register register, DataType dataType)
 			throws CodeUnitInsertionException {
+		TraceRegisterUtils.requireByteBound(register);
 		return create(lifespan, register.getAddress(), dataType,
-			TraceRegisterUtils.byteLengthOf(register));
+			register.getNumBytes());
 	}
 }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/memory/TraceMemoryRegisterSpace.java

@@ -54,13 +54,50 @@ public interface TraceMemoryRegisterSpace extends TraceMemorySpace {
 		return getStates(snap, TraceRegisterUtils.rangeForRegister(register));
 	}
 
+	/**
+	 * Set the value of a register at the given snap
+	 * 
+	 * <p>
+	 * <b>IMPORTANT:</b> The trace database cannot track the state ({@link TraceMemoryState#KNOWN},
+	 * etc.) with per-bit accuracy. It only has byte precision. If the given value specifies, e.g.,
+	 * only a single bit, then the entire byte will become marked {@link TraceMemoryState#KNOWN},
+	 * even though the remaining 7 bits could technically be unknown.
+	 * 
+	 * @param snap the snap
+	 * @param value the register value
+	 * @return the number of bytes written
+	 */
 	default int setValue(long snap, RegisterValue value) {
-		ByteBuffer buf = TraceRegisterUtils.bufferForValue(value);
-		return putBytes(snap, value.getRegister().getAddress(), buf);
+		if (!value.hasAnyValue()) {
+			return 0;
+		}
+		Register reg = value.getRegister();
+		if (!value.hasValue() || !TraceRegisterUtils.isByteBound(reg)) {
+			RegisterValue old = getValue(snap, reg.getBaseRegister());
+			// Do not use .getRegisterValue, as that will zero unmasked bits
+			// Instead, we'll pass the original register to bufferForValue
+			value = old.combineValues(value);
+		}
+		ByteBuffer buf = TraceRegisterUtils.bufferForValue(reg, value);
+		return putBytes(snap, reg.getAddress(), buf);
 	}
 
+	/**
+	 * Write bytes at the given snap and register address
+	 * 
+	 * <p>
+	 * Note that bit-masked registers are not properly heeded. If the caller wishes to preserve
+	 * non-masked bits, it must first retrieve the current value and combine it with the desired
+	 * value. The caller must also account for any bit shift in the passed buffer. Alternatively,
+	 * consider {@link #setValue(long, RegisterValue)}.
+	 * 
+	 * @param snap the snap
+	 * @param register the register to modify
+	 * @param buf the buffer of bytes to write
+	 * @return the number of bytes written
+	 */
 	default int putBytes(long snap, Register register, ByteBuffer buf) {
-		int byteLength = TraceRegisterUtils.byteLengthOf(register);
+		int byteLength = register.getNumBytes();
 		int limit = buf.limit();
 		buf.limit(Math.min(limit, buf.position() + byteLength));
 		int result = putBytes(snap, register.getAddress(), buf);
@@ -79,7 +116,7 @@ public interface TraceMemoryRegisterSpace extends TraceMemorySpace {
 	}
 
 	default int getBytes(long snap, Register register, ByteBuffer buf) {
-		int byteLength = TraceRegisterUtils.byteLengthOf(register);
+		int byteLength = register.getNumBytes();
 		int limit = buf.limit();
 		buf.limit(Math.min(limit, buf.position() + byteLength));
 		int result = getBytes(snap, register.getAddress(), buf);
@@ -87,8 +124,21 @@ public interface TraceMemoryRegisterSpace extends TraceMemorySpace {
 		return result;
 	}
 
+	/**
+	 * Remove a value from the given time and register
+	 * 
+	 * <p>
+	 * <b>IMPORANT:</b> The trace database cannot track the state ({@link TraceMemoryState#KNOWN},
+	 * etc.) with per-bit accuracy. It only has byte precision. If the given register specifies,
+	 * e.g., only a single bit, then the entire byte will become marked
+	 * {@link TraceMemoryState#UNKNOWN}, even though the remaining 7 bits could technically be
+	 * known.
+	 * 
+	 * @param snap the snap
+	 * @param register the register
+	 */
 	default void removeValue(long snap, Register register) {
-		int byteLength = TraceRegisterUtils.byteLengthOf(register);
+		int byteLength = register.getNumBytes();
 		removeBytes(snap, register.getAddress(), byteLength);
 	}
 

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

@@ -34,16 +34,7 @@ public enum TraceRegisterUtils {
 
 	public static AddressRange rangeForRegister(Register register) {
 		Address address = register.getAddress();
-		return new AddressRangeImpl(address, address.add(register.getMinimumByteSize() - 1));
-	}
-
-	public static int byteLengthOf(Register register) {
-		int bitLength = register.getBitLength();
-		if ((bitLength & 7) != 0) {
-			throw new IllegalArgumentException(
-				"Cannot work with sub-byte boundaries. Consider using the base register.");
-		}
-		return bitLength >> 3;
+		return new AddressRangeImpl(address, address.add(register.getNumBytes() - 1));
 	}
 
 	public static byte[] padOrTruncate(byte[] arr, int length) {
@@ -58,33 +49,22 @@ public enum TraceRegisterUtils {
 		return Arrays.copyOfRange(arr, arr.length - length, arr.length);
 	}
 
-	public static ByteBuffer bufferForValue(RegisterValue value) {
-		byte[] arr = value.getSignedValue().toByteArray();
-		int byteLength = byteLengthOf(value.getRegister());
-		arr = padOrTruncate(arr, byteLength);
-		if (!value.getRegister().isBigEndian()) {
-			ArrayUtils.reverse(arr);
+	public static ByteBuffer bufferForValue(Register reg, RegisterValue value) {
+		byte[] bytes = value.toBytes().clone();
+		int start = bytes.length / 2;
+		// NB: I guess contextreg is always big?
+		if (!reg.isBigEndian() && !reg.isProcessorContext()) {
+			ArrayUtils.reverse(bytes, start, bytes.length);
 		}
-		return ByteBuffer.wrap(arr);
-	}
-
-	public static int computeMaskOffset(byte[] arr) {
-		for (int i = 0; i < arr.length; i++) {
-			switch (arr[i]) {
-				case -1:
-					return i;
-				case 0:
-					continue;
-				default:
-					throw new IllegalArgumentException(
-						"Can only handle sub-registers on byte boundaries");
-			}
-		}
-		throw new IllegalArgumentException("No value");
+		int offset = TraceRegisterUtils.computeMaskOffset(reg);
+		return ByteBuffer.wrap(bytes, start + offset, reg.getNumBytes());
 	}
 
 	public static int computeMaskOffset(Register reg) {
-		return computeMaskOffset(reg.getBaseMask());
+		if (reg.isBaseRegister()) {
+			return 0;
+		}
+		return reg.getOffset() - reg.getBaseRegister().getOffset();
 	}
 
 	public static int computeMaskOffset(RegisterValue value) {
@@ -162,21 +142,38 @@ public enum TraceRegisterUtils {
 		return regs.getValue(snap, reg.getBaseRegister()).combineValues(rv);
 	}
 
-	public static RegisterValue getRegisterValue(Register register,
+	public static RegisterValue getRegisterValue(Register reg,
 			BiConsumer<Address, ByteBuffer> readAction) {
-		int byteLength = TraceRegisterUtils.byteLengthOf(register);
-		byte[] mask = register.getBaseMask();
+		/*
+		 * The byte array for reg values spans the whole base register, but we'd like to avoid
+		 * over-reading, so we'll zero in on the bytes actually included in the mask. We'll then
+		 * have to handle endianness and such. The regval instance should then apply the actual mask
+		 * for the sub-register, if applicable.
+		 */
+		int byteLength = reg.getNumBytes();
+		byte[] mask = reg.getBaseMask();
 		ByteBuffer buf = ByteBuffer.allocate(mask.length * 2);
 		buf.put(mask);
-		int maskOffset = TraceRegisterUtils.computeMaskOffset(mask);
+		int maskOffset = TraceRegisterUtils.computeMaskOffset(reg);
 		int startVal = buf.position() + maskOffset;
 		buf.position(startVal);
 		buf.limit(buf.position() + byteLength);
-		readAction.accept(register.getAddress(), buf);
+		readAction.accept(reg.getAddress(), buf);
 		byte[] arr = buf.array();
-		if (!register.isBigEndian()) {
-			ArrayUtils.reverse(arr, startVal, startVal + byteLength);
+		if (!reg.isBigEndian() && !reg.isProcessorContext()) {
+			ArrayUtils.reverse(arr, mask.length, buf.capacity());
+		}
+		return new RegisterValue(reg, arr);
+	}
+
+	public static boolean isByteBound(Register register) {
+		return register.getLeastSignificantBit() % 8 == 0 && register.getBitLength() % 8 == 0;
+	}
+
+	public static void requireByteBound(Register register) {
+		if (!isByteBound(register)) {
+			throw new IllegalArgumentException(
+				"Cannot work with sub-byte registers. Consider a parent, instead.");
 		}
-		return new RegisterValue(register, arr);
 	}
 }

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

@@ -49,6 +49,8 @@ import ghidra.trace.database.memory.DBTraceMemoryManager;
 import ghidra.trace.database.symbol.DBTraceReference;
 import ghidra.trace.database.thread.DBTraceThread;
 import ghidra.trace.database.thread.DBTraceThreadManager;
+import ghidra.trace.model.ImmutableTraceAddressSnapRange;
+import ghidra.trace.model.TraceAddressSnapRange;
 import ghidra.trace.model.language.TraceGuestLanguage;
 import ghidra.util.Msg;
 import ghidra.util.database.DBOpenMode;
@@ -94,6 +96,10 @@ public class ToyDBTraceBuilder implements AutoCloseable {
 		return new AddressRangeImpl(addr(start), addr(end));
 	}
 
+	public TraceAddressSnapRange srange(long snap, long start, long end) {
+		return new ImmutableTraceAddressSnapRange(addr(start), addr(end), snap, snap);
+	}
+
 	public AddressRange drng(long start, long end) {
 		return new AddressRangeImpl(data(start), data(end));
 	}

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

@@ -15,11 +15,20 @@
  */
 package ghidra.trace.database.memory;
 
+import org.junit.Test;
+
 import ghidra.program.model.lang.LanguageID;
+import ghidra.trace.util.LanguageTestWatcher.TestLanguage;
 
 public class DBTraceMemoryManagerBETest extends AbstractDBTraceMemoryManagerTest {
 	@Override
 	protected LanguageID getLanguageID() {
 		return new LanguageID("Toy:BE:64:default");
 	}
+
+	@TestLanguage("Toy:BE:32:builder")
+	@Test
+	public void testRegisterBits() throws Exception {
+		runTestRegisterBits();
+	}
 }

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

@@ -15,11 +15,20 @@
  */
 package ghidra.trace.database.memory;
 
+import org.junit.Test;
+
 import ghidra.program.model.lang.LanguageID;
+import ghidra.trace.util.LanguageTestWatcher.TestLanguage;
 
 public class DBTraceMemoryManagerLETest extends AbstractDBTraceMemoryManagerTest {
 	@Override
 	protected LanguageID getLanguageID() {
 		return new LanguageID("Toy:LE:64:default");
 	}
+
+	@TestLanguage("Toy:LE:32:builder")
+	@Test
+	public void testRegisterBits() throws Exception {
+		runTestRegisterBits();
+	}
 }

Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/program/DBTraceDisassemblerIntegrationTest.java

@@ -19,12 +19,9 @@ import static org.junit.Assert.assertEquals;
 
 import java.io.File;
 import java.io.IOException;
-import java.lang.annotation.*;
 import java.util.Set;
 
 import org.junit.*;
-import org.junit.rules.TestWatcher;
-import org.junit.runner.Description;
 
 import com.google.common.collect.Range;
 
@@ -45,6 +42,8 @@ import ghidra.trace.database.memory.DBTraceMemoryManager;
 import ghidra.trace.database.memory.DBTraceMemorySpace;
 import ghidra.trace.model.memory.TraceMemoryFlag;
 import ghidra.trace.model.memory.TraceOverlappedRegionException;
+import ghidra.trace.util.LanguageTestWatcher;
+import ghidra.trace.util.LanguageTestWatcher.TestLanguage;
 import ghidra.util.database.UndoableTransaction;
 import ghidra.util.exception.*;
 import ghidra.util.task.ConsoleTaskMonitor;
@@ -54,35 +53,8 @@ public class DBTraceDisassemblerIntegrationTest extends AbstractGhidraHeadlessIn
 	protected ToyDBTraceBuilder b;
 	protected DBTraceVariableSnapProgramView view;
 
-	@Target(ElementType.METHOD)
-	@Retention(RetentionPolicy.RUNTIME)
-	public @interface TestLanguage {
-		String value();
-	}
-
-	public static class LanguageWatcher extends TestWatcher {
-		String language = ProgramBuilder._TOY64_BE;
-
-		@Override
-		protected void starting(Description description) {
-			language = computeLanguage(description);
-		}
-
-		private String computeLanguage(Description description) {
-			TestLanguage annot = description.getAnnotation(TestLanguage.class);
-			if (annot == null) {
-				return ProgramBuilder._TOY64_BE;
-			}
-			return annot.value();
-		}
-
-		public String getLanguage() {
-			return language;
-		}
-	}
-
 	@Rule
-	public LanguageWatcher testLanguage = new LanguageWatcher();
+	public LanguageTestWatcher testLanguage = new LanguageTestWatcher();
 
 	@Before
 	public void setUp() throws IOException {

Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/util/LanguageTestWatcher.java

@@ -0,0 +1,55 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.trace.util;
+
+import java.lang.annotation.*;
+
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+
+import ghidra.program.database.ProgramBuilder;
+
+public class LanguageTestWatcher extends TestWatcher {
+
+	@Target(ElementType.METHOD)
+	@Retention(RetentionPolicy.RUNTIME)
+	public @interface TestLanguage {
+		String value();
+	}
+
+	protected String language;
+
+	public LanguageTestWatcher() {
+		this(ProgramBuilder._TOY64_BE);
+	}
+
+	public LanguageTestWatcher(String defaultLanguage) {
+		this.language = defaultLanguage;
+	}
+
+	@Override
+	protected void starting(Description description) {
+		TestLanguage annot = description.getAnnotation(TestLanguage.class);
+		if (annot == null) {
+			return;
+		}
+		language = annot.value();
+	}
+
+	public String getLanguage() {
+		return language;
+	}
+}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/Register.java

@@ -21,11 +21,10 @@ import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSpace;
 
 /**
- * Class to represent a processor register.  To sort of handle bit registers, a
- * special addressing convention is used.  First the upper bit is set.  Second, the
- * next 3 bits are used to specify what bit position within a byte that this register
- * bit exists at.  Finally, the rest of the address is the address of the byte where
- * the register bit lives.
+ * Class to represent a processor register. To sort of handle bit registers, a special addressing
+ * convention is used. First the upper bit is set. Second, the next 3 bits are used to specify what
+ * bit position within a byte that this register bit exists at. Finally, the rest of the address is
+ * the address of the byte where the register bit lives.
  */
 public class Register implements java.io.Serializable, Comparable<Register> {
 
@@ -61,7 +60,7 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 	private Register baseRegister;
 	private String group;
 
-	/**Set of valid lane sizes**/
+	/** Set of valid lane sizes **/
 	private TreeSet<Integer> laneSizes;
 
 	/**
@@ -72,10 +71,10 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 	 * @param address the address in register space of this register
 	 * @param numBytes the size (in bytes) of this register
 	 * @param bigEndian true if the most significant bytes are associated with the lowest register
-	 * addresses, and false if the least significant bytes are associated with the lowest register 
-	 * addresses. 
-	 * @param typeFlags the type(s) of this Register  (TYPE_NONE, TYPE_FP, TYPE_SP, 
-	 * 	TYPE_PC, TYPE_CONTEXT, TYPE_ZERO);)
+	 *            addresses, and false if the least significant bytes are associated with the lowest
+	 *            register addresses.
+	 * @param typeFlags the type(s) of this Register (TYPE_NONE, TYPE_FP, TYPE_SP, TYPE_PC,
+	 *            TYPE_CONTEXT, TYPE_ZERO);)
 	 */
 	public Register(String name, String description, Address address, int numBytes,
 			boolean bigEndian, int typeFlags) {
@@ -128,6 +127,7 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Add register alias
+	 * 
 	 * @param aliasReg
 	 */
 	void addAlias(String alias) {
@@ -142,6 +142,7 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Remove register alias
+	 * 
 	 * @param alias
 	 */
 	void removeAlias(String alias) {
@@ -151,9 +152,8 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 	}
 
 	/**
-	 * Return register aliases.
-	 * NOTE: This is generally only supported for
-	 * context register fields.
+	 * Return register aliases. NOTE: This is generally only supported for context register fields.
+	 * 
 	 * @return register aliases or null
 	 */
 	public Iterable<String> getAliases() {
@@ -201,6 +201,19 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 		return (bitLength + 7) / 8;
 	}
 
+	/**
+	 * Returns the number of bytes spanned by this Register.
+	 * 
+	 * <p>
+	 * Compare to {{@link #getMinimumByteSize()}: Suppose a 5-bit register spans 2 bytes: 1 bit in
+	 * the first byte, and the remaining 4 in the following byte. Its value can still be stored in 1
+	 * byte, which is what {@link #getMinimumByteSize()} returns; however, its storage still spans 2
+	 * bytes of the base register, which is what this method returns.
+	 */
+	public int getNumBytes() {
+		return numBytes;
+	}
+
 	/**
 	 * Returns the offset into the register space for this register
 	 */
@@ -210,6 +223,7 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Returns the bit offset from the register address for this register.
+	 * 
 	 * @return the bit offset from the register address for this register.
 	 */
 	public int getLeastSignificantBit() {
@@ -224,8 +238,7 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 	}
 
 	/**
-	 * Returns true for a register whose context value should
-	 * follow the disassembly flow.
+	 * Returns true for a register whose context value should follow the disassembly flow.
 	 */
 	public boolean followsFlow() {
 		return (typeFlags & TYPE_DOES_NOT_FOLLOW_FLOW) == 0;
@@ -335,8 +348,8 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 	}
 
 	/**
-	 * Returns list of children registers sorted by
-	 * lest-significant bit-offset within this register.
+	 * Returns list of children registers sorted by lest-significant bit-offset within this
+	 * register.
 	 */
 	public List<Register> getChildRegisters() {
 		return new ArrayList<>(childRegisters);
@@ -400,6 +413,7 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Returns the mask that indicates which bits in the base register apply to this register.
+	 * 
 	 * @return the mask that indicates which bits in the base register apply to this register
 	 */
 	public byte[] getBaseMask() {
@@ -445,11 +459,11 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 	}
 
 	/**
-	 * Determines if reg is contained within this register.
-	 * Method does not work for bit registers (e.g., context-bits)
+	 * Determines if reg is contained within this register. Method does not work for bit registers
+	 * (e.g., context-bits)
+	 * 
 	 * @param reg another register
-	 * @return true if reg equals this register or is contained
-	 * within it. 
+	 * @return true if reg equals this register or is contained within it.
 	 */
 	public boolean contains(Register reg) {
 		if (equals(reg)) {
@@ -472,8 +486,9 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Returns true if this is a vector register
-	 * @return true precisely when {@code this} is a full vector register (i.e., a register that can be
-	 * used as input or output for a SIMD operation).
+	 * 
+	 * @return true precisely when {@code this} is a full vector register (i.e., a register that can
+	 *         be used as input or output for a SIMD operation).
 	 */
 	public boolean isVectorRegister() {
 		return (typeFlags & TYPE_VECTOR) != 0;
@@ -481,8 +496,10 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Determines whether {@code laneSizeInBytes} is a valid lane size for this register.
+	 * 
 	 * @param laneSizeInBytes lane size to check, measured in bytes
-	 * @return true precisely when {@code this} is a vector register and {@code laneSizeInBytes} is a valid lane size.
+	 * @return true precisely when {@code this} is a vector register and {@code laneSizeInBytes} is
+	 *         a valid lane size.
 	 */
 	public boolean isValidLaneSize(int laneSizeInBytes) {
 		if (!isVectorRegister()) {
@@ -496,7 +513,9 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Returns the sorted array of lane sizes for this register, measured in bytes.
-	 * @return array of lane sizes, or {@code null} if {@code this} is not a vector register or no lane sizes have been set.
+	 * 
+	 * @return array of lane sizes, or {@code null} if {@code this} is not a vector register or no
+	 *         lane sizes have been set.
 	 */
 	public int[] getLaneSizes() {
 		if (laneSizes == null) {
@@ -512,6 +531,7 @@ public class Register implements java.io.Serializable, Comparable<Register> {
 
 	/**
 	 * Adds a lane size.
+	 * 
 	 * @param laneSizeInBytes lane size to add
 	 * @throws UnsupportedOperationException if register is unable to support the definition of
 	 *             lanes.