Merge remote-tracking branch 'origin/GP-0_Dan_testFixes-20220331-1'

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerLocationLabel.java

@@ -49,6 +49,7 @@ public class DebuggerLocationLabel extends JLabel {
 			listenFor(TraceMemoryRegionChangeType.LIFESPAN_CHANGED, this::regionChanged);
 			listenFor(TraceMemoryRegionChangeType.DELETED, this::regionChanged);
 
+			listenFor(TraceModuleChangeType.ADDED, this::moduleChanged);
 			listenFor(TraceModuleChangeType.CHANGED, this::moduleChanged);
 			listenFor(TraceModuleChangeType.LIFESPAN_CHANGED, this::moduleChanged);
 			listenFor(TraceModuleChangeType.DELETED, this::moduleChanged);

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/copying/DebuggerCopyIntoProgramDialog.java

@@ -787,8 +787,7 @@ public class DebuggerCopyIntoProgramDialog extends DialogComponentProvider {
 	}
 
 	protected void executeEntry(RangeEntry entry, Program dest, TraceRecorder recorder,
-			TaskMonitor monitor)
-			throws Exception {
+			TaskMonitor monitor) throws Exception {
 		MemoryBlock block = executeEntryBlock(entry, dest, monitor);
 		Address dstMin = entry.getDstRange().getMinAddress();
 		if (block.isOverlay()) {

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/pcode/BranchPcodeRow.java

@@ -27,7 +27,7 @@ public class BranchPcodeRow implements PcodeRow {
 	}
 
 	@Override
-	public int getSequence() {
+	public Integer getSequence() {
 		return sequence;
 	}
 

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/pcode/EnumPcodeRow.java

@@ -27,7 +27,7 @@ public enum EnumPcodeRow implements PcodeRow {
 	}
 
 	@Override
-	public int getSequence() {
+	public Integer getSequence() {
 		return 0;
 	}
 

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/pcode/FallthroughPcodeRow.java

@@ -25,7 +25,7 @@ public class FallthroughPcodeRow implements PcodeRow {
 	}
 
 	@Override
-	public int getSequence() {
+	public Integer getSequence() {
 		return sequence;
 	}
 

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/pcode/OpPcodeRow.java

@@ -32,7 +32,10 @@ public class OpPcodeRow implements PcodeRow {
 	}
 
 	@Override
-	public int getSequence() {
+	public Integer getSequence() {
+		if (op == null) {
+			return null;
+		}
 		return op.getSeqnum().getTime();
 	}
 
@@ -41,6 +44,7 @@ public class OpPcodeRow implements PcodeRow {
 		return code;
 	}
 
+	@Override
 	public boolean isNext() {
 		return isNext;
 	}

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/pcode/PcodeRow.java

@@ -18,7 +18,7 @@ package ghidra.app.plugin.core.debug.gui.pcode;
 import ghidra.program.model.pcode.PcodeOp;
 
 public interface PcodeRow {
-	int getSequence();
+	Integer getSequence();
 
 	String getCode();
 

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

@@ -231,6 +231,10 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 					info.trackTraceBreakpoint(breakpoint, c, false);
 				}
 			}
+			catch (TrackedTooSoonException e) {
+				Msg.info(this, "Ignoring " + breakpoint +
+					" added until service has finished loading its trace");
+			}
 		}
 
 		private void breakpointChanged(TraceBreakpoint breakpoint) {
@@ -242,6 +246,10 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 					info.trackTraceBreakpoint(breakpoint, c, true);
 				}
 			}
+			catch (TrackedTooSoonException e) {
+				Msg.info(this, "Ignoring " + breakpoint +
+					" changed until service has finished loading its trace");
+			}
 		}
 
 		private void breakpointLifespanChanged(TraceAddressSpace spaceIsNull,
@@ -266,6 +274,10 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 						info.trackTraceBreakpoint(breakpoint, c, false);
 					}
 				}
+				catch (TrackedTooSoonException e) {
+					Msg.info(this, "Ignoring " + breakpoint +
+						" span changed until service has finished loading its trace");
+				}
 			}
 		}
 
