Merge remote-tracking branch 'origin/patch'

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/disassemble/AbstractTracePatchInstructionAction.java

@@ -25,6 +25,7 @@ import ghidra.app.plugin.core.assembler.AssemblyDualTextField;
 import ghidra.app.plugin.core.assembler.PatchInstructionAction;
 import ghidra.app.services.DebuggerControlService;
 import ghidra.app.services.DebuggerControlService.StateEditor;
+import ghidra.framework.cmd.BackgroundCommand;
 import ghidra.program.model.address.*;
 import ghidra.program.model.lang.*;
 import ghidra.program.model.listing.CodeUnit;
@@ -32,6 +33,8 @@ import ghidra.program.model.mem.MemoryAccessException;
 import ghidra.program.util.DefaultLanguageService;
 import ghidra.trace.model.guest.TracePlatform;
 import ghidra.trace.model.program.TraceProgramView;
+import ghidra.util.Msg;
+import ghidra.util.task.TaskMonitor;
 
 public abstract class AbstractTracePatchInstructionAction extends PatchInstructionAction {
 	protected final DebuggerDisassemblerPlugin plugin;
@@ -116,15 +119,18 @@ public abstract class AbstractTracePatchInstructionAction extends PatchInstructi
 		return Assemblers.getAssembler(language);
 	}
 
