GP-5885 - Updated the Next Instruction action to jump to the the function entry when in the function header

Ghidra/Features/Base/src/main/help/help/topics/Navigation/Navigation.htm

@@ -649,6 +649,20 @@
         <P>To move the cursor to the next instruction click on the Navigate by Instruction icon,
         <IMG src="images/I.gif" alt="I" border="0">. This icon is disabled when no more
         instructions exist in the current search direction.</P>
+		<P>
+		If already on an instruction, <B>on the address field</B>, then this action will find the 
+		next data or undefined data before finding the next instruction. This allows users to jump 
+		between ranges of instructions.
+		</P>
+		
+		<BLOCKQUOTE>
+	        <P><IMG src="help/shared/note.yellow.png" alt="Note" border="0">When performing this 
+				action while not on the address field, then the action will first go to the address
+				field for the current address if an instruction exists.  Subsequent invocations will
+				then work as described above.  This is convenient for jumping from a function 
+				signature to the function entry point.  
+		    </P>
+	    </BLOCKQUOTE>
 
         <P>When inverted, this task, if on an instruction, will attempt to navigate to the next
         data or undefined data. If not on an instruction, then this task will find the next

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/AbstractNextPreviousAction.java

@@ -78,7 +78,7 @@ public abstract class AbstractNextPreviousAction extends NavigatableContextActio
 	protected abstract KeyStroke getKeyStroke();
 
 	@Override
