GP-4133 improved support for __return_storage_ptr__ param which

transitions return storage to VOID.

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/ApplyFunctionSignatureCmd.java

@@ -213,14 +213,14 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 				returnDt = prepareDataType(returnDt, targetDtm, dtCleaner);
 			}
 
-			List<Parameter> params = createParameters(compilerSpec, conventionName, args);
+			ReturnParameterImpl returnParam = new ReturnParameterImpl(returnDt, program);
+			List<Parameter> params =
+				createParameters(compilerSpec, conventionName, args, returnParam);
 
 			SymbolTable symbolTable = program.getSymbolTable();
 
 			adjustParameterNamesToAvoidConflicts(symbolTable, func, params);
 
-			ReturnParameterImpl returnParam = new ReturnParameterImpl(returnDt, program);
-
 			func.updateFunction(conventionName, returnParam, params,
 				FunctionUpdateType.DYNAMIC_STORAGE_FORMAL_PARAMS, false, source);
 			func.setVarArgs(signature.hasVarArgs());
@@ -247,7 +247,9 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 	}
 
 	private List<Parameter> createParameters(CompilerSpec compilerSpec, String conventionName,
-			ParameterDefinition[] args) throws InvalidInputException {
+			ParameterDefinition[] args, Parameter returnParam) throws InvalidInputException {
+
+		DataType returnDt = returnParam.getDataType();
 
 		int firstParamIndex = getIndexOfFirstParameter(conventionName, args);
 
@@ -256,11 +258,15 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 		DataTypeManager dtm = program.getDataTypeManager();
 		for (int i = firstParamIndex; i < args.length; i++) {
 			String name = args[i].getName();
+			DataType type = args[i].getDataType().clone(dtm);
 			if (Function.RETURN_PTR_PARAM_NAME.equals(name)) {
-				continue; // discard what should be an auto-param
+				if ((type instanceof Pointer) &&
+					(type.isEquivalent(returnDt) || VoidDataType.dataType.isEquivalent(returnDt))) {
+					returnParam.setDataType(((Pointer) type).getDataType(), source);
+				}
+				continue; // remove what should be an auto-param
 			}
 
-			DataType type = args[i].getDataType().clone(dtm);
 			if (settleCTypes) {
 				type = settleCDataType(type, dtm);
 			}
@@ -269,6 +275,7 @@ public class ApplyFunctionSignatureCmd extends BackgroundCommand {
 			param.setComment(args[i].getComment());
 			params.add(param);
 		}
+
 		return params;
 	}
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/ParameterDB.java

@@ -54,10 +54,10 @@ class ParameterDB extends VariableDB implements Parameter {
 	@Override
 	public DataType getDataType() {
 		DataType dt = getFormalDataType();
-		VariableStorage varStorage = getVariableStorage();
+		if (isForcedIndirect()) {
-		if (varStorage.isForcedIndirect()) {
 			Program program = function.getProgram();
 			DataTypeManager dtm = program.getDataTypeManager();
+			VariableStorage varStorage = getVariableStorage();
 			int ptrSize = varStorage.size();
 			if (ptrSize != dtm.getDataOrganization().getPointerSize()) {
 				dt = dtm.getPointer(dt, ptrSize);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/ReturnParameterDB.java

@@ -18,6 +18,7 @@ package ghidra.program.database.function;
 import java.io.IOException;
 
 import ghidra.program.model.data.*;
+import ghidra.program.model.lang.DynamicVariableStorage;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.symbol.SourceType;
 import ghidra.program.util.ChangeManager;
@@ -45,8 +46,8 @@ public class ReturnParameterDB extends ParameterDB {
 	}
 
 	@Override
-	public void setName(String name, SourceType source) throws DuplicateNameException,
+	public void setName(String name, SourceType source)
-			InvalidInputException {
+			throws DuplicateNameException, InvalidInputException {
 		throw new UnsupportedOperationException();
 	}
 
@@ -81,8 +82,7 @@ public class ReturnParameterDB extends ParameterDB {
 				newStorage = VariableStorage.UNASSIGNED_STORAGE;
 			}
 			Program program = function.getProgram();
-			type =
+			type = VariableUtilities.checkDataType(type,
-				VariableUtilities.checkDataType(type,
 				newStorage.isVoidStorage() || newStorage.isUnassignedStorage(), getLength(),
 				program);
 			if (!newStorage.isUnassignedStorage()) {
@@ -163,6 +163,19 @@ public class ReturnParameterDB extends ParameterDB {
 		return dataType;
 	}
 
+	@Override
+	public DataType getDataType() {
+		if (storage == DynamicVariableStorage.INDIRECT_VOID_STORAGE) {
+			return VoidDataType.dataType;
+		}
+		return super.getDataType();
+	}
+
+	@Override
+	public boolean isForcedIndirect() {
+		return storage.isForcedIndirect();
+	}
+
 	@Override
 	public SourceType getSource() {
 		if (dataType == null || Undefined.isUndefined(dataType)) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/DynamicVariableStorage.java

@@ -23,9 +23,26 @@ import ghidra.util.exception.InvalidInputException;
 
 public class DynamicVariableStorage extends VariableStorage {
 
+	/**
+	 * <code>INDIRECT_VOID_STORAGE</code> used to identify return storage which is "mapped"
+	 * with a data-type of void but was forced indirect with the corresponding use of a
+	 * hidden return-storage-parameter.
+	 */
+	public static final DynamicVariableStorage INDIRECT_VOID_STORAGE = new DynamicVariableStorage();
+
 	private AutoParameterType autoParamType;
 	private boolean forcedIndirect;
 	private boolean isUnassigned = false;
+	private boolean isVoid = false;
+
+	/**
+	 * Construct void return storage with forced-indirect flag.
+	 */
+	private DynamicVariableStorage() {
+		super();
+		forcedIndirect = true;
+		isVoid = true;
+	}
 
 	/**
 	 * Construct Unassigned dynamic variable storage with an optional auto-parameter type
@@ -58,8 +75,7 @@ public class DynamicVariableStorage extends VariableStorage {
 	 * @throws InvalidInputException
 	 */
 	public DynamicVariableStorage(ProgramArchitecture program, AutoParameterType autoParamType,
-			Address address,
+			Address address, int size) throws InvalidInputException {
-			int size) throws InvalidInputException {
 		super(program, address, size);
 		this.autoParamType = autoParamType;
 	}
@@ -72,8 +88,7 @@ public class DynamicVariableStorage extends VariableStorage {
 	 * @throws InvalidInputException if specified varnodes violate storage restrictions
 	 */
 	public DynamicVariableStorage(ProgramArchitecture program, AutoParameterType autoParamType,
-			Varnode... varnodes)
+			Varnode... varnodes) throws InvalidInputException {
-			throws InvalidInputException {
 		super(program, varnodes);
 		this.autoParamType = autoParamType;
 	}
@@ -88,8 +103,7 @@ public class DynamicVariableStorage extends VariableStorage {
 	 * @throws InvalidInputException
 	 */
 	public DynamicVariableStorage(ProgramArchitecture program, boolean forcedIndirect,
-			Address address, int size)
+			Address address, int size) throws InvalidInputException {
-			throws InvalidInputException {
 		super(program, address, size);
 		this.forcedIndirect = forcedIndirect;
 	}
@@ -103,8 +117,7 @@ public class DynamicVariableStorage extends VariableStorage {
 	 * @throws InvalidInputException if specified varnodes violate storage restrictions
 	 */
 	public DynamicVariableStorage(ProgramArchitecture program, boolean forcedIndirect,
-			Varnode... varnodes)
+			Varnode... varnodes) throws InvalidInputException {
-			throws InvalidInputException {
 		super(program, varnodes);
 		this.forcedIndirect = forcedIndirect;
 	}
@@ -124,6 +137,11 @@ public class DynamicVariableStorage extends VariableStorage {
 		return isUnassigned;
 	}
 
+	@Override
+	public boolean isVoidStorage() {
+		return isVoid;
+	}
+
 	@Override
 	public AutoParameterType getAutoParameterType() {
 		return autoParamType;
@@ -132,7 +150,7 @@ public class DynamicVariableStorage extends VariableStorage {
 	@Override
 	public String toString() {
 		String str = super.toString();
-		if (forcedIndirect) {
+		if (forcedIndirect && varnodes != null) {
 			str = str + " (ptr)";
 		}
 		if (autoParamType != null) {
@@ -146,8 +164,10 @@ public class DynamicVariableStorage extends VariableStorage {
 	 * NOTE: The {@link #isUnassignedStorage()} method should be used to
 	 * detect this type of storage.
 	 * @param autoParamType auto-parameter type or null if not applicable
+	 * @return Unassigned dynamic variable storage
 	 */
-	public static DynamicVariableStorage getUnassignedDynamicStorage(AutoParameterType autoParamType) {
+	public static DynamicVariableStorage getUnassignedDynamicStorage(
+			AutoParameterType autoParamType) {
 		return new DynamicVariableStorage(autoParamType);
 	}
 
@@ -157,6 +177,7 @@ public class DynamicVariableStorage extends VariableStorage {
 	 * detect this type of storage.
 	 * @param forcedIndirect if true indicates that the parameter has been forced to pass 
 	 * as a pointer instead of its raw type
+	 * @return Unassigned dynamic variable storage
 	 */
 	public static DynamicVariableStorage getUnassignedDynamicStorage(boolean forcedIndirect) {
 		return new DynamicVariableStorage(forcedIndirect);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/ParamListStandardOut.java

@@ -60,8 +60,8 @@ public class ParamListStandardOut extends ParamListStandard {
 			else {
 				assignAddressFallback(StorageClass.PTR, pointerType, false, status, store);
 				store.type = pointerType;
-				store.isIndirect = true;	// Signal that there is a hidden return
 			}
+			store.isIndirect = true;	// Signal that there is a hidden return
 			if (addAutoParams) {
 				ParameterPieces hiddenRet = new ParameterPieces();
 				hiddenRet.type = pointerType;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/ParameterPieces.java

@@ -66,6 +66,9 @@ public class ParameterPieces {
 			type = DataType.DEFAULT;
 		}
 		if (VoidDataType.isVoidDataType(type)) {
+			if (isIndirect) {
+				return DynamicVariableStorage.INDIRECT_VOID_STORAGE;
+			}
 			return VariableStorage.VOID_STORAGE;
 		}
 		int sz = type.getLength();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Function.java

@@ -36,9 +36,8 @@ import ghidra.util.task.TaskMonitor;
 public interface Function extends Namespace {
 
 	public static final String DEFAULT_PARAM_PREFIX = "param_";
-	public static final String THIS_PARAM_NAME = AutoParameterType.THIS.getDisplayName();
+	public static final String THIS_PARAM_NAME = "this";
-	public static final String RETURN_PTR_PARAM_NAME =
+	public static final String RETURN_PTR_PARAM_NAME = "__return_storage_ptr__";
-		AutoParameterType.RETURN_STORAGE_PTR.getDisplayName();
 	public static final int DEFAULT_PARAM_PREFIX_LEN = DEFAULT_PARAM_PREFIX.length();
 	public static final String DEFAULT_LOCAL_PREFIX = "local_";
 	public static final String DEFAULT_LOCAL_RESERVED_PREFIX = "local_res";
@@ -681,6 +680,7 @@ public interface Function extends Namespace {
 	 * a recursive search is generally needed (see {@link #getFunctionThunkAddresses(boolean)}).
 	 * This method form may be removed in a future release.
 	 */
+	@Deprecated
 	public default Address[] getFunctionThunkAddresses() {
 		return getFunctionThunkAddresses(false);
 	}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableStorage.java

@@ -294,8 +294,7 @@ public class VariableStorage implements Comparable<VariableStorage> {
 					"Variable storage incompatible with program, address space not found: " +
 						curSpace.getName());
 			}
-			newVarnodes[i] =
+			newVarnodes[i] = new Varnode(newSpace.getAddress(v[i].getOffset()), v[i].getSize());
-				new Varnode(newSpace.getAddress(v[i].getOffset()), v[i].getSize());
 		}
 		return new VariableStorage(newProgramArch, newVarnodes);
 	}
@@ -625,9 +624,9 @@ public class VariableStorage implements Comparable<VariableStorage> {
 		if (varnodes == null || otherVarnodes == null) {
 			return false;
 		}
-		for (int i = 0; i < varnodes.length; i++) {
+		for (Varnode varnode : varnodes) {
-			for (int j = 0; j < otherVarnodes.length; j++) {
+			for (Varnode otherVarnode : otherVarnodes) {
-				if (varnodes[i].intersects(otherVarnodes[j])) {
+				if (varnode.intersects(otherVarnode)) {
 					return true;
 				}
 			}
@@ -644,8 +643,8 @@ public class VariableStorage implements Comparable<VariableStorage> {
 		if (varnodes == null || set == null || set.isEmpty()) {
 			return false;
 		}
-		for (int i = 0; i < varnodes.length; i++) {
+		for (Varnode varnode : varnodes) {
-			if (varnodes[i].intersects(set)) {
+			if (varnode.intersects(set)) {
 				return true;
 			}
 		}
@@ -662,8 +661,8 @@ public class VariableStorage implements Comparable<VariableStorage> {
 			return false;
 		}
 		Varnode regVarnode = new Varnode(reg.getAddress(), reg.getMinimumByteSize());
-		for (int i = 0; i < varnodes.length; i++) {
+		for (Varnode varnode : varnodes) {
-			if (varnodes[i].intersects(regVarnode)) {
+			if (varnode.intersects(regVarnode)) {
 				return true;
 			}
 		}
@@ -679,8 +678,8 @@ public class VariableStorage implements Comparable<VariableStorage> {
 		if (varnodes == null) {
 			return false;
 		}
-		for (int i = 0; i < varnodes.length; i++) {
+		for (Varnode varnode : varnodes) {
-			if (varnodes[i].contains(address)) {
+			if (varnode.contains(address)) {
 				return true;
 			}
 		}
@@ -816,8 +815,8 @@ public class VariableStorage implements Comparable<VariableStorage> {
 			list = null;
 		}
 		if (list == null) {
-			throw new InvalidInputException("Invalid varnode serialization: '" + serialization +
+			throw new InvalidInputException(
-				"'");
+				"Invalid varnode serialization: '" + serialization + "'");
 		}
 		return list;
 	}
@@ -869,9 +868,10 @@ public class VariableStorage implements Comparable<VariableStorage> {
 					if (space.isRegisterSpace()) {
 						long offset = Long.parseUnsignedLong(offsetStr, 16);
 						int size = Integer.parseInt(sizeStr);
-						Address oldRegAddr =
+						Address oldRegAddr = translator.getOldLanguage()
-							translator.getOldLanguage().getAddressFactory().getRegisterSpace().getAddress(
+								.getAddressFactory()
-								offset);
+								.getRegisterSpace()
+								.getAddress(offset);
 						String newOffsetStr =
 							translateRegisterVarnodeOffset(oldRegAddr, size, translator);
 						if (newOffsetStr != null) {
@@ -895,8 +895,8 @@ public class VariableStorage implements Comparable<VariableStorage> {
 		}
 
 		if (strBuilder == null) {
-			throw new InvalidInputException("Invalid varnode serialization: '" + serialization +
+			throw new InvalidInputException(
-				"'");
+				"Invalid varnode serialization: '" + serialization + "'");
 		}
 
 		return strBuilder.toString();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableUtilities.java

@@ -163,7 +163,7 @@ public class VariableUtilities {
 	/**
 	 * Determine the appropriate data type for an automatic parameter
 	 * @param function function whose auto param datatype is to be determined
-	 * @param returnDataType function's return datatype
+	 * @param returnDataType function's formal return datatype
 	 * @param storage variable storage for an auto-parameter (isAutoStorage should be true)
 	 * @return auto-parameter data type
 	 */