GP-0: Fixes for tests post GP-2676/3028

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java

@@ -230,7 +230,7 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 				return;
 			}
 			try {
-				info.trackTraceBreakpoint(c.a, tb, false);
+				info.trackTraceBreakpoint(c.a, tb, getMode(info.trace), false);
 			}
 			catch (TrackedTooSoonException e) {
 				Msg.info(this,
@@ -243,7 +243,7 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 				return;
 			}
 			try {
-				info.trackTraceBreakpoint(c.a, tb, true);
+				info.trackTraceBreakpoint(c.a, tb, getMode(info.trace), true);
 			}
 			catch (TrackedTooSoonException e) {
 				Msg.info(this,
@@ -269,7 +269,7 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 			}
 			else {
 				try {
-					info.trackTraceBreakpoint(c.a, tb, false);
+					info.trackTraceBreakpoint(c.a, tb, getMode(info.trace), false);
 				}
 				catch (TrackedTooSoonException e) {
 					Msg.info(this, "Ignoring " + tb +
@@ -553,28 +553,17 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 			}
 			Collection<TraceBreakpoint> visible = new ArrayList<>();
 			for (AddressRange range : trace.getBaseAddressFactory().getAddressSet()) {
-				Collection<? extends TraceBreakpoint> breaks = trace
+				visible.addAll(trace.getBreakpointManager()
-						.getBreakpointManager()
+						.getBreakpointsIntersecting(Lifespan.at(snap), range));
-						.getBreakpointsIntersecting(Lifespan.at(snap), range);
-				if (mode.useEmulatedBreakpoints()) {
-					visible.addAll(breaks);
-				}
-				else {
-					for (TraceBreakpoint tb : breaks) {
-						if (recorder.getTargetBreakpoint(tb) != null) {
-							visible.add(tb);
-						}
-					}
-				}
 			}
-			trackTraceBreakpoints(a, visible);
+			trackTraceBreakpoints(a, visible, mode);
 		}
 
 		protected void trackTraceBreakpoints(AddCollector a,
-				Collection<TraceBreakpoint> breakpoints) {
+				Collection<TraceBreakpoint> breakpoints, StateEditingMode mode) {
 			for (TraceBreakpoint tb : breakpoints) {
 				try {
-					trackTraceBreakpoint(a, tb, false);
+					trackTraceBreakpoint(a, tb, mode, false);
 				}
 				catch (TrackedTooSoonException e) {
 					// This can still happen during reload (on OBJECT_RESTORED)
@@ -597,8 +586,13 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 				new DefaultTraceLocation(trace, null, Lifespan.at(snap), tb.getMinAddress()));
 		}
 
-		protected void trackTraceBreakpoint(AddCollector a, TraceBreakpoint tb, boolean forceUpdate)
+		protected void trackTraceBreakpoint(AddCollector a, TraceBreakpoint tb,
+				StateEditingMode mode, boolean forceUpdate)
 				throws TrackedTooSoonException {
+			if (!mode.useEmulatedBreakpoints() &&
+				(recorder == null || recorder.getTargetBreakpoint(tb) == null)) {
+				return;
+			}
 			Address traceAddr = tb.getMinAddress();
 			ProgramLocation progLoc = computeStaticLocation(tb);
 			LogicalBreakpointInternal lb;

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java

@@ -807,7 +807,9 @@ public class DebuggerTraceManagerServicePlugin extends Plugin
 			}
 			varView.setSnap(snap);
 			fireLocationEvent(coordinates, cause);
-		}, SwingExecutorService.LATER); // Not MAYBE_NOW lest snap events get out of order
+		}, cause == ActivationCause.EMU_STATE_EDIT
+				? SwingExecutorService.MAYBE_NOW // ProgramView may call .get on Swing thread
+				: SwingExecutorService.LATER); // Respect event order
 	}
 
 	protected void fireLocationEvent(DebuggerCoordinates coordinates, ActivationCause cause) {

Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerTraceManagerService.java

@@ -61,6 +61,10 @@ public interface DebuggerTraceManagerService {
 		 * The change was driven by the recorder advancing a snapshot
 		 */
 		FOLLOW_PRESENT,
+		/**
+		 * The tool is activating scratch coordinates to display an emulator state change
+		 */
+		EMU_STATE_EDIT,
 		/**
 		 * The change was caused by a change to the mapper selection, probably indirectly by the
 		 * user

Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/StateEditingMode.java

@@ -319,7 +319,7 @@ public enum StateEditingMode {
 					throw new AssertionError(e);
 				}
 			}
-			return traceManager.activateAndNotify(withTime, ActivationCause.USER, false);
+			return traceManager.activateAndNotify(withTime, ActivationCause.EMU_STATE_EDIT, false);
 		}
 
 		@Override

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServiceTest.java

@@ -15,7 +15,6 @@
  */
 package ghidra.app.plugin.core.debug.service.breakpoint;
 
-import static ghidra.lifecycle.Unfinished.TODO;
 import static org.junit.Assert.*;
 
 import java.util.*;
@@ -26,6 +25,7 @@ import org.junit.*;
 
 import generic.Unique;
 import ghidra.app.plugin.core.debug.gui.AbstractGhidraHeadedDebuggerGUITest;
+import ghidra.app.plugin.core.debug.service.editing.DebuggerStateEditingServicePlugin;
 import ghidra.app.plugin.core.debug.service.modules.DebuggerStaticMappingUtils;
 import ghidra.app.services.*;
 import ghidra.app.services.LogicalBreakpoint.State;
@@ -547,16 +547,6 @@ public class DebuggerLogicalBreakpointServiceTest extends AbstractGhidraHeadedDe
 		assertTrue(breakpointService.getAllBreakpoints().isEmpty());
 	}
 
-	/**
-	 * TODO: When "resume recording" is implemented, consider that a "new" trace may already have
-	 * breakpoints
-	 */
-	@Test
-	@Ignore
-	public void testRecordTraceWithBreakpoints() {
-		TODO();
-	}
-
 	@Test
 	public void testRecordTraceThenOpenTraceThenAddBreakpoint() throws Throwable {
 		startRecorder1();
@@ -720,16 +710,6 @@ public class DebuggerLogicalBreakpointServiceTest extends AbstractGhidraHeadedDe
 		assertLogicalBreakpointForMappedSoftwareBreakpoint(trace);
 	}
 
-	/**
-	 * TODO: When "resume recording" is implemented, consider that a "new" trace may already have
-	 * mappings
-	 */
-	@Test
-	@Ignore
-	public void testOpenProgramWithBookmarkThenRecordTraceWithMapping() {
-		TODO();
-	}
-
 	@Test
 	public void testOpenProgramThenAddBookmarksThenRecordTraceThenAddMapping() throws Throwable {
 		startRecorder1();
@@ -1639,11 +1619,15 @@ public class DebuggerLogicalBreakpointServiceTest extends AbstractGhidraHeadedDe
 	@Test
 	public void testAddTraceBreakpointSetSleighThenMapThenSaveToProgramCopiesSleigh()
 			throws Throwable {
+		DebuggerStateEditingService editingService =
+			addPlugin(tool, DebuggerStateEditingServicePlugin.class);
+
 		// TODO: What if already mapped?
 		// Not sure I care about tb.setEmuSleigh() out of band
 
 		createTrace();
 		traceManager.openTrace(tb.trace);
+		editingService.setCurrentMode(tb.trace, StateEditingMode.RW_EMULATOR);
 		createProgramFromTrace();
 		intoProject(program);
 		programManager.openProgram(program);

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/editing/DebuggerStateEditingServiceTest.java

@@ -454,39 +454,24 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 	}
 
 	@Test
-	public void testWriteTargetMemoryNotPresentErr() throws Throwable {
+	public void testWriteTargetRequiresPresent() throws Throwable {
 		TraceRecorder recorder = recordAndWaitSync();
 		traceManager.openTrace(tb.trace);
 		activateTrace();
 		traceManager.activateThread(recorder.getTraceThread(mb.testThread1));
 		waitForSwing();
-		editingService.setCurrentMode(recorder.getTrace(), StateEditingMode.RW_TARGET);
 
-		traceManager.activateSnap(traceManager.getCurrentSnap() - 1);
+		editingService.setCurrentMode(tb.trace, StateEditingMode.RW_TARGET);
-
-		StateEditor editor = createStateEditor();
-		assertFalse(editor.isVariableEditable(tb.addr(0x00400000), 4));
-		expecting(MemoryAccessException.class, () -> {
-			waitOn(editor.setVariable(tb.addr(0x00400000), tb.arr(1, 2, 3, 4)));
-		});
-	}
-
-	@Test
-	public void testWriteTargetRegisterNotPresentErr() throws Throwable {
-		TraceRecorder recorder = recordAndWaitSync();
-		traceManager.openTrace(tb.trace);
-		activateTrace();
-		traceManager.activateThread(recorder.getTraceThread(mb.testThread1));
 		waitForSwing();
-		editingService.setCurrentMode(recorder.getTrace(), StateEditingMode.RW_TARGET);
+		assertEquals(recorder.getSnap(), traceManager.getCurrentSnap());
 
 		traceManager.activateSnap(traceManager.getCurrentSnap() - 1);
+		waitForSwing();
+		assertEquals(StateEditingMode.RW_EMULATOR, editingService.getCurrentMode(tb.trace));
 
-		StateEditor editor = createStateEditor();
+		editingService.setCurrentMode(tb.trace, StateEditingMode.RW_TARGET);
-		assertFalse(editor.isRegisterEditable(r0));
+		waitForSwing();
-		expecting(MemoryAccessException.class, () -> {
+		assertEquals(recorder.getSnap(), traceManager.getCurrentSnap());
-			waitOn(editor.setRegister(rv1234));
-		});
 	}
 
 	@Test
@@ -494,6 +479,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		createAndOpenTrace();
 		activateTrace();
 		editingService.setCurrentMode(tb.trace, StateEditingMode.RW_TARGET);
+		waitForSwing();
 
 		StateEditor editor = createStateEditor();
 		assertFalse(editor.isVariableEditable(tb.addr(0x00400000), 4));
@@ -507,6 +493,7 @@ public class DebuggerStateEditingServiceTest extends AbstractGhidraHeadedDebugge
 		createAndOpenTrace();
 		activateTrace();
 		editingService.setCurrentMode(tb.trace, StateEditingMode.RW_TARGET);
+		waitForSwing();
 
 		StateEditor editor = createStateEditor();
 		assertFalse(editor.isRegisterEditable(r0));