GP-0: Fix tests, esp., hang in testLaunchLocalGdb

Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/rmi/trace/TraceRmiHandler.java

@@ -43,7 +43,6 @@ import ghidra.dbg.target.schema.TargetObjectSchema.SchemaName;
 import ghidra.dbg.target.schema.XmlSchemaContext;
 import ghidra.dbg.util.PathPattern;
 import ghidra.dbg.util.PathUtils;
-import ghidra.debug.api.control.ControlMode;
 import ghidra.debug.api.target.ActionName;
 import ghidra.debug.api.tracemgr.DebuggerCoordinates;
 import ghidra.debug.api.tracermi.*;
@@ -788,6 +787,14 @@ public class TraceRmiHandler implements TraceRmiConnection {
 		return makeArgument(ent.getKey(), ent.getValue());
 	}
 
+	protected boolean followsPresent(Trace trace) {
+		DebuggerControlService controlService = this.controlService;
+		if (controlService == null) {
+			return true;
+		}
+		return controlService.getCurrentMode(trace).followsPresent();
+	}
+
 	protected ReplyActivate handleActivate(RequestActivate req) {
 		OpenTrace open = requireOpenTrace(req.getOid());
 		TraceObject object = open.getObject(req.getObject(), true);
@@ -795,8 +802,7 @@ public class TraceRmiHandler implements TraceRmiConnection {
 		if (coords.getTrace() != open.trace) {
 			coords = DebuggerCoordinates.NOWHERE;
 		}
-		ControlMode mode = controlService.getCurrentMode(open.trace);
+		if (open.lastSnapshot != null && followsPresent(open.trace)) {
-		if (open.lastSnapshot != null && mode.followsPresent()) {
 			coords = coords.snap(open.lastSnapshot.getKey());
 		}
 		DebuggerCoordinates finalCoords = coords.object(object);
@@ -972,6 +978,7 @@ public class TraceRmiHandler implements TraceRmiConnection {
 						.getValuePaths(toLifespan(req.getSpan()),
 							toPathPattern(req.getPattern()))
 						.map(TraceRmiHandler::makeValDesc)
+						.sorted(Comparator.comparing(ValDesc::getKey))
 						.toList())
 				.build();
 	}

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/target/DBTraceObject.java

@@ -36,6 +36,7 @@ import ghidra.trace.database.module.*;
 import ghidra.trace.database.stack.DBTraceObjectStack;
 import ghidra.trace.database.stack.DBTraceObjectStackFrame;
 import ghidra.trace.database.target.InternalTraceObjectValue.ValueLifespanSetter;
+import ghidra.trace.database.target.ValueSpace.EntryKeyDimension;
 import ghidra.trace.database.target.ValueSpace.SnapDimension;
 import ghidra.trace.database.target.visitors.*;
 import ghidra.trace.database.target.visitors.TreeTraversal.Visitor;
@@ -366,7 +367,10 @@ public class DBTraceObject extends DBAnnotatedObject implements TraceObject {
 	}
 
 	protected Collection<? extends InternalTraceObjectValue> doGetValues(Lifespan lifespan) {
-		return manager.valueMap.reduce(TraceObjectValueQuery.values(this, lifespan)).values();
+		return manager.valueMap
+				.reduce(TraceObjectValueQuery.values(this, lifespan)
+						.starting(EntryKeyDimension.FORWARD))
+				.values();
 	}
 
 	protected Collection<? extends InternalTraceObjectValue> cachedDoGetValues(Lifespan lifespan) {

Ghidra/Framework/Pty/src/main/java/ghidra/pty/linux/FdInputStream.java

@@ -33,7 +33,7 @@ public class FdInputStream extends InputStream {
 	private static final PosixC LIB_POSIX = PosixC.INSTANCE;
 
 	private final int fd;
-	private boolean closed = false;
+	private volatile boolean closed = false;
 
 	/**
 	 * Wrap the given file descriptor in an {@link InputStream}
@@ -85,7 +85,8 @@ public class FdInputStream extends InputStream {
 	}
 
 	@Override
-	public synchronized void close() throws IOException {
+	public void close() throws IOException {
 		closed = true;
+		// NB. The Pty is responsible for closing the fd
 	}
 }

Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/GdbCommandsTest.java

@@ -858,6 +858,7 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest {
 			tb = new ToyDBTraceBuilder((Trace) mdo.get());
 			assertEquals("""
 					Parent          Key       Span     Value           Type
+					Test.Objects[1] vaddr     [0,+inf) ram:deadbeef    ADDRESS
 					Test.Objects[1] vbool     [0,+inf) True            BOOL
 					Test.Objects[1] vboolarr  [0,+inf) [True, False]   BOOL_ARR
 					Test.Objects[1] vbyte     [0,+inf) 1               BYTE
@@ -871,8 +872,7 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest {
 					Test.Objects[1] vobj      [0,+inf) Test.Objects[1] OBJECT
 					Test.Objects[1] vshort    [0,+inf) 2               SHORT
 					Test.Objects[1] vshortarr [0,+inf) [1, 2, 3]       SHORT_ARR
-					Test.Objects[1] vstring   [0,+inf) 'Hello'         STRING
+					Test.Objects[1] vstring   [0,+inf) 'Hello'         STRING""",
-					Test.Objects[1] vaddr     [0,+inf) ram:deadbeef    ADDRESS""",
 				extractOutSection(out, "---GetValues---"));
 		}
 	}
@@ -1012,7 +1012,7 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest {
 				%s
 				ghidra trace connect %s
 				file bash
-				start
+				starti
 				ghidra trace start
 				ghidra trace tx-start "Tx"
 				break main
@@ -1031,26 +1031,27 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest {
 					.getValuePaths(Lifespan.at(0),
 						PathPredicates.parse("Inferiors[1].Breakpoints[]"))
 					.map(p -> p.getLastEntry())
+					.sorted(Comparator.comparing(TraceObjectValue::getEntryKey))
 					.toList();
 			assertEquals(5, infBreakLocVals.size());
 			AddressRange rangeMain =
 				infBreakLocVals.get(0).getChild().getValue(0, "_range").castValue();
 			Address main = rangeMain.getMinAddress();
 
-			// The temporary breakpoint uses up number 1
+			// NB. starti avoid use of temporary main breakpoint
-			assertBreakLoc(infBreakLocVals.get(0), "[2.1]", main, 1,
+			assertBreakLoc(infBreakLocVals.get(0), "[1.1]", main, 1,
 				Set.of(TraceBreakpointKind.SW_EXECUTE),
 				"main");
-			assertBreakLoc(infBreakLocVals.get(1), "[3.1]", main.add(10), 1,
+			assertBreakLoc(infBreakLocVals.get(1), "[2.1]", main.add(10), 1,
 				Set.of(TraceBreakpointKind.HW_EXECUTE),
 				"*main+10");
-			assertBreakLoc(infBreakLocVals.get(2), "[4.1]", main.add(20), 1,
+			assertBreakLoc(infBreakLocVals.get(2), "[3.1]", main.add(20), 1,
 				Set.of(TraceBreakpointKind.WRITE),
 				"-location *((char*)(&main+20))");
-			assertBreakLoc(infBreakLocVals.get(3), "[5.1]", main.add(30), 8,
+			assertBreakLoc(infBreakLocVals.get(3), "[4.1]", main.add(30), 8,
 				Set.of(TraceBreakpointKind.READ),
 				"-location *((char(*)[8])(&main+30))");
-			assertBreakLoc(infBreakLocVals.get(4), "[6.1]", main.add(40), 5,
+			assertBreakLoc(infBreakLocVals.get(4), "[5.1]", main.add(40), 5,
 				Set.of(TraceBreakpointKind.READ, TraceBreakpointKind.WRITE),
 				"-location *((char(*)[5])(&main+40))");
 		}

Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/GdbMethodsTest.java

@@ -109,7 +109,7 @@ public class GdbMethodsTest extends AbstractGdbTraceRmiTest {
 					ghidra trace start
 					%s
 					ghidra trace tx-open "Fake" 'ghidra trace create-obj Breakpoints'
-					start"""
+					starti"""
 					.formatted(INSTRUMENT_STOPPED));
 			RemoteMethod refreshBreakpoints = conn.getMethod("refresh_breakpoints");
 			try (ManagedDomainObject mdo = openDomainObject("/New Traces/gdb/bash")) {
@@ -129,26 +129,27 @@ public class GdbMethodsTest extends AbstractGdbTraceRmiTest {
 						.getValuePaths(Lifespan.at(0),
 							PathPredicates.parse("Inferiors[1].Breakpoints[]"))
 						.map(p -> p.getLastEntry())
+						.sorted(Comparator.comparing(TraceObjectValue::getEntryKey))
 						.toList();
 				assertEquals(5, infBreakLocVals.size());
 				AddressRange rangeMain =
 					infBreakLocVals.get(0).getChild().getValue(0, "_range").castValue();
 				Address main = rangeMain.getMinAddress();
 
-				// The temporary breakpoint uses up number 1
+				// NB. starti avoid use of temporary main breakpoint
-				assertBreakLoc(infBreakLocVals.get(0), "[2.1]", main, 1,
+				assertBreakLoc(infBreakLocVals.get(0), "[1.1]", main, 1,
 					Set.of(TraceBreakpointKind.SW_EXECUTE),
 					"main");
-				assertBreakLoc(infBreakLocVals.get(1), "[3.1]", main.add(10), 1,
+				assertBreakLoc(infBreakLocVals.get(1), "[2.1]", main.add(10), 1,
 					Set.of(TraceBreakpointKind.HW_EXECUTE),
 					"*main+10");
-				assertBreakLoc(infBreakLocVals.get(2), "[4.1]", main.add(20), 1,
+				assertBreakLoc(infBreakLocVals.get(2), "[3.1]", main.add(20), 1,
 					Set.of(TraceBreakpointKind.WRITE),
 					"-location *((char*)(&main+20))");
-				assertBreakLoc(infBreakLocVals.get(3), "[5.1]", main.add(30), 8,
+				assertBreakLoc(infBreakLocVals.get(3), "[4.1]", main.add(30), 8,
 					Set.of(TraceBreakpointKind.READ),
 					"-location *((char(*)[8])(&main+30))");
-				assertBreakLoc(infBreakLocVals.get(4), "[6.1]", main.add(40), 5,
+				assertBreakLoc(infBreakLocVals.get(4), "[5.1]", main.add(40), 5,
 					Set.of(TraceBreakpointKind.READ, TraceBreakpointKind.WRITE),
 					"-location *((char(*)[5])(&main+40))");
 			}
@@ -163,7 +164,7 @@ public class GdbMethodsTest extends AbstractGdbTraceRmiTest {
 					ghidra trace start
 					%s
 					ghidra trace tx-open "Fake" 'ghidra trace create-obj Inferiors[1].Breakpoints'
-					start"""
+					starti"""
 					.formatted(INSTRUMENT_STOPPED));
 			RemoteMethod refreshInfBreakpoints = conn.getMethod("refresh_inf_breakpoints");
 			try (ManagedDomainObject mdo = openDomainObject("/New Traces/gdb/bash")) {
@@ -183,26 +184,27 @@ public class GdbMethodsTest extends AbstractGdbTraceRmiTest {
 						.getValuePaths(Lifespan.at(0),
 							PathPredicates.parse("Inferiors[1].Breakpoints[]"))
 						.map(p -> p.getLastEntry())
+						.sorted(Comparator.comparing(TraceObjectValue::getEntryKey))
 						.toList();
 				assertEquals(5, infBreakLocVals.size());
 				AddressRange rangeMain =
 					infBreakLocVals.get(0).getChild().getValue(0, "_range").castValue();
 				Address main = rangeMain.getMinAddress();
 
-				// The temporary breakpoint uses up number 1
+				// NB. starti avoid use of temporary main breakpoint
-				assertBreakLoc(infBreakLocVals.get(0), "[2.1]", main, 1,
+				assertBreakLoc(infBreakLocVals.get(0), "[1.1]", main, 1,
 					Set.of(TraceBreakpointKind.SW_EXECUTE),
 					"main");
-				assertBreakLoc(infBreakLocVals.get(1), "[3.1]", main.add(10), 1,
+				assertBreakLoc(infBreakLocVals.get(1), "[2.1]", main.add(10), 1,
 					Set.of(TraceBreakpointKind.HW_EXECUTE),
 					"*main+10");
-				assertBreakLoc(infBreakLocVals.get(2), "[4.1]", main.add(20), 1,
+				assertBreakLoc(infBreakLocVals.get(2), "[3.1]", main.add(20), 1,
 					Set.of(TraceBreakpointKind.WRITE),
 					"-location *((char*)(&main+20))");
-				assertBreakLoc(infBreakLocVals.get(3), "[5.1]", main.add(30), 8,
+				assertBreakLoc(infBreakLocVals.get(3), "[4.1]", main.add(30), 8,
 					Set.of(TraceBreakpointKind.READ),
 					"-location *((char(*)[8])(&main+30))");
-				assertBreakLoc(infBreakLocVals.get(4), "[6.1]", main.add(40), 5,
+				assertBreakLoc(infBreakLocVals.get(4), "[5.1]", main.add(40), 5,
 					Set.of(TraceBreakpointKind.READ, TraceBreakpointKind.WRITE),
 					"-location *((char(*)[5])(&main+40))");
 			}
@@ -735,7 +737,7 @@ public class GdbMethodsTest extends AbstractGdbTraceRmiTest {
 					resume.invoke(Map.of("inferior", inf1));
 					waitRunning();
 
-					interrupt.invoke(Map.of());
+					interrupt.invoke(Map.of("inferior", inf1));
 					waitStopped();
 				}
 
@@ -851,7 +853,7 @@ public class GdbMethodsTest extends AbstractGdbTraceRmiTest {
 					%s
 					start"""
 					.formatted(INSTRUMENT_STOPPED));
-			RemoteMethod stepAdvance = conn.getMethod("step_advance");
+			RemoteMethod stepAdvance = conn.getMethod("Advance");
 			try (ManagedDomainObject mdo = openDomainObject("/New Traces/gdb/bash")) {
 				tb = new ToyDBTraceBuilder((Trace) mdo.get());
 				waitStopped();
@@ -879,7 +881,7 @@ public class GdbMethodsTest extends AbstractGdbTraceRmiTest {
 					%s
 					start"""
 					.formatted(INSTRUMENT_STOPPED));
-			RemoteMethod stepReturn = conn.getMethod("step_return");
+			RemoteMethod stepReturn = conn.getMethod("Return");
 			try (ManagedDomainObject mdo = openDomainObject("/New Traces/gdb/bash")) {
 				tb = new ToyDBTraceBuilder((Trace) mdo.get());
 				waitStopped();

Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/TraceRmiLauncherServicePluginTest.java

@@ -57,6 +57,7 @@ public class TraceRmiLauncherServicePluginTest extends AbstractGhidraHeadedDebug
 					Map<String, ?> arguments, RelPrompt relPrompt) {
 				Map<String, Object> args = new HashMap<>(arguments);
 				args.put("arg:1", file);
+				args.put("env:OPT_START_CMD", "starti");
 				return args;
 			}
 		};