Merge remote-tracking branch 'origin/GP-1720_ghidra007_Remove_Extraneous_Make_thiscalls--SQUASHED' into patch

Ghidra/Features/Base/ghidra_scripts/GraphClassesScript.java

@@ -323,6 +323,7 @@ public class GraphClassesScript extends GhidraScript {
 				.defaultVertexColor(WebColors.PURPLE)
 				.defaultEdgeColor(WebColors.PURPLE)
 				.defaultLayoutAlgorithm("Compact Hierarchical")
+				.maxNodeCount(1000)
 				.build();
 
 		display.setGraph(graph, graphOptions,

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

@@ -27,11 +27,9 @@ import ghidra.program.model.block.CodeBlock;
 import ghidra.program.model.block.IsolatedEntrySubModel;
 import ghidra.program.model.data.*;
 import ghidra.program.model.listing.*;
-import ghidra.program.model.listing.Function.FunctionUpdateType;
+import ghidra.program.model.mem.*;
-import ghidra.program.model.mem.DumbMemBufferImpl;
-import ghidra.program.model.mem.MemoryAccessException;
 import ghidra.program.model.symbol.*;
-import ghidra.util.exception.*;
+import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
 public class ExtendedFlatProgramAPI extends FlatProgramAPI {
@@ -279,34 +277,9 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 	}
 
 	/**
-	 * Method to determine if the given instruction is a terminating instruction (it return, ...)
+	 * Method to create the first function after the last terminating function before the given 
-	 * @param instruction the given instruction
+	 * address which is in the middle of undefined bytes
-	 * @return true if the given instruction is a terminating instruction, false otherwise
+	 * @param address the given address
-	 */
-	public boolean isTerminatingInstruction(Instruction instruction) {
-
-		FlowType flowType = instruction.getFlowType();
-		FlowType overrideType =
-			FlowOverride.getModifiedFlowType(flowType, instruction.getFlowOverride());
-
-		if (flowType.isTerminal() || overrideType.isTerminal()) {
-			return true;
-		}
-		Function functionContaining = getFunctionContaining(instruction.getAddress());
-		if (functionContaining != null) {
-			if (functionContaining.isThunk() && flowType.isJump()) {
-				return true;
-			}
-			if (flowType.isJump()) {
-				return true;
-			}
-		}
-		return false;
-	}
-
-	/**
-	 * Method to create the first function after the last terminating function before the given address
-	 * @param address the given addres
 	 * @param expectedFiller the expected filler byte value
 	 * @return the created function or null if not created
 	 */
@@ -314,12 +287,33 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 
 		PseudoDisassembler pseudoDisassembler = new PseudoDisassembler(currentProgram);
 
+		// skip any undefineds and get the defined instruction before the given address
 		Instruction instructionBefore = getInstructionBefore(address);
 
-		if (instructionBefore == null || !isTerminatingInstruction(instructionBefore)) {
+		if (instructionBefore == null) {
 			return null;
 		}
 
+		Address instBeforeAddr = instructionBefore.getAddress();
+
+		Memory memory = currentProgram.getMemory();
+		if (!memory.getBlock(address).equals(memory.getBlock(instBeforeAddr))) {
+			return null;
+		}
+
+		// set some arbritrary limit on how far back to go
+		if (address.subtract(instBeforeAddr) > 2000) {
+			return null;
+		}
+
+		// if the instruction before all the undefines bytes doesn't indicate that it is the end 
+		// of a function or an end of a range of a function then return
+		FlowType flowType = instructionBefore.getFlowType();
+		if (!flowType.isTerminal() && !flowType.isJump()) {
+			return null;
+		}
+
+		//get the last address in the instruction
 		Address maxAddress = instructionBefore.getMaxAddress();
 		int maxLen = (int) (address.getOffset() - maxAddress.getOffset());
 		if (maxLen <= 0) {
@@ -328,6 +322,7 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 
 		int offset = 1;
 
+		// skip the filler
 		Byte filler;
 		try {
 			filler = getByte(maxAddress.add(offset));
@@ -345,6 +340,8 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 
 		Address functionStart = maxAddress.add(offset);
 
+		// check to see if the address after the instruction and filler is the start of a valid 
+		// subroutine
 		if (!pseudoDisassembler.isValidSubroutine(functionStart, true)) {
 			return null;
 		}
@@ -352,6 +349,7 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 			disassemble(functionStart);
 		}
 
+		// if so, create a function there
 		Function function = createFunction(functionStart, null);
 
 		return function;
@@ -451,7 +449,6 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 
 		Byte filler = determineFillerByte();
 		if (filler == null) {
-			//	println("Can't determine filler byte so cannot create undefined functions");
 			return;
 		}
 
@@ -469,7 +466,6 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 
 			Function newFunction = createFunctionBefore(address, filler);
 			if (newFunction == null) {
-				//println("Can't find function containing " + address.toString());
 				continue;
 			}
 
@@ -542,27 +538,6 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 		return subroutineAddresses;
 	}
 
-	/**
-	 * Method to make the given function a thiscall
-	 * @param function the given function
-	 * @throws InvalidInputException if issues setting return type
-	 * @throws DuplicateNameException if try to create same symbol name already in namespace
-	 */
-	public void makeFunctionThiscall(Function function)
-			throws InvalidInputException, DuplicateNameException {
-
-		if (function.getCallingConventionName().equals("__thiscall")) {
-			return;
-		}
-
-		// FIXME: if you pass a Function arg you should use its program not currentProgram
-		ReturnParameterImpl returnType =
-			new ReturnParameterImpl(function.getSignature().getReturnType(), currentProgram);
-
-		function.updateFunction("__thiscall", returnType,
-			FunctionUpdateType.DYNAMIC_STORAGE_ALL_PARAMS, true, function.getSignatureSource(),
-			function.getParameters());
-	}
 
 	/**
 	 * Method to get a list of symbols either matching exactly (if exact flag is true) or containing (if exact flag is false) the given symbol name
@@ -1009,7 +984,7 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 	}
 
 	/**
-	 * Method to add the given comment to an existing plate comment unless it already exists in the comment
+	 * Method to add the given string to a plate comment unless the string already exists in it. 
 	 * @param address the given address
 	 * @param comment the comment to add to the plate comment at the given address
 	 */
@@ -1018,6 +993,7 @@ public class ExtendedFlatProgramAPI extends FlatProgramAPI {
 		String plateComment = getPlateComment(address);
 
 		if (plateComment == null) {
+			setPlateComment(address, comment);
 			return;
 		}
 

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

@@ -1820,6 +1820,9 @@ public class RTTIGccClassRecoverer extends RTTIClassRecoverer {
 
 		//	findRealVBaseFunctions(recoveredClasses);
 
+		// make constructors and destructors this calls
+		makeConstructorsAndDestructorsThiscalls(recoveredClasses);
+
 	}
 
 	private StructureDataType createClassTypeInfoStructure() {

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

@@ -1333,6 +1333,9 @@ public class RTTIWindowsClassRecoverer extends RTTIClassRecoverer {
 
 		findRealVBaseFunctions(recoveredClasses);
 
+		// make constructors and destructors _thiscalls 
+		makeConstructorsAndDestructorsThiscalls(recoveredClasses);
+
 	}
 
 	/**

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

@@ -33,6 +33,7 @@ import ghidra.program.database.data.DataTypeUtilities;
 import ghidra.program.flatapi.FlatProgramAPI;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
+import ghidra.program.model.lang.CompilerSpec;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.listing.Function.FunctionUpdateType;
 import ghidra.program.model.mem.MemoryBlock;
@@ -110,8 +111,9 @@ public class RecoveredClassHelper {
 	List<Function> allDestructors = new ArrayList<Function>();
 	List<Function> allInlinedConstructors = new ArrayList<Function>();
 	List<Function> allInlinedDestructors = new ArrayList<Function>();
+	List<Function> nonClassInlines = new ArrayList<Function>();
 
-	List<Namespace> badFIDNamespaces = new ArrayList<Namespace>();
+	Set<Namespace> badFIDNamespaces = new HashSet<Namespace>();
 	List<Structure> badFIDStructures = new ArrayList<Structure>();
 
 	List<Function> badFIDFunctions = new ArrayList<Function>();
@@ -1951,9 +1953,6 @@ public class RecoveredClassHelper {
 			createVftableOrderMapping(recoveredClass);
 		}
 		
-		// If not already, make function a thiscall
-		extendedFlatAPI.makeFunctionThiscall(constructorFunction);
-
 		recoveredClass.addConstructor(constructorFunction);
 		addToAllConstructors(constructorFunction);
 	}
@@ -1969,9 +1968,6 @@ public class RecoveredClassHelper {
 			Function inlinedConstructorFunction)
 			throws InvalidInputException, DuplicateNameException {
 
-		//If not already, make function a thiscall
-		extendedFlatAPI.makeFunctionThiscall(inlinedConstructorFunction);
-
 		recoveredClass.addInlinedConstructor(inlinedConstructorFunction);
 		addToAllInlinedConstructors(inlinedConstructorFunction);
 	}
@@ -1986,9 +1982,6 @@ public class RecoveredClassHelper {
 	public void addDestructorToClass(RecoveredClass recoveredClass, Function destructorFunction)
 			throws InvalidInputException, DuplicateNameException {
 
-		//If not already, make function a thiscall
-		extendedFlatAPI.makeFunctionThiscall(destructorFunction);
-
 		recoveredClass.addDestructor(destructorFunction);
 		addToAllDestructors(destructorFunction);
 	}
@@ -2004,9 +1997,6 @@ public class RecoveredClassHelper {
 			Function inlinedDestructorFunction)
 			throws InvalidInputException, DuplicateNameException {
 
-		//If not already, make function a thiscall
-		extendedFlatAPI.makeFunctionThiscall(inlinedDestructorFunction);
-
 		recoveredClass.addInlinedDestructor(inlinedDestructorFunction);
 		addToAllInlinedDestructors(inlinedDestructorFunction);
 	}
@@ -2255,14 +2245,14 @@ public class RecoveredClassHelper {
 	public void makeFunctionThiscall(Function function)
 			throws InvalidInputException, DuplicateNameException {
 
-		if (function.getCallingConventionName().equals("__thiscall")) {
+		if (function.getCallingConventionName().equals(CompilerSpec.CALLING_CONVENTION_thiscall)) {
 			return;
 		}
 
 		ReturnParameterImpl returnType =
 			new ReturnParameterImpl(function.getSignature().getReturnType(), program);
 
-		function.updateFunction("__thiscall", returnType,
+		function.updateFunction(CompilerSpec.CALLING_CONVENTION_thiscall, returnType,
 			FunctionUpdateType.DYNAMIC_STORAGE_ALL_PARAMS, true, function.getSignatureSource(),
 			function.getParameters());
 	}
@@ -3145,11 +3135,10 @@ public class RecoveredClassHelper {
 						notInFunctionVftableRefs.add(addr);
 					}
 					else {
-						boolean functionCreated =
+						boolean functionCreated = extendedFlatAPI.createFunction(program, addr);
-							extendedFlatAPI.createFunction(prog, addr);
+							if (!functionCreated) {
-						if (!functionCreated) {
+								notInFunctionVftableRefs.add(addr);
-							notInFunctionVftableRefs.add(addr);
+							}
-						}
 					}
 				}
 			}
@@ -3806,11 +3795,13 @@ public class RecoveredClassHelper {
 		if (bookmarkComment.contains("Single Match")) {
 
 			Symbol symbol = symbolTable.getPrimarySymbol(functionAddress);
+
 			if (symbol != null && symbol.getSource() == SourceType.ANALYSIS &&
 				!symbol.getName().equals(name) && !symbol.getParentNamespace().equals(namespace)) {
 				// add to list of bad namespaces to be cleaned up later 
-				if (!badFIDNamespaces.contains(symbol.getParentNamespace())) {
+				Namespace parentNamespace = symbol.getParentNamespace();
-					badFIDNamespaces.add(symbol.getParentNamespace());
+				if (!parentNamespace.isGlobal()) {
+					badFIDNamespaces.add(parentNamespace);
 				}
 				extendedFlatAPI.addUniqueStringToPlateComment(functionAddress,
 					"***** Removed Bad FID Symbol *****");
@@ -4426,6 +4417,11 @@ public class RecoveredClassHelper {
 			monitor.checkCanceled();
 			Namespace badNamespace = badNamespaceIterator.next();
 
+			// global namespace shouldn't be on list but check anyway
+			if (badNamespace.isGlobal()) {
+				continue;
+			}
+
 			// delete empty namespace and parent namespaces
 			if (!extendedFlatAPI.hasSymbolsInNamespace(badNamespace)) {
 				removeEmptyNamespaces(badNamespace);
@@ -5965,6 +5961,7 @@ public class RecoveredClassHelper {
 
 							// remove from the allConstructors too
 							addInlinedConstructorToClass(recoveredClass, constructor);
+							nonClassInlines.add(constructor);
 							constructorIterator.remove();
 							removeFromAllConstructors(constructor);
 
@@ -6966,6 +6963,40 @@ public class RecoveredClassHelper {
 		}
 	}
 
+	public void makeConstructorsAndDestructorsThiscalls(List<RecoveredClass> recoveredClasses)
+			throws CancelledException, InvalidInputException, DuplicateNameException {
+
+		if (recoveredClasses.isEmpty()) {
+			return;
+		}
+
+		List<Function> allConstructorDestructorFunctions = new ArrayList<Function>();
+
+		for (RecoveredClass recoveredClass : recoveredClasses) {
+
+			monitor.checkCanceled();
+
+			allConstructorDestructorFunctions.addAll(recoveredClass.getConstructorList());
+			allConstructorDestructorFunctions.addAll(recoveredClass.getDestructorList());
+			allConstructorDestructorFunctions.addAll(recoveredClass.getInlinedConstructorList());
+			allConstructorDestructorFunctions.addAll(recoveredClass.getInlinedDestructorList());
+		}
+
+		if (allConstructorDestructorFunctions.isEmpty()) {
+			return;
+		}
+
+		// remove the inlines that are not in their expected class -- still want the inline
+		// comments later in processing but don't make them this calls
+		allConstructorDestructorFunctions.removeAll(nonClassInlines);
+
+		for (Function function : allConstructorDestructorFunctions) {
+			monitor.checkCanceled();
+			makeFunctionThiscall(function);
+		}
+
+	}
+
 	/**
 	 * Method to create <class_name>_data structure for given class
 	 * @param recoveredClass the class