Merge remote-tracking branch

'origin/GP-3450_Dan_dbgStructHovers--SQUASHED' (#5337)

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/stack/vars/VariableValueHoverService.java

@@ -408,7 +408,7 @@ public class VariableValueHoverService extends AbstractConfigurableHover
 				return fillRegister(ins, register);
 			}
 			Address refAddress = opLoc.getRefAddress();
-			if (operand instanceof Scalar scalar && refAddress != null) {
+			if (operand instanceof Scalar && refAddress != null) {
 				return fillReference(ins, refAddress);
 			}
 			if (operand instanceof Address address) {
@@ -431,11 +431,14 @@ public class VariableValueHoverService extends AbstractConfigurableHover
 
 		public CompletableFuture<VariableValueTable> fillPcodeOp(Function function, String name,
 				DataType type, PcodeOp op, AddressSetView symbolStorage) {
+			Program program = function.getProgram();
+			boolean requiresFrame = applyCopyHeuristic(program, op, symbolStorage,
+				VariableValueUtils::requiresFrame, VariableValueUtils::requiresFrame);
 			return executeBackground(monitor -> {
-				UnwoundFrame<WatchValue> frame = VariableValueUtils.requiresFrame(op, symbolStorage)
+				UnwoundFrame<WatchValue> frame = requiresFrame
 						? eval.getStackFrame(function, warnings, monitor, true)
 						: eval.getGlobalsFakeFrame();
-				return fillFrameOp(frame, function.getProgram(), name, type, op, symbolStorage);
+				return fillFrameOp(frame, program, name, type, op, symbolStorage);
 			});
 		}
 
@@ -448,7 +451,9 @@ public class VariableValueHoverService extends AbstractConfigurableHover
 			table.add(new IntegerRow(value));
 			if (type != DataType.DEFAULT) {
 				String repr = eval.getRepresentation(frame, address, value, type);
-				table.add(new ValueRow(repr, value.state()));
+				if (repr != null) {
+					table.add(new ValueRow(repr, value.state()));
+				}
 			}
 			return table;
 		}
@@ -465,6 +470,33 @@ public class VariableValueHoverService extends AbstractConfigurableHover
 			return fillWatchValue(frame, storage.getMinAddress(), type, value);
 		}
 
+		interface CopyCase<T> {
+			T evaluate(Program program, Varnode varnode, AddressSetView symbolStorage);
+		}
+
+		interface DefaultCase<T> {
+			T evaluate(Program program, PcodeOp op, AddressSetView symbolStorage);
+		}
+
+		/**
+		 * XXX: This is a heuristic I'm not sure about. I'm going to assume a token whose p-code op
+		 * is a {@link PcodeOp#COPY} is an assignment statement. I suppose I could check that I'm on
+		 * the LHS of an assignment operator, but that could be difficult and complex.... In any
+		 * case, if it's an assignment, I'm going to evaluate the output of the output operand
+		 * instead. Trouble is, that may just traverse back over the copy, as the copy is the
+		 * defining operator. It might only work if I can guarantee the output is part of the symbol
+		 * storage.
+		 * 
+		 */
+		protected <T> T applyCopyHeuristic(Program program, PcodeOp op,
+				AddressSetView symbolStorage,
+				CopyCase<T> copyCase, DefaultCase<T> defaultCase) {
+			return switch (op.getOpcode()) {
+				case PcodeOp.COPY -> copyCase.evaluate(program, op.getOutput(), symbolStorage);
+				default -> defaultCase.evaluate(program, op, symbolStorage);
+			};
+		}
+
 		public VariableValueTable fillFrameOp(UnwoundFrame<WatchValue> frame, Program program,
 				String name, DataType type, PcodeOp op, AddressSetView symbolStorage) {
 			table.add(new NameRow(name));
@@ -472,7 +504,10 @@ public class VariableValueHoverService extends AbstractConfigurableHover
 				table.add(new FrameRow(frame));
 			}
 			table.add(new TypeRow(type));
