Repeated use of GoToService.goTo within same function triggers new decompile

[alandtse]

Describe the bug A clear and concise description of the bug. I am using the ret-sync plugin with Ghidra that is using the GoToService.goTo() command to navigate a program in Ghidra. Ret-Sync allows syncing of a debugger with Ghidra so every step results in a new call to Ghidra to the next address. However, in Ghidra the repeated calls to goto for the next address results in the Decompile window trying to decompile the function again. This decompilation breaks the sync of the Ghidra Listing and Decompiler window.

To Reproduce Steps to reproduce the behavior:

1. Use Ret-Sync plugin with debugger (x64debug) and sync with Ghidra.
2. Step through program in debugger.
3. Ghidra Listing window will step but Decompiler window will reset due to decompile attempt.

Expected behavior A clear and concise description of what you expected to happen. GoTos within the same function block should not trigger a new decompilation attempt and maintain the sync between the Listing window and Decompiler window.

Screenshots If applicable, add screenshots to help explain your problem.

Attachments If applicable, please attach any files that caused problems or log files generated by the software.

Environment (please complete the following information):

• OS: [e.g. macOS 10.14.2] Windows 10
• Java Version: [e.g. 11.0] OpenJDK 11 (I can get the specific if needed).
• Ghidra Version: [e.g. 10.1.4] 10.1.4
• Ghidra Origin: [e.g. official GitHub distro, third party distro, locally built] GitHub release

Additional context Add any other context about the problem here. I'm not sure if the Ghidra UI allows reproduction of this by issuing subsequent gotos, but will check and update.

[dragonmacher]

Naturally, providing assistance without the actual code will be difficult. That being said, there are many goTo() methods on the service. Can you specify which method(s) are being called?

Update: The Decompiler has code to ensure that location changes within the body of a function to not trigger a re-decompile. There could be a few things triggering this, the most likely of which is multiple calls to the GoToServce, triggering an oscillation. It is possible there some subtle bug exists in Ghidra. We would need to be able to reproduce the issue in order to fix it.

[alandtse]

Thanks. I thought I had linked the specific function call in the ret-sync plugin. I went ahead and pasted the references to the GoTo service that I can see in the plugin. I'm rusty with Java but it looks like the base GoToService with boolean goTo​([ProgramLocation](https://ghidra.re/ghidra_docs/api/ghidra/program/util/ProgramLocation.html) loc)?

import ghidra.app.services.GoToService;
...
servicesRequired = {
                ProgramManager.class,
                ConsoleService.class,
                CodeViewerService.class,
                GoToService.class },
...
GoToService gs;
...
        gs = tool.getService(GoToService.class);

...
    void gotoLoc(long base, long offset) {
        Address dest = null;

        if (!syncEnabled)
            return;

        dest = rebase(base, offset);

        if (dest != null) {
            gs.goTo(dest);
            clrs.setPrevAddr(dest);
        }
    }

On further review, I'm seeing the issue when the decompiled function itself tends to be on the larger size and it's more severe if I step through the program quickly which results in multiple calls to gs.goTo(dest);.

Perhaps it's tied to a decompile that hasn't completed? Instead of finishing the decompile, the next goto may restart the decompile request? I believe that may be one of the trigger cases but I also believe I've seen it when the function has already been decompiled.

As for the specific code I'm stepping through, I'm reverse engineering different versions of Skyrim. But I imagine any sufficiently complex code may be able to trigger it.

Please let me know if I can provide any further information.

[dragonmacher]

Perhaps it's tied to a decompile that hasn't completed? Instead of finishing the decompile, the next goto may restart the decompile request?

It feels like some variation of this. The code below shows that we attempt to update the in-process decompilation. However, no such attempt is made if there is another pending decompilation to execute after the active one. In that case, the pending request is updated to be that of the function containing the location being requested.

It seems like there could be a scenario where rapid navigation happens such that:

• request 1 triggers decompilation 1
• request 2 arrives for a different function; a pending decompilation 2 is triggered
• request 3 arrives and is contained in the original in-process decompilation 1

