diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/register/DebuggerRegistersProvider.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/register/DebuggerRegistersProvider.java index 27206bae6c..ca8e4ec87a 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/register/DebuggerRegistersProvider.java +++ b/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 diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServicePlugin.java index dff8ea1ee6..6477158de8 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServicePlugin.java +++ b/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(); diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultTraceRecorder.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultTraceRecorder.java index 0332d4a762..4bed4ab24b 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultTraceRecorder.java +++ b/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 onTarget = getRegisterMapper(thread).getRegistersOnTarget(); + for (; register != null; register = register.getParentRegister()) { + if (onTarget.contains(register)) { + return register; + } + } + return null; + } + @Override public CompletableFuture writeThreadRegisters(TracePlatform platform, TraceThread thread, int frameLevel, Map values) { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/record/ObjectBasedTraceRecorder.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/record/ObjectBasedTraceRecorder.java index 25e93b24a1..fdd73a1ae2 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/record/ObjectBasedTraceRecorder.java +++ b/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 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 writeThreadRegisters(TracePlatform platform, TraceThread thread, int frameLevel, Map values) { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/TraceRecorder.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/TraceRecorder.java index 153b411884..795b6545cc 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/TraceRecorder.java +++ b/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, - true); + Register parent = isRegisterOnTarget(platform, thread, frameLevel, register); + 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) { - Collection onTarget = getRegisterMapper(thread).getRegistersOnTarget(); - return onTarget.contains(register) || onTarget.contains(register.getBaseRegister()); - } + Register isRegisterOnTarget(TracePlatform platform, TraceThread thread, int frameLevel, + Register register); /** * 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; } /** diff --git a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/watch/DebuggerWatchesProviderTest.java b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/watch/DebuggerWatchesProviderTest.java index 9749da7307..70a6b01b67 100644 --- a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/watch/DebuggerWatchesProviderTest.java +++ b/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")); diff --git a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServiceTest.java b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServiceTest.java index d5eef80f06..eec650971e 100644 --- a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServiceTest.java +++ b/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 + * + *

+ * 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 expecting(Class 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)); + }); } } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/guest/InternalTracePlatform.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/guest/InternalTracePlatform.java index b5d9c740d7..8f5ad69e37 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/guest/InternalTracePlatform.java +++ b/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 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 path, + Register register) { + return getConventionalRegisterPath(schema, path, + getConventionalRegisterObjectName(register)); + } + @Override default PathMatcher getConventionalRegisterPath(TraceObject container, Register register) { return getConventionalRegisterPath(container.getTargetSchema(), diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemorySpace.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemorySpace.java index 1159102161..6da1e52972 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemorySpace.java +++ b/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); diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/space/AbstractDBTraceSpaceBasedManager.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/space/AbstractDBTraceSpaceBasedManager.java index 3842673e28..53a7fcd0c7 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/space/AbstractDBTraceSpaceBasedManager.java +++ b/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 frame = ImmutablePair.of(thread, frameLevel); if (!createIfAbsent) { diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/guest/TracePlatform.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/guest/TracePlatform.java index 4ec5161426..06d6b077a2 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/guest/TracePlatform.java +++ b/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 path, + String name); + /** * Get the expected path where an object defining the register value would be * diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/TraceRegisterUtils.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/TraceRegisterUtils.java index 84fc8720b2..db7fef1375 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/TraceRegisterUtils.java +++ b/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, - TracePlatform platform, long snap, TraceMemorySpace regs, boolean requireKnown) { + public static RegisterValue combineWithTraceParentRegisterValue(Register parent, + 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) {