GP-2761: Fix register editing under new conventions when names don't match case.

This commit is contained in:
Dan 2022-11-30 11:14:25 -05:00
parent 8d6cf5e310
commit e6c3713d3c
12 changed files with 226 additions and 54 deletions

View file

@ -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

View file

@ -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();

View file

@ -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) {

View file

@ -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) {

View file

@ -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<Register> 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;
}
/**

View file

@ -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"));

View file

@ -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));
});
}
}