From bdf3c1d2f6c0740a07d0a368dc3f099b7f571ca3 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Fri, 22 Aug 2025 09:15:56 -0400 Subject: [PATCH] GP-5885 - Updated the Next Instruction action to jump to the the function entry when in the function header --- .../help/topics/Navigation/Navigation.htm | 14 ++++++ .../AbstractNextPreviousAction.java | 47 +++++++++++------ .../NextPreviousInstructionAction.java | 50 +++++++++++++++++-- .../NextPrevCodeUnitPluginTest.java | 31 ++++++++++++ .../core/navigation/NextPrevFunctionTest.java | 1 - .../core/functiongraph/FGActionManager.java | 4 +- .../vertex/ListingGraphComponentPanel.java | 2 +- 7 files changed, 126 insertions(+), 23 deletions(-) diff --git a/Ghidra/Features/Base/src/main/help/help/topics/Navigation/Navigation.htm b/Ghidra/Features/Base/src/main/help/help/topics/Navigation/Navigation.htm index 6a83576484..a808584dd6 100644 --- a/Ghidra/Features/Base/src/main/help/help/topics/Navigation/Navigation.htm +++ b/Ghidra/Features/Base/src/main/help/help/topics/Navigation/Navigation.htm @@ -649,6 +649,20 @@

To move the cursor to the next instruction click on the Navigate by Instruction icon, I. This icon is disabled when no more instructions exist in the current search direction.

+

+ If already on an instruction, on the address field, 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. +

+ +
+

NoteWhen 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. +

+

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 diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/AbstractNextPreviousAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/AbstractNextPreviousAction.java index 476c4fe9c9..b61b4197fc 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/AbstractNextPreviousAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/AbstractNextPreviousAction.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -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()) - : getPreviousAddress(monitor, context.getProgram(), context.getAddress()); + ? getNextAddress(monitor, context) + : 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; - } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NextPreviousInstructionAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NextPreviousInstructionAction.java index 31f2352568..1718d66cda 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NextPreviousInstructionAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NextPreviousInstructionAction.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -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 { diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevCodeUnitPluginTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevCodeUnitPluginTest.java index f40cd0a7fb..e583d0d22c 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevCodeUnitPluginTest.java +++ b/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 { diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevFunctionTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevFunctionTest.java index 355b795962..bf779f4569 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/navigation/NextPrevFunctionTest.java +++ b/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(); diff --git a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java index f120cf2018..998ca3ab43 100644 --- a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/FGActionManager.java +++ b/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); diff --git a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/graph/vertex/ListingGraphComponentPanel.java b/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/graph/vertex/ListingGraphComponentPanel.java index 7c8d7cc079..5415228c46 100644 --- a/Ghidra/Features/FunctionGraph/src/main/java/ghidra/app/plugin/core/functiongraph/graph/vertex/ListingGraphComponentPanel.java +++ b/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"));