From 34e67ea90940718b7cf2665e5a8866a0f7f33f8f Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Wed, 7 Apr 2021 14:55:39 -0400 Subject: [PATCH] GP-0: Fixing DebuggerTraceManagerService tests --- .../service/model/DefaultTraceRecorder.java | 4 +-- .../DebuggerTraceManagerServicePlugin.java | 8 ++--- .../DebuggerTraceManagerServiceTest.java | 35 ++++++++++++------- .../main/java/ghidra/dbg/util/ValueUtils.java | 17 --------- .../dbg/model/TestDebuggerModelBuilder.java | 5 +-- .../ghidra/dbg/model/TestTargetStack.java | 9 ++--- .../TestTargetStackFrameHasRegisterBank.java | 5 +-- .../TestTargetStackFrameIsRegisterBank.java | 5 +-- .../ghidra/dbg/model/test_schema.xml | 14 -------- 9 files changed, 42 insertions(+), 60 deletions(-) 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 f1bd6c693c..2e9401d7d3 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 @@ -236,8 +236,8 @@ public class DefaultTraceRecorder implements TraceRecorder { @Override public TraceStackFrame getTraceStackFrame(TargetStackFrame frame) { - // THIS IS UNUSED - return null; + // Not the most efficient, but only used in testing. + return getTraceStackFrameForSuccessor(frame); } @Override diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java index 140ad0bd7e..65a36aea63 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java @@ -984,10 +984,10 @@ public class DebuggerTraceManagerServicePlugin extends Plugin return thread; } } - if (!Objects.equals(prev.getTrace(), resolved.getTrace())) { - return recorder.getTarget(); - } - return null; + //if (!Objects.equals(prev.getTrace(), resolved.getTrace())) { + return recorder.getTarget(); + //} + //return null; } @Override diff --git a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServiceTest.java b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServiceTest.java index f6f4b67c6e..9d82efc7ec 100644 --- a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServiceTest.java +++ b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServiceTest.java @@ -33,6 +33,7 @@ import ghidra.dbg.model.TestTargetStackFrameHasRegisterBank; import ghidra.framework.model.DomainFile; import ghidra.trace.database.thread.DBTraceThread; import ghidra.trace.model.Trace; +import ghidra.trace.model.thread.TraceThread; import ghidra.util.SystemUtilities; import ghidra.util.database.UndoableTransaction; @@ -307,30 +308,30 @@ public class DebuggerTraceManagerServiceTest extends AbstractGhidraHeadedDebugge traceManager.activateTrace(trace); waitForSwing(); - assertEquals(0, recorder.getSnap()); - assertEquals(0, traceManager.getCurrentSnap()); + long initSnap = recorder.getSnap(); + assertEquals(initSnap, traceManager.getCurrentSnap()); recorder.forceSnapshot(); waitForSwing(); - assertEquals(1, recorder.getSnap()); - assertEquals(1, traceManager.getCurrentSnap()); + assertEquals(initSnap + 1, recorder.getSnap()); + assertEquals(initSnap + 1, traceManager.getCurrentSnap()); traceManager.setAutoActivatePresent(false); recorder.forceSnapshot(); waitForSwing(); - assertEquals(2, recorder.getSnap()); - assertEquals(1, traceManager.getCurrentSnap()); + assertEquals(initSnap + 2, recorder.getSnap()); + assertEquals(initSnap + 1, traceManager.getCurrentSnap()); traceManager.setAutoActivatePresent(true); recorder.forceSnapshot(); waitForSwing(); - assertEquals(3, recorder.getSnap()); - assertEquals(3, traceManager.getCurrentSnap()); + assertEquals(initSnap + 3, recorder.getSnap()); + assertEquals(initSnap + 3, traceManager.getCurrentSnap()); } @Test @@ -344,6 +345,8 @@ public class DebuggerTraceManagerServiceTest extends AbstractGhidraHeadedDebugge new TestDebuggerTargetTraceMapper(mb.testProcess1)); Trace trace = recorder.getTrace(); + waitForValue(() -> modelService.getTarget(trace)); + traceManager.openTrace(trace); waitForSwing(); @@ -356,16 +359,22 @@ public class DebuggerTraceManagerServiceTest extends AbstractGhidraHeadedDebugge assertNull(traceManager.getCurrentThread()); assertEquals(mb.testProcess1, mb.testModel.session.getFocus()); - traceManager.activateThread(recorder.getTraceThread(mb.testThread1)); + TraceThread thread = waitForValue(() -> recorder.getTraceThread(mb.testThread1)); + traceManager.activateThread(thread); waitForSwing(); assertEquals(mb.testThread1, mb.testModel.session.getFocus()); TestTargetStack stack = mb.testThread1.addStack(); // Note, push simply moves the data, the new frame still has the higher index - TestTargetStackFrameHasRegisterBank frame0 = stack.pushFrameHasBank(); - TestTargetStackFrameHasRegisterBank frame1 = stack.pushFrameHasBank(); + TestTargetStackFrameHasRegisterBank frame0 = stack.pushFrameHasBank(mb.addr(0x00400000)); + TestTargetStackFrameHasRegisterBank frame1 = stack.pushFrameHasBank(mb.addr(0x00400100)); waitForDomainObject(trace); + // Eww. I'm starting to think this could be cheating, considering focus sync at launch + waitForValue(() -> recorder.getTraceStackFrame(frame0)); + waitForValue(() -> recorder.getTraceStackFrame(frame1)); + waitForValue(() -> recorder.getTargetStackFrame(thread, 0)); + waitForValue(() -> recorder.getTargetStackFrame(thread, 1)); // Starting with 0 results in no change in coordinates, so ignored traceManager.activateFrame(1); @@ -418,8 +427,8 @@ public class DebuggerTraceManagerServiceTest extends AbstractGhidraHeadedDebugge TestTargetStack stack = mb.testThread1.addStack(); // Note, push simply moves the data, the new frame still has the higher index - TestTargetStackFrameHasRegisterBank frame0 = stack.pushFrameHasBank(); - TestTargetStackFrameHasRegisterBank frame1 = stack.pushFrameHasBank(); + TestTargetStackFrameHasRegisterBank frame0 = stack.pushFrameHasBank(mb.addr(0x00400000)); + TestTargetStackFrameHasRegisterBank frame1 = stack.pushFrameHasBank(mb.addr(0x00400100)); waitForDomainObject(trace); // Starting with 0 results in no change in coordinates, so ignored diff --git a/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/util/ValueUtils.java b/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/util/ValueUtils.java index 3760c58ca7..01c271c0f5 100644 --- a/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/util/ValueUtils.java +++ b/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/util/ValueUtils.java @@ -15,8 +15,6 @@ */ package ghidra.dbg.util; -import java.util.Collection; - import ghidra.util.Msg; public enum ValueUtils { @@ -44,19 +42,4 @@ public enum ValueUtils { } return cls.cast(val); } - - public static boolean expectBoolean(Object val, Object logObj, String attributeName, - boolean fallback, boolean required) { - Boolean exp = expectType(val, Boolean.class, logObj, attributeName, null, required); - if (exp == null) { - return fallback; - } - return exp; - } - - @SuppressWarnings("unchecked") - public static , E> Class colOf(Class colType, - Class elemType) { - return (Class) colType; - } } diff --git a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestDebuggerModelBuilder.java b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestDebuggerModelBuilder.java index 4c7f09eaf7..cf3ad8bc3e 100644 --- a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestDebuggerModelBuilder.java +++ b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestDebuggerModelBuilder.java @@ -92,7 +92,7 @@ public class TestDebuggerModelBuilder { * Create register banks which are the top frame of stacks attributed to the threads. */ public void createTestThreadStacksAndFramesAreRegisterBanks() { - applyThreadRegisterBankConvention(t -> t.addStack().pushFrameIsBank()); + applyThreadRegisterBankConvention(t -> t.addStack().pushFrameIsBank(addr(0x00400000))); } /** @@ -100,6 +100,7 @@ public class TestDebuggerModelBuilder { * threads. */ public void createTestThreadStacksAndFramesHaveRegisterBanks() { - applyThreadRegisterBankConvention(t -> t.addStack().pushFrameHasBank().getBank()); + applyThreadRegisterBankConvention( + t -> t.addStack().pushFrameHasBank(addr(0x00400000)).getBank()); } } diff --git a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestTargetStack.java b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestTargetStack.java index 8ef6956db6..c7d6dc9abd 100644 --- a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestTargetStack.java +++ b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestTargetStack.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; import ghidra.dbg.target.TargetStack; +import ghidra.program.model.address.Address; public class TestTargetStack extends DefaultTestTargetObject implements TargetStack { @@ -42,8 +43,8 @@ public class TestTargetStack extends DefaultTestTargetObject - - - -