-			WatchValue value = frame.evaluate(program, op, symbolStorage);
+
+			WatchValue value =
+				applyCopyHeuristic(program, op, symbolStorage, frame::evaluate, frame::evaluate);
+
 			// TODO: What if the type is dynamic with non-fixed size?
 			if (type.getLength() != value.length()) {
 				value = frame.zext(value, type.getLength());

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/stack/vars/VariableValueUtils.java

@@ -362,16 +362,31 @@ public enum VariableValueUtils {
 		return new RequiresFrameEvaluator(symbolStorage).evaluateStorage(program, storage);
 	}
 
+	/**
+	 * Check if evaluation of the given varnode will require a frame
+	 * 
+	 * @param program the program containing the variable storage
+	 * @param varnode the varnode to evaluate
+	 * @param symbolStorage the leaves of evaluation, usually storage used by symbols in scope. See
+	 *            {@link #collectSymbolStorage(ClangLine)}
+	 * @return true if a frame is required, false otherwise
+	 */
+	public static boolean requiresFrame(Program program, Varnode varnode,
+			AddressSetView symbolStorage) {
+		return new RequiresFrameEvaluator(symbolStorage).evaluateVarnode(program, varnode);
+	}
+
 	/**
 	 * Check if evaluation of the given p-code op will require a frame
 	 * 
+	 * @param program the program containing the variable storage
 	 * @param op the op whose output to evaluation
 	 * @param symbolStorage the leaves of evaluation, usually storage used by symbols in scope. See
 	 *            {@link #collectSymbolStorage(ClangLine)}
 	 * @return true if a frame is required, false otherwise
 	 */
-	public static boolean requiresFrame(PcodeOp op, AddressSetView symbolStorage) {
+	public static boolean requiresFrame(Program program, PcodeOp op, AddressSetView symbolStorage) {
-		return new RequiresFrameEvaluator(symbolStorage).evaluateOp(null, op);
+		return new RequiresFrameEvaluator(symbolStorage).evaluateOp(program, op);
 	}
 
 	/**
@@ -523,7 +538,7 @@ public enum VariableValueUtils {
 	 * It's not the greatest, but any variable to be evaluated should only be expressed in terms of
 	 * symbols on the same line (at least by the decompiler's definition, wrapping shouldn't count
 	 * against us). This can be used to determine where evaluation should cease descending into
-	 * defining p-code ops. See {@link #requiresFrame(PcodeOp, AddressSetView)}, and
+	 * defining p-code ops. See {@link #requiresFrame(Program, PcodeOp, AddressSetView)}, and
 	 * {@link UnwoundFrame#evaluate(Program, PcodeOp, AddressSetView)}.
 	 * 
 	 * @param line the line
@@ -879,7 +894,11 @@ public enum VariableValueUtils {
 			}
 			Settings settings = type.getDefaultSettings();
 			if (address.isStackAddress()) {
-				address = frame.getBasePointer().add(address.getOffset());
+				Address base = frame.getBasePointer();
+				if (base == null) {
+					return null;
+				}
+				address = base.add(address.getOffset());
 				if (frame instanceof ListingUnwoundFrame listingFrame) {
 					settings = listingFrame.getComponentContaining(address);
 				}

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

@@ -731,9 +731,9 @@ public class DebuggerTraceManagerServicePlugin extends Plugin
 			return CompletableFuture.completedFuture(found);
 		}
 		if (emulationService == null) {
-			throw new IllegalStateException(
+			Msg.warn(this, "Cannot navigate to coordinates with execution schedules, " +
-				"Cannot navigate to coordinates with execution schedules, " +
+				"because the emulation service is not available.");
-					"because the emulation service is not available.");
+			return CompletableFuture.completedFuture(null);
 		}
 		return emulationService.backgroundEmulate(coordinates.getPlatform(), coordinates.getTime());
 	}
@@ -746,6 +746,9 @@ public class DebuggerTraceManagerServicePlugin extends Plugin
 			return AsyncUtils.nil();
 		}
 		return materialize(coordinates).thenAcceptAsync(snap -> {
+			if (snap == null) {
+				return; // The tool is probably closing
+			}
 			if (!coordinates.equals(current)) {
 				return; // We navigated elsewhere before emulation completed
 			}

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/AbstractUnwoundFrame.java

@@ -297,26 +297,29 @@ public abstract class AbstractUnwoundFrame<T> implements UnwoundFrame<T> {
 			Reason.INSPECT);
 	}
 
-	@Override
+	protected FrameVarnodeEvaluator<T> newEvaluator(AddressSetView symbolStorage) {
-	public T evaluate(Program program, VariableStorage storage, AddressSetView symbolStorage) {
 		SavedRegisterMap registerMap = computeRegisterMap();
-		return new FrameVarnodeEvaluator<T>(state.getArithmetic(), symbolStorage) {
+		return new FrameVarnodeEvaluator<>(state.getArithmetic(), symbolStorage) {
 			@Override
 			protected T evaluateMemory(Address address, int size) {
 				return registerMap.getVar(state, address, size, Reason.INSPECT);
 			}
-		}.evaluateStorage(program, storage);
+		};
+	}
+
+	@Override
+	public T evaluate(Program program, VariableStorage storage, AddressSetView symbolStorage) {
+		return newEvaluator(symbolStorage).evaluateStorage(program, storage);
+	}
+
+	@Override
+	public T evaluate(Program program, Varnode varnode, AddressSetView symbolStorage) {
+		return newEvaluator(symbolStorage).evaluateVarnode(program, varnode);
 	}
 
 	@Override
 	public T evaluate(Program program, PcodeOp op, AddressSetView symbolStorage) {
-		SavedRegisterMap registerMap = computeRegisterMap();
+		return newEvaluator(symbolStorage).evaluateOp(program, op);
-		return new FrameVarnodeEvaluator<T>(state.getArithmetic(), symbolStorage) {
-			@Override
-			protected T evaluateMemory(Address address, int size) {
-				return registerMap.getVar(state, address, size, Reason.INSPECT);
-			}
-		}.evaluateOp(program, op);
 	}
 
 	@Override

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/UnwoundFrame.java

@@ -26,6 +26,7 @@ import ghidra.program.model.address.AddressSetView;
 import ghidra.program.model.lang.Register;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.pcode.PcodeOp;
+import ghidra.program.model.pcode.Varnode;
 
 /**
  * A frame that has been unwound through analysis or annotated in the listing
@@ -153,8 +154,8 @@ public interface UnwoundFrame<T> {
 	 * 
 	 * <p>
 	 * Each varnode's value is simply retrieved from the state, in contrast to
-	 * {@link #evaluate(VariableStorage, AddressSetView)}, which ascends to varnodes' defining
+	 * {@link #evaluate(Program, VariableStorage, AddressSetView)}, which ascends to varnodes'
-	 * p-code ops.
+	 * defining p-code ops.
 	 * 
 	 * <p>
 	 * <b>WARNING:</b> Never invoke this method from the Swing thread. The state could be associated
@@ -173,7 +174,7 @@ public interface UnwoundFrame<T> {
 	 * <b>WARNING:</b> Never invoke this method from the Swing thread. The state could be associated
 	 * with a live session, and this may block to retrieve live state.
 	 * 
-	 * @see #getValue(VariableStorage)
+	 * @see #getValue(Program, VariableStorage)
 	 * @param variable the variable
 	 * @return the value
 	 */
@@ -197,13 +198,13 @@ public interface UnwoundFrame<T> {
 	 * Evaluate the given storage, following defining p-code ops until symbol storage is reached
 	 * 
 	 * <p>
-	 * This behaves similarly to {@link #getValue(VariableStorage)}, except this one will ascend
+	 * This behaves similarly to {@link #getValue(Program, VariableStorage)}, except this one will
-	 * recursively to each varnode's defining p-code op. The recursion terminates when a varnode is
+	 * ascend recursively to each varnode's defining p-code op. The recursion terminates when a
-	 * contained in the given symbol storage. The symbol storage is usually collected by examining
+	 * varnode is contained in the given symbol storage. The symbol storage is usually collected by
-	 * the tokens on the same line, searching for ones that represent "high symbols." This ensures
+	 * examining the tokens on the same line, searching for ones that represent "high symbols." This
-	 * that any temporary storage used by the original program in the evaluation of, e.g., a field
+	 * ensures that any temporary storage used by the original program in the evaluation of, e.g., a
-	 * access, are not read from the current state but re-evaluated in terms of the symbols' current
+	 * field access, are not read from the current state but re-evaluated in terms of the symbols'
-	 * values.
+	 * current values.
 	 * 
 	 * <p>
 	 * <b>WARNING:</b> Never invoke this method from the Swing thread. The state could be associated
@@ -217,6 +218,20 @@ public interface UnwoundFrame<T> {
 	 */
 	T evaluate(Program program, VariableStorage storage, AddressSetView symbolStorage);
 
+	/**
+	 * Evaluate the given varnode, following defining p-code ops until symbol storage is reached
+	 * 
+	 * <p>
+	 * <b>WARNING:</b> Never invoke this method from the Swing thread. The state could be associated
+	 * with a live session, and this may block to retrieve live state.
+	 * 
+	 * @param program the program containing the varnode
+	 * @param varnode the varnode
+	 * @param symbolStorage the terminal storage, usually that of symbols
+	 * @return the value
+	 */
+	T evaluate(Program program, Varnode varnode, AddressSetView symbolStorage);
+
 	/**
 	 * Evaluate the output for the given p-code op, ascending until symbol storage is reached
 	 * 
@@ -224,7 +239,7 @@ public interface UnwoundFrame<T> {
 	 * <b>WARNING:</b> Never invoke this method from the Swing thread. The state could be associated
 	 * with a live session, and this may block to retrieve live state.
 	 * 
-	 * @see #evaluate(VariableStorage, AddressSetView)
+	 * @see #evaluate(Program, VariableStorage, AddressSetView)
 	 * @param program the program containing the op
 	 * @param op the op
 	 * @param symbolStorage the terminal storage, usually that of symbols
@@ -251,7 +266,7 @@ public interface UnwoundFrame<T> {
 	/**
 	 * Set the value of the given variable
 	 * 
-	 * @see #setValue(StateEditor, VariableStorage, BigInteger)
+	 * @see #setValue(StateEditor, Program, VariableStorage, BigInteger)
 	 * @param editor the editor for setting values
 	 * @param variable the variable to modify
 	 * @param value the desired value

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/stack/StackUnwinderTest.java

@@ -513,6 +513,54 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest {
 		}
 	}
 
+	protected Function createFillStackStructProgramX86_64() throws Throwable {
+		createProgram("x86:LE:64:default", "gcc");
+		intoProject(program);
+
+		try (Transaction tx = program.openTransaction("Assemble")) {
+			ProgramBasedDataTypeManager dtm = program.getDataTypeManager();
+			Structure structure = new StructureDataType("MyStruct", 0, dtm);
+			structure.add(DWordDataType.dataType, "f1", "");
+			structure.add(DWordDataType.dataType, "f2", "");
+			structure.add(QWordDataType.dataType, "f3", "");
+			structure =
+				(Structure) dtm.addDataType(structure, DataTypeConflictHandler.DEFAULT_HANDLER);
+
+			Address entry = addr(program, 0x00400000);
+			program.getMemory()
+					.createInitializedBlock(".text", entry, 0x1000, (byte) 0, monitor, false);
+
+			Assembler asm = Assemblers.getAssembler(program.getLanguage(), NO_16BIT_CALLS);
+			AssemblyBuffer buf = new AssemblyBuffer(asm, entry);
+
+			buf.assemble("PUSH RBP");
+			buf.assemble("MOV RBP, RSP");
+			buf.assemble("SUB RSP, 0x20");
+			buf.assemble("MOV dword ptr [RBP + -0x10], 0x4");
+			buf.assemble("MOV dword ptr [RBP + -0xc], 0xdeadbeef");
+			buf.assemble("MOV qword ptr [RBP + -0x8], 0xcafebab");
+			buf.assemble("LEA RDI, [RBP + -0x10]");
+			buf.assemble("CALL 0x00400040");
+			buf.assemble("MOV RSP, RBP");
+			buf.assemble("POP RBP");
+			buf.assemble("RET");
+			Address end = buf.getNext();
+
+			program.getMemory().setBytes(entry, buf.getBytes());
+
+			Disassembler dis = Disassembler.getDisassembler(program, monitor, null);
+			dis.disassemble(entry, null);
+
+			Function funFillStruct = program.getFunctionManager()
+					.createFunction("fillStruct", entry, new AddressSet(entry, end.previous()),
+						SourceType.ANALYSIS);
+			funFillStruct.addLocalVariable(new LocalVariableImpl("s", structure, -0x18, program),
+				SourceType.ANALYSIS);
+
+			return funFillStruct;
+		}
+	}
+
 	protected Function createFillStructArrayProgramX86_32() throws Throwable {
 		createProgram("x86:LE:32:default", "gcc");
 		intoProject(program);
@@ -1552,7 +1600,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest {
 					if (!(field instanceof ClangTextField clangField)) {
 						continue;
 					}
-					if (!fieldText.equals(field.getText())) {
+					if (!fieldText.equals(field.getText().trim())) {
 						continue;
 					}
 					int numRows = field.getNumRows();
@@ -1738,6 +1786,43 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest {
 			RowKey.WARNINGS, "IGNORED"), table);
 	}
 
+	@Test
+	public void testStructureSetInStackHighVarStructField() throws Throwable {
+		addPlugins();
+		// PC Tracking interferes with goTo
+		listingPlugin.setTrackingSpec(NoneLocationTrackingSpec.INSTANCE);
+		VariableValueHoverPlugin valuesPlugin = addPlugin(tool, VariableValueHoverPlugin.class);
+		VariableValueHoverService valuesService = valuesPlugin.getHoverService();
+		Function function = createFillStackStructProgramX86_64();
+		Address entry = function.getEntryPoint();
+
+		programManager.openProgram(program);
+
+		// Not necessary to actually run. Just examine values at entry.
+		useTrace(ProgramEmulationUtils.launchEmulationTrace(program, entry, this));
+		tb.trace.release(this);
+		TraceThread thread = Unique.assertOne(tb.trace.getThreadManager().getAllThreads());
+		traceManager.openTrace(tb.trace);
+		traceManager.activateThread(thread);
+		waitForSwing();
+
+		goTo(staticListing, new ProgramLocation(program, entry));
+		HoverLocation loc = findTokenLocation(function, "f1", "s.f1 = 4;");
+		VariableValueTable table = getVariableValueTable(valuesService, loc.pLoc,
+			traceManager.getCurrent(), loc.fLoc, loc.field);
+
+		assertTable(Map.ofEntries(
+			Map.entry(RowKey.NAME, "Name: f1"),
+			Map.entry(RowKey.FRAME, "Frame: 0 fillStruct pc=00400000 sp=00005000 base=00005000"),
+			Map.entry(RowKey.TYPE, "Type: dword"),
+			Map.entry(RowKey.LOCATION, "Location: 00004fe8:4"),
+			Map.entry(RowKey.BYTES, "Bytes: (UNKNOWN) 00 00 00 00"),
+			Map.entry(RowKey.INTEGER, "Integer: (UNKNOWN) 0"),
+			Map.entry(RowKey.VALUE, "Value: (UNKNOWN) 0h"),
+			Map.entry(RowKey.WARNINGS, "IGNORED")),
+			table);
+	}
+
 	@Test
 	public void testStructurePointerRegisterHighVarStruct() throws Throwable {
 		addPlugins();