If this happens, we make no attempt to see if 'request 3' can be satisfied with the in-process decompilation 1. If we did that, that may prevent some of the bouncing you are seeing. In practice, most decompilation is sufficiently fast that we likely will not see this in normal use.

	/**
	 * Requests a new decompile be scheduled.  If a current decompile is already in progress,
	 * the new request is checked to see if represents the same function. If so, only the
	 * location of the current decompile is updated and the current decompile is allowed to continue.
	 * Otherwise a new DecompileRunnable is created and scheduled to run using the updateManager.
	 * When the updateMangers runs, it will stop any current decompiles and begin the new decompile.
	 * @param program The program containing the function to be decompiled.
	 * @param location the location in the program to be decompiled and positioned to.
	 * @param debugFile if non-null, creates decompile debug output to this file.
	 * @param forceDecompile true forces a new decompile to be scheduled even if the current job
	 * is the same function.
	 */
	synchronized void decompile(Program program, ProgramLocation location,
			ViewerPosition viewerPosition, File debugFile, boolean forceDecompile) {

		DecompileRunnable newDecompileRunnable =
			new DecompileRunnable(program, location, debugFile, viewerPosition, this);

		if (forceDecompile) {
			cancelAll();
			setPendingRunnable(newDecompileRunnable);
			return;
		}

		if (updateCurrentRunnable(newDecompileRunnable)) {
			return;
		}

		setPendingRunnable(newDecompileRunnable);
	}

[alandtse]

It seems like there could be a scenario where rapid navigation happens such that:

• request 1 triggers decompilation 1
• request 2 arrives for a different function; a pending decompilation 2 is triggered
• request 3 arrives and is contained in the original in-process decompilation 1

Thanks. I'll pay attention to see if that is what is happening. However, I'm a bit doubtful as I mainly notice it as I step through the same function (stepping over any function calls) so it's not flipping between two functions that I have noticed.

I'll also try to note whether this behavior occurs because it may have hit a slow decompiling function. You mentioned a queuing behavior which could result in a long decompile basically blocking any newer decompiles with a steadily increasing queue as more functions are reached.

Now that you've pointed me toward the code at issue, I may be able to investigate further to test the theory and potentially do a PR.

One last note, the reason I noticed the problem is because the decompile window stops syncing and sometimes will scroll back to the top (I assume due to a completed decompile). If it's a problem that there's a long queue, perhaps a visual greying of the text of the decompile window would help indicate that the listing window and decompile are no longer synced.

[dragonmacher]

You mentioned a queuing behavior which could result in a long decompile basically blocking any newer decompiles with a steadily increasing queue as more functions are reached.

The isn't an open-ended queue. At most there are just 2 items: the in-process decompilation and 1 pending decompilation. Any further requests will replace the pending request with the new request.

If it's a problem that there's a long queue, perhaps a visual greying of the text of the decompile window would help indicate that the listing window and decompile are no longer synced.

When all is working as expected, this should not be needed. However, we could possibly revisit what is displayed while new decompilation is taking place.

Now that you've pointed me toward the code at issue, I may be able to investigate further to test the theory and potentially do a PR.

Let us know if you figure out a way for us to reproduce it. I'm sure we could improve the internals for this particular use case.

[alandtse]

One potential cause I've figured out is assigning registers appears to trigger the decompiler for stepping since apparently the decompiler may restructure based on those values. I was manually assigning registers based on the debugger and was planning to also add that feature to retsync, but if that causes decompiles it's probably not ideal.

That said, I've now confirmed the behavior even when all registers are cleared and I'm still in the same function. I haven't had time to build Ghidra yet to really debug but thought I'd at least report the register assignment behavior.

[homes410]

Ghidra that is using the GoToService.goTo() command to navigate a program in Ghidra. Ret-Sync allows syncing of a debugger with Ghidra so every step results in a new call to Ghidra to the next address. However, in Ghidra the repeated calls to goto for the next address results in the Decompile window trying to decompile the function again. This decompilation breaks the sync of the Ghidra Listing and Decompiler window.

 Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/DecompileData.java 
index e4d243958..b4c855c12 100644
@@ -22,9 +22,11 @@ import ghidra.program.model.address.AddressSpace;
 import ghidra.program.model.listing.Function;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.pcode.HighFunction;
+import ghidra.program.model.pcode.PcodeOpAST;
 import ghidra.program.util.ProgramLocation;
 
 import java.io.File;
+import java.util.Iterator;
 
 import docking.widgets.fieldpanel.support.ViewerPosition;
 
@@ -118,7 +120,12 @@ public class DecompileData {
 		if (address == null) {
 			return false;
 		}
-		return function.getBody().contains(address);
+		AddressSpace funcSpace = function.getEntryPoint().getAddressSpace();
+		if (funcSpace.isOverlaySpace() && address.getAddressSpace().equals(funcSpace)) {
+			address = address.getPhysicalAddress();
+		}
+		Iterator<PcodeOpAST> pcodeOps = decompileResults.getHighFunction().getPcodeOps(address);
+		return pcodeOps.hasNext();
 	}
 
 	public AddressSpace getFunctionSpace() {

But above patch will trigger refresh when PC address is set to NOP instruction. But You can mod the ret-sync plugin to filter out addresses with those specific instruction to avoid refresh.