-	@Override
+	class PatchInstructionCommand extends BackgroundCommand<TraceProgramView> {
-	protected void applyPatch(byte[] data) throws MemoryAccessException {
+		private final byte[] data;
-		TraceProgramView view = getView();
+
-		if (view == null) {
+		public PatchInstructionCommand(byte[] data) {
-			return;
+			this.data = data;
 		}
+
+		@Override
+		public boolean applyTo(TraceProgramView view, TaskMonitor monitor) {
 			DebuggerControlService controlService = tool.getService(DebuggerControlService.class);
 			if (controlService == null) {
-			return;
+				return true;
 			}
 			StateEditor editor = controlService.createStateEditor(view);
 			Address address = getAddress();
@@ -138,7 +144,9 @@ public abstract class AbstractTracePatchInstructionAction extends PatchInstructi
 				editor.setVariable(address, data).get(1, TimeUnit.SECONDS);
 			}
 			catch (InterruptedException | ExecutionException | TimeoutException e) {
-			throw new MemoryAccessException("Couldn't patch", e);
+				setStatusMsg("Couldn't patch: " + e);
+				Msg.error(this, "Couldn't patch", e);
+				return false;
 			}
 
 			AddressSetView set = new AddressSet(address, address.add(data.length - 1));
@@ -147,6 +155,19 @@ public abstract class AbstractTracePatchInstructionAction extends PatchInstructi
 				dis.setInitialContext(contextValue);
 			}
 			dis.run(tool, view);
+
+			return true;
+		}
+	}
+
+	@Override
+	protected void applyPatch(byte[] data) throws MemoryAccessException {
+		TraceProgramView view = getView();
+		if (view == null) {
+			return;
+		}
+		PatchInstructionCommand patch = new PatchInstructionCommand(data);
+		patch.run(tool, view);
 	}
 
 	protected TraceProgramView getView() {

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/disassemble/TracePatchDataAction.java

@@ -20,6 +20,7 @@ import java.util.concurrent.*;
 import ghidra.app.plugin.core.assembler.PatchDataAction;
 import ghidra.app.services.DebuggerControlService;
 import ghidra.app.services.DebuggerControlService.StateEditor;
+import ghidra.framework.cmd.BackgroundCommand;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressRange;
 import ghidra.program.model.listing.CodeUnit;
@@ -27,6 +28,8 @@ import ghidra.program.model.mem.MemoryAccessException;
 import ghidra.program.model.util.CodeUnitInsertionException;
 import ghidra.trace.model.listing.TraceData;
 import ghidra.trace.model.program.TraceProgramView;
+import ghidra.util.Msg;
+import ghidra.util.task.TaskMonitor;
 
 public class TracePatchDataAction extends PatchDataAction {
 	protected final DebuggerDisassemblerPlugin plugin;
@@ -41,25 +44,42 @@ public class TracePatchDataAction extends PatchDataAction {
 		return super.isApplicableToUnit(cu) && cu instanceof TraceData;
 	}
 
-	@Override
+	class PatchDataCommand extends BackgroundCommand<TraceProgramView> {
-	protected void applyPatch(AddressRange rng, byte[] encoded)
+		private final byte[] encoded;
-			throws MemoryAccessException, CodeUnitInsertionException {
+
-		if (!(getProgram() instanceof TraceProgramView view)) {
+		public PatchDataCommand(byte[] encoded) {
-			return;
+			this.encoded = encoded;
 		}
+
+		@Override
+		public boolean applyTo(TraceProgramView view, TaskMonitor monitor) {
 			DebuggerControlService controlService = tool.getService(DebuggerControlService.class);
 			if (controlService == null) {
-			return;
+				return true;
 			}
 			StateEditor editor = controlService.createStateEditor(view);
 			Address address = getAddress();
 
 			try {
 				editor.setVariable(address, encoded).get(1, TimeUnit.SECONDS);
+				// Let the trace do everything regarding existing units
+				return true;
 			}
 			catch (InterruptedException | ExecutionException | TimeoutException e) {
-			throw new MemoryAccessException("Couldn't patch", e);
+				setStatusMsg("Couldn't patch: " + e);
+				Msg.error(this, "Couldn't patch", e);
+				return false;
 			}
-		// Let the trace do everything regarding existing units
+		}
+	}
+
+	@Override
+	protected void applyPatch(AddressRange rng, byte[] encoded)
+			throws MemoryAccessException, CodeUnitInsertionException {
+		if (!(getProgram() instanceof TraceProgramView view)) {
+			return;
+		}
+		PatchDataCommand patch = new PatchDataCommand(encoded);
+		patch.run(tool, view);
 	}
 }

Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/control/DebuggerControlPluginTest.java

@@ -466,15 +466,19 @@ public class DebuggerControlPluginTest extends AbstractGhidraHeadedDebuggerInteg
 
 		assertTrue(
 			helper.patchInstructionAction.isAddToPopup(listingProvider.getActionContext(null)));
+		long snapBefore = traceManager.getCurrent().getViewSnap();
 		Instruction ins =
 			helper.patchInstructionAt(tb.addr(0x00400123), "imm r0,#0x0", "imm r0,#0x3d2");
 		assertEquals(2, ins.getLength());
 
+		waitForPass(() -> assertNotEquals(snapBefore, traceManager.getCurrent().getViewSnap()));
 		long snap = traceManager.getCurrent().getViewSnap();
 		assertTrue(Lifespan.isScratch(snap));
 		byte[] bytes = new byte[2];
+		waitForPass(noExc(() -> {
 			view.getMemory().getBytes(tb.addr(0x00400123), bytes);
 			assertArrayEquals(tb.arr(0x30, 0xd2), bytes);
+		}));
 	}
 
 	@Test
@@ -517,6 +521,7 @@ public class DebuggerControlPluginTest extends AbstractGhidraHeadedDebuggerInteg
 
 		goTo(listingProvider.getListingPanel(), new ProgramLocation(view, tb.addr(0x00400123)));
 		assertTrue(helper.patchDataAction.isAddToPopup(listingProvider.getActionContext(null)));
+		long snapBefore = traceManager.getCurrent().getViewSnap();
 
 		/**
 		 * TODO: There's a bug in the trace forking: Data units are not replaced when bytes changed.
@@ -525,11 +530,14 @@ public class DebuggerControlPluginTest extends AbstractGhidraHeadedDebuggerInteg
 		/*Data data =*/ helper.patchDataAt(tb.addr(0x00400123), "0h", "5h");
 		// assertEquals(2, data.getLength());
 
+		waitForPass(() -> assertNotEquals(snapBefore, traceManager.getCurrent().getViewSnap()));
 		long snap = traceManager.getCurrent().getViewSnap();
 		assertTrue(Lifespan.isScratch(snap));
 		byte[] bytes = new byte[2];
+		waitForPass(noExc(() -> {
 			view.getMemory().getBytes(tb.addr(0x00400123), bytes);
 			assertArrayEquals(tb.arr(0, 5), bytes);
+		}));
 	}
 
 	@Test