GP-3235 improved auto-assignment of VOID storage to void return type for

function

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java

@@ -47,6 +47,7 @@ import ghidra.app.util.cparser.C.CParserUtils;
 import ghidra.app.util.viewer.field.ListingColors.FunctionColors;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.DataType;
+import ghidra.program.model.data.VoidDataType;
 import ghidra.program.model.listing.Function;
 import ghidra.program.model.listing.VariableStorage;
 import ghidra.program.model.symbol.ExternalLocation;
@@ -747,8 +748,15 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
 						setStatusText("Return name may not be modified");
 					}
 					else if ("Storage".equals(getColumnName(column))) {
-						setStatusText(
+						boolean blockVoidStorageEdit = (rowData.getIndex() == null) &&
-							"Enable 'Use Custom Storage' to allow editing of Parameter and Return Storage");
+							VoidDataType.isVoidDataType(rowData.getFormalDataType());
+						if (!blockVoidStorageEdit) {
+							setStatusText(
+								"Enable 'Use Custom Storage' to allow editing of Parameter and Return Storage");
+						}
+						else {
+							setStatusText("Void return storage may not be modified");
+						}
 					}
 				}
 			}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java

@@ -96,6 +96,13 @@ public class FunctionEditorModel {
 
 		returnInfo = new ParamInfo(this, function.getReturn());
 
+		// check for void storage correction
+		if (VoidDataType.isVoidDataType(returnInfo.getDataType()) &&
+			returnInfo.getStorage() != VariableStorage.VOID_STORAGE) {
+			returnInfo.setStorage(VariableStorage.VOID_STORAGE);
+			modelChanged = true;
+		}
+
 		autoParamCount = 0;
 		Parameter[] params = function.getParameters();
 		for (Parameter parameter : params) {
@@ -265,7 +272,7 @@ public class FunctionEditorModel {
 		}
 		else if (!allowCustomStorage &&
 			Function.UNKNOWN_CALLING_CONVENTION_STRING.equals(callingConventionName)) {
-			if (!(returnType instanceof VoidDataType && parameters.isEmpty())) {
+			if (!(VoidDataType.isVoidDataType(returnType) && parameters.isEmpty())) {
 				statusText =
 					"Warning: No calling convention specified. Ghidra may automatically assign one later.";
 			}
@@ -338,17 +345,9 @@ public class FunctionEditorModel {
 		return true;
 	}
 
-	private DataType getBaseDataType(DataType dataType) {
-		if (dataType instanceof TypeDef) {
-			return ((TypeDef) dataType).getBaseDataType();
-		}
-		return dataType;
-	}
-
 	private boolean hasValidReturnType() {
 		DataType returnType = returnInfo.getDataType();
-		DataType baseType = getBaseDataType(returnType);
+		if (VoidDataType.isVoidDataType(returnType)) {
-		if (baseType instanceof VoidDataType) {
 			return true;
 		}
 		if (returnType.getLength() <= 0) {
@@ -377,7 +376,7 @@ public class FunctionEditorModel {
 
 	private boolean isValidParamType(ParamInfo param) {
 		DataType dataType = param.getDataType();
-		if (dataType.isEquivalent(VoidDataType.dataType)) {
+		if (VoidDataType.isVoidDataType(dataType)) {
 			statusText = "\"void\" is not allowed as a parameter datatype.";
 			return false;
 		}
@@ -800,7 +799,7 @@ public class FunctionEditorModel {
 
 		param.setFormalDataType(formalDataType.clone(program.getDataTypeManager()));
 		if (allowCustomStorage) {
-			if (isReturn && (formalDataType instanceof VoidDataType)) {
+			if (isReturn && VoidDataType.isVoidDataType(formalDataType)) {
 				param.setStorage(VariableStorage.VOID_STORAGE);
 			}
 			else {
@@ -1099,10 +1098,8 @@ public class FunctionEditorModel {
 
 		setCallingConventionName(functionDefinition.getCallingConventionName());
 
-		if (!isSameSize(returnInfo.getFormalDataType(), functionDefinition.getReturnType())) {
+		DataType returnDt = functionDefinition.getReturnType();
-			returnInfo.setStorage(VariableStorage.UNASSIGNED_STORAGE);
+		returnInfo.setFormalDataType(returnDt);
-		}
-		returnInfo.setFormalDataType(functionDefinition.getReturnType());
 
 		List<ParamInfo> oldParams = parameters;
 		parameters = new ArrayList<>();
@@ -1117,6 +1114,13 @@ public class FunctionEditorModel {
 		fixupOrdinals();
 
 		if (allowCustomStorage) {
+			if (VoidDataType.isVoidDataType(returnDt)) {
+				returnInfo.setStorage(VariableStorage.VOID_STORAGE);
+			}
+			else if (!isSameSize(returnInfo.getFormalDataType(),
+				functionDefinition.getReturnType())) {
+				returnInfo.setStorage(VariableStorage.UNASSIGNED_STORAGE);
+			}
 			reconcileCustomStorage(oldParams, parameters);
 		}
 		else {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionVariableData.java

@@ -19,6 +19,10 @@ import ghidra.program.model.data.DataType;
 import ghidra.program.model.listing.VariableStorage;
 
 interface FunctionVariableData {
+
+	/**
+	 * {@return parameter ordinal or null for return variable.}
+	 */
 	public Integer getIndex();
 
 	public void setStorage(VariableStorage storage);

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParameterTableModel.java

@@ -20,6 +20,7 @@ import java.util.List;
 
 import docking.widgets.table.AbstractGTableModel;
 import ghidra.program.model.data.DataType;
+import ghidra.program.model.data.VoidDataType;
 import ghidra.program.model.listing.VariableStorage;
 
 class ParameterTableModel extends AbstractGTableModel<FunctionVariableData> {
@@ -223,7 +224,15 @@ class ParameterTableModel extends AbstractGTableModel<FunctionVariableData> {
 
 		@Override
 		public boolean isCellEditable(int rowIndex) {
-			return canCustomizeStorage;
+			FunctionVariableData rowData = getRowObject(rowIndex);
+			if (rowData == null || !canCustomizeStorage) {
+				return false;
+			}
+			if (rowData.getIndex() == null) {
+				// return parameter - don't permit storage edit for void type
+				return !VoidDataType.isVoidDataType(rowData.getFormalDataType());
+			}
+			return true;
 		}
 
 		@Override

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/exceptionhandlers/gcc/DwarfDecoderFactory.java

@@ -208,7 +208,7 @@ public class DwarfDecoderFactory {
 
 		@Override
 		public DataType getDataType(Program program) {
-			return new VoidDataType();
+			return VoidDataType.dataType;
 		}
 	}
 
@@ -370,7 +370,7 @@ public class DwarfDecoderFactory {
 
 		@Override
 		public DataType getDataType(Program program) {
-			return new VoidDataType();
+			return VoidDataType.dataType;
 		}
 	}
 

Ghidra/Features/Base/src/main/java/ghidra/util/UndefinedFunction.java

@@ -327,11 +327,8 @@ public class UndefinedFunction implements Function {
 	public Parameter getReturn() {
 		try {
 			DataType dt = getReturnType();
-			if (dt instanceof TypeDef) {
-				dt = ((TypeDef) dt).getBaseDataType();
-			}
 			return new ReturnParameterImpl(dt,
-				(dt instanceof VoidDataType) ? VariableStorage.VOID_STORAGE
+				VoidDataType.isVoidDataType(dt) ? VariableStorage.VOID_STORAGE
 						: VariableStorage.UNASSIGNED_STORAGE,
 				getProgram());
 		}

Ghidra/Features/Base/src/main/java/ghidra/util/state/ResultsState.java

@@ -2168,7 +2168,7 @@ public class ResultsState {
 		}
 		else {
 			returnType = func.getReturnType();
-			if (VoidDataType.dataType.isEquivalent(returnType)) {
+			if (VoidDataType.isVoidDataType(returnType)) {
 				return;
 			}
 		}

Ghidra/Features/Decompiler/ghidra_scripts/classrecovery/RecoveredClassHelper.java

@@ -3288,8 +3288,7 @@ public class RecoveredClassHelper {
 			}
 
 			if (returnType.equals("void")) {
-				DataType voidDataType = new VoidDataType();
+				constructorFunction.setReturnType(VoidDataType.dataType, SourceType.ANALYSIS);
-				constructorFunction.setReturnType(voidDataType, SourceType.ANALYSIS);
 			}
 			else if (returnType.contains("*")) {
 				DataType classPointerDataType = dataTypeManager.getPointer(classStruct);
@@ -3353,7 +3352,7 @@ public class RecoveredClassHelper {
 					true);
 			}
 
-			destructorFunction.setReturnType(new VoidDataType(), SourceType.ANALYSIS);
+			destructorFunction.setReturnType(VoidDataType.dataType, SourceType.ANALYSIS);
 		}
 	}
 
@@ -3400,7 +3399,7 @@ public class RecoveredClassHelper {
 					true, true);
 			}
 
-			vbaseDestructorFunction.setReturnType(new VoidDataType(), SourceType.ANALYSIS);
+			vbaseDestructorFunction.setReturnType(VoidDataType.dataType, SourceType.ANALYSIS);
 		}
 
 	}
@@ -6220,7 +6219,7 @@ public class RecoveredClassHelper {
 						if (!atexitCalledFunctions.contains(calledFunction)) {
 							atexitCalledFunctions.add(calledFunction);
 						}
-						calledFunction.setReturnType(new VoidDataType(), SourceType.ANALYSIS);
+						calledFunction.setReturnType(VoidDataType.dataType, SourceType.ANALYSIS);
 					}
 					else {
 						if (!atexitCalledFunctions.contains(calledFunction)) {

Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/PdbPrimitiveTypeApplicator.java

@@ -79,8 +79,7 @@ public class PdbPrimitiveTypeApplicator {
 
 	DataType getVoidType() {
 		if (voidGhidraPrimitive == null) {
-			DataType dataType = new VoidDataType(getDataTypeManager());
+			voidGhidraPrimitive = resolve(VoidDataType.dataType);
-			voidGhidraPrimitive = resolve(dataType);
 		}
 		return voidGhidraPrimitive;
 	}

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

@@ -392,7 +392,10 @@ public class FunctionDB extends DatabaseObject implements Function {
 				return;
 			}
 			type = type.clone(program.getDataTypeManager());
-			if (storage.isValid() && (storage.size() != type.getLength())) {
+			if (VoidDataType.isVoidDataType(type)) {
+				storage = VariableStorage.VOID_STORAGE;
+			}
+			else if (storage.isValid() && (storage.size() != type.getLength())) {
 				try {
 					storage = VariableUtilities.resizeStorage(storage, type, true, this);
 				}
@@ -811,13 +814,9 @@ public class FunctionDB extends DatabaseObject implements Function {
 		}
 
 		dataTypes[0] = returnParam.getFormalDataType();
-		DataType baseType = dataTypes[0];
+		returnParam.setDynamicStorage(
-		if (baseType instanceof TypeDef) {
+					VoidDataType.isVoidDataType(dataTypes[0]) ? VariableStorage.VOID_STORAGE
-			baseType = ((TypeDef) baseType).getBaseDataType();
+							: VariableStorage.UNASSIGNED_STORAGE);
-		}
-		returnParam
-				.setDynamicStorage((baseType instanceof VoidDataType) ? VariableStorage.VOID_STORAGE
-						: VariableStorage.UNASSIGNED_STORAGE);
 
 		PrototypeModel callingConvention = getCallingConvention();
 		if (callingConvention == null) {
@@ -913,12 +912,8 @@ public class FunctionDB extends DatabaseObject implements Function {
 			ReturnParameterDB rtnParam = getReturn();
 			if (rtnParam.getVariableStorage().isBadStorage()) {
 				DataType dt = rtnParam.getDataType();
-				DataType baseType = dt;
-				if (baseType instanceof TypeDef) {
-					baseType = ((TypeDef) baseType).getBaseDataType();
-				}
 				VariableStorage storage =
-					(baseType instanceof VoidDataType) ? VariableStorage.VOID_STORAGE
+					VoidDataType.isVoidDataType(dt) ? VariableStorage.VOID_STORAGE
 							: VariableStorage.UNASSIGNED_STORAGE;
 				rtnParam.setStorageAndDataType(storage, dt);
 			}

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

@@ -22,10 +22,6 @@ import ghidra.program.model.listing.*;
 
 class ParameterDB extends VariableDB implements Parameter {
 
-	/**
-	 * @param function
-	 * @param s
-	 */
 	ParameterDB(FunctionDB function, SymbolDB s) {
 		super(function, s);
 	}

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

@@ -28,16 +28,17 @@ public class ReturnParameterDB extends ParameterDB {
 
 	private DataType dataType;
 
-	/**
-	 * @param function
-	 * @param s
-	 */
 	ReturnParameterDB(FunctionDB function, DataType dt, VariableStorage storage) {
 		super(function, null);
 		this.dataType = dt;
 		this.storage = storage;
 	}
 
+	@Override
+	protected boolean isVoidAllowed() {
+		return true;
+	}
+
 	@Override
 	public String getName() {
 		return RETURN_NAME;
@@ -123,15 +124,10 @@ public class ReturnParameterDB extends ParameterDB {
 			VariableStorage newStorage = VariableStorage.UNASSIGNED_STORAGE;
 			boolean hasCustomVariableStorage = function.hasCustomVariableStorage();
 			if (hasCustomVariableStorage) {
-				DataType baseType = type;
-				if (baseType instanceof TypeDef) {
-					baseType = ((TypeDef) baseType).getBaseDataType();
-				}
 				try {
-					newStorage =
+					newStorage = VoidDataType.isVoidDataType(type) ? VariableStorage.VOID_STORAGE
-						(baseType instanceof VoidDataType) ? VariableStorage.VOID_STORAGE
+							: VariableUtilities.resizeStorage(getVariableStorage(), type,
-								: VariableUtilities.resizeStorage(getVariableStorage(), type,
+								alignStack, function);
-									alignStack, function);
 					VariableUtilities.checkStorage(newStorage, type, force);
 				}
 				catch (InvalidInputException e) {

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

@@ -23,8 +23,8 @@ import ghidra.program.database.data.DataTypeUtilities;
 import ghidra.program.database.symbol.SymbolDB;
 import ghidra.program.database.symbol.VariableSymbolDB;
 import ghidra.program.model.address.Address;
-import ghidra.program.model.data.AbstractFloatDataType;
 import ghidra.program.model.data.DataType;
+import ghidra.program.model.data.VoidDataType;
 import ghidra.program.model.lang.Register;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.pcode.Varnode;
@@ -53,12 +53,21 @@ public abstract class VariableDB implements Variable {
 		this.functionMgr = function.getFunctionManager();
 	}
 
+	protected boolean isVoidAllowed() {
+		return false;
+	}
+
 	@Override
-	public boolean isValid() {
+	public final boolean isValid() {
 		VariableStorage variableStorage = getVariableStorage();
 		DataType dt = getDataType();
-		return variableStorage.isValid() &&
+		if (VoidDataType.isVoidDataType(dt)) {
-			((dt instanceof AbstractFloatDataType) || variableStorage.size() == dt.getLength());
+			return isVoidAllowed() && variableStorage.isVoidStorage();
+		}
+		if (dt.getLength() <= 0 || !variableStorage.isValid()) {
+			return false;
+		}
+		return variableStorage.size() >= dt.getLength();
 	}
 
 	@Override

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/VoidDataType.java

@@ -93,4 +93,20 @@ public class VoidDataType extends BuiltIn {
 		return null; // standard C name and type
 	}
 
+	/**
+	 * Determine if the specified {@link DataType} is a {@link VoidDataType} after 
+	 * stripping away any {@link TypeDef}.
+	 * @param dt datatype to be tested
+	 * @return true if dt is a void type
+	 */
+	public static boolean isVoidDataType(DataType dt) {
+		if (dt == null) {
+			return false;
+		}
+		if (dt instanceof TypeDef t) {
+			dt = t.getBaseDataType();
+		}
+		return dt instanceof VoidDataType;
+	}
+
 }

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

@@ -80,11 +80,7 @@ public class ParamListStandard implements ParamList {
 		if (tp == null) {
 			tp = DataType.DEFAULT;
 		}
-		DataType baseType = tp;
+		if (VoidDataType.isVoidDataType(tp)) {
-		if (baseType instanceof TypeDef) {
-			baseType = ((TypeDef) baseType).getBaseDataType();
-		}
-		if (baseType instanceof VoidDataType) {
 			return VariableStorage.VOID_STORAGE;
 		}
 		int sz = tp.getLength();

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

@@ -264,9 +264,13 @@ abstract class VariableImpl implements Variable {
 
 	@Override
 	public final boolean isValid() {
-		DataType dt = getDataType();
+		if (VoidDataType.isVoidDataType(dataType)) {
-		return variableStorage.isValid() &&
+			return isVoidAllowed() && variableStorage.isVoidStorage();
-			((dt instanceof AbstractFloatDataType) || variableStorage.size() == dt.getLength());
+		}
+		if (dataType.getLength() <= 0 || !variableStorage.isValid()) {
+			return false;
+		}
+		return variableStorage.size() >= dataType.getLength();
 	}
 
 	@Override
@@ -302,11 +306,7 @@ abstract class VariableImpl implements Variable {
 	public void setDataType(DataType type, SourceType source) throws InvalidInputException {
 		type =
 			VariableUtilities.checkDataType(type, isVoidAllowed(), dataType.getLength(), program);
-		DataType baseType = type;
+		variableStorage = VoidDataType.isVoidDataType(type) ? VariableStorage.VOID_STORAGE
-		if (baseType instanceof TypeDef) {
-			baseType = ((TypeDef) baseType).getBaseDataType();
-		}
-		variableStorage = (baseType instanceof VoidDataType) ? VariableStorage.VOID_STORAGE
 				: resizeStorage(variableStorage, type);
 		dataType = type;
 	}

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

@@ -372,6 +372,17 @@ public class VariableUtilities {
 	 */
 	public static VariableStorage resizeStorage(VariableStorage curStorage, DataType dataType,
 			boolean alignStack, Function function) throws InvalidInputException {
+
+		if (dataType instanceof TypeDef td) {
+			dataType = td.getBaseDataType();
+		}
+		if (dataType instanceof VoidDataType) {
+			return VariableStorage.VOID_STORAGE;
+		}
+		if (dataType instanceof AbstractFloatDataType) {
+			return curStorage; // do not constrain or attempt resize of float storage
+		}
+
 		if (!curStorage.isValid()) {
 			return curStorage;
 		}
@@ -380,17 +391,11 @@ public class VariableUtilities {
 		if (curSize == newSize) {
 			return curStorage;
 		}
+
 		if (curSize == 0 || curStorage.isUniqueStorage() || curStorage.isHashStorage()) {
 			throw new InvalidInputException("Storage can't be resized: " + curStorage.toString());
 		}
 
-		if (dataType instanceof TypeDef) {
-			dataType = ((TypeDef) dataType).getBaseDataType();
-		}
-		if (dataType instanceof AbstractFloatDataType) {
-			return curStorage; // do not constrain or attempt resize of float storage
-		}
-
 		if (newSize > curSize) {
 			return expandStorage(curStorage, newSize, dataType, alignStack, function);
 		}