-	public void actionPerformed(final NavigatableActionContext context) {
+	public void actionPerformed(NavigatableActionContext context) {
 		Task t = new Task("Searching for " + doGetNavigationTypeName(), true, false, true) {
 			@Override
 			public void run(TaskMonitor monitor) {
@@ -88,7 +88,7 @@ public abstract class AbstractNextPreviousAction extends NavigatableContextActio
 		new TaskLauncher(t);
 	}
 
-	void gotoNextPrevious(TaskMonitor monitor, final NavigatableActionContext context) {
+	private void gotoNextPrevious(TaskMonitor monitor, NavigatableActionContext context) {
 
 		try {
 			boolean direction = isForward;
@@ -97,8 +97,8 @@ public abstract class AbstractNextPreviousAction extends NavigatableContextActio
 			}
 
 			Address address = direction
-					? getNextAddress(monitor, context.getProgram(), context.getAddress())
+					? getNextAddress(monitor, context)
-					: getPreviousAddress(monitor, context.getProgram(), context.getAddress());
+					: getPreviousAddress(monitor, context);
 
 			Swing.runLater(() -> gotoAddress(context, address));
 		}
@@ -107,6 +107,32 @@ public abstract class AbstractNextPreviousAction extends NavigatableContextActio
 		}
 	}
 
+	protected Address getNextAddress(TaskMonitor monitor, NavigatableActionContext context)
+			throws CancelledException {
+
+		// default for clients that do not need the context
+		Program program = context.getProgram();
+		Address address = context.getAddress();
+		return getNextAddress(monitor, program, address);
+	}
+
+	protected Address getPreviousAddress(TaskMonitor monitor, NavigatableActionContext context)
+			throws CancelledException {
+
+		// default for clients that do not need the context
+		Program program = context.getProgram();
+		Address address = context.getAddress();
+		return getPreviousAddress(monitor, program, address);
+	}
+
+	abstract protected Address getNextAddress(TaskMonitor monitor, Program program, Address address)
+			throws CancelledException;
+
+	abstract protected Address getPreviousAddress(TaskMonitor monitor, Program program,
+			Address address) throws CancelledException;
+
+	abstract protected String getNavigationTypeName();
+
 	private void gotoAddress(NavigatableActionContext actionContext, Address address) {
 		if (address == null) {
 			tool.setStatusInfo("Unable to locate another \"" + doGetNavigationTypeName() +
@@ -120,7 +146,6 @@ public abstract class AbstractNextPreviousAction extends NavigatableContextActio
 			Navigatable navigatable = actionContext.getNavigatable();
 			gotoAddress(service, navigatable, address);
 		}
-
 	}
 
 	protected void gotoAddress(GoToService service, Navigatable navigatable, Address address) {
@@ -145,16 +170,8 @@ public abstract class AbstractNextPreviousAction extends NavigatableContextActio
 		return getNavigationTypeName();
 	}
 
-	abstract protected String getNavigationTypeName();
-
 	protected String getInvertedNavigationTypeName() {
 		return "Non-" + getNavigationTypeName();
 	}
 
-	abstract protected Address getNextAddress(TaskMonitor monitor, Program program, Address address)
-			throws CancelledException;
-
-	abstract protected Address getPreviousAddress(TaskMonitor monitor, Program program,
-			Address address) throws CancelledException;
-
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NextPreviousInstructionAction.java

@@ -18,13 +18,17 @@ package ghidra.app.plugin.core.navigation;
 import java.awt.event.InputEvent;
 import java.awt.event.KeyEvent;
 
+import javax.help.UnsupportedOperationException;
 import javax.swing.Icon;
 import javax.swing.KeyStroke;
 
 import generic.theme.GIcon;
+import ghidra.app.context.NavigatableActionContext;
 import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.listing.*;
+import ghidra.program.util.AddressFieldLocation;
+import ghidra.program.util.ProgramLocation;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
@@ -53,13 +57,20 @@ public class NextPreviousInstructionAction extends AbstractNextPreviousAction {
 	}
 
 	@Override
-	protected Address getNextAddress(TaskMonitor monitor, Program program, Address address)
+	protected Address getNextAddress(TaskMonitor monitor, NavigatableActionContext context)
 			throws CancelledException {
 
+		Program program = context.getProgram();
+		Address address = context.getAddress();
 		if (isInverted) {
 			return getNextNonInstructionAddress(monitor, program, address);
 		}
 
+		// check for known special cases
+		if (useCurrentInstruction(context)) {
+			return address;
+		}
+
 		if (isInstructionAt(program, address)) {
 			// on an instruction, find a non-instruction before finding the next instruction
 			address = getAddressOfNextPreviousNonInstruction(monitor, program, address, true);
@@ -69,10 +80,27 @@ public class NextPreviousInstructionAction extends AbstractNextPreviousAction {
 		return getAddressOfNextInstructionAfter(program, address);
 	}
 
+	private boolean useCurrentInstruction(NavigatableActionContext context) {
+		// Jumping to the next instruction below the current instruction is not useful.  When on an
+		// instruction, find the next non-instruction and then look for the next instruction after
+		// that.  We do not want to do this when there is an instruction at an address, but it is
+		// not close to the cursor, such as when on a function signature.  In that case, jump to the
+		// instruction at that same address.  In the case when the cursor is on the function
+		// signature, this will allow the user to quickly jump to the entry address field.
+		// 
+		// Each time this action is executed, it places the cursor on the address field.  Use that
+		// as a signal that we should go to the next instruction.  This allows the example outlined
+		// above to work, with only minor intrusion to the user's workflow.
+		ProgramLocation location = context.getLocation();
+		return !(location instanceof AddressFieldLocation);
+	}
+
 	@Override
-	protected Address getPreviousAddress(TaskMonitor monitor, Program program, Address address)
+	protected Address getPreviousAddress(TaskMonitor monitor, NavigatableActionContext context)
 			throws CancelledException {
 
+		Program program = context.getProgram();
+		Address address = context.getAddress();
 		if (isInverted) {
 			return getPreviousNonInstructionAddress(monitor, program, address);
 		}
@@ -86,6 +114,20 @@ public class NextPreviousInstructionAction extends AbstractNextPreviousAction {
 		return getAddressOfPreviousInstructionBefore(program, address);
 	}
 
+	@Override
+	protected Address getNextAddress(TaskMonitor monitor, Program program, Address address)
+			throws CancelledException {
+		// use getNextAddress(NavigatableActionContext, TaskMonitor) instead
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	protected Address getPreviousAddress(TaskMonitor monitor, Program program, Address address)
+			throws CancelledException {
+		// use getPreviousAddress(NavigatableActionContext, TaskMonitor) instead
+		throw new UnsupportedOperationException();
+	}
+
 	private Address getNextNonInstructionAddress(TaskMonitor monitor, Program program,
 			Address address) throws CancelledException {
 

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevCodeUnitPluginTest.java

@@ -31,6 +31,7 @@ import ghidra.app.cmd.disassemble.DisassembleCommand;
 import ghidra.app.plugin.core.bookmark.BookmarkEditCmd;
 import ghidra.app.plugin.core.codebrowser.CodeBrowserPlugin;
 import ghidra.app.services.GoToService;
+import ghidra.app.util.viewer.field.FunctionSignatureFieldFactory;
 import ghidra.framework.cmd.CompoundCmd;
 import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.database.ProgramBuilder;
@@ -38,6 +39,7 @@ import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.mem.Memory;
+import ghidra.program.model.symbol.Symbol;
 import ghidra.test.*;
 import ghidra.util.task.TaskMonitor;
 import resources.Icons;
@@ -173,6 +175,35 @@ public class NextPrevCodeUnitPluginTest extends AbstractGhidraHeadedIntegrationT
 		assertAddress("0x100294d");
 	}
 
+	@Test
+	public void testSearchInstruction_WhenOnFunctionSignature() throws Exception {
+
+		//
+		// Test that the Next Instruction action will go to the Address Field when inside of a 
+		// function signature.
+		//
+
+		Symbol symbol = getUniqueSymbol(program, "FUN_01002239");
+		Address functionEntry = symbol.getAddress();
+
+		assertTrue(cb.goToField(functionEntry, FunctionSignatureFieldFactory.FIELD_NAME, 0, 0));
+
+		nextInstruction();
+
+		// address is the same, but the field has changed from the signature to the bytes field
+		assertAddress(functionEntry);
+
+		nextInstruction();
+		assertAddress("0x1002cf5");
+
+		toggleDirection();
+
+		// going backwards the address before the function will be chosen
+		nextInstruction();
+		assertAddress("0x100294d");
+
+	}
+
 	@Test
 	public void testSearchNotInstruction() throws Exception {
 

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevFunctionTest.java

@@ -47,7 +47,6 @@ public class NextPrevFunctionTest extends AbstractGhidraHeadedIntegrationTest {
 		return addrFactory.getAddress(address);
 	}
 
-	@SuppressWarnings("unchecked") // we know that bookmarks is of type String
 	@Before
 	public void setUp() throws Exception {
 		env = new TestEnv();

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java

@@ -388,7 +388,7 @@ public class FGActionManager {
 		fullViewAction.setHelpLocation(
 			new HelpLocation("FunctionGraphPlugin", "Vertex_Action_Full_View"));
 
-		DockingAction xrefsAction = new DockingAction("Jump to a XRef", owner) {
+		DockingAction xrefsAction = new DockingAction("Jump to XRef", owner) {
 			@Override
 			public void actionPerformed(ActionContext context) {
 				controller.showXRefsDialog();
@@ -410,7 +410,7 @@ public class FGActionManager {
 				return true;
 			}
 		};
-		menuData = new MenuData(new String[] { "Jump to a XRef" }, popupMutateGroup1);
+		menuData = new MenuData(new String[] { "Jump to XRef" }, popupMutateGroup1);
 		menuData.setIcon(XREFS_ICON);
 		menuData.setMenuSubGroup(Integer.toString(vertexGroupingSubgroupOffset++));
 		xrefsAction.setPopupMenuData(menuData);

Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/graph/vertex/ListingGraphComponentPanel.java

@@ -331,7 +331,7 @@ public class ListingGraphComponentPanel extends AbstractGraphComponentPanel {
 				controller.showXRefsDialog();
 			}
 		};
-		xrefsAction.setDescription("Jump to a XRef");
+		xrefsAction.setDescription("Jump to XRef");
 		Icon imageIcon = new GIcon("icon.plugin.functiongraph.action.vertex.xrefs");
 		xrefsAction.setToolBarData(new ToolBarData(imageIcon, firstGroup));
 		xrefsAction.setHelpLocation(new HelpLocation("FunctionGraphPlugin", "Vertex_Action_XRefs"));