GP-6670 a few improvements to gcc class recovery script

Ghidra/Features/Decompiler/ghidra_scripts/RecoverClassesFromRTTIScript.java

@@ -215,6 +215,8 @@ public class RecoverClassesFromRTTIScript extends GhidraScript {
 				return;
 			}
 
+			nameVfunctions = true;
+
 			recoverClassesFromRTTI = new RTTIGccClassRecoverer(currentProgram, state.getTool(),
 				this, BOOKMARK_FOUND_FUNCTIONS, USE_SHORT_TEMPLATE_NAMES_IN_STRUCTURE_FIELDS,
 				nameVfunctions, MAKE_VFUNCTIONS_THISCALLS, hasDebugSymbols, monitor);

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

@@ -299,6 +299,12 @@ public class DecompilerScriptUtils {
 	 * @return a new address with the specified offset in the default address space
 	 */
 	public final Address toAddr(long offset) {
+
+		long maxOffset = program.getMaxAddress().getOffset();
+
+		if (Long.compareUnsigned(offset, maxOffset) > 0) {
+			return null;
+		}
 		return program.getAddressFactory().getDefaultAddressSpace().getAddress(offset);
 	}
 

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

@@ -1623,8 +1623,14 @@ public class RTTIGccClassRecoverer extends RTTIClassRecoverer {
 			internalString = "internal_";
 		}
 
-		symbolTable.createLabel(vtable.getVfunctionTop(),
-			internalString + constructionString + VFTABLE_LABEL, classNamespace,
+		// check for non-ideal primary symbol that another analyzer may have created and remove it 
+		// so it can be replaced with a better one
+		Symbol primaryVftableSymbol = symbolTable.getPrimarySymbol(vtable.getVfunctionTop());
+		if (primaryVftableSymbol != null && primaryVftableSymbol.getName().startsWith("vfTable_")) {
+			primaryVftableSymbol.delete();
+		}
+		String vftableName = internalString + constructionString + VFTABLE_LABEL;
+		symbolTable.createLabel(vtable.getVfunctionTop(), vftableName, classNamespace,
 			SourceType.ANALYSIS);
 
 	}
@@ -3283,7 +3289,7 @@ public class RTTIGccClassRecoverer extends RTTIClassRecoverer {
 		assignConstructorsAndDestructorsUsingExistingNameNew(recoveredClasses);
 
 		// find gcc destructors in top of vftables
-		findVftableDestructors(recoveredClasses);
+		findVftableDestructors();
 
 		// figure out which are inlined and put on separate list to be processed later
 		separateInlinedConstructorDestructors(recoveredClasses);
@@ -3365,8 +3371,7 @@ public class RTTIGccClassRecoverer extends RTTIClassRecoverer {
 		}
 	}
 
-	private void findVftableDestructors(List<RecoveredClass> recoveredClasses)
-			throws CancelledException {
+	private void findVftableDestructors() throws CancelledException {
 
 		for (RecoveredClass recoveredClass : recoveredClasses) {
 
@@ -3384,82 +3389,129 @@ public class RTTIGccClassRecoverer extends RTTIClassRecoverer {
 					continue;
 				}
 
-				Function firstVfunction = virtualFunctions.get(0);
-				Function secondVfunction = virtualFunctions.get(1);
-
-				Address callingAddressOfFirstVfunction =
-					getCallingAddress(secondVfunction, firstVfunction);
-				if (callingAddressOfFirstVfunction == null) {
+				Function deletingDestructor = getDeletingDestructor(virtualFunctions);
+				if (deletingDestructor == null) {
 					continue;
 				}
 
-				// TODO: eventually work into new op delete discovery
-				Address callingAddrOfOpDelete =
-					getCallingAddress(secondVfunction, "operator.delete");
-				if (callingAddrOfOpDelete == null) {
+				// find case where deleting destructor only calls operator.delete and the 
+				// destructor is just a RET instruction
+				if (getFunctionCallMap(deletingDestructor, true).keySet().size() == 1) {
+					for (Function vfunction : virtualFunctions) {
+						monitor.checkCancelled();
+
+						if (hasOnlyReturnInstruction(vfunction)) {
+							recoveredClass.addDestructor(vfunction);
+							recoveredClass.addDeletingDestructor(deletingDestructor);
+							break;
+						}
+					}
 					continue;
 				}
 
-				// if firsrVfunction is called before op delete then valid set of
-				// destructor/deleting destructor
-				if (callingAddrOfOpDelete.getOffset() > callingAddressOfFirstVfunction
-						.getOffset()) {
-					recoveredClass.addDestructor(firstVfunction);
-					recoveredClass.addDeletingDestructor(secondVfunction);
+				// find case where deleting destructor calls destructor then operator.delete
+				for (Function vfunction : virtualFunctions) {
+					monitor.checkCancelled();
+
+					// skip deleting destructor - won't call itself
+					if (vfunction.equals(deletingDestructor)) {
+						continue;
+					}
+
+					Address callingAddessOfDestructor =
+						getCallingAddress(deletingDestructor, vfunction);
+					if (callingAddessOfDestructor != null) {
+						Address callingAddrOfOpDelete =
+							getCallingAddress(deletingDestructor, "operator.delete");
+
+						if (callingAddrOfOpDelete.compareTo(callingAddessOfDestructor) > 0) {
+							recoveredClass.addDestructor(vfunction);
+							recoveredClass.addDeletingDestructor(deletingDestructor);
+						}
+
+					}
 				}
 
 			}
 		}
 	}
 
+	private boolean hasOnlyReturnInstruction(Function function) {
+
+		InstructionIterator instructions =
+			program.getListing().getInstructions(function.getBody(), true);
+
+		// get the first instruction
+		if (instructions.hasNext()) {
+			Instruction instruction = instructions.next();
+			if (instruction.getFlowType().isTerminal() && !instructions.hasNext()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private Function getDeletingDestructor(List<Function> functions) throws CancelledException {
+
+		for (Function function : functions) {
+			monitor.checkCancelled();
+
+			Map<Address, Function> map = getFunctionCallMap(function, true);
+
+			for (Address address : map.keySet()) {
+				monitor.checkCancelled();
+
+				Function calledFunction = map.get(address);
+				if (calledFunction == null) {
+					continue;
+				}
+
+				if (calledFunction.getName().equals("operator.delete")) {
+					return function;
+				}
+			}
+		}
+
+		return null;
+	}
+
 	private Address getCallingAddress(Function function, Function expectedCalledFunction)
 			throws CancelledException {
 
-		InstructionIterator instructions =
-			function.getProgram().getListing().getInstructions(function.getBody(), true);
+		Map<Address, Function> map = getFunctionCallMap(function, true);//should this be false?
 
-		while (instructions.hasNext()) {
+		for (Address address : map.keySet()) {
 			monitor.checkCancelled();
-			Instruction instruction = instructions.next();
-			if (instruction.getFlowType().isCall()) {
 
-				Function calledFunction =
-					extendedFlatAPI.getReferencedFunction(instruction.getMinAddress(), false);
+			Function calledFunction = map.get(address);
+			if (calledFunction == null) {
+				continue;
+			}
 
-				if (calledFunction == null) {
-					continue;
-				}
-				if (calledFunction.equals(expectedCalledFunction)) {
-					return instruction.getAddress();
-				}
+			if (calledFunction.equals(expectedCalledFunction)) {
+				return address;
 			}
 		}
 		return null;
-
 	}
 
-	private Address getCallingAddress(Function function, String expectedCalledFunctionName)
-			throws CancelledException {
+	private Address getCallingAddress(Function function, String name) throws CancelledException {
 
-		InstructionIterator instructions =
-			function.getProgram().getListing().getInstructions(function.getBody(), true);
+		Map<Address, Function> map = getFunctionCallMap(function, false);
 
-		while (instructions.hasNext()) {
+		for (Address address : map.keySet()) {
 			monitor.checkCancelled();
-			Instruction instruction = instructions.next();
-			if (instruction.getFlowType().isCall()) {
 
-				Function calledFunction =
-					extendedFlatAPI.getReferencedFunction(instruction.getMinAddress(), false);
+			Function calledFunction = map.get(address);
+			if (calledFunction == null) {
+				continue;
+			}
 
-				if (calledFunction != null &&
-					calledFunction.getName().equals(expectedCalledFunctionName)) {
-					return instruction.getAddress();
-				}
+			if (calledFunction.getName().equals(name)) {
+				return address;
 			}
 		}
 		return null;
-
 	}
 
 	private void removeFromIndeterminateLists(List<RecoveredClass> recoveredClasses,

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

@@ -23,6 +23,7 @@ import ghidra.program.model.address.Address;
 import ghidra.program.model.data.*;
 import ghidra.program.model.listing.Function;
 import ghidra.program.model.symbol.Namespace;
+import ghidra.util.Msg;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
@@ -205,9 +206,12 @@ public class RecoveredClass {
 		// error if try to add different address to same offset
 		Address address = classOffsetToVftableMap.get(offset);
 		if (!address.equals(vftableAddress)) {
-			throw new Exception(name + " trying to add different vftable address (old: " +
-				vftableAddress.toString() + " new: " + address.toString() + ")  to same offset " +
-				offset);
+//			throw new Exception(name + " trying to add different vftable address (old: " +
+//				vftableAddress.toString() + " new: " + address.toString() + ")  to same offset " +
+//				offset);
+			Msg.debug(this, name + " trying to add different vftable address (old: " +
+				vftableAddress.toString() + " new: " + address.toString() + ") to same offset");
+
 		}
 
 	}

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

@@ -465,9 +465,11 @@ public class RecoveredClassHelper {
 					continue;
 				}
 
-				if (calledFunction.isExternal()) {
-					continue;
-				}
+				// TODO: might redo to have separate call maps that do/don't include external
+				// keeping this here for reminder
+//				if (calledFunction.isExternal()) {
+//					continue;
+//				}
 
 				// include the null functions in map so things using map can get accurate count
 				// of number of CALL instructions even if the call reg type
@@ -4470,6 +4472,7 @@ public class RecoveredClassHelper {
 			if (vftablePointerDataType == null) {
 				Msg.debug(this,
 					"vftablePointerDataType is null for vftableAddress: " + vftableAddress);
+				continue;
 			}
 
 			DataType vftableDataType = vftablePointerDataType.getDataType();

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

@@ -317,7 +317,7 @@ public class Vtable {
 		// if it has a primary non-default symbol and it isn't "vftable" then it isn't a vftable
 		Symbol primarySymbol = symbolTable.getPrimarySymbol(topAddress);
 		if (primarySymbol != null && primarySymbol.getSource() != SourceType.DEFAULT &&
-			!primarySymbol.getName().contains("vftable")) {
+			!primarySymbol.getName().toLowerCase().contains("vftable")) {
 			return numFunctionPointers;
 		}
 		MemoryBlock currentBlock = program.getMemory().getBlock(topAddress);