Merge remote-tracking branch 'origin/GP-5763_emteere_NoReturnExternalsINT3--SQUASHED'

Ghidra/Features/Base/ghidra_scripts/FixupNoReturnFunctionsScript.java

@@ -41,6 +41,7 @@ import ghidra.app.script.GhidraScript;
 import ghidra.app.tablechooser.*;
 import ghidra.program.model.address.*;
 import ghidra.program.model.block.*;
+import ghidra.program.model.lang.Processor;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.symbol.*;
 import ghidra.util.exception.CancelledException;
@@ -49,10 +50,15 @@ public class FixupNoReturnFunctionsScript extends GhidraScript {
 
 	IssueEntries entryList = null;
 	
+	private final static String X86_NAME = "x86";
+	boolean isX86;
+
 	@Override
 	public void run() throws Exception {
 		Program cp = currentProgram;
 		
+		isX86 = checkForX86(cp);
+
 		TableChooserExecutor executor = createTableExecutor();
 
 		if (this.isRunningHeadless()) {
@@ -89,6 +95,11 @@ public class FixupNoReturnFunctionsScript extends GhidraScript {
 		}
 	}
 
+	private boolean checkForX86(Program cp) {
+		return cp.getLanguage().getProcessor().equals(
+			Processor.findOrPossiblyCreateProcessor(X86_NAME));
+	}
+
 	private void configureTableColumns(TableChooserDialog dialog) {
 		StringColumnDisplay explanationColumn = new StringColumnDisplay() {
 			@Override
@@ -373,10 +384,22 @@ public class FixupNoReturnFunctionsScript extends GhidraScript {
 		//  
 		//  
 		FunctionManager functionManager = currentProgram.getFunctionManager();
-		FunctionIterator functionIter = functionManager.getFunctions(true);
+		
 		AddressSet set = new AddressSet();
 		HashSet<Function> suspectNoReturnFunctions = new HashSet<Function>();
 
+		FunctionIterator functionIter = functionManager.getFunctions(true);
+		checkFunctions(cp, functionIter, noReturnEntries, set, suspectNoReturnFunctions);
+
+		FunctionIterator externalFunctionIter = functionManager.getExternalFunctions();
+		checkFunctions(cp, externalFunctionIter, noReturnEntries, set, suspectNoReturnFunctions);
+		
+		return set;
+	}
+
+	public void checkFunctions(Program cp, FunctionIterator functionIter,
+			IssueEntries noReturnEntries, AddressSet set,
+			HashSet<Function> suspectNoReturnFunctions) throws CancelledException {
 		while (functionIter.hasNext()) {
 			Function candidateNoReturnfunction = functionIter.next();
 			noReturnEntries.setMessage("Checking function: " + candidateNoReturnfunction.getName());
@@ -428,8 +451,6 @@ public class FixupNoReturnFunctionsScript extends GhidraScript {
 				suspectNoReturnFunctions.add(candidateNoReturnfunction);
 			}
 		}
-
-		return set;
 	}
 
 	private boolean testCalledFunctionsNonReturning(Function candidateNonReturningFunction,
@@ -462,8 +483,8 @@ public class FixupNoReturnFunctionsScript extends GhidraScript {
 		FunctionManager funcManager = currentProgram.getFunctionManager();
 		Listing listing = currentProgram.getListing();
 		while (fallThru != null) {
-			if (funcManager.getFunctionAt(fallThru) != null) {
+			Function fallThruFunction = funcManager.getFunctionAt(fallThru);
-
+			if (fallThruFunction != null) {
 				NoReturnLocations location = new NoReturnLocations(currentProgram,
 					ref.getToAddress(), ref.getFromAddress(), "Function defined after call");
 				dialog.add(location);
@@ -490,11 +511,15 @@ public class FixupNoReturnFunctionsScript extends GhidraScript {
 					// or references.  This is especially true if there is only one
 					// example for a calling reference.
 					if (callingFunc != null) {
+						Address fromAddress = reference.getFromAddress();
 						Function function =
-							funcManager.getFunctionContaining(reference.getFromAddress());
+							funcManager.getFunctionContaining(fromAddress);
-						if (callingFunc.equals(function)) {
+						// The reference must come from an address within this function
+						// before this function call (reference fromAddress)
+						// this should get rid of spurious data references from other functions
+						if ((fromAddress.compareTo(fallThru) < 0) && callingFunc.equals(function)) {
 							NoReturnLocations location = new NoReturnLocations(currentProgram,
-								ref.getToAddress(), ref.getFromAddress(),
+								ref.getToAddress(), fromAddress,
 								"Data Reference from same function after call");
 							dialog.add(location);
 							return true;
@@ -517,6 +542,15 @@ public class FixupNoReturnFunctionsScript extends GhidraScript {
 				dialog.add(location);
 				return true;
 			}
+			if (isX86) {
+				Instruction fallInstr = listing.getInstructionContaining(fallThru);
+				if (fallInstr != null && fallInstr.getMnemonicString().equals("INT3")) {
+					NoReturnLocations location = new NoReturnLocations(currentProgram,
+						ref.getToAddress(), ref.getFromAddress(), "INT3 interrupt after call");
+					dialog.add(location);
+					return true;
+				}
+			}
 			fallThru = null;
 			if (block.getFlowType().isFallthrough()) {
 				CodeBlockReferenceIterator dests = block.getDestinations(monitor);

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/FindNoReturnFunctionsAnalyzer.java

@@ -25,8 +25,7 @@ import ghidra.app.util.importer.MessageLog;
 import ghidra.framework.options.Options;
 import ghidra.program.model.address.*;
 import ghidra.program.model.block.*;
-import ghidra.program.model.lang.GhidraLanguagePropertyKeys;
+import ghidra.program.model.lang.*;
-import ghidra.program.model.lang.Language;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.pcode.PcodeOp;
 import ghidra.program.model.pcode.Varnode;
@@ -80,6 +79,9 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 	private Address lastGetNextFuncAddress = null;  // last addr used for getNextFunction()
 	private Address nextFunction = null;            // last return nextFunction
 	
+	private final static String X86_NAME = "x86";
+	boolean isX86;
+
 	public FindNoReturnFunctionsAnalyzer() {
 		this(NAME, DESCRIPTION, AnalyzerType.INSTRUCTION_ANALYZER);
 	}
@@ -106,6 +108,8 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 			this.reasonList = new ArrayList<>();
 			lastGetNextFuncAddress = null;
 			
+			isX86 = checkForX86(program);
+
 			monitor.setMessage("NoReturn - Finding non-returning functions");
 
 			AddressSet noReturnSet = new AddressSet();
@@ -150,6 +154,11 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 		return true;
 	}
 	
+	private boolean checkForX86(Program cp) {
+		return cp.getLanguage().getProcessor().equals(
+			Processor.findOrPossiblyCreateProcessor(X86_NAME));
+	}
+
 	/**
 	 * repair any damaged locations
 	 * 
@@ -358,7 +367,10 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 			}
 
 			// detected a calling issue, check other instructions calling the same place
-			Address[] flows = inst.getFlows();
+			Address[] flows = getAllFlows(inst);
+			if (flows == null) {
+				continue;
+			}
 			for (Address target : flows) {
 
 				int count = 1;
@@ -558,6 +570,17 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 				return true;
 			}
 
+			// on x86 INT3 after a call indicates a non-returning call from alignment padding
+			if (isX86) {
+				Instruction fallInstr = listing.getInstructionContaining(fallThru);
+				if (fallInstr != null &&  fallInstr.getMnemonicString().equals("INT3")) {
+					NoReturnLocations location =
+							new NoReturnLocations(target, fallThru, "INT3 interrupt after call");
+					reasonList.add(location);
+					return true;
+				}
+			}
+			
 			// get the next instruction in fallthru chain
 			fallThru = null;
 			if (instr.getFlowType().isFallthrough()) {
@@ -567,6 +590,38 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 		return false;
 	}
 	
+	/**
+	 * Get all flows that have already been added to instruction.
+	 * If there are none and this is an indirect, get the function at
+	 * the end of the read.
+	 * @param callInst
+	 * @return all flows
+	 */
+	private Address[] getAllFlows(Instruction callInst) {
+		Address[] flows = callInst.getFlows();
+		if (flows != null && flows.length > 0) {
+			return flows;
+		}
+		FlowType flowType = callInst.getFlowType();
+		if (!flowType.isCall() || !flowType.isIndirect()) {
+			return flows;
+		}
+		// if haven't found any flows yet, check for a read of a location that refers
+		// to a function.
+		Reference[] referencesFrom = callInst.getReferencesFrom();
+		for (Reference reference : referencesFrom) {
+			if (reference.getReferenceType().isRead()) {
+				Function functionAt = program.getFunctionManager().getFunctionAt(reference.getToAddress());
+				if (functionAt != null) {
+					flows = new Address[1];
+					flows[0] = reference.getToAddress();
+					return flows;
+				}
+			}
+		}
+		return flows;
+	}
+	
 	/**
 	 * Return true if fallThru address has inconsistent (data/call) references to it.
 	 * Adds the reason for non-returning reason to no return locations list.
@@ -585,8 +640,8 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 				Reference reference = refIterTo.next();
 				RefType refType = reference.getReferenceType();
 				if (refType.isRead() || refType.isWrite()) {
-					// look at function the reference is coming from
+					// Check function the reference is coming from
-					// is the function the same as the call is in
+					// is the same function as the call is in
 					//    This is a better indicator of non-returning
 					// Random references from another function could be bad disassembly
 					// or references.  This is especially true if there is only one
@@ -595,9 +650,13 @@ public class FindNoReturnFunctionsAnalyzer extends AbstractAnalyzer {
 					// TODO: if this is done before functions are created from calls
 					//       then this check will do nothing
 					if (callingFunc != null) {
+						Address fromAddress = reference.getFromAddress();
 						Function function =
-							funcManager.getFunctionContaining(reference.getFromAddress());
+							funcManager.getFunctionContaining(fromAddress);
-						if (callingFunc.equals(function)) {
+						// The reference must come from an address within this function
+						// before this the function call (reference fromAddress)
+						// this should get rid of considering spurious/bad data references from other functions
+						if ((fromAddress.compareTo(addr) < 0) && callingFunc.equals(function)) {
 							NoReturnLocations location =
 								new NoReturnLocations(calledAddr, reference.getToAddress(),
 									"Data Reference from same function after call");