Merge remote-tracking branch 'origin/GP-2761_Dan_registerEditsNewConventions--SQUASHED'

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/register/DebuggerRegistersProvider.java

@@ -879,12 +879,9 @@ public class DebuggerRegistersProvider extends ComponentProviderAdapter
 			return;
 		}
 		StateEditor editor = editingService.createStateEditor(current);
-		if (!editor.isRegisterEditable(rv.getRegister())) {
-			rv = combineWithTraceBaseRegisterValue(rv);
-		}
 		if (!editor.isRegisterEditable(rv.getRegister())) {
 			Msg.showError(this, getComponent(), "Edit Register",
-				"Neither the register nor its base can be edited.");
+				"Neither the register nor any parent can be edited.");
 			return;
 		}
 
@@ -905,13 +902,6 @@ public class DebuggerRegistersProvider extends ComponentProviderAdapter
 		return;
 	}
 
-	private RegisterValue combineWithTraceBaseRegisterValue(RegisterValue rv) {
-		TraceMemorySpace regs = getRegisterMemorySpace(false);
-		TracePlatform platform = current.getPlatform();
-		long snap = current.getViewSnap();
-		return TraceRegisterUtils.combineWithTraceBaseRegisterValue(rv, platform, snap, regs, true);
-	}
-
 	/**
 	 * TODO: Make this smart enough to replace a component type when applicable? NOTE: Would require
 	 * cloning the type to avoid effects elsewhere. Maybe just keep a dedicated data type for this

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServicePlugin.java

@@ -93,7 +93,8 @@ public class DebuggerStateEditingServicePlugin extends AbstractDebuggerPlugin
 				return false;
 			}
 			TraceRecorder recorder = coordinates.getRecorder();
-			return recorder.isVariableOnTarget(coordinates.getThread(), address, length);
+			return recorder.isVariableOnTarget(coordinates.getPlatform(), coordinates.getThread(),
+				coordinates.getFrame(), address, length);
 		}
 
 		protected boolean isTraceVariableEditable(DebuggerCoordinates coordinates, Address address,
@@ -103,6 +104,10 @@ public class DebuggerStateEditingServicePlugin extends AbstractDebuggerPlugin
 
 		protected boolean isEmulatorVariableEditable(DebuggerCoordinates coordinates,
 				Address address, int length) {
+			if (coordinates.getThread() == null) {
+				// A limitation in TraceSchedule, which is used to manifest patches
+				return false;
+			}
 			if (!isTraceVariableEditable(coordinates, address, length)) {
 				return false;
 			}
@@ -198,6 +203,7 @@ public class DebuggerStateEditingServicePlugin extends AbstractDebuggerPlugin
 			TraceThread thread = coordinates.getThread();
 			if (thread == null) {
 				// TODO: Well, technically, only for register edits
+				// It's a limitation in TraceSchedule. Every step requires a thread
 				throw new IllegalArgumentException("Emulator edits require a thread.");
 			}
 			Language language = coordinates.getPlatform().getLanguage();

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultTraceRecorder.java

@@ -546,6 +546,19 @@ public class DefaultTraceRecorder implements TraceRecorder {
 		return processRecorder.writeProcessMemory(start, data);
 	}
 
+	@Override
+	public Register isRegisterOnTarget(TracePlatform platform, TraceThread thread, int frameLevel,
+			Register register) {
+		// NOTE: This pays no heed to frameLevel, but caller does require level==0 for now.
+		Collection<Register> onTarget = getRegisterMapper(thread).getRegistersOnTarget();
+		for (; register != null; register = register.getParentRegister()) {
+			if (onTarget.contains(register)) {
+				return register;
+			}
+		}
+		return null;
+	}
+
 	@Override
 	public CompletableFuture<Void> writeThreadRegisters(TracePlatform platform, TraceThread thread,
 			int frameLevel, Map<Register, RegisterValue> values) {

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/record/ObjectBasedTraceRecorder.java

@@ -29,6 +29,7 @@ import ghidra.app.services.TraceRecorderListener;
 import ghidra.async.AsyncFence;
 import ghidra.async.AsyncUtils;
 import ghidra.dbg.AnnotatedDebuggerAttributeListener;
+import ghidra.dbg.DebuggerObjectModel;
 import ghidra.dbg.error.DebuggerMemoryAccessException;
 import ghidra.dbg.error.DebuggerModelAccessException;
 import ghidra.dbg.target.*;
@@ -541,6 +542,73 @@ public class ObjectBasedTraceRecorder implements TraceRecorder {
 		return Utils.bigIntegerToBytes(value, byteLength, true);
 	}
 
+	protected TargetRegisterBank isExactRegisterOnTarget(TracePlatform platform,
+			TargetRegisterContainer regContainer, String name) {
+		PathMatcher matcher =
+			platform.getConventionalRegisterPath(regContainer.getSchema(), List.of(), name);
+		for (TargetObject targetObject : matcher.getCachedSuccessors(regContainer).values()) {
+			if (!(targetObject instanceof TargetRegister targetRegister)) {
+				continue;
+			}
+			DebuggerObjectModel model = targetRegister.getModel();
+			List<String> pathBank = model.getRootSchema()
+					.searchForAncestor(TargetRegisterBank.class, targetRegister.getPath());
+			if (pathBank == null ||
+				!(model.getModelObject(pathBank) instanceof TargetRegisterBank targetBank)) {
+				continue;
+			}
+			return targetBank;
+		}
+		return null;
+	}
+
+	protected TargetRegisterBank isExactRegisterOnTarget(TracePlatform platform, TraceThread thread,
+			int frameLevel, Register register) {
+		TargetRegisterContainer regContainer = getTargetRegisterContainer(thread, frameLevel);
+		if (regContainer == null) {
+			return null;
+		}
+		TargetRegisterBank result;
+		String name = platform.getConventionalRegisterObjectName(register);
+		result = isExactRegisterOnTarget(platform, regContainer, name);
+		if (result != null) {
+			return result;
+		}
+		// Not totally case insensitive, but the sane cases
+		String upperName = name.toUpperCase();
+		if (!name.equals(upperName)) {
+			result = isExactRegisterOnTarget(platform, regContainer, upperName);
+			if (result != null) {
+				return result;
+			}
+		}
+		String lowerName = name.toLowerCase();
+		if (!name.equals(lowerName)) {
+			result = isExactRegisterOnTarget(platform, regContainer, lowerName);
+			if (result != null) {
+				return result;
+			}
+		}
+		return null;
+	}
+
+	@Override
+	public Register isRegisterOnTarget(TracePlatform platform, TraceThread thread, int frameLevel,
+			Register register) {
+		for (; register != null; register = register.getParentRegister()) {
+			TargetRegisterBank targetBank =
+				isExactRegisterOnTarget(platform, thread, frameLevel, register);
+			if (targetBank != null) {
+				/**
+				 * TODO: A way to ask the target which registers are modifiable, but
+				 * "isRegisterOnTarget" does not necessarily imply for writing
+				 */
+				return register;
+			}
+		}
+		return null;
+	}
+
 	@Override
 	public CompletableFuture<Void> writeThreadRegisters(TracePlatform platform, TraceThread thread,
 			int frameLevel, Map<Register, RegisterValue> values) {

Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/TraceRecorder.java

@@ -553,22 +553,23 @@ public interface TraceRecorder {
 			Utils.bytesToBigInteger(data, data.length, register.isBigEndian(), false));
 		TraceMemorySpace regs =
 			getTrace().getMemoryManager().getMemoryRegisterSpace(thread, frameLevel, false);
-		rv = TraceRegisterUtils.combineWithTraceBaseRegisterValue(rv, platform, getSnap(), regs,
+		Register parent = isRegisterOnTarget(platform, thread, frameLevel, register);
-			true);
+		rv = TraceRegisterUtils.combineWithTraceParentRegisterValue(parent, rv, platform, getSnap(),
+			regs, true);
 		return writeThreadRegisters(platform, thread, frameLevel, Map.of(rv.getRegister(), rv));
 	}
 
 	/**
 	 * Check if the given register exists on target (is mappable) for the given thread
 	 * 
+	 * @param platform the platform whose language defines the registers
 	 * @param thread the thread whose registers to examine
+	 * @param frameLevel the frame, usually 0.
 	 * @param register the register to check
-	 * @return true if the given register is known for the given thread on target
+	 * @return the smallest parent register known for the given thread on target, or null
 	 */
