diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwindWarning.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwindWarning.java index 07bf9fc86b..f38eaffbbe 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwindWarning.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwindWarning.java @@ -238,6 +238,20 @@ public interface StackUnwindWarning { } } + /** + * While interpreting p-code, we encountered an internal branch but ignored it. + * + * @param seq the sequence number of the op + */ + public record IgnoredInternalFlowStackUnwindWarning(SequenceNumber seq) + implements StackUnwindWarning { + @Override + public String getMessage() { + return "Ignored control flow internal to instruction at %s:%d" + .formatted(seq.getTarget(), seq.getTime()); + } + } + /** * A custom warning, either because a specific type is too onerous, or because the message was * deserialized and the specific type and info cannot be recovered. diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwinder.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwinder.java index cd803448bf..05b49231e9 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwinder.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/StackUnwinder.java @@ -99,7 +99,7 @@ public class StackUnwinder { final AddressSpace codeSpace; private final Register sp; - record ThreadAndSnap(TraceThread thread, Long viewSnap) {} + record ThreadAndSnap(TraceThread thread, long viewSnap) {} private Map>> unwound = new HashMap<>(); @@ -186,7 +186,7 @@ public class StackUnwinder { ThreadAndSnap tas = new ThreadAndSnap(coord.getThread(), coord.getViewSnap()); TreeMap> treeMap = unwound.computeIfAbsent( - tas, t -> new TreeMap>()); + tas, _ -> new TreeMap>()); AnalysisUnwoundFrame savedFrame = treeMap.get(coord.getFrame()); if (savedFrame != null) { // Short circuit here if possible to avoid recomputing UnwindInfo @@ -373,7 +373,8 @@ public class StackUnwinder { } /** - * A convenience method + * A convenience method + * * @return the deepest level */ public int getRecoveredFrameCount() { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/SymPcodeExecutor.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/SymPcodeExecutor.java index 0f46a853b1..dac7d9180c 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/SymPcodeExecutor.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/stack/SymPcodeExecutor.java @@ -31,13 +31,11 @@ import ghidra.program.model.lang.*; import ghidra.program.model.listing.*; import ghidra.program.model.mem.MemoryAccessException; import ghidra.program.model.pcode.*; -import ghidra.util.Msg; import ghidra.util.exception.*; import ghidra.util.task.TaskMonitor; /** * The interpreter of p-code ops in the domain of {@link Sym} - * *

* This is used for static analysis by executing specific basic blocks. As such, it should never be * expected to interpret a conditional jump. (TODO: This rule might be violated if a fall-through @@ -101,16 +99,21 @@ public class SymPcodeExecutor extends PcodeExecutor { @Override public void stepOp(PcodeOp op, PcodeFrame frame, PcodeUseropLibrary library) { - // TODO: This function can probably be removed after GP-6707 is complete try { monitor.checkCancelled(); } catch (CancelledException e) { - throw new PcodeExecutionException("Monitor was cancelled", frame, e); + throw new PcodeExecutionException("Cancelled", frame, e); } super.stepOp(op, frame, library); } + @Override + protected void branchInternal(PcodeOp op, PcodeFrame frame, int relative) { + warnings.add(new IgnoredInternalFlowStackUnwindWarning(op.getSeqnum())); + // Don't call super! Could put us in a loop + } + /** * Attempt to figure the stack depth change for a given function * diff --git a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/stack/StackUnwinderTest.java b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/stack/StackUnwinderTest.java index da3318c56d..36b1692f73 100644 --- a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/stack/StackUnwinderTest.java +++ b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/stack/StackUnwinderTest.java @@ -21,8 +21,7 @@ import java.io.IOException; import java.math.BigInteger; import java.nio.ByteBuffer; import java.util.*; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; import java.util.function.Predicate; import org.junit.Ignore; @@ -75,6 +74,7 @@ import ghidra.program.model.lang.*; import ghidra.program.model.listing.*; import ghidra.program.model.listing.Function.FunctionUpdateType; import ghidra.program.model.mem.MemoryBlock; +import ghidra.program.model.pcode.SequenceNumber; import ghidra.program.model.scalar.Scalar; import ghidra.program.model.symbol.RefType; import ghidra.program.model.symbol.SourceType; @@ -89,6 +89,7 @@ import ghidra.trace.model.thread.TraceThread; import ghidra.trace.model.time.schedule.Scheduler; import ghidra.util.Msg; import ghidra.util.NumericUtilities; +import ghidra.util.exception.CancelledException; import junit.framework.AssertionFailedError; public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { @@ -146,7 +147,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { protected Function createSumSquaresProgramX86_32() throws Throwable { createProgram("x86:LE:32:default", "gcc"); intoProject(program); - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { Address entry = addr(program, 0x00400000); program.getMemory() .createInitializedBlock(".text", entry, 0x1000, (byte) 0, monitor, false); @@ -209,7 +210,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { protected Function createFibonacciProgramX86_32() throws Throwable { createProgram("x86:LE:32:default", "gcc"); intoProject(program); - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { Address entry = addr(program, 0x00400000); program.getMemory() .createInitializedBlock(".text", entry, 0x1000, (byte) 0, monitor, false); @@ -284,7 +285,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { createProgram("x86:LE:32:default", "gcc"); intoProject(program); Address entry; - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { entry = addr(program, 0x00400000); Address externs = addr(program, 0x00700000); program.getMemory() @@ -346,7 +347,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { createProgram("x86:LE:32:default", "gcc"); intoProject(program); Address entry; - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { entry = addr(program, 0x00400000); program.getMemory() .createInitializedBlock(".text", entry, 0x1000, (byte) 0, monitor, false); @@ -401,7 +402,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { createProgram("x86:LE:32:default", "gcc"); intoProject(program); - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { Address entry = addr(program, 0x00400000); program.getMemory() .createInitializedBlock(".text", entry, 0x1000, (byte) 0, monitor, false); @@ -438,7 +439,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { createProgram("x86:LE:32:default", "gcc"); intoProject(program); - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { ProgramBasedDataTypeManager dtm = program.getDataTypeManager(); Structure structure = new StructureDataType("MyStruct", 0, dtm); structure.add(WordDataType.dataType, "y", ""); @@ -520,7 +521,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { createProgram("x86:LE:64:default", "gcc"); intoProject(program); - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { ProgramBasedDataTypeManager dtm = program.getDataTypeManager(); Structure structure = new StructureDataType("MyStruct", 0, dtm); structure.add(DWordDataType.dataType, "f1", ""); @@ -568,7 +569,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { createProgram("x86:LE:32:default", "gcc"); intoProject(program); - try (Transaction tx = program.openTransaction("Assemble")) { + try (Transaction _ = program.openTransaction("Assemble")) { ProgramBasedDataTypeManager dtm = program.getDataTypeManager(); Structure structure = new StructureDataType("MyStruct", 0, dtm); structure.add(WordDataType.dataType, "y", ""); @@ -768,6 +769,73 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { infoAtBody); } + /** + * This test is interesting because it involves an instruction with an internal p-code loop + * + * @throws Throwable because + */ + @Test + public void testComputeUnwindInfoWithTzcnt() throws Throwable { + addPlugin(tool, CodeBrowserPlugin.class); + addPlugin(tool, DecompilePlugin.class); + + createProgram("x86:LE:64:default", "gcc"); + intoProject(program); + Address entry; + Function function; + try (Transaction _ = program.openTransaction("Assemble")) { + 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("TZCNT EAX, EDI"); + buf.assemble("RET"); + + byte[] bytes = buf.getBytes(); + program.getMemory().setBytes(entry, bytes); + + Disassembler dis = Disassembler.getDisassembler(program, monitor, null); + dis.disassemble(entry, null); + + function = program.getFunctionManager() + .createFunction("tzcnt", entry, + new AddressSet(entry, entry.add(bytes.length - 1)), + SourceType.USER_DEFINED); + function.updateFunction("default", + new ReturnParameterImpl(IntegerDataType.dataType, program), + List.of(new ParameterImpl("n", IntegerDataType.dataType, program)), + FunctionUpdateType.DYNAMIC_STORAGE_FORMAL_PARAMS, true, SourceType.ANALYSIS); + } + + programManager.openProgram(program); + + UnwindAnalysis ua = new UnwindAnalysis(program); + + CompletableFuture futureInfo = CompletableFuture.supplyAsync(() -> { + try { + return ua.computeUnwindInfo(entry, monitor); + } + catch (CancelledException e) { + throw new AssertionError(e); + } + }); + try { + futureInfo.get(1, TimeUnit.SECONDS); + } + catch (TimeoutException e) { + monitor.cancel(); + } + UnwindInfo infoAtEntry = + Objects.requireNonNull(futureInfo.getNow(null), "Probably timed out"); + assertEquals(new UnwindInfo(function, 0L, 8L, stack(0), -1, + Map.of(), new StackUnwindWarningSet( + // NOTE: A bit brittle, since the TZCNT p-code may change + new IgnoredInternalFlowStackUnwindWarning(new SequenceNumber(entry, 8))), + null), infoAtEntry); + } + @Test public void testComputeUnwindInfoWithArmBx() throws Throwable { addPlugin(tool, CodeBrowserPlugin.class); @@ -830,7 +898,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { waitOn(frameAtSetup.setValue(editor, param1, BigInteger.valueOf(4))); waitForTasks(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { tb.trace.getBreakpointManager() .addBreakpoint("Breakpoints[0]", Lifespan.nowOn(0), retInstr, Set.of(), CommonSet.SWX.kinds(), true, "capture return value"); @@ -892,7 +960,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { waitForTasks(); TraceBreakpointLocation bptUnwind; - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { bptUnwind = tb.trace.getBreakpointManager() .addBreakpoint("Breakpoints[0]", Lifespan.nowOn(0), retInstr, Set.of(), CommonSet.SWX.kinds(), true, "unwind stack"); @@ -929,7 +997,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { } } - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { bptUnwind.delete(); } @@ -981,7 +1049,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { long spAtSetup = regs.getValue(0, sp).getUnsignedValue().longValueExact(); TraceBreakpointLocation bptUnwind; - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { bptUnwind = tb.trace.getBreakpointManager() .addBreakpoint("Breakpoints[0]", Lifespan.nowOn(0), entry, Set.of(), CommonSet.SWX.kinds(), true, "unwind stack"); @@ -1063,7 +1131,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { waitForSwing(); DebuggerCoordinates atSetup = traceManager.getCurrent(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { new UnwindStackCommand(tool, atSetup).applyTo(tb.trace, monitor); } waitForDomainObject(tb.trace); @@ -1124,7 +1192,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { waitOn(frameAtSetup.setReturnAddress(editor, tb.addr(0xdeadbeef))); waitForTasks(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { tb.trace.getBreakpointManager() .addBreakpoint("Breakpoints[0]", Lifespan.nowOn(0), retInstr, Set.of(), CommonSet.SWX.kinds(), true, "unwind stack"); @@ -1137,7 +1205,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { traceManager.activateTime(result.schedule()); waitForTasks(); DebuggerCoordinates tallest = traceManager.getCurrent(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { new UnwindStackCommand(tool, tallest).applyTo(tb.trace, monitor); } waitForDomainObject(tb.trace); @@ -1164,7 +1232,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { Register sp = program.getCompilerSpec().getStackPointer(); waitOn(editor.setRegister(new RegisterValue(sp, BigInteger.valueOf(0x4ff0)))); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { tb.trace.getBreakpointManager() .addBreakpoint("Breakpoints[0]", Lifespan.nowOn(0), retInstr, Set.of(), CommonSet.SWX.kinds(), true, "unwind stack"); @@ -1178,7 +1246,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { traceManager.activateTime(result.schedule()); waitForTasks(); DebuggerCoordinates atRet = traceManager.getCurrent(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { new UnwindStackCommand(tool, atRet).applyTo(tb.trace, monitor); } waitForDomainObject(tb.trace); @@ -1205,7 +1273,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { Register sp = program.getCompilerSpec().getStackPointer(); waitOn(editor.setRegister(new RegisterValue(sp, BigInteger.valueOf(0x4ff0)))); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { tb.trace.getBreakpointManager() .addBreakpoint("Breakpoints[0]", Lifespan.nowOn(0), retInstr, Set.of(), CommonSet.SWX.kinds(), true, "unwind stack"); @@ -1219,7 +1287,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { traceManager.activateTime(result.schedule()); waitForTasks(); DebuggerCoordinates atRet = traceManager.getCurrent(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { new UnwindStackCommand(tool, atRet).applyTo(tb.trace, monitor); } waitForDomainObject(tb.trace); @@ -1246,7 +1314,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { Register sp = program.getCompilerSpec().getStackPointer(); waitOn(editor.setRegister(new RegisterValue(sp, BigInteger.valueOf(0x4ff0)))); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { tb.trace.getBreakpointManager() .addBreakpoint("Breakpoints[0]", Lifespan.nowOn(0), retInstr, Set.of(), CommonSet.SWX.kinds(), true, "unwind stack"); @@ -1260,7 +1328,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { traceManager.activateTime(result.schedule()); waitForTasks(); DebuggerCoordinates atRet = traceManager.getCurrent(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { new UnwindStackCommand(tool, atRet).applyTo(tb.trace, monitor); } waitForDomainObject(tb.trace); @@ -1448,7 +1516,7 @@ public class StackUnwinderTest extends AbstractGhidraHeadedDebuggerTest { TraceLocation dynLoc = mappingService.getOpenMappedLocation(tb.trace, new ProgramLocation(program, stIns.getAddress()), current.getSnap()); Address dynamicAddress = dynLoc.getAddress(); - try (Transaction tx = tb.startTransaction()) { + try (Transaction _ = tb.startTransaction()) { int length = stIns.getLength(); assertEquals(length, tb.trace.getMemoryManager()