From 00bbee3163df17e3d6bf4f93a0675f0cabf6d00f Mon Sep 17 00:00:00 2001
From: emteere <47253321+emteere@users.noreply.github.com>
Date: Thu, 23 Nov 2023 09:36:22 -0500
Subject: [PATCH] GP-3962 Fix for test failures from delayslot with branches
 flow following, also fixed backward flow following through delayslots with
 branches

---
 .../select/flow/FollowDelaySlotFlowTest.java  |  4 +--
 .../program/database/code/InstructionDB.java  | 29 ++++++++++---------
 .../program/model/listing/Instruction.java    | 23 +++++++++++++++
 3 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowDelaySlotFlowTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowDelaySlotFlowTest.java
index 82d0854e13..eaf64d2eba 100644
--- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowDelaySlotFlowTest.java
+++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowDelaySlotFlowTest.java
@@ -175,7 +175,7 @@ public class FollowDelaySlotFlowTest extends AbstractFollowFlowTest {
 		AddressSet flowAddresses = followFlow.getFlowAddressSet(TaskMonitor.DUMMY);
 
 		AddressSet expectedAddresses = new AddressSet();
-		expectedAddresses.add(address(10), address(15));
+		expectedAddresses.add(address(10), address(17));
 		expectedAddresses.add(address(20), address(25));
 
 		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
@@ -580,7 +580,7 @@ public class FollowDelaySlotFlowTest extends AbstractFollowFlowTest {
 		AddressSet flowAddresses = followFlow.getFlowToAddressSet(TaskMonitor.DUMMY);
 
 		AddressSet expectedAddresses = new AddressSet();
-		expectedAddresses.add(address(16), address(17));
+		expectedAddresses.add(address(14), address(17));
 
 		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
 
diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/InstructionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/InstructionDB.java
index aa399b9aaf..2261e92ed4 100644
--- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/InstructionDB.java
+++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/InstructionDB.java
@@ -220,7 +220,7 @@ public class InstructionDB extends CodeUnitDB implements Instruction, Instructio
 			}
 
 			do {
-				// skip past delay slot instructions
+				// skip past delay slot instructions which satisfy specific conditions
 				try {
 					instr = program.getListing()
 							.getInstructionContaining(
@@ -229,28 +229,31 @@ public class InstructionDB extends CodeUnitDB implements Instruction, Instructio
 				catch (AddressOverflowException e) {
 					return null;
 				}
-				// if an instruction is in a delay slot and has references to it,
-				// consider this the fallfrom instruction
-				if (instr != null && instr.isInDelaySlot() && instr.hasFallthrough()) {
-					if (program.getSymbolTable().hasSymbol(instr.getMinAddress())) {
-						break;
-					}
-				}
 			}
-			while (instr != null && instr.isInDelaySlot());
+			// Continue walking instructions backwards if a delay-slot instruction is found and 
+			// either the delay slot instruction does not fallthrough or it does not have a 
+			// ref or label on it.
+			while (instr != null && instr.isInDelaySlot() &&
+				(!instr.hasFallthrough() ||
+					!program.getSymbolTable().hasSymbol(instr.getMinAddress())));
 			
 			if (instr == null) {
 				return null;
 			}
 
-			// If this instruction is in a delay slot,
-			// it is assumed to always fall from the delay-slotted
-			// instruction regardless of its fall-through
 			if (this.isInDelaySlot()) {
+				// If this instruction is within delay-slot, return a null fall-from address if 
+				// previous instruction (i.e., instruction with delay slot, found above)
+				// does not have a fallthrough and this instruction has a ref or label on it.
+				if (!instr.hasFallthrough() &&
+					program.getSymbolTable().hasSymbol(this.getMinAddress())) {
+					return null;
+				}
+				// Return previous instruction's address (i.e., instruction with delay slot, found above)
 				return instr.getMinAddress();
 			}
 
-			// No delay slot, but check if the instruction falls into this one.
+			// No delay-slot, but check if the instruction falls into this one.
 			Address fallAddr = instr.getFallThrough();
 			if (fallAddr != null && fallAddr.equals(address)) {
 				return instr.getMinAddress();
diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Instruction.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Instruction.java
index ec3d777ad1..7e9c84ab8a 100644
--- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Instruction.java
+++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Instruction.java
@@ -143,6 +143,29 @@ public interface Instruction extends CodeUnit, ProcessorContext {
 	 * this instruction.
 	 * This is useful for handling instructions that are found
 	 * in a delay slot.
+	 * 
+	 * Note: if an instruction is in a delayslot, then it may have
+	 *    a branch into the delayslot, which is handled as follows
+	 *    
+	 * <pre>
+	 * JMPIF Y, X
+	 *   lab:
+	 *     _ADD         getFallFrom() = JMPIF
+	 * MOV              getFallFrom() = _ADD
+	 * 
+	 * JMP Y, X
+	 *   lab:
+	 *     _ADD         getFallFrom() = null
+	 * MOV              getFallFrom() = _ADD
+	 *
+	 * JMPIF Y, X
+	 *     _ADD         getFallFrom() = JMPIF
+	 * MOV              getFallFrom() = JMPIF
+	 *   
+	 * JMP Y, X
+	 *     _ADD         getFallFrom() = JMP
+	 * MOV              getFallFrom() = null
+	 *</pre>
 	 */
 	public Address getFallFrom();