diff --git a/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/evt/DbgDebuggeeStateChangeEvent.java b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/evt/DbgDebuggeeStateChangeEvent.java new file mode 100644 index 0000000000..f9bb205d47 --- /dev/null +++ b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/evt/DbgDebuggeeStateChangeEvent.java @@ -0,0 +1,48 @@ +/* ### + * IP: GHIDRA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package agent.dbgeng.manager.evt; + +import agent.dbgeng.dbgeng.DebugClient.ChangeDebuggeeState; +import ghidra.comm.util.BitmaskSet; + +/** + * The event corresponding with ChangedDebuggeeState + */ +public class DbgDebuggeeStateChangeEvent extends AbstractDbgEvent { + private final BitmaskSet flags; + private final long argument; + + /** + * The selected flags must be specified by dbgeng. + * + * @param flags dbgeng-provided id + * @param argument event-specific argument + */ + public DbgDebuggeeStateChangeEvent(BitmaskSet flags, long argument) { + super((int) flags.getBitmask()); + this.flags = flags; + this.argument = argument; + } + + public BitmaskSet getFlags() { + return flags; + } + + public long getArgument() { + return argument; + } + +} diff --git a/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgDebugEventCallbacksAdapter.java b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgDebugEventCallbacksAdapter.java index d836efd2d4..1541d09cde 100644 --- a/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgDebugEventCallbacksAdapter.java +++ b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgDebugEventCallbacksAdapter.java @@ -18,8 +18,7 @@ package agent.dbgeng.manager.impl; import java.nio.file.Paths; import agent.dbgeng.dbgeng.*; -import agent.dbgeng.dbgeng.DebugClient.ChangeEngineState; -import agent.dbgeng.dbgeng.DebugClient.DebugStatus; +import agent.dbgeng.dbgeng.DebugClient.*; import agent.dbgeng.dbgeng.util.DebugEventCallbacksAdapter; import agent.dbgeng.manager.DbgState; import agent.dbgeng.manager.evt.*; @@ -132,13 +131,14 @@ public class DbgDebugEventCallbacksAdapter extends DebugEventCallbacksAdapter { return checkInterrupt(DebugStatus.NO_CHANGE); } - /* @Override public DebugStatus changeDebuggeeState(BitmaskSet flags, long argument) { - System.err.println("CHANGE_DEBUGGEE_STATE: " + flags + ":" + argument); - return DebugStatus.NO_CHANGE; + //System.err.println("CHANGE_DEBUGGEE_STATE: " + flags + ":" + argument); + return checkInterrupt( + manager.processEvent(new DbgDebuggeeStateChangeEvent(flags, argument))); } - + + /* @Override public DebugStatus sessionStatus(SessionStatus status) { System.err.println("SESSION_STATUS: " + status); diff --git a/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgManagerImpl.java b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgManagerImpl.java index 3d13198ac9..0550cfd7e2 100644 --- a/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgManagerImpl.java +++ b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/manager/impl/DbgManagerImpl.java @@ -15,7 +15,7 @@ */ package agent.dbgeng.manager.impl; -import static ghidra.async.AsyncUtils.*; +import static ghidra.async.AsyncUtils.sequence; import java.util.*; import java.util.concurrent.CompletableFuture; @@ -594,6 +594,7 @@ public class DbgManagerImpl implements DbgManager { handlerMap.put(DbgStateChangedEvent.class, this::processStateChanged); handlerMap.put(DbgSessionSelectedEvent.class, this::processSessionSelected); handlerMap.put(DbgSystemsEvent.class, this::processSystemsEvent); + handlerMap.putVoid(DbgDebuggeeStateChangeEvent.class, this::processDebuggeeStateChanged); handlerMap.putVoid(DbgCommandDoneEvent.class, this::processDefault); handlerMap.putVoid(DbgStoppedEvent.class, this::processDefault); handlerMap.putVoid(DbgRunningEvent.class, this::processDefault); @@ -1034,6 +1035,12 @@ public class DbgManagerImpl implements DbgManager { return statusMap.get(evt.getClass()); } + protected void processDebuggeeStateChanged(DbgDebuggeeStateChangeEvent evt, Void v) { + if (evt.getFlags().contains(ChangeDebuggeeState.DATA)) { + getEventListeners().fire.memoryChanged(currentProcess, 0L, 0, evt.getCause()); + } + } + protected void processConsoleOutput(DbgConsoleOutputEvent evt, Void v) { getEventListeners().fire.consoleOutput(evt.getInfo(), evt.getMask()); } diff --git a/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/model/impl/DbgModelTargetProcessImpl.java b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/model/impl/DbgModelTargetProcessImpl.java index 59292626b8..87194d3931 100644 --- a/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/model/impl/DbgModelTargetProcessImpl.java +++ b/Ghidra/Debug/Debugger-agent-dbgeng/src/main/java/agent/dbgeng/model/impl/DbgModelTargetProcessImpl.java @@ -224,6 +224,13 @@ public class DbgModelTargetProcessImpl extends DbgModelTargetObjectImpl } } + @Override + public void memoryChanged(DbgProcess proc, long addr, int len, DbgCause cause) { + if (proc.equals(this.process)) { + listeners.fire.invalidateCacheRequested(memory); + } + } + @Override public CompletableFuture setActive() { DbgManagerImpl manager = getManager(); diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelImpl.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelImpl.java index 6304afa018..631bf2d4c2 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelImpl.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelImpl.java @@ -111,7 +111,6 @@ public class GdbModelImpl extends AbstractDebuggerObjectModel { break; } case RUNNING: { - session.invalidateMemoryAndRegisterCaches(); session.setAccessible(false); break; } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java index 799c53938a..ecaaaf5120 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java @@ -441,9 +441,13 @@ public class GdbModelTargetInferior inferiorRunning(sco.getReason()); List params = new ArrayList<>(); gatherThreads(params, sco.getAffectedThreads()); + if (targetEventThread == null && !params.isEmpty()) { + targetEventThread = threads.getTargetThread(sco.getAffectedThreads().iterator().next()); + } if (targetEventThread != null) { impl.session.getListeners().fire.event(impl.session, targetEventThread, TargetEventType.RUNNING, "Running", params); + invalidateMemoryAndRegisterCaches(); } } if (sco.getState() != GdbState.STOPPED) { diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetProcessMemory.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetProcessMemory.java index 660a0e7403..9aa00cb375 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetProcessMemory.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetProcessMemory.java @@ -181,6 +181,8 @@ public class GdbModelTargetProcessMemory }); } + // TODO: Seems this is only called when sco.getState() == STOPPED. + // Maybe should name it such public CompletableFuture stateChanged(GdbStateChangeRecord sco) { return requestElements(false).thenCompose(__ -> { AsyncFence fence = new AsyncFence(); diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java index 88db498b61..992b9a2b09 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java @@ -77,6 +77,7 @@ import ghidra.program.util.ProgramSelection; import ghidra.trace.model.*; import ghidra.trace.model.Trace.*; import ghidra.trace.model.memory.TraceMemoryRegion; +import ghidra.trace.model.memory.TraceMemoryState; import ghidra.trace.model.modules.*; import ghidra.trace.model.program.TraceProgramView; import ghidra.trace.model.program.TraceVariableSnapProgramView; @@ -257,6 +258,8 @@ public class DebuggerListingProvider extends CodeViewerProvider implements Listi listenFor(TraceSectionChangeType.ADDED, this::sectionChanged); listenFor(TraceSectionChangeType.CHANGED, this::sectionChanged); listenFor(TraceSectionChangeType.DELETED, this::sectionChanged); + + listenFor(TraceMemoryStateChangeType.CHANGED, this::memStateChanged); } private void snapshotAdded(TraceSnapshot snapshot) { @@ -301,6 +304,22 @@ public class DebuggerListingProvider extends CodeViewerProvider implements Listi private void sectionChanged(TraceSection section) { updateLabelDebouncer.contact(null); } + + private void memStateChanged(TraceAddressSnapRange range, TraceMemoryState oldIsNull, + TraceMemoryState newState) { + if (current.getView() == null) { + return; + } + if (!range.getLifespan().contains(current.getSnap())) { + return; + } + // TODO: Debounce this? + getListingPanel().getFieldPanel().repaint(); + + if (newState == TraceMemoryState.UNKNOWN) { + doAutoReadMemory(); + } + } } protected class ForAccessRecorderListener implements TraceRecorderListener { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultThreadRecorder.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultThreadRecorder.java index cfc9d349f8..c3742aedb3 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultThreadRecorder.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DefaultThreadRecorder.java @@ -26,8 +26,7 @@ import ghidra.async.AsyncUtils; import ghidra.dbg.target.*; import ghidra.dbg.target.TargetExecutionStateful.TargetExecutionState; import ghidra.dbg.util.PathUtils; -import ghidra.program.model.address.Address; -import ghidra.program.model.address.AddressRange; +import ghidra.program.model.address.*; import ghidra.program.model.data.Pointer; import ghidra.program.model.lang.*; import ghidra.trace.model.Trace; @@ -331,6 +330,24 @@ public class DefaultThreadRecorder implements ManagedThreadRecorder { }, getTargetThread().getJoinedPath(".")); } + @Override + public void invalidateRegisterValues(TargetRegisterBank bank) { + int frameLevel = stackRecorder.getSuccessorFrameLevel(bank); + long snap = recorder.getSnap(); + String path = bank.getJoinedPath("."); + + recorder.parTx.execute("Registers invalidated: " + path, () -> { + TraceMemoryRegisterSpace regSpace = + memoryManager.getMemoryRegisterSpace(traceThread, frameLevel, false); + if (regSpace == null) { + return; + } + AddressSpace as = regSpace.getAddressSpace(); + regSpace.setState(snap, as.getMinAddress(), as.getMaxAddress(), + TraceMemoryState.UNKNOWN); + }, path); + } + public CompletableFuture writeThreadRegisters(int frameLevel, Map values) { if (!regMapper.getRegistersOnTarget().containsAll(values.keySet())) { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/TraceEventListener.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/TraceEventListener.java index 1d2f5ffd4d..e8700d8758 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/TraceEventListener.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/TraceEventListener.java @@ -28,8 +28,7 @@ import ghidra.dbg.target.TargetEventScope.TargetEventType; import ghidra.dbg.target.TargetExecutionStateful.TargetExecutionState; import ghidra.dbg.util.DebuggerCallbackReorderer; import ghidra.dbg.util.PathUtils; -import ghidra.program.model.address.Address; -import ghidra.program.model.address.AddressRange; +import ghidra.program.model.address.*; import ghidra.trace.model.Trace; import ghidra.trace.model.memory.TraceMemoryManager; import ghidra.trace.model.memory.TraceMemoryState; @@ -50,6 +49,8 @@ public class TraceEventListener extends AnnotatedDebuggerAttributeListener { protected final DebuggerCallbackReorderer reorderer = new DebuggerCallbackReorderer(this); protected final PrivatelyQueuedListener queue; + private boolean ignoreInvalidation = false; + public TraceEventListener(TraceObjectManager collection) { super(MethodHandles.lookup()); this.recorder = collection.getRecorder(); @@ -97,13 +98,6 @@ public class TraceEventListener extends AnnotatedDebuggerAttributeListener { private boolean eventApplies(TargetObject eventThread, TargetEventType type, List parameters) { - if (type == TargetEventType.RUNNING) { - return false; - /** - * TODO: Perhaps some configuration for this later. It's kind of interesting to record - * the RUNNING event time, but it gets pedantic when these exist between steps. - */ - } if (eventThread != null) { return successor(eventThread); } @@ -131,8 +125,21 @@ public class TraceEventListener extends AnnotatedDebuggerAttributeListener { if (!eventApplies(eventThread, type, parameters)) { return; } + if (type == TargetEventType.RUNNING) { + /** + * Do not permit the current snapshot to be invalidated on account of the target + * running. When the STOP occurs, a new (completely UNKNOWN) snapshot is generated. + */ + ignoreInvalidation = true; + return; + /** + * TODO: Perhaps some configuration for this later. It's kind of interesting to record + * the RUNNING event time, but it gets pedantic when these exist between steps. + */ + } ManagedThreadRecorder rec = recorder.getThreadRecorder(eventThread); recorder.createSnapshot(description, rec == null ? null : rec.getTraceThread(), null); + ignoreInvalidation = false; if (type == TargetEventType.MODULE_LOADED) { long snap = recorder.getSnap(); @@ -180,6 +187,30 @@ public class TraceEventListener extends AnnotatedDebuggerAttributeListener { } } + @Override + public void invalidateCacheRequested(TargetObject object) { + if (!valid) { + return; + } + if (ignoreInvalidation) { + return; + } + if (object instanceof TargetRegisterBank) { + ManagedThreadRecorder rec = recorder.getThreadRecorderForSuccessor(object); + if (rec != null) { + rec.invalidateRegisterValues((TargetRegisterBank) object); + } + } + if (object instanceof TargetMemory) { + long snap = recorder.getSnap(); + String path = object.getJoinedPath("."); + recorder.parTx.execute("Memory invalidated: " + path, () -> { + AddressSet set = trace.getBaseLanguage().getAddressFactory().getAddressSet(); + memoryManager.setState(snap, set, TraceMemoryState.UNKNOWN); + }, path); + } + } + @Override public void registersUpdated(TargetObject bank, Map updates) { if (!valid) { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/interfaces/ManagedThreadRecorder.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/interfaces/ManagedThreadRecorder.java index f0a5aad404..2c7eb4d730 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/interfaces/ManagedThreadRecorder.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/interfaces/ManagedThreadRecorder.java @@ -39,6 +39,8 @@ public interface ManagedThreadRecorder extends AbstractTraceRecorder { public void recordRegisterValues(TargetRegisterBank bank, Map updates); + public void invalidateRegisterValues(TargetRegisterBank bank); + public boolean objectRemoved(TargetObject removed); public void stateChanged(TargetExecutionState state); diff --git a/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/DebuggerModelListener.java b/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/DebuggerModelListener.java index 46b11407b9..f29c2cff9e 100644 --- a/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/DebuggerModelListener.java +++ b/Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/DebuggerModelListener.java @@ -304,7 +304,7 @@ public interface DebuggerModelListener { * it need only report reads or writes which updated that cache. However, that cache must be * invalidated whenever any other event occurs which could change register values, e.g., the * target stepping or running. See {@link #invalidateCacheRequested(TargetObject)}. If the - * implementation doe not employ a cache, then it must report every successful + * implementation does not employ a cache, then it must report every successful * client-driven read or write. If the implementation can detect debugger-driven * register reads and writes, then it recommended to call this method for those events. However, * this method must not be called for target-driven register changes, except diff --git a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelSteppableTest.java b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelSteppableTest.java index 5e58a303e1..d724cf1c09 100644 --- a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelSteppableTest.java +++ b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelSteppableTest.java @@ -15,10 +15,15 @@ */ package ghidra.dbg.test; -import static org.junit.Assert.*; -import static org.junit.Assume.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.junit.Assume.assumeNotNull; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import org.junit.Test; @@ -27,9 +32,15 @@ import ghidra.async.AsyncDebouncer; import ghidra.async.AsyncTimer; import ghidra.dbg.DebugModelConventions.AsyncState; import ghidra.dbg.DebuggerModelListener; -import ghidra.dbg.target.*; import ghidra.dbg.target.TargetEventScope.TargetEventType; +import ghidra.dbg.target.TargetExecutionStateful; import ghidra.dbg.target.TargetExecutionStateful.TargetExecutionState; +import ghidra.dbg.target.TargetMemory; +import ghidra.dbg.target.TargetObject; +import ghidra.dbg.target.TargetRegisterBank; +import ghidra.dbg.target.TargetSteppable; +import ghidra.dbg.target.TargetThread; +import ghidra.util.Msg; /** * Tests the functionality of a single-stepping a target @@ -116,7 +127,8 @@ public abstract class AbstractDebuggerModelSteppableTest extends AbstractDebugge EVENT_RUNNING, EVENT_STOPPED, REGS_UPDATED, - CACHE_INVALIDATED; + REGS_CACHE_INVALIDATED, + MEM_CACHE_INVALIDATED; } /** @@ -172,8 +184,14 @@ public abstract class AbstractDebuggerModelSteppableTest extends AbstractDebugge @Override public void invalidateCacheRequested(TargetObject object) { synchronized (callbacks) { - callbacks.add(CallbackType.CACHE_INVALIDATED); - log.add("invalidateCacheRequested()"); + if (object instanceof TargetRegisterBank) { + callbacks.add(CallbackType.REGS_CACHE_INVALIDATED); + log.add("invalidateCacheRequested(TargetRegisterBank)"); + } + else if (object instanceof TargetMemory) { + callbacks.add(CallbackType.MEM_CACHE_INVALIDATED); + log.add("invalidateCacheRequested(TargetMemory)"); + } } debouncer.contact(null); } @@ -215,17 +233,99 @@ public abstract class AbstractDebuggerModelSteppableTest extends AbstractDebugge callbacks = List.copyOf(listener.callbacks); } - int stoppedIdx = callbacks.indexOf(CallbackType.EVENT_STOPPED); - assertNotEquals(-1, stoppedIdx); - List follows = callbacks.subList(stoppedIdx + 1, callbacks.size()); - assertFalse("Observed multiple event(STOPPED/OTHER) callbacks for one step", - follows.contains(CallbackType.EVENT_STOPPED)); - int regsUpdatedIdx = callbacks.indexOf(CallbackType.REGS_UPDATED); - assertNotEquals("Did not observe a registersUpdated() callback", -1, regsUpdatedIdx); - assertTrue("registersUpdated() must follow event(STOPPED/OTHER)", - regsUpdatedIdx > stoppedIdx); - int invalidatedIdx = follows.indexOf(CallbackType.CACHE_INVALIDATED); - assertTrue("Observed an invalidateCacheRequest() after registersUpdated()", - invalidatedIdx < regsUpdatedIdx); // absent or precedes + Msg.info(this, "Observations: " + callbacks); + + boolean observedRunning = false; + boolean observedStopped = false; + boolean observedRegsUpdated = false; + boolean observedInvalidateRegs = false; + boolean observedInvalidateMem = false; + for (CallbackType cb : callbacks) { + switch (cb) { + case EVENT_RUNNING: + if (observedRunning) { + fail("Observed a second event(RUNNING)."); + } + observedRunning = true; + break; + case EVENT_STOPPED: + if (observedStopped) { + fail("Observed a second event(STOPPED)."); + } + observedStopped = true; + break; + case REGS_UPDATED: + if (!observedStopped) { + fail("Observed registersUpdated() before event(STOPPED)."); + } + observedRegsUpdated = true; + break; + case REGS_CACHE_INVALIDATED: + if (!observedStopped) { + if (observedRunning) { + // Recorder will ignore, but it still counts for flushing model + break; + } + /** + * Spurious cache invalidations are not permitted before the recorder would + * create a new snap. It knows to ignore invalidations that occur between + * event(RUNNING) and event(STOPPED). + */ + fail("Observed a spurious invalidateCacheRequested(Regs)."); + } + else if (observedInvalidateRegs) { + Msg.warn(this, + "Observed an extra invalidateCacheRequested(Regs) after event(STOPPED)."); + } + else if (observedRegsUpdated) { + // The invalidate would effectively undo the update + fail("Observed invalidateCacheRequested(Regs) after registersUpdated()."); + } + else { + observedInvalidateRegs = true; + } + break; + case MEM_CACHE_INVALIDATED: + if (!observedStopped) { + if (observedRunning) { + // Recorder will ignore, but it still counts for flushing model + observedInvalidateMem = true; + break; + } + /** + * Spurious cache invalidations are not permitted before the recorder would + * create a new snap. It knows to ignore invalidations that occur between + * event(RUNNING) and event(STOPPED). + */ + fail("Observed a spurious invalidateCacheRequested(Mem)."); + } + else if (observedInvalidateMem) { + Msg.warn(this, + "Observed an extra invalidateCacheRequested(Mem) after event(STOPPED)."); + } + else { + observedInvalidateMem = true; + } + break; + default: + throw new AssertionError(); + } + } + + if (!observedStopped) { + fail("Never observed event(STOPPED)."); + } + if (!observedRegsUpdated && !observedInvalidateRegs) { + fail("Observed neither invalidateCacheRequested(Regs) nor registersUpdated()."); + } + if (!observedInvalidateMem) { + /** + * NOTE: even though STOPPED advances the snapshot, we still require + * invalidateCacheRequested(Mem) to ensure the model's memory cache is not stale. + * Whether or not your model employs an internal cache, this is required, because your + * model may be accessed via a proxy model (e.g., GADP) that uses a cache. + */ + fail("Never observed invalidateCacheRequested(Mem)."); + } } } 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 bd8ef5e850..418b5a545d 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 @@ -328,9 +328,19 @@ public class DBTraceMemorySpace implements Unfinished, TraceMemorySpace, DBTrace } protected void checkState(TraceMemoryState state) { - if (state == null || state == TraceMemoryState.UNKNOWN) { + /** + * TODO: I don't remember why I prohibited this originally. It seems some technicality in + * calling something "last known?" We might revisit and specify that definition, in the face + * of memory being invalidated while the target is suspended. It seems appropriate to leave + * the stale bytes in the trace, but change the state to UNKNOWN. Do those stale bytes still + * count as last known? Aside from getBytes, I don't see anything that requires a precise + * distinction. We might be careful how this interacts with removeBytes, though.... Well, + * AFAICT, it doesn't depend on state markings. For now, I'm going to allow it. We'll see + * what happens. Changing this had no effect on the unit tests :/ . + */ + /*if (state == null || state == TraceMemoryState.UNKNOWN) { throw new IllegalArgumentException("Cannot erase memory state without removing bytes"); - } + }*/ } @Override