@@ -362,7 +374,7 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 				long length, Collection<TraceBreakpointKind> kinds);
 
 		protected LogicalBreakpointInternal getOrCreateLogicalBreakpointFor(Address address,
-				TraceBreakpoint breakpoint, AddCollector c) {
+				TraceBreakpoint breakpoint, AddCollector c) throws TrackedTooSoonException {
 			Set<LogicalBreakpointInternal> set =
 				breakpointsByAddress.computeIfAbsent(address, a -> new HashSet<>());
 			for (LogicalBreakpointInternal lb : set) {
@@ -543,7 +555,12 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 		protected void trackTraceBreakpoints(Collection<TraceBreakpoint> breakpoints,
 				AddCollector collector) {
 			for (TraceBreakpoint b : breakpoints) {
-				trackTraceBreakpoint(b, collector, false);
+				try {
+					trackTraceBreakpoint(b, collector, false);
+				}
+				catch (TrackedTooSoonException e) {
+					throw new AssertionError(e);
+				}
 			}
 		}
 
@@ -562,7 +579,7 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 		}
 
 		protected void trackTraceBreakpoint(TraceBreakpoint breakpoint, AddCollector c,
-				boolean forceUpdate) {
+				boolean forceUpdate) throws TrackedTooSoonException {
 			Address traceAddr = breakpoint.getMinAddress();
 			ProgramLocation progLoc = computeStaticLocation(breakpoint);
 			LogicalBreakpointInternal lb;

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

@@ -451,6 +451,7 @@ public interface LogicalBreakpointInternal extends LogicalBreakpoint {
 	/**
 	 * Check if this logical breakpoint can subsume the given candidate trace breakpoint
 	 * 
+	 * <p>
 	 * Note that logical breakpoints only include trace breakpoints for traces being actively
 	 * recorded. All statuses regarding trace breakpoints are derived from the target breakpoints,
 	 * i.e., they show the present status, regardless of the view's current time. A separate
@@ -458,8 +459,9 @@ public interface LogicalBreakpointInternal extends LogicalBreakpoint {
 	 * 
 	 * @param breakpoint the trace breakpoint to check
 	 * @return true if it can be aggregated.
+	 * @throws TrackedTooSoonException if the containing trace is still being added to the manager
 	 */
-	boolean canMerge(TraceBreakpoint breakpoint);
+	boolean canMerge(TraceBreakpoint breakpoint) throws TrackedTooSoonException;
 
 	boolean trackBreakpoint(Bookmark bookmark);
 

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

@@ -350,10 +350,18 @@ public class MappedLogicalBreakpoint implements LogicalBreakpointInternal {
 	}
 
 	@Override
-	public boolean canMerge(TraceBreakpoint breakpoint) {
+	public boolean canMerge(TraceBreakpoint breakpoint) throws TrackedTooSoonException {
 		TraceBreakpointSet breaks = traceBreaks.get(breakpoint.getTrace());
 		if (breaks == null) {
-			throw new AssertionError();
+			/**
+			 * This happens when the trace is first added to the manager, between the listener being
+			 * installed and the current breakpoints being loaded. We received a breakpoint-changed
+			 * event for a trace that hasn't been loaded, so we don't see its break set in this
+			 * logical breakpoint. The solution is "easy": Just punt, and it'll get generated later.
+			 * It's not enough to return false, because that will generate another logical
+			 * breakpoint, which may actually duplicate this one.
+			 */
+			throw new TrackedTooSoonException();
 		}
 		if (length != breakpoint.getLength()) {
 			return false;

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

@@ -0,0 +1,20 @@
+/* ###
+ * 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 ghidra.app.plugin.core.debug.service.breakpoint;
+
+public class TrackedTooSoonException extends Exception {
+
+}

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/copying/DebuggerCopyActionsPluginTest.java

@@ -19,7 +19,6 @@ import static org.junit.Assert.*;
 
 import java.util.Arrays;
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -39,6 +38,7 @@ import ghidra.app.plugin.core.debug.gui.listing.DebuggerListingProvider;
 import ghidra.app.plugin.core.debug.service.modules.DebuggerStaticMappingServicePlugin;
 import ghidra.app.services.ActionSource;
 import ghidra.app.services.DebuggerStaticMappingService;
+import ghidra.async.AsyncTestUtils;
 import ghidra.dbg.DebuggerModelListener;
 import ghidra.dbg.target.TargetObject;
 import ghidra.program.model.address.*;
@@ -53,7 +53,8 @@ import ghidra.trace.model.memory.TraceMemoryFlag;
 import ghidra.util.database.UndoableTransaction;
 
 @Category(NightlyCategory.class)
-public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerGUITest {
+public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerGUITest
+		implements AsyncTestUtils {
 
 	DebuggerCopyActionsPlugin copyActionsPlugin;
 	DebuggerListingPlugin listingPlugin;
@@ -87,7 +88,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 	}
 
 	@Test
-	public void testActionCopyIntoCurrentProgramWithoutRelocationCreateBlocks() throws Exception {
+	public void testActionCopyIntoCurrentProgramWithoutRelocationCreateBlocks() throws Throwable {
 		assertDisabled(copyActionsPlugin.actionCopyIntoCurrentProgram);
 
 		createProgram();
@@ -120,7 +121,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 		assertEquals(".text", entry.getBlockName());
 		assertTrue(entry.isCreate());
 		dialog.okCallback();
-		dialog.lastTask.get(1000, TimeUnit.MILLISECONDS);
+		waitOn(dialog.lastTask);
 		waitForSwing();
 
 		MemoryBlock text = Unique.assertOne(Arrays.asList(program.getMemory().getBlocks()));
@@ -128,7 +129,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 	}
 
 	@Test
-	public void testActionCopyIntoCurrentProgramWithoutRelocationCrossLanguage() throws Exception {
+	public void testActionCopyIntoCurrentProgramWithoutRelocationCrossLanguage() throws Throwable {
 		assertDisabled(copyActionsPlugin.actionCopyIntoCurrentProgram);
 
 		createProgram(getSLEIGH_X86_LANGUAGE());
@@ -201,7 +202,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 		assertTrue(entry.isCreate());
 
 		dialog.okCallback();
-		dialog.lastTask.get(1000, TimeUnit.MILLISECONDS);
+		waitOn(dialog.lastTask);
 		waitForSwing();
 
 		byte[] dest = new byte[4];
@@ -210,7 +211,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 	}
 
 	@Test
-	public void testActionCopyIntoCurrentProgramWithRelocationExistingBlocks() throws Exception {
+	public void testActionCopyIntoCurrentProgramWithRelocationExistingBlocks() throws Throwable {
 		assertDisabled(copyActionsPlugin.actionCopyIntoCurrentProgram);
 
 		createAndOpenTrace();
@@ -264,7 +265,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 		assertEquals(".text *", entry.getBlockName());
 		assertFalse(entry.isCreate());
 		dialog.okCallback();
-		dialog.lastTask.get(1000, TimeUnit.MILLISECONDS);
+		waitOn(dialog.lastTask);
 		waitForSwing();
 
 		MemoryBlock text = Unique.assertOne(Arrays.asList(program.getMemory().getBlocks()));
@@ -272,7 +273,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 	}
 
 	@Test
-	public void testActionCopyIntoCurrentProgramWithRelocationOverlayBlocks() throws Exception {
+	public void testActionCopyIntoCurrentProgramWithRelocationOverlayBlocks() throws Throwable {
 		assertDisabled(copyActionsPlugin.actionCopyIntoCurrentProgram);
 
 		createAndOpenTrace();
@@ -327,7 +328,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 		assertEquals(".text_2", entry.getBlockName());
 		assertTrue(entry.isCreate());
 		dialog.okCallback();
-		dialog.lastTask.get(1000, TimeUnit.MILLISECONDS);
+		waitOn(dialog.lastTask);
 		waitForSwing();
 
 		MemoryBlock text2 =
@@ -337,7 +338,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 	}
 
 	@Test
-	public void testActionCopyIntoNewProgram() throws Exception {
+	public void testActionCopyIntoNewProgram() throws Throwable {
 		assertDisabled(copyActionsPlugin.actionCopyIntoNewProgram);
 
 		createAndOpenTrace();
@@ -366,7 +367,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 		assertTrue(entry.isCreate());
 		entry.setBlockName(".my_text");
 		dialog.okCallback();
-		dialog.lastTask.get(1000, TimeUnit.MILLISECONDS);
+		waitOn(dialog.lastTask);
 		waitForSwing();
 
 		// Declare my own, or the @After will try to release it erroneously
@@ -379,7 +380,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 	}
 
 	@Test
-	public void testActionCopyIntoNewProgramAdjacentRegions() throws Exception {
+	public void testActionCopyIntoNewProgramAdjacentRegions() throws Throwable {
 		assertDisabled(copyActionsPlugin.actionCopyIntoNewProgram);
 
 		createAndOpenTrace();
@@ -422,7 +423,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 		assertTrue(entry.isCreate());
 
 		dialog.okCallback();
-		dialog.lastTask.get(1000, TimeUnit.MILLISECONDS);
+		waitOn(dialog.lastTask);
 		waitForSwing();
 
 		// Declare my own, or the @After will try to release it erroneously
@@ -431,7 +432,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 	}
 
 	@Test
-	public void testActionCopyIntoNewProgramCaptureLive() throws Exception {
+	public void testActionCopyIntoNewProgramCaptureLive() throws Throwable {
 		assertDisabled(copyActionsPlugin.actionCopyIntoNewProgram);
 
 		createTestModel();
@@ -482,7 +483,7 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 
 		assertEquals(0, listener.count);
 		dialog.okCallback();
-		dialog.lastTask.get(10000, TimeUnit.MILLISECONDS);
+		waitOn(dialog.lastTask);
 		waitForSwing();
 		assertEquals(16, listener.count);
 
@@ -494,7 +495,13 @@ public class DebuggerCopyActionsPluginTest extends AbstractGhidraHeadedDebuggerG
 		assertEquals(tb.addr(stSpace, 0x55550000), text.getStart());
 		assertEquals(".my_text", text.getName());
 		byte[] arr = new byte[8];
-		text.getBytes(tb.addr(stSpace, 0x55550000), arr);
-		assertArrayEquals(tb.arr(1, 2, 3, 4, 5, 6, 7, 8), arr);
+		/**
+		 * While waitOn will ensure the read request completes, it doesn't ensure the recorder has
+		 * actually written the result to the database, yet.
+		 */
+		waitForPass(noExc(() -> {
+			text.getBytes(tb.addr(stSpace, 0x55550000), arr);
+			assertArrayEquals(tb.arr(1, 2, 3, 4, 5, 6, 7, 8), arr);
+		}));
 	}
 }