-	default boolean isRegisterOnTarget(TraceThread thread, Register register) {
+	Register isRegisterOnTarget(TracePlatform platform, TraceThread thread, int frameLevel,
-		Collection<Register> onTarget = getRegisterMapper(thread).getRegistersOnTarget();
+			Register register);
-		return onTarget.contains(register) || onTarget.contains(register.getBaseRegister());
-	}
 
 	/**
 	 * Check if the given trace address exists in target memory
@@ -583,21 +584,29 @@ public interface TraceRecorder {
 	/**
 	 * Check if a given variable (register or memory) exists on target
 	 * 
+	 * @param platform the platform whose language defines the registers
 	 * @param thread if a register, the thread whose registers to examine
+	 * @param frameLevel the frame, usually 0.
 	 * @param address the address of the variable
 	 * @param size the size of the variable. Ignored for memory
 	 * @return true if the variable can be mapped to the target
 	 */
-	default boolean isVariableOnTarget(TraceThread thread, Address address, int size) {
+	default boolean isVariableOnTarget(TracePlatform platform, TraceThread thread, int frameLevel,
+			Address address, int size) {
 		if (address.isMemoryAddress()) {
 			return isMemoryOnTarget(address);
 		}
-		Register register = getTrace().getBaseLanguage().getRegister(address, size);
+		Register register = platform.getLanguage().getRegister(address, size);
 		if (register == null) {
 			throw new IllegalArgumentException("Cannot identify the (single) register: " + address);
 		}
 
-		return isRegisterOnTarget(thread, register);
+		// TODO: Can any debugger modify regs up the stack?
+		if (frameLevel != 0) {
+			return false;
+		}
+
+		return isRegisterOnTarget(platform, thread, frameLevel, register) != null;
 	}
 
 	/**

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/watch/DebuggerWatchesProviderTest.java

@@ -588,7 +588,7 @@ public class DebuggerWatchesProviderTest extends AbstractGhidraHeadedDebuggerGUI
 		WatchRow row = prepareTestEditTarget("r1");
 		TraceThread thread = recorder.getTraceThread(mb.testThread1);
 		// Sanity check
-		assertFalse(recorder.isRegisterOnTarget(thread, r1));
+		assertNull(recorder.isRegisterOnTarget(tb.host, thread, 0, r1));
 
 		assertFalse(row.isRawValueEditable());
 		runSwingWithException(() -> row.setRawValueString("0x1234"));

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServiceTest.java

@@ -30,6 +30,7 @@ import ghidra.app.services.DebuggerStateEditingService;
 import ghidra.app.services.DebuggerStateEditingService.StateEditingMode;
 import ghidra.app.services.DebuggerStateEditingService.StateEditor;
 import ghidra.app.services.TraceRecorder;
+import ghidra.async.AsyncUtils.TemperamentalRunnable;
 import ghidra.dbg.target.TargetRegisterBank;
 import ghidra.program.model.lang.*;
 import ghidra.program.model.mem.MemoryAccessException;
@@ -61,6 +62,27 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		return tb.trace.getPlatformManager().getHostPlatform();
 	}
 
+	/**
+	 * Verify that the given action (usually a lambda) throws an exception
+	 * 
+	 * <p>
+	 * This fulfills the same use case as the {@link Test#expected()} attribute, but allows more
+	 * precise verification of which code in the test causes the exception.
+	 */
+	<E extends Throwable> E expecting(Class<E> cls, TemperamentalRunnable action) {
+		try {
+			action.run();
+			fail("Expected exception type " + cls + ", but got no error.");
+		}
+		catch (Throwable e) {
+			if (cls.isInstance(e)) {
+				return cls.cast(e);
+			}
+			fail("Expection exception type " + cls + ", but got " + e);
+		}
+		throw new AssertionError();
+	}
+
 	@Before
 	public void setUpEditorTest() throws Exception {
 		editingService = addPlugin(tool, DebuggerStateEditingServicePlugin.class);
@@ -72,7 +94,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		rvHigh1234 = new RegisterValue(r0h, BigInteger.valueOf(1234));
 	}
 
-	@Test(expected = IllegalArgumentException.class)
+	@Test
 	public void testWriteEmuMemoryNoThreadErr() throws Throwable {
 		/**
 		 * TODO: It'd be nice if this worked, since memory edits don't really require a thread
@@ -86,10 +108,13 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		editingService.setCurrentMode(tb.trace, StateEditingMode.WRITE_EMULATOR);
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		assertFalse(editor.isVariableEditable(tb.addr(0x00400000), 4));
+		expecting(IllegalArgumentException.class, () -> {
+			waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		});
 	}
 
-	@Test(expected = IllegalArgumentException.class)
+	@Test
 	public void testWriteEmuRegisterNoThreadErr() throws Throwable {
 		createAndOpenTrace();
 		editingService.setCurrentMode(tb.trace, StateEditingMode.WRITE_EMULATOR);
@@ -98,7 +123,10 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setRegister(rv1234));
+		assertFalse(editor.isRegisterEditable(r0));
+		expecting(IllegalArgumentException.class, () -> {
+			waitOn(editor.setRegister(rv1234));
+		});
 	}
 
 	@Test
@@ -114,6 +142,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isVariableEditable(tb.addr(0x00400000), 4));
 		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
 		waitForSwing();
 
@@ -140,6 +169,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isRegisterEditable(r0));
 		waitOn(editor.setRegister(rv1234));
 		waitForSwing();
 
@@ -178,6 +208,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForPass(() -> assertEquals(step1, traceManager.getCurrent().getTime()));
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isVariableEditable(tb.addr(0x00600000), 4));
 		waitOn(editor.setVariable(tb.addr(0x00600000), tb.arr(1, 2, 3, 4)));
 		waitForSwing();
 
@@ -216,6 +247,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForPass(() -> assertEquals(step1, traceManager.getCurrent().getTime()));
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isRegisterEditable(r0));
 		waitOn(editor.setRegister(rv1234));
 		waitForSwing();
 
@@ -243,7 +275,9 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isVariableEditable(tb.addr(0x00400000), 4));
 		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		assertTrue(editor.isVariableEditable(tb.addr(0x00400002), 4));
 		waitOn(editor.setVariable(tb.addr(0x00400002), tb.arr(5, 6, 7, 8)));
 		waitForSwing();
 
@@ -271,6 +305,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isRegisterEditable(r0));
 		waitOn(editor.setRegister(rv1234));
 		waitOn(editor.setRegister(rv5678));
 		waitForSwing();
@@ -295,6 +330,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isVariableEditable(tb.addr(0x00400000), 4));
 		// NB. Editor creates its own transaction
 		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
 		waitForSwing();
@@ -308,7 +344,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		assertArrayEquals(tb.arr(1, 2, 3, 4), buf.array());
 	}
 
-	@Test(expected = IllegalArgumentException.class)
+	@Test
 	public void testWriteTraceRegisterNoThreadErr() throws Throwable {
 		// NB. Definitely no thread required
 		createAndOpenTrace();
@@ -317,8 +353,11 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
+		assertFalse(editor.isRegisterEditable(r0));
 		// NB. Editor creates its own transaction
-		waitOn(editor.setRegister(rv1234));
+		expecting(IllegalArgumentException.class, () -> {
+			waitOn(editor.setRegister(rv1234));
+		});
 	}
 
 	@Test
@@ -336,6 +375,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		waitForSwing();
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isRegisterEditable(r0));
 		// NB. Editor creates its own transaction
 		waitOn(editor.setRegister(rv1234));
 		waitForSwing();
@@ -360,6 +400,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		editingService.setCurrentMode(recorder.getTrace(), StateEditingMode.WRITE_TARGET);
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isVariableEditable(tb.addr(0x00400000), 4));
 		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
 
 		assertArrayEquals(mb.arr(1, 2, 3, 4),
@@ -379,6 +420,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		editingService.setCurrentMode(recorder.getTrace(), StateEditingMode.WRITE_TARGET);
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isRegisterEditable(r0));
 		waitOn(editor.setRegister(rv1234));
 
 		assertArrayEquals(mb.arr(0, 0, 0, 0, 0, 0, 4, 0xd2), waitOn(bank.readRegister("r0")));
@@ -397,6 +439,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		editingService.setCurrentMode(recorder.getTrace(), StateEditingMode.WRITE_TARGET);
 
 		StateEditor editor = createStateEditor();
+		assertTrue(editor.isRegisterEditable(r0));
 		waitOn(editor.setRegister(rv1234));
 		waitForPass(() -> {
 			TraceMemorySpace regs =
@@ -405,12 +448,13 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 			RegisterValue value = regs.getValue(getPlatform(), traceManager.getCurrentSnap(), r0);
 			assertEquals(rv1234, value);
 		});
+		assertTrue(editor.isRegisterEditable(r0h));
 		waitOn(editor.setRegister(rvHigh1234));
 
 		assertArrayEquals(mb.arr(0, 0, 4, 0xd2, 0, 0, 4, 0xd2), waitOn(bank.readRegister("r0")));
 	}
 
-	@Test(expected = MemoryAccessException.class)
+	@Test
 	public void testWriteTargetMemoryNotPresentErr() throws Throwable {
 		TraceRecorder recorder = recordAndWaitSync();
 		traceManager.openTrace(tb.trace);
@@ -422,10 +466,13 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		traceManager.activateSnap(traceManager.getCurrentSnap() - 1);
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		assertFalse(editor.isVariableEditable(tb.addr(0x00400000), 4));
+		expecting(MemoryAccessException.class, () -> {
+			waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		});
 	}
 
-	@Test(expected = MemoryAccessException.class)
+	@Test
 	public void testWriteTargetRegisterNotPresentErr() throws Throwable {
 		TraceRecorder recorder = recordAndWaitSync();
 		traceManager.openTrace(tb.trace);
@@ -437,46 +484,61 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		traceManager.activateSnap(traceManager.getCurrentSnap() - 1);
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setRegister(rv1234));
+		assertFalse(editor.isRegisterEditable(r0));
+		expecting(MemoryAccessException.class, () -> {
+			waitOn(editor.setRegister(rv1234));
+		});
 	}
 
-	@Test(expected = MemoryAccessException.class)
+	@Test
 	public void testWriteTargetMemoryNotAliveErr() throws Throwable {
 		createAndOpenTrace();
 		activateTrace();
 		editingService.setCurrentMode(tb.trace, StateEditingMode.WRITE_TARGET);
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		assertFalse(editor.isVariableEditable(tb.addr(0x00400000), 4));
+		expecting(MemoryAccessException.class, () -> {
+			waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		});
 	}
 
-	@Test(expected = MemoryAccessException.class)
+	@Test
 	public void testWriteTargetRegisterNotAliveErr() throws Throwable {
 		createAndOpenTrace();
 		activateTrace();
 		editingService.setCurrentMode(tb.trace, StateEditingMode.WRITE_TARGET);
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setRegister(rv1234));
+		assertFalse(editor.isRegisterEditable(r0));
+		expecting(MemoryAccessException.class, () -> {
+			waitOn(editor.setRegister(rv1234));
+		});
 	}
 
-	@Test(expected = MemoryAccessException.class)
+	@Test
 	public void testWriteReadOnlyMemoryErr() throws Throwable {
 		createAndOpenTrace();
 		activateTrace();
 		editingService.setCurrentMode(tb.trace, StateEditingMode.READ_ONLY);
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		assertFalse(editor.isVariableEditable(tb.addr(0x00400000), 4));
+		expecting(MemoryAccessException.class, () -> {
+			waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
+		});
 	}
 
-	@Test(expected = MemoryAccessException.class)
+	@Test
 	public void testWriteReadOnlyRegisterErr() throws Throwable {
 		createAndOpenTrace();
 		activateTrace();
 		editingService.setCurrentMode(tb.trace, StateEditingMode.READ_ONLY);
 
 		StateEditor editor = createStateEditor();
-		waitOn(editor.setRegister(rv1234));
+		assertFalse(editor.isRegisterEditable(r0));
+		expecting(MemoryAccessException.class, () -> {
+			waitOn(editor.setRegister(rv1234));
+		});
 	}
 }

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

@@ -102,15 +102,21 @@ public interface InternalTracePlatform extends TracePlatform {
 
 	@Override
 	default PathMatcher getConventionalRegisterPath(TargetObjectSchema schema, List<String> path,
-			Register register) {
+			String name) {
 		PathMatcher matcher = schema.searchFor(TargetRegister.class, path, true);
 		if (matcher.isEmpty()) {
 			return matcher;
 		}
-		String name = getConventionalRegisterObjectName(register);
 		return matcher.applyKeys(Align.RIGHT, List.of(name));
 	}
 
+	@Override
+	default PathMatcher getConventionalRegisterPath(TargetObjectSchema schema, List<String> path,
+			Register register) {
+		return getConventionalRegisterPath(schema, path,
+			getConventionalRegisterObjectName(register));
+	}
+
 	@Override
 	default PathMatcher getConventionalRegisterPath(TraceObject container, Register register) {
 		return getConventionalRegisterPath(container.getTargetSchema(),

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

@@ -270,7 +270,7 @@ public class DBTraceMemorySpace
 
 	@Override
 	public DBTraceCodeSpace getCodeSpace(boolean createIfAbsent) {
-		if (space.isRegisterSpace()) {
+		if (space.isRegisterSpace() && !space.isOverlaySpace()) {
 			return trace.getCodeManager().getCodeRegisterSpace(thread, frameLevel, createIfAbsent);
 		}
 		return trace.getCodeManager().getCodeSpace(space, createIfAbsent);

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

@@ -40,7 +40,6 @@ import ghidra.util.LockHold;
 import ghidra.util.Msg;
 import ghidra.util.database.*;
 import ghidra.util.database.annot.*;
-import ghidra.util.exception.DuplicateNameException;
 import ghidra.util.exception.VersionException;
 import ghidra.util.task.TaskMonitor;
 
@@ -207,8 +206,9 @@ public abstract class AbstractDBTraceSpaceBasedManager<M extends DBTraceSpaceBas
 
 	protected M getForRegisterSpace(TraceThread thread, int frameLevel, boolean createIfAbsent) {
 		trace.getThreadManager().assertIsMine(thread);
-		if (thread instanceof TraceObjectThread objThread) {
+		if (trace.getObjectManager().hasSchema()) {
-			return getForRegisterSpaceObjectThread(objThread, frameLevel, createIfAbsent);
+			return getForRegisterSpaceObjectThread((TraceObjectThread) thread, frameLevel,
+				createIfAbsent);
 		}
 		Pair<TraceThread, Integer> frame = ImmutablePair.of(thread, frameLevel);
 		if (!createIfAbsent) {

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/guest/TracePlatform.java

@@ -182,6 +182,17 @@ public interface TracePlatform {
 	 */
 	String getConventionalRegisterObjectName(Register register);
 
+	/**
+	 * Get the expected path where an object defining the register value would be
+	 * 
+	 * @param schema the schema of the register container
+	 * @param path the path to the register container
+	 * @param name the name of the register on the target
+	 * @return the path matcher, possibly empty
+	 */
+	PathMatcher getConventionalRegisterPath(TargetObjectSchema schema, List<String> path,
+			String name);
+
 	/**
 	 * Get the expected path where an object defining the register value would be
 	 * 

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

@@ -140,24 +140,31 @@ public enum TraceRegisterUtils {
 		return new RegisterValue(register, addr.getOffsetAsBigInteger());
 	}
 
-	public static RegisterValue combineWithTraceBaseRegisterValue(RegisterValue rv,
+	public static RegisterValue combineWithTraceParentRegisterValue(Register parent,
-			TracePlatform platform, long snap, TraceMemorySpace regs, boolean requireKnown) {
+			RegisterValue rv, TracePlatform platform, long snap, TraceMemorySpace regs,
+			boolean requireKnown) {
 		Register reg = rv.getRegister();
-		if (reg.isBaseRegister()) {
+		if (reg == parent) {
 			return rv;
 		}
 		if (regs == null) {
 			if (requireKnown) {
-				throw new IllegalStateException("Must fetch base register before setting a child");
+				throw new IllegalStateException("Must fetch " + parent + " before setting " + reg);
 			}
-			return rv.getBaseRegisterValue();
+			return rv.getRegisterValue(parent);
 		}
 		if (requireKnown) {
 			if (TraceMemoryState.KNOWN != regs.getState(platform, snap, reg.getBaseRegister())) {
-				throw new IllegalStateException("Must fetch base register before setting a child");
+				throw new IllegalStateException("Must fetch " + parent + " before setting " + reg);
 			}
 		}
-		return regs.getValue(platform, snap, reg.getBaseRegister()).combineValues(rv);
+		return regs.getValue(platform, snap, parent).combineValues(rv);
+	}
+
+	public static RegisterValue combineWithTraceBaseRegisterValue(RegisterValue rv,
+			TracePlatform platform, long snap, TraceMemorySpace regs, boolean requireKnown) {
+		return combineWithTraceParentRegisterValue(rv.getRegister().getBaseRegister(), rv, platform,
+			snap, regs, requireKnown);
 	}
 
 	public static ByteBuffer prepareBuffer(Register register) {