Merge remote-tracking branch 'origin/GP-4872_ghidra007_rtti_add_check_for_existing_constructor_return_type--SQUASHED'

Ghidra/Features/Decompiler/ghidra_scripts/classrecovery/DecompilerScriptUtils.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.
@@ -16,10 +16,8 @@
 //DO NOT RUN. THIS IS NOT A SCRIPT! THIS IS A CLASS THAT IS USED BY SCRIPTS. 
 package classrecovery;
 
-import docking.options.OptionsService;
 import ghidra.app.decompiler.*;
 import ghidra.app.decompiler.component.DecompilerUtils;
-import ghidra.framework.options.ToolOptions;
 import ghidra.framework.plugintool.ServiceProvider;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.DataType;
@@ -27,7 +25,10 @@ import ghidra.program.model.data.ParameterDefinition;
 import ghidra.program.model.listing.Function;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.pcode.*;
-import ghidra.util.exception.CancelledException;
+import ghidra.program.model.pcode.HighFunctionDBUtil.ReturnCommitOption;
+import ghidra.program.model.symbol.SourceType;
+import ghidra.util.Msg;
+import ghidra.util.exception.*;
 import ghidra.util.task.TaskMonitor;
 
 public class DecompilerScriptUtils {
@@ -109,6 +110,30 @@ public class DecompilerScriptUtils {
 		return decompRes.getHighFunction().getFunctionPrototype().getReturnType();
 	}
 
+	public void commitFunction(Function function) {
+		DecompileResults decompRes = decompInterface.decompileFunction(function,
+			decompInterface.getOptions().getDefaultTimeout(), monitor);
+
+		if (decompRes == null || decompRes.getHighFunction() == null ||
+			decompRes.getHighFunction().getFunctionPrototype() == null) {
+			Msg.debug(this, "Couldn't commit params - null high function");
+			return;
+		}
+
+		try {
+			HighFunctionDBUtil.commitParamsToDatabase(decompRes.getHighFunction(), true,
+				ReturnCommitOption.COMMIT, SourceType.ANALYSIS);
+		}
+		catch (DuplicateNameException e) {
+			Msg.debug(this, "Couldn't commit params " + e);
+			return;
+		}
+		catch (InvalidInputException e) {
+			Msg.debug(this, "Couldn't commit params " + e);
+			return;
+		}
+	}
+
 	/**
 	 * Method to retrieve the function signature string from the decompiler function prototype. NOTE:
 	 * if there is a this param, it will not be included.

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

@@ -479,7 +479,8 @@ public class RecoveredClassHelper {
 		return functionToLoadPcodeOps.get(function);
 	}
 
-	public Set<Function> getAllVfunctions(List<Address> vftableAddresses) throws CancelledException {
+	public Set<Function> getAllVfunctions(List<Address> vftableAddresses)
+			throws CancelledException {
 		if (vftableAddresses.isEmpty()) {
 			return Collections.emptySet();
 		}
@@ -3294,6 +3295,14 @@ public class RecoveredClassHelper {
 	public void addConstructorsToClassNamespace(RecoveredClass recoveredClass,
 			Structure classStruct) throws Exception {
 
+		DataType undefinedDT = null;
+		if (defaultPointerSize == 4) {
+			undefinedDT = new Undefined4DataType();
+		}
+		if (defaultPointerSize == 8) {
+			undefinedDT = new Undefined8DataType();
+		}
+
 		Namespace classNamespace = recoveredClass.getClassNamespace();
 		String className = recoveredClass.getName();
 
@@ -3310,70 +3319,53 @@ public class RecoveredClassHelper {
 					true);
 			}
 
-			// if current decompiler function return type is a pointer then set the return type
+			// commit what the decompiler knows first so that retyping will not
-			// to a pointer to the class structure, otherwise if it is a void, make it a void so the
+			// completely overwrite decompiler with listing signature
-			// listing has void too, otherwise, leave it as is, probably a void
+			decompilerUtils.commitFunction(constructorFunction);
-			String returnType = getReturnTypeFromDecompiler(constructorFunction);
 
-			// Set error bookmark, add error message, and  get the listing return type if the
+			HighFunction highFunction = decompilerUtils.getHighFunction(constructorFunction);
-			// decompiler return type is null
+			if (highFunction == null) {
-			if (returnType == null) {
+				String msg =
+					"Decompiler Error: Failed to decompile function possibly due to the addition of class structure. ";
 
-				String msg1 = "Decompiler Error: Failed to decompile function";
+				Msg.debug(this, msg + constructorFunction.getEntryPoint());
-				String msg2 = ", possibly due to the addition of class structure.";
-
-				Msg.debug(this, msg1 + " at " + constructorFunction.getEntryPoint() + msg2);
 
 				program.getBookmarkManager()
 						.setBookmark(constructorFunction.getEntryPoint(), BookmarkType.ERROR,
-							"Decompiler Error", msg1 + msg2);
+							"Decompiler Error", msg);
-
+				continue;
-				// get the return type from the listing and in some cases it will
-				// indicate the correct type to help determine the below type to add
-				returnType = constructorFunction.getReturnType().getDisplayName();
 			}
 
-			if (returnType.equals("void")) {
+			DataType returnType = highFunction.getFunctionPrototype().getReturnType();
-				constructorFunction.setReturnType(VoidDataType.dataType, SourceType.ANALYSIS);
+			if (returnType == null) {
+				Msg.debug(this,
+					"ERROR: Return type is null " + constructorFunction.getEntryPoint());
+				continue;
 			}
-			else if (returnType.contains("*")) {
+
-				DataType classPointerDataType = dataTypeManager.getPointer(classStruct);
+			// if a FID function and isn't void or * change it to undefined so the decompiler will
-				constructorFunction.setReturnType(classPointerDataType, SourceType.ANALYSIS);
-			}
-			// if neither and it is a FID function change it to undefined so the decompiler will
 			// recompute it
-			else if (isFidFunction(constructorFunction)) {
+			String returnTypeString = returnType.getDisplayName();
-				DataType undefinedDT = null;
+			if (isFidFunction(constructorFunction) && returnTypeString != "void" &&
-				if (defaultPointerSize == 4) {
+				!returnTypeString.contains("*")) {
-					undefinedDT = new Undefined4DataType();
+
-				}
-				if (defaultPointerSize == 8) {
-					undefinedDT = new Undefined8DataType();
-				}
 				if (undefinedDT != null) {
 					constructorFunction.setReturnType(undefinedDT, SourceType.ANALYSIS);
 				}
 			}
 
+			// if return type is a pointer then make sure it is the class structure 
+			if (returnType.getDisplayName().contains("*")) {
+				DataType classPointerDataType = dataTypeManager.getPointer(classStruct);
+				if (!returnType.isEquivalent(classPointerDataType)) {
+					constructorFunction.setReturnType(classPointerDataType,
+						SourceType.ANALYSIS);
+				}
+			}
+
 		}
 	}
 
-	/**
-	 * Get the return value from the decompiler signature for the given function
-	 * @param function the given function
-	 * @return the decompiler return value for the given function
-	 */
-	private String getReturnTypeFromDecompiler(Function function) {
-
-		DataType decompilerReturnType = decompilerUtils.getDecompilerReturnType(function);
-
-		if (decompilerReturnType == null) {
-			return null;
-		}
-
-		return decompilerReturnType.getDisplayName();
-	}
-
 	/**
 	 * Method to name class destructors and add them to class namespace
 	 * @param recoveredClass current class
@@ -3399,8 +3391,7 @@ public class RecoveredClassHelper {
 				createNewSymbolAtFunction(destructorFunction, destructorName, classNamespace, true,
 					true);
 			}
-
+			decompilerUtils.commitFunction(destructorFunction);
-			destructorFunction.setReturnType(VoidDataType.dataType, SourceType.ANALYSIS);
 		}
 	}
 
@@ -3426,6 +3417,7 @@ public class RecoveredClassHelper {
 
 			createNewSymbolAtFunction(destructorFunction, destructorName, classNamespace, false,
 				false);
+			decompilerUtils.commitFunction(destructorFunction);
 		}
 	}
 
@@ -3450,8 +3442,7 @@ public class RecoveredClassHelper {
 				createNewSymbolAtFunction(vbaseDestructorFunction, destructorName, classNamespace,
 					true, true);
 			}
-
+			decompilerUtils.commitFunction(vbaseDestructorFunction);
-			vbaseDestructorFunction.setReturnType(VoidDataType.dataType, SourceType.ANALYSIS);
 		}
 
 	}
@@ -3606,6 +3597,7 @@ public class RecoveredClassHelper {
 						function.getEntryPoint().toString());
 				}
 				return;
+
 			}
 
 			symbol = lcmd.getSymbol();
@@ -4674,6 +4666,7 @@ public class RecoveredClassHelper {
 					createNewSymbolAtFunction(vfunction, vfunctionName, classNamespace, setPrimary,
 						removeBadFID);
 				}
+				decompilerUtils.commitFunction(vfunction);
 			}
 		}
 	}
@@ -5040,6 +5033,10 @@ public class RecoveredClassHelper {
 			if (nameVfunctions) {
 				createNewSymbolAtFunction(indeterminateFunction,
 					className + "_Constructor_or_Destructor", classNamespace, false, false);
+				// in this case since indeterminate, only commit if script names it
+				// if name flag is not set then it will have correct name from debug and be handled 
+				// in other methods (ie addConst, addDest)
+				decompilerUtils.commitFunction(indeterminateFunction);
 			}
 
 		}