From 60c5da018dca2bd846b7eb7a8ab02272ee69d98f Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Tue, 24 Jan 2023 10:22:30 -0500 Subject: [PATCH] GP-0: Fix timing and null thread issues in tests --- .../service/model/record/ObjectBasedTraceRecorder.java | 5 +++-- .../src/main/java/ghidra/app/services/TraceRecorder.java | 6 +++++- .../service/editing/DebuggerStateEditingServiceTest.java | 8 +++++--- 3 files changed, 13 insertions(+), 6 deletions(-) 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 ae8d82b45a..25c01291d3 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 @@ -427,7 +427,8 @@ public class ObjectBasedTraceRecorder implements TraceRecorder { @Override public Set getTargetRegisterBanks(TraceThread thread, int frameLevel) { - return Set.of(objectRecorder.getTargetFrameInterface(thread, frameLevel, TargetRegisterBank.class)); + return Set.of( + objectRecorder.getTargetFrameInterface(thread, frameLevel, TargetRegisterBank.class)); } @Override @@ -504,7 +505,7 @@ public class ObjectBasedTraceRecorder implements TraceRecorder { protected TargetRegisterContainer getTargetRegisterContainer(TraceThread thread, int frameLevel) { if (!(thread instanceof TraceObjectThread tot)) { - throw new AssertionError(); + throw new AssertionError("thread = " + thread); } TraceObject objThread = tot.getObject(); TraceObject regContainer = objThread.queryRegisterContainer(frameLevel); 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 9cd3a0c9cd..c991846141 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 @@ -527,7 +527,8 @@ public interface TraceRecorder { return writeMemory(address, data); } if (address.isRegisterAddress()) { - return writeRegister(platform, thread, frameLevel, address, data); + return writeRegister(platform, Objects.requireNonNull(thread), frameLevel, address, + data); } throw new IllegalArgumentException("Address is not in a recognized space: " + address); } @@ -599,6 +600,9 @@ public interface TraceRecorder { if (address.isMemoryAddress()) { return isMemoryOnTarget(address); } + if (thread == null) { // register-space addresses require a thread + return false; + } Register register = platform.getLanguage().getRegister(address, size); if (register == null) { throw new IllegalArgumentException("Cannot identify the (single) register: " + address); 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 b6c374aee1..0e8a107fd1 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 @@ -432,13 +432,13 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge (TargetRegisterBank) mb.testThread1.getCachedAttribute("RegisterBank"); traceManager.openTrace(tb.trace); activateTrace(); - TraceThread thread = recorder.getTraceThread(mb.testThread1); + TraceThread thread = waitForValue(() -> recorder.getTraceThread(mb.testThread1)); traceManager.activateThread(thread); waitForSwing(); editingService.setCurrentMode(recorder.getTrace(), StateEditingMode.RW_TARGET); StateEditor editor = createStateEditor(); - assertTrue(editor.isRegisterEditable(r0)); + waitForPass(() -> assertTrue(editor.isRegisterEditable(r0))); waitOn(editor.setRegister(rv1234)); waitForPass(() -> { TraceMemorySpace regs = @@ -447,7 +447,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge RegisterValue value = regs.getValue(getPlatform(), traceManager.getCurrentSnap(), r0); assertEquals(rv1234, value); }); - assertTrue(editor.isRegisterEditable(r0h)); + waitForPass(() -> assertTrue(editor.isRegisterEditable(r0h))); waitOn(editor.setRegister(rvHigh1234)); assertArrayEquals(mb.arr(0, 0, 4, 0xd2, 0, 0, 4, 0xd2), waitOn(bank.readRegister("r0"))); @@ -478,6 +478,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge public void testWriteTargetMemoryNotAliveErr() throws Throwable { createAndOpenTrace(); activateTrace(); + waitForSwing(); editingService.setCurrentMode(tb.trace, StateEditingMode.RW_TARGET); waitForSwing(); @@ -492,6 +493,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge public void testWriteTargetRegisterNotAliveErr() throws Throwable { createAndOpenTrace(); activateTrace(); + waitForSwing(); editingService.setCurrentMode(tb.trace, StateEditingMode.RW_TARGET); waitForSwing();