GT-2932 - Decompiler - review fixes

Ghidra/Features/Base/src/main/java/ghidra/app/merge/tool/ListingMergePanel.java

@@ -476,6 +476,12 @@ public class ListingMergePanel extends JPanel
 			return false;
 		}
 
+		@Override
+		public boolean goToExternalLocation(Navigatable navigatable, ExternalLocation externalLoc,
+				boolean checkNavigationOption) {
+			return false;
+		}
+
 		@Override
 		public GoToOverrideService getOverrideService() {
 			return null;

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/gotoquery/GoToHelper.java

@@ -17,6 +17,8 @@ package ghidra.app.plugin.core.gotoquery;
 
 import java.util.Stack;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.widgets.OptionDialog;
 import ghidra.app.cmd.refs.SetExternalNameCmd;
 import ghidra.app.nav.Navigatable;
@@ -96,6 +98,9 @@ public class GoToHelper {
 			}
 			ExternalLocation externalLoc =
 				program.getExternalManager().getExternalLocation(externalSym);
+
+			// TODO - this seems like a mistake to always pass 'false' here; please doc why we
+			//        wish to ignore the user options for when to navigate to external programs
 			return goToExternalLinkage(navigatable, externalLoc, false);
 		}
 
@@ -248,7 +253,7 @@ public class GoToHelper {
 	 * external program association has not yet been established, the user will be prompted to make
 	 * an association if they choose before completing the navigation.
 	 * @param nav Navigatable
-	 * @param externalLoc external location
+	 * @param externalLocation external location
 	 * @param checkNavigationOption if true the {@link NavigationOptions#isGotoExternalProgramEnabled}
 	 * option will be used to determine if navigation to the external program will be
 	 * attempted, or if navigation to the external linkage location within the current
@@ -257,32 +262,47 @@ public class GoToHelper {
 	 * @return true if navigation to the external program was successful or navigation to a
 	 * linkage location was performed.
 	 */
-	public boolean goToExternalLocation(Navigatable nav, ExternalLocation externalLoc,
+	public boolean goToExternalLocation(Navigatable nav, ExternalLocation externalLocation,
 			boolean checkNavigationOption) {
 
 		if (checkNavigationOption && !navOptions.isGotoExternalProgramEnabled()) {
-			return goToExternalLinkage(nav, externalLoc, true);
+			return goToExternalLinkage(nav, externalLocation, true);
 		}
 
-		Symbol externalSym = externalLoc.getSymbol();
+		Program externalProgram = openExternalProgram(externalLocation);
+		if (externalProgram == null) {
+			return false;
+		}
+
+		// try the address first if it exists
+		Address addr = externalLocation.getAddress();
+		if (addr != null && externalProgram.getMemory().contains(addr)) {
+			goTo(nav, new AddressFieldLocation(externalProgram, addr), externalProgram);
+			return true;
+		}
+
+		// then try the symbol
+		Symbol symbol = getExternalSymbol(externalProgram, externalLocation);
+		if (symbol != null) {
+			goTo(nav, symbol.getProgramLocation(), externalProgram);
+			return true;
+		}
+
+		performDefaultExternalProgramNavigation(nav, externalLocation, externalProgram, addr);
+		return false; // return false because we did not go to the requested address
+	}
+
+	private Program openExternalProgram(ExternalLocation externalLocation) {
+
+		if (externalLocation == null) {
+			return null;
+		}
+
+		Symbol externalSym = externalLocation.getSymbol();
 		Program program = externalSym.getProgram();
-
-		ExternalManager extMgr = program.getExternalManager();
-		String extProgName = externalLoc.getLibraryName();
-		if (Library.UNKNOWN.equals(extProgName)) {
-			tool.setStatusInfo(" External location refers to " + Library.UNKNOWN +
-				" library. Unable to " + "perform navigation.", true);
-			return false;
-		}
-		String pathName = extMgr.getExternalLibraryPath(extProgName);
-		if (pathName == null || pathName.length() == 0) {
-			createExternalAssociation(program, extProgName);
-			pathName = extMgr.getExternalLibraryPath(extProgName);
-		}
-		if (pathName == null || pathName.length() == 0) {
-			tool.setStatusInfo(
-				" External location is not resolved. Unable to " + "perform navigation.", true);
-			return false;
+		String pathName = getExternalLibraryPath(externalLocation, program);
+		if (pathName == null) {
+			return null;
 		}
 
 		ProjectData pd = tool.getProject().getProjectData();
@@ -290,36 +310,20 @@ public class GoToHelper {
 		ProgramManager service = tool.getService(ProgramManager.class);
 		if (domainFile == null || service == null) {
 			tool.setStatusInfo("Unable to navigate to external location. " +
-				"Destination program [" + pathName + "] does not exist.");
-			return false;
-		}
-		Program externalProgram = service.openProgram(domainFile, -1, ProgramManager.OPEN_VISIBLE);
-		if (externalProgram == null) {
-			return false;
+				"Destination program [" + externalLocation + "] does not exist.");
+			return null;
 		}
 
-		String label = externalLoc.getOriginalImportedName();
-		if (label == null) {
-			label = externalLoc.getLabel();
-		}
-		Address addr = externalLoc.getAddress();
+		return service.openProgram(domainFile, -1, ProgramManager.OPEN_VISIBLE);
+	}
 
-		// try the address first if it exists.
-		if (addr != null && externalProgram.getMemory().contains(addr)) {
-			goTo(nav, new AddressFieldLocation(externalProgram, addr), externalProgram);
-			return true;
-		}
+	private void performDefaultExternalProgramNavigation(Navigatable nav,
+			ExternalLocation externalLocation, Program externalProgram, Address addr) {
 
-		Symbol symbol = getExternalSymbol(externalProgram, externalLoc);
-		if (symbol != null) {
-			goTo(nav, symbol.getProgramLocation(), externalProgram);
-			return true;
-		}
-
-		// failed to navigate to address or symbol
+		// failed to navigate to address or symbol; alert the user
 		if (addr != null) {
-			if (externalLoc.getSymbol().getSource() != SourceType.DEFAULT) {
-				tool.setStatusInfo("Symbol [" + getExternalName(externalLoc) +
+			if (externalLocation.getSymbol().getSource() != SourceType.DEFAULT) {
+				tool.setStatusInfo("Symbol [" + getExternalName(externalLocation) +
 					"] does not exist, and address [" + addr + "] is not in memory.", true);
 			}
 			else {
@@ -327,14 +331,39 @@ public class GoToHelper {
 			}
 		}
 		else {
-			tool.setStatusInfo("Symbol [" + getExternalName(externalLoc) + "] does not exist.",
+			tool.setStatusInfo("Symbol [" + getExternalName(externalLocation) + "] does not exist.",
 				true);
 		}
 
 		// navigate to top of external program
-		addr = externalProgram.getMinAddress();
-		goTo(nav, new AddressFieldLocation(externalProgram, addr), externalProgram);
-		return false; // return false because we did not go to the requested address
+		AddressFieldLocation location =
+			new AddressFieldLocation(externalProgram, externalProgram.getMinAddress());
+		goTo(nav, location, externalProgram);
+	}
+
+	private String getExternalLibraryPath(ExternalLocation externalLocation, Program program) {
+
+		ExternalManager externalManager = program.getExternalManager();
+		String extProgName = externalLocation.getLibraryName();
+		if (Library.UNKNOWN.equals(extProgName)) {
+			tool.setStatusInfo(" External location refers to " + Library.UNKNOWN +
+				" library. Unable to " + "perform navigation.", true);
+			return null;
+		}
+
+		String pathName = externalManager.getExternalLibraryPath(extProgName);
+		if (StringUtils.isBlank(pathName)) {
+			createExternalAssociation(program, extProgName);
+			pathName = externalManager.getExternalLibraryPath(extProgName);
+		}
+
+		if (StringUtils.isBlank(pathName)) {
+			tool.setStatusInfo(" External location is not resolved. Unable to perform navigation.",
+				true);
+			return null;
+		}
+
+		return pathName;
 	}
 
 	private Stack<String> getExternalNamespaceStack(ExternalLocation externalLoc) {
@@ -375,9 +404,7 @@ public class GoToHelper {
 
 		// assume global symbol name if mangled name (same as original name) exists
 		String label = externalLoc.getOriginalImportedName();
-
 		SymbolPath symbolPath;
-
 		if (label == null) {
 			// use alternate symbol (may or may not be mangled)
 			Symbol s = externalLoc.getSymbol();

Ghidra/Features/Base/src/main/java/ghidra/app/services/GoToService.java

@@ -105,7 +105,7 @@ public interface GoToService {
 	/**
 	 * Navigate to either the external program location or address linkage location.  
 	 * Specific behavior may vary based upon implementation.
-	 * @param nav Navigatable
+	
 	 * @param externalLoc external location
 	 * @param checkNavigationOption if true the service navigation 
 	 * option will be used to determine if navigation to the external program will be 
@@ -118,6 +118,23 @@ public interface GoToService {
 	public boolean goToExternalLocation(ExternalLocation externalLoc,
 			boolean checkNavigationOption);
 
+	/**
+	 * Navigate to either the external program location or address linkage location.  
+	 * Specific behavior may vary based upon implementation.
+	 * 
+	 * @param navigatable Navigatable
+	 * @param externalLoc external location
+	 * @param checkNavigationOption if true the service navigation 
+	 * option will be used to determine if navigation to the external program will be 
+	 * attempted, or if navigation to the external linkage location within the current
+	 * program will be attempted.  If false, the implementations default behavior
+	 * will be performed.
+	 * @return true if either navigation to the external program or to a
+	 * linkage location was completed successfully.
+	 */
+	public boolean goToExternalLocation(Navigatable navigatable, ExternalLocation externalLoc,
+			boolean checkNavigationOption);
+
 	/**
 	 * Parses the input string as either: 
 	 * 	an address/symbol expression (0x1000+5,  or LAB1000+5)  
@@ -131,10 +148,10 @@ public interface GoToService {
 	 * 
 	 * The listener will be notified after query and will indicate the query status.
 	 * 
-	 * @param queryInput this input string to parse.
 	 * @param fromAddr The address used to determine the scope of the query
-	 * @param caseSensitive if true, the search must match case.
+	 * @param queryData the query input data
 	 * @param listener the listener that will be notified when the query completes.
+	 * @param monitor the task monitor
 	 * @return true if the queryInput is found or appears to be a wildcard search.
 	 */
 	public boolean goToQuery(Address fromAddr, QueryData queryData, GoToServiceListener listener,

Ghidra/Features/Base/src/main/java/ghidra/app/util/navigation/GoToServiceImpl.java

@@ -125,6 +125,12 @@ public class GoToServiceImpl implements GoToService {
 		return helper.goToExternalLocation(defaultNavigatable, extLoc, checkNavigationOption);
 	}
 
+	@Override
+	public boolean goToExternalLocation(Navigatable navigatable, ExternalLocation extLoc,
+			boolean checkNavigationOption) {
+		return helper.goToExternalLocation(navigatable, extLoc, checkNavigationOption);
+	}
+
 	@Override
 	public boolean goToQuery(Navigatable navigatable, Address fromAddr, QueryData queryData,
 			GoToServiceListener listener, TaskMonitor monitor) {
@@ -133,9 +139,8 @@ public class GoToServiceImpl implements GoToService {
 			navigatable = defaultNavigatable;
 		}
 
-		GoToQuery query =
-			new GoToQuery(navigatable, plugin, this, queryData, fromAddr, listener,
-				helper.getOptions(), monitor);
+		GoToQuery query = new GoToQuery(navigatable, plugin, this, queryData, fromAddr, listener,
+			helper.getOptions(), monitor);
 
 		return query.processQuery();
 	}

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/field/OperandFieldHelper.java

@@ -401,8 +401,8 @@ abstract class OperandFieldHelper extends FieldFactory {
 		for (int opIndex = 0; opIndex < numOperands; opIndex++) {
 			OperandRepresentationList operandRepresentationList =
 				codeUnitFormat.getOperandRepresentationList(inst, opIndex);
-			addElementsForOperand(inst, elements, opIndex, operandRepresentationList,
-				characterOffset);
+			characterOffset = addElementsForOperand(inst, elements, opIndex,
+				operandRepresentationList, characterOffset);
 			characterOffset = 0;
 		}
 
@@ -504,8 +504,8 @@ abstract class OperandFieldHelper extends FieldFactory {
 	}
 
 	private boolean containsNonPrimary(Reference[] refs) {
-		for (Reference ref : refs) {
-			if (!ref.isPrimary()) {
+		for (int i = 0; i < refs.length; i++) {
+			if (!refs[i].isPrimary()) {
 				return true;
 			}
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/field/OperandFieldMouseHandler.java

@@ -46,10 +46,6 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 
 	private final static Class<?>[] SUPPORTED_CLASSES = new Class[] { OperandFieldLocation.class };
 
-	/**
-	 * @see FieldMouseHandlerExtension#fieldElementClicked(Object, Navigatable,
-	 *      MouseEvent, ServiceProvider)
-	 */
 	@Override
 	public boolean fieldElementClicked(Object clickedObject, Navigatable navigatable,
 			ProgramLocation location, MouseEvent mouseEvent, ServiceProvider serviceProvider) {
@@ -101,9 +97,6 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 		return false;
 	}
 
-	/**
-	 * @see FieldMouseHandlerExtension#getSupportedProgramLocations()
-	 */
 	@Override
 	public Class<?>[] getSupportedProgramLocations() {
 		return SUPPORTED_CLASSES;
@@ -130,19 +123,55 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 
 	private boolean checkExternalReference(Navigatable navigatable, CodeUnit codeUnit,
 			OperandFieldLocation loc, GoToService goToService) {
+
 		Address refAddr = loc.getRefAddress();
 		if (refAddr == null || !refAddr.isExternalAddress()) {
-			return false;
+			return checkExternalThunkFunctionReference(navigatable, codeUnit, loc, goToService);
 		}
+
 		Program program = codeUnit.getProgram();
 		Symbol s = program.getSymbolTable().getPrimarySymbol(refAddr);
 		if (s == null) {
 			return false;
 		}
+
 		ExternalLocation extLoc = program.getExternalManager().getExternalLocation(s);
 		return goToService.goToExternalLocation(extLoc, true);
 	}
 
+	private boolean checkExternalThunkFunctionReference(Navigatable navigatable, CodeUnit codeUnit,
+			OperandFieldLocation loc, GoToService goToService) {
+
+		Address refAddr = loc.getRefAddress();
+		Program program = codeUnit.getProgram();
+		Symbol s = program.getSymbolTable().getPrimarySymbol(refAddr);
+		if (s == null) {
+			return false;
+		}
+
+		SymbolType type = s.getSymbolType();
+		if (type != SymbolType.FUNCTION) {
+			return false;
+		}
+
+		Function refFunction = (Function) s.getObject();
+		Function thunked = refFunction.getThunkedFunction(true);
+		if (thunked == null) {
+			return false;
+		}
+
+		if (thunked.getBody().contains(codeUnit.getAddress())) {
+			// this handles the unlikely case where the user double-clicks a reference to a 
+			// local thunk label--don't navigate externally
+			return false;
+		}
+
+		Symbol thunkedSymbol = thunked.getSymbol();
+		ExternalLocation extLoc = program.getExternalManager().getExternalLocation(thunkedSymbol);
+		boolean success = goToService.goToExternalLocation(extLoc, true);
+		return success;
+	}
+
 	private boolean checkVariableReference(Navigatable navigatable, CodeUnit codeUnit,
 			OperandFieldLocation loc, GoToService goToService) {
 
@@ -150,40 +179,18 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 			return false;
 		}
 
-		VariableOffset variableOffset = loc.getVariableOffset();
-		if (variableOffset != null) {
-			Variable variable = variableOffset.getVariable();
-			if (variable != null) {
-				goToService.goTo(navigatable,
-					new VariableNameFieldLocation(navigatable.getProgram(), variable, 0),
-					navigatable.getProgram());
-				return true;
-			}
+		if (goToExplicitOperandVariable(navigatable, loc, goToService)) {
+			return true;
 		}
 
 		Program p = codeUnit.getProgram();
 		Address cuAddr = codeUnit.getMinAddress();
-		Address refAddr = loc.getRefAddress();
-		Function func = p.getFunctionManager().getFunctionContaining(cuAddr);
-		if (func == null) {
+		Function function = p.getFunctionManager().getFunctionContaining(cuAddr);
+		if (function == null) {
 			return false;
 		}
 
-		ProgramLocation varLoc = null;
-		if (refAddr != null && !refAddr.isStackAddress()) {
-			Register reg = p.getRegister(refAddr, 1);
-			if (reg != null) {
-				Variable variable = p.getFunctionManager().getReferencedVariable(cuAddr, refAddr,
-					reg.getMinimumByteSize(),
-					!isWrite((Instruction) codeUnit, loc.getOperandIndex(), reg));
-				if (variable != null) {
-					varLoc = new VariableNameFieldLocation(navigatable.getProgram(), variable, 0);
-				}
-			}
-		}
-
-		if (varLoc != null) {
-			goToService.goTo(navigatable, varLoc, navigatable.getProgram());
+		if (goToRegisterVariable(navigatable, codeUnit, loc, goToService)) {
 			return true;
 		}
 
@@ -195,20 +202,71 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 		Variable variable = p.getFunctionManager().getReferencedVariable(cuAddr,
 			reference.getToAddress(), 0, reference.getReferenceType().isRead());
 		if (variable != null) {
-			varLoc = new VariableNameFieldLocation(navigatable.getProgram(), variable, 0);
-			goToService.goTo(navigatable, varLoc, navigatable.getProgram());
+			ProgramLocation pl =
+				new VariableNameFieldLocation(navigatable.getProgram(), variable, 0);
+			goToService.goTo(navigatable, pl, navigatable.getProgram());
 			return true;
 		}
 
 		if (reference.isStackReference()) {
-			ProgramLocation fnLoc = new FunctionSignatureFieldLocation(p, func.getEntryPoint(),
-				null, 0, func.getPrototypeString(false, false));
-			goToService.goTo(navigatable, fnLoc, navigatable.getProgram());
+			ProgramLocation pl = new FunctionSignatureFieldLocation(p, function.getEntryPoint(),
+				null, 0, function.getPrototypeString(false, false));
+			goToService.goTo(navigatable, pl, navigatable.getProgram());
 			return true;
 		}
 		return false;
 	}
 
+	private boolean goToRegisterVariable(Navigatable navigatable, CodeUnit codeUnit,
+			OperandFieldLocation loc, GoToService goToService) {
+
+		Address refAddr = loc.getRefAddress();
+		Address cuAddr = codeUnit.getMinAddress();
+		if (refAddr == null || refAddr.isStackAddress()) {
+			return false;
+		}
+
+		Program p = codeUnit.getProgram();
+		Register reg = p.getRegister(refAddr, 1);
+		if (reg == null) {
+			return false;
+		}
+
+		Variable variable = p.getFunctionManager().getReferencedVariable(cuAddr, refAddr,
+			reg.getMinimumByteSize(), !isWrite((Instruction) codeUnit, loc.getOperandIndex(), reg));
+		if (variable == null) {
+			return false;
+		}
+
+		ProgramLocation pl = new VariableNameFieldLocation(navigatable.getProgram(), variable, 0);
+		goToService.goTo(navigatable, pl, navigatable.getProgram());
+		return true;
+	}
+
+	/**
+	 * Navigate to the variable, when directly supplied by the field location
+	 * 
+	 * @param navigatable the navigatable to which we should navigate
+	 * @param loc the source location
+	 * @param goToService the GoTo service
+	 * @return true if we decide to attempt navigation
+	 */
+	private boolean goToExplicitOperandVariable(Navigatable navigatable, OperandFieldLocation loc,
+			GoToService goToService) {
+
+		VariableOffset variableOffset = loc.getVariableOffset();
+		if (variableOffset != null) {
+			Variable variable = variableOffset.getVariable();
+			if (variable != null) {
+				goToService.goTo(navigatable,
+					new VariableNameFieldLocation(navigatable.getProgram(), variable, 0),
+					navigatable.getProgram());
+				return true;
+			}
+		}
+		return false;
+	}
+
 	private boolean isWrite(Instruction inst, int operandIndex, Register reg) {
 		for (Object obj : inst.getResultObjects()) {
 			if (obj == reg) {
@@ -220,12 +278,12 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 
 	private boolean checkMemRefs(Navigatable navigatable, CodeUnit codeUnit,
 			OperandFieldLocation loc, ServiceProvider serviceProvider) {
+
 		Address refAddr = loc.getRefAddress();
 		if (refAddr == null || !refAddr.isMemoryAddress()) {
 			return false;
 		}
 
-		int opIndex = loc.getOperandIndex();
 		Reference[] refs = codeUnit.getOperandReferences(loc.getOperandIndex());
 		Address[] addrs = getAddressesForReferences(refs, codeUnit, serviceProvider);
 		if (addrs.length == 0) {
@@ -251,26 +309,8 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 			return true;
 		}
 
-		Address gotoAddr = null;
-		if (addrs.length == 1) {
-			gotoAddr = addrs[0];
-		}
-		else {
-			gotoAddr = codeUnit.getAddress(opIndex);
-			if (gotoAddr == null) {
-				Scalar scalar = codeUnit.getScalar(opIndex);
-				if (scalar != null) {
-					Address minAddress = codeUnit.getMinAddress();
-					try {
-						gotoAddr = minAddress.getNewAddress(scalar.getUnsignedValue(), true);
-					}
-					catch (Exception e) {
-						// handled below by checking for null
-					}
-				}
-			}
-		}
-
+		// 1 address found
+		Address gotoAddr = addrs[0];
 		if (gotoAddr == null) {
 			return false;
 		}
@@ -294,27 +334,28 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 			ServiceProvider serviceProvider, boolean skipExternal) {
 		Address address = reference.getToAddress();
 		RefType refType = reference.getReferenceType();
-		if (refType.isIndirect()) {
-			Program program = codeUnit.getProgram();
-			Data data = program.getListing().getDefinedDataAt(address);
-			Address indirectAddrress = null;
-			if (data != null) {
-				if (data.isPointer()) {
-					Reference ref = data.getPrimaryReference(0);
-					indirectAddrress = ref != null ? ref.getToAddress() : (Address) data.getValue();
-				}
-			}
-			else {
-				PseudoDisassembler pdis = new PseudoDisassembler(program);
-				indirectAddrress = pdis.getIndirectAddr(address);
-			}
-			if (indirectAddrress != null &&
-				(!indirectAddrress.isExternalAddress() || !skipExternal) &&
-				followIndirectReference(serviceProvider, indirectAddrress, program)) {
-				return indirectAddrress;
-			}
+		if (!refType.isIndirect()) {
+			return address;
 		}
 
+		Program program = codeUnit.getProgram();
+		Data data = program.getListing().getDefinedDataAt(address);
+		Address indirectAddrress = null;
+		if (data != null) {
+			if (data.isPointer()) {
+				Reference ref = data.getPrimaryReference(0);
+				indirectAddrress = ref != null ? ref.getToAddress() : (Address) data.getValue();
+			}
+		}
+		else {
+			PseudoDisassembler pdis = new PseudoDisassembler(program);
+			indirectAddrress = pdis.getIndirectAddr(address);
+		}
+
+		if (indirectAddrress != null && (!indirectAddrress.isExternalAddress() || !skipExternal) &&
+			followIndirectReference(serviceProvider, indirectAddrress, program)) {
+			return indirectAddrress;
+		}
 		return address;
 	}
 
@@ -324,6 +365,7 @@ public class OperandFieldMouseHandler implements FieldMouseHandlerExtension {
 		if (optionsService == null) {
 			return false;
 		}
+
 		NavigationOptions navOptions = new NavigationOptions(optionsService);
 		try {
 			if (!navOptions.isFollowIndirectionEnabled()) {

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/listingpanel/DualListingGoToService.java

@@ -73,7 +73,7 @@ class DualListingGoToService implements GoToService {
 
 	@Override
 	public boolean goTo(Navigatable navigatable, ProgramLocation loc, Program program) {
-			return dualGoTo(loc);
+		return dualGoTo(loc);
 	}
 
 	@Override
@@ -95,8 +95,8 @@ class DualListingGoToService implements GoToService {
 		AddressSetView addresses =
 			isLeftSide ? dualListing.getLeftAddresses() : dualListing.getRightAddresses();
 		if (!addresses.contains(addr)) {
-			dualListing.setStatusInfo("\"" + addr.toString() +
-				"\" is outside the current listing's view.");
+			dualListing.setStatusInfo(
+				"\"" + addr.toString() + "\" is outside the current listing's view.");
 			return false;
 		}
 		return true;
@@ -150,6 +150,13 @@ class DualListingGoToService implements GoToService {
 			"Connot Go To an external address from a dual listing view.");
 	}
 
+	@Override
+	public boolean goToExternalLocation(Navigatable navigatable, ExternalLocation extLoc,
+			boolean checkNavigationOption) {
+		throw new UnsupportedOperationException(
+			"Connot Go To an external address from a dual listing view.");
+	}
+
 	@Override
 	public boolean goToQuery(Address fromAddr, QueryData queryData, GoToServiceListener listener,
 			TaskMonitor monitor) {

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/listingpanel/ListingPanel.java

@@ -519,10 +519,8 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Moves the cursor to the given program location and repositions the scrollbar to
-	 * show that location in the screen
-	 * 
-	 * @param loc the location to move to
-	 * @return true if the 'go to' was successful
+	 * show that location in the screen.
+	 * @param loc the location to move to.
 	 */
 	public boolean goTo(ProgramLocation loc) {
 		return goTo(loc, true);
@@ -538,7 +536,6 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	 *                             the given location will be placed in the center of the screen;
 	 *                             when the parameter is false, then the screen will be scrolled
 	 *                             only enough to show the cursor.
-	 * @return true if the 'go to' was successful
 	 */
 	public boolean goTo(ProgramLocation loc, boolean centerWhenNotVisible) {
 		final FieldLocation floc = getFieldLocation(loc);
@@ -558,27 +555,15 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 		return true;
 	}
 
-	/** 
-	 * Scroll the view of the listing to the given location 
-	 * @param location the location 
-	 */
+	/** Scroll the view of the listing to the given location. */
 	public void scrollTo(ProgramLocation location) {
 		FieldLocation fieldLocation = getFieldLocation(location);
-		if (fieldLocation == null) {
-			return; // this can happen when using restricted views
-		}
 		fieldPanel.scrollTo(fieldLocation);
 	}
 
-	/** 
-	 * Center the view of the listing around the given location 
-	 * @param location the location 
-	 */
+	/** Center the view of the listing around the given location. */
 	public void center(ProgramLocation location) {
 		FieldLocation fieldLocation = getFieldLocation(location);
-		if (fieldLocation == null) {
-			return; // this can happen when using restricted views
-		}
 		fieldPanel.center(fieldLocation);
 	}
 

Ghidra/Features/Base/src/main/java/ghidra/program/model/listing/CodeUnitFormat.java

@@ -17,8 +17,6 @@ package ghidra.program.model.listing;
 
 import java.util.*;
 
-import org.apache.commons.lang3.StringUtils;
-
 import ghidra.app.util.NamespaceUtils;
 import ghidra.app.util.viewer.field.CommentUtils;
 import ghidra.program.model.address.*;
@@ -38,8 +36,6 @@ import ghidra.util.MathUtilities;
 
 public class CodeUnitFormat {
 
-	private static final String EMPTY = "";
-
 	protected static final String PLUS = "+";
 	protected static final String UNDERSCORE = "_";
 
@@ -116,39 +112,37 @@ public class CodeUnitFormat {
 	 */
 	public String getRepresentationString(CodeUnit cu, boolean includeEOLcomment) {
 
-		StringBuilder buffy = new StringBuilder(getMnemonicRepresentation(cu));
+		StringBuffer stringBuffer = new StringBuffer(getMnemonicRepresentation(cu));
 		if (cu instanceof Instruction) {
 			Instruction instr = (Instruction) cu;
 			int n = instr.getNumOperands();
-			if (n > 1) {
-				buffy.append(' ');
-			}
-
 			for (int i = 0; i < n; i++) {
-				String sep = instr.getSeparator(i);
-				buffy.append(StringUtils.isBlank(sep) ? EMPTY : sep);
-				buffy.append(getOperandRepresentationString(cu, i));
+				if (i == 0) {
+					stringBuffer.append(" ");
+				}
+				else {
+					String separator = instr.getSeparator(i);
+					if (separator != null && separator.length() != 0) {
+						stringBuffer.append(separator);
+					}
+				}
+				stringBuffer.append(getOperandRepresentationString(cu, i));
 			}
-
-			// grab any trailing separator at n (index + 1; see Instruction.getSeparator())
-			String sep = instr.getSeparator(n);
-			buffy.append(StringUtils.isBlank(sep) ? EMPTY : sep);
 		}
 		else { // data always has one operand
-			buffy.append(' ');
-			buffy.append(getOperandRepresentationString(cu, 0));
+			stringBuffer.append(" ");
+			stringBuffer.append(getOperandRepresentationString(cu, 0));
 		}
-
 		if (includeEOLcomment) {
 			String eolComment = cu.getComment(CodeUnit.EOL_COMMENT);
 			if (eolComment != null) {
 				// fixup annotations
 				eolComment = CommentUtils.getDisplayString(eolComment, cu.getProgram());
-				buffy.append("  // ");
-				buffy.append(eolComment);
+				stringBuffer.append("  // ");
+				stringBuffer.append(eolComment);
 			}
 		}
-		return buffy.toString();
+		return stringBuffer.toString();
 	}
 
 	/**
@@ -158,19 +152,19 @@ public class CodeUnitFormat {
 	 * @return mnemonic representation
 	 */
 	public String getMnemonicRepresentation(CodeUnit cu) {
-		StringBuilder buffy = new StringBuilder();
+		StringBuffer stringBuffer = new StringBuffer();
 		String mnemonic = cu.getMnemonicString();
 		if (options.showDataMutability && (cu instanceof Data) && mnemonic != null) {
 			Data d = (Data) cu;
 			if (d.isConstant()) {
-				buffy.append("const ");
+				stringBuffer.append("const ");
 			}
 			else if (d.isVolatile()) {
-				buffy.append("volatile ");
+				stringBuffer.append("volatile ");
 			}
 		}
-		buffy.append(mnemonic);
-		return buffy.toString();
+		stringBuffer.append(mnemonic);
+		return stringBuffer.toString();
 	}
 
 	/**
@@ -279,7 +273,7 @@ public class CodeUnitFormat {
 	 * Perform register markup with explicit and implied register variable
 	 * reference.
 	 * 
-	 * @param instr instruction
+	 * @param inst instruction
 	 * @param opIndex
 	 * @param func function containing instruction
 	 * @param primaryRef primary reference or null
@@ -452,17 +446,18 @@ public class CodeUnitFormat {
 	 * @param opIndex operand index
 	 * @param func function containing instruction
 	 * @param primaryRef primary reference
-	 * @param representationList the representation objects
+	 * @param referencedVariable optional variable associated with reference
+	 * @param regIndexMap register index map
+	 * @param representationList
 	 * @return true if primaryRef was included in scalar mark-up
 	 */
 	private boolean performAddressMarkup(Instruction instr, int opIndex, Function func,
 			Reference primaryRef, List<Object> representationList) {
-
 		if (primaryRef == null || !primaryRef.isMemoryReference()) {
 			return false;
 		}
-
 		Address refAddr = primaryRef.getToAddress();
+
 		int size = representationList.size();
 		for (int i = 0; i < size; i++) {
 			Object obj = representationList.get(i);
@@ -1138,9 +1133,13 @@ public class CodeUnitFormat {
 	 * Get a representation object corresponding to the specified reference.
 	 * Format options are considered when generating label.
 	 * 
-	 * @param cu the code unit
-	 * @param ref the reference
+	 * @param cu
+	 * @param ref
 	 * @param var variable which corresponds to reference or null
+	 * @param showIndirectValue if true, indirect memory references which refer
+	 *            to a pointer will get an additional "=value" appended where
+	 *            value corresponds to data pointed to by the referenced
+	 *            pointer.
 	 * @return reference representation object
 	 */
 	private Object getReferenceRepresentation(CodeUnit cu, Reference ref, Variable var) {
@@ -1418,6 +1417,9 @@ public class CodeUnitFormat {
 		return symbol.getName();
 	}
 
+	/**
+	 * Returns ShowBlockName setting
+	 */
 	public ShowBlockName getShowBlockName() {
 		return options.showBlockName;
 	}

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/codebrowser/AbstractCodeBrowserNavigationTest.java

@@ -45,6 +45,7 @@ public class AbstractCodeBrowserNavigationTest extends AbstractGhidraHeadedInteg
 	private DockingActionIf nextFunction;
 	private DockingActionIf prevFunction;
 
+	protected ToyProgramBuilder builder;
 	protected Program program;
 	protected CodeBrowserPlugin cb;
 
@@ -83,7 +84,7 @@ public class AbstractCodeBrowserNavigationTest extends AbstractGhidraHeadedInteg
 	}
 
 	private Program buildProgram() throws Exception {
-		ToyProgramBuilder builder = new ToyProgramBuilder("Test", true, this);
+		builder = new ToyProgramBuilder("Test", true, this);
 		builder.createMemory(".text", "0x1001000", 0x6600);
 		builder.createMemory(".data", "0x1008000", 0x600);
 		builder.createMemory(".rsrc", "0x100a000", 0x5400);
@@ -98,7 +99,6 @@ public class AbstractCodeBrowserNavigationTest extends AbstractGhidraHeadedInteg
 
 		builder.applyDataType("0x1001000", PointerDataType.dataType, 1);
 		builder.createExternalReference("0x1001000", "ADVAPI32.dll", "IsTextUnicode", 0); // linkage location
-
 		builder.createExternalReference("0x1001020", "ADVAPI32.dll", "IsTextUnicode", 0); // without pointer
 
 		builder.addBytesBranch("1004000", "1004010");

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/codebrowser/ExternalCodeBrowserNavigationTest.java

@@ -63,8 +63,22 @@ public class ExternalCodeBrowserNavigationTest extends AbstractCodeBrowserNaviga
 
 		int txId = program.startTransaction("Set Path");
 		program.getExternalManager().setExternalPath("ADVAPI32.dll", "/FILE1", true);
-		program.endTransaction(txId, true);
 
+		//
+		// Create a call reference to a thunk function
+		// Create a thunk function to an external location
+		//
+		String arbitraryAddress = "0x1001000";
+		ExternalLocation externalLocation = builder.createExternalFunction(arbitraryAddress,
+			"ADVAPI32.dll", "externalFunctionXyz", "_Zxyz");
+		String thunkAddress = "0x1006300";
+		Function thunk = builder.createFunction(thunkAddress);
+		thunk.setThunkedFunction(externalLocation.getFunction());
+
+		builder.createMemoryCallReference("0x1001030", thunkAddress);
+
+		program.endTransaction(txId, true);
+		program.flushEvents();
 		waitForSwing();
 	}
 
@@ -180,17 +194,35 @@ public class ExternalCodeBrowserNavigationTest extends AbstractCodeBrowserNaviga
 
 		cb.goTo(new OperandFieldLocation(program, addr("1001020"), null, null, null, 0, 0));
 		assertEquals(addr("1001020"), cb.getCurrentAddress());
-
 		assertEquals("FILE2", program.getDomainFile().getName());
 
 		// verify that navigation to the external program, address 0x1001888, is performed
 		// since navigation initiated from linkage location
 		click(cb, 2);
 		assertEquals(addr("1001888"), cb.getCurrentAddress());
-
 		assertEquals("FILE1", lastNavigationProgram.getDomainFile().getName());
 		assertEquals(addr("1001888"), lastNavigationLocation.getAddress());
+	}
 
+	@Test
+	public void testOperandExternalProgramNavigation_OnThunk() throws Exception {
+
+		getTool().getOptions("Navigation").setEnum("External Navigation",
+			NavigationOptions.ExternalNavigationEnum.NavigateToExternalProgram);
+
+		String fromAddress = "1001030";
+		cb.goTo(new OperandFieldLocation(program, addr(fromAddress), null, null, null, 0, 0));
+		assertEquals(addr(fromAddress), cb.getCurrentAddress());
+		assertEquals("FILE2", program.getDomainFile().getName());
+
+		// verify that navigation to the external program, address 0x1001888, is performed
+		// since navigation initiated from linkage location
+		click(cb, 2);
+
+		String otherAddress = "0x01001000";
+		assertEquals(addr(otherAddress), cb.getCurrentAddress());
+		assertEquals("FILE1", lastNavigationProgram.getDomainFile().getName());
+		assertEquals(addr(otherAddress), lastNavigationLocation.getAddress());
 	}
 
 	/**
@@ -270,6 +302,7 @@ public class ExternalCodeBrowserNavigationTest extends AbstractCodeBrowserNaviga
 		@Mock
 		public boolean goTo(Invocation inv, final Navigatable navigatable, ProgramLocation loc,
 				Program p) {
+
 			// Track last navigation location
 			lastNavigationLocation = loc;
 			lastNavigationProgram = p;

Ghidra/Features/Base/src/test/java/ghidra/app/services/TestDummyGoToService.java

@@ -84,6 +84,13 @@ public class TestDummyGoToService implements GoToService {
 		return false;
 	}
 
+	@Override
+	public boolean goToExternalLocation(Navigatable navigatable, ExternalLocation externalLoc,
+			boolean checkNavigationOption) {
+		// stub
+		return false;
+	}
+
 	@Override
 	public boolean goToQuery(Address fromAddr, QueryData queryData, GoToServiceListener listener,
 			TaskMonitor monitor) {