Merge remote-tracking branch

'origin/GP-2534_emteere_SharedFunctionContiguousOnByDefault--SQUASHED'
(Closes #678, Closes #4573)

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/analysis/SharedReturnAnalysisCmd.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -266,11 +265,30 @@ public class SharedReturnAnalysisCmd extends BackgroundCommand {
 			processFunctionJumpReferences(program, entry, monitor);
 		}
 		else {
+			// check if there is any fallthru flow to the potential entry point
+			if (hasFallThruTo(program, entry)) {
+				return;
+			}
 			AutoAnalysisManager analysisMgr = AutoAnalysisManager.getAnalysisManager(program);
 			analysisMgr.createFunction(entry, false);
 		}
 	}
 
+	private boolean hasFallThruTo(Program program, Address location) {
+		Instruction instr= program.getListing().getInstructionAt(location);
+		if (instr == null) {
+			return true;
+		}
+		Address fallFrom = instr.getFallFrom();
+		if (fallFrom != null) {
+			Instruction fallInstr = program.getListing().getInstructionContaining(fallFrom);
+			if (fallInstr != null && fallInstr.getFallThrough().equals(location)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	private void checkAboveFunction(Symbol functionSymbol, AddressSet jumpScanSet) {
 
 		Program program = functionSymbol.getProgram();
@@ -352,7 +370,12 @@ public class SharedReturnAnalysisCmd extends BackgroundCommand {
 
 	private void processFunctionJumpReferences(Program program, Address entry, TaskMonitor monitor)
 			throws CancelledException {
-
+		
+		// check if there is any fallthru flow to the entry point
+		if (hasFallThruTo(program, entry)) {
+			return;
+		}
+		
 		// since reference fixup will occur when flow override is done,
 		// avoid concurrent modification during reference iterator use
 		// by building list of jump references
@@ -361,6 +384,8 @@ public class SharedReturnAnalysisCmd extends BackgroundCommand {
 			return;
 		}
 
+		FunctionManager funcMgr = program.getFunctionManager();
+		
 		for (Reference ref : fnRefList) {
 			monitor.checkCanceled();
 			Instruction instr = program.getListing().getInstructionAt(ref.getFromAddress());
@@ -371,17 +396,29 @@ public class SharedReturnAnalysisCmd extends BackgroundCommand {
 			if (checkRef == null) {
 				continue;
 			}
+			
 			// if there is a function at this address, this is a thunk
 			//    Handle differently
-			if (program.getFunctionManager().getFunctionAt(instr.getMinAddress()) != null) {
+			Address refInstrAddr = instr.getMinAddress();
+			
+			if (funcMgr.getFunctionAt(refInstrAddr) != null) {
 				continue;
 			}
+			
+			// if this instruction is contained in the body of the function
+			// then it is just an internal jump reference to the top of the
+			// function
+			Function functionContaining = funcMgr.getFunctionContaining(refInstrAddr);
+			if (functionContaining != null && functionContaining.getEntryPoint().equals(entry)) {
+				continue;
+			}
+			
 			if (checkRef.getToAddress().equals(ref.getToAddress())) {
 				if (instr.getFlowOverride() != FlowOverride.NONE) {
 					continue;
 				}
 				SetFlowOverrideCmd cmd =
-					new SetFlowOverrideCmd(instr.getMinAddress(), FlowOverride.CALL_RETURN);
+					new SetFlowOverrideCmd(refInstrAddr, FlowOverride.CALL_RETURN);
 				cmd.applyTo(program);
 			}
 		}

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

@@ -20,9 +20,8 @@ import ghidra.app.services.*;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.framework.options.Options;
 import ghidra.program.model.address.AddressSetView;
-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.listing.Program;
 import ghidra.util.HelpLocation;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
@@ -32,7 +31,7 @@ import ghidra.util.task.TaskMonitor;
  * associated branching instruction flow to a CALL-RETURN
  */
 public class SharedReturnAnalyzer extends AbstractAnalyzer {
-
+	
 	private static final String NAME = "Shared Return Calls";
 	protected static final String DESCRIPTION =
 		"Converts branches to calls, followed by an immediate return, when the destination is a function.  " +
@@ -54,12 +53,11 @@ public class SharedReturnAnalyzer extends AbstractAnalyzer {
 		"Signals to allow conditional jumps to be consider for " +
 			"shared return jumps to other functions.";
 
-	private final static boolean OPTION_DEFAULT_ASSUME_CONTIGUOUS_FUNCTIONS_ENABLED = false;
+	private final static boolean OPTION_DEFAULT_ASSUME_CONTIGUOUS_FUNCTIONS_ENABLED = true;
 	private final static boolean OPTION_DEFAULT_CONSIDER_CONDITIONAL_BRANCHES_ENABLED = false;
 
 	private boolean assumeContiguousFunctions = OPTION_DEFAULT_ASSUME_CONTIGUOUS_FUNCTIONS_ENABLED;
-	private boolean considerConditionalBranches =
+	private boolean considerConditionalBranches = OPTION_DEFAULT_CONSIDER_CONDITIONAL_BRANCHES_ENABLED;
-		OPTION_DEFAULT_CONSIDER_CONDITIONAL_BRANCHES_ENABLED;
 
 	public SharedReturnAnalyzer() {
 		this(NAME, DESCRIPTION, AnalyzerType.FUNCTION_ANALYZER);
@@ -88,6 +86,12 @@ public class SharedReturnAnalyzer extends AbstractAnalyzer {
 
 		boolean sharedReturnEnabled = language.getPropertyAsBoolean(
 			GhidraLanguagePropertyKeys.ENABLE_SHARED_RETURN_ANALYSIS, true);
+	
+		// If the language (in the .pspec file) overrides this setting, use that value
+		boolean contiguousFunctionsEnabled = language.getPropertyAsBoolean(
+			GhidraLanguagePropertyKeys.ENABLE_ASSUME_CONTIGUOUS_FUNCTIONS_ONLY, assumeContiguousFunctions);
+		
+		assumeContiguousFunctions = contiguousFunctionsEnabled;
 
 		return sharedReturnEnabled;
 	}
@@ -98,11 +102,11 @@ public class SharedReturnAnalyzer extends AbstractAnalyzer {
 			"Auto_Analysis_Option_Instructions");
 
 		options.registerOption(OPTION_NAME_ASSUME_CONTIGUOUS_FUNCTIONS,
-			OPTION_DEFAULT_ASSUME_CONTIGUOUS_FUNCTIONS_ENABLED, helpLocation,
+			assumeContiguousFunctions, helpLocation,
 			OPTION_DESCRIPTION_ASSUME_CONTIGUOUS_FUNCTIONS);
 
 		options.registerOption(OPTION_NAME_CONSIDER_CONDITIONAL_BRANCHES_FUNCTIONS,
-			OPTION_DEFAULT_CONSIDER_CONDITIONAL_BRANCHES_ENABLED, helpLocation,
+			considerConditionalBranches, helpLocation,
 			OPTION_DESCRIPTION_CONSIDER_CONDITIONAL_BRANCHES_FUNCTIONS);
 	}
 
@@ -110,11 +114,10 @@ public class SharedReturnAnalyzer extends AbstractAnalyzer {
 	public void optionsChanged(Options options, Program program) {
 
 		assumeContiguousFunctions = options.getBoolean(OPTION_NAME_ASSUME_CONTIGUOUS_FUNCTIONS,
-			OPTION_DEFAULT_ASSUME_CONTIGUOUS_FUNCTIONS_ENABLED);
+			assumeContiguousFunctions);
 
-		considerConditionalBranches =
+		considerConditionalBranches = options.getBoolean(OPTION_NAME_CONSIDER_CONDITIONAL_BRANCHES_FUNCTIONS,
-			options.getBoolean(OPTION_NAME_CONSIDER_CONDITIONAL_BRANCHES_FUNCTIONS,
+				considerConditionalBranches);
-				OPTION_DEFAULT_CONSIDER_CONDITIONAL_BRANCHES_ENABLED);
 
 	}
 

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

@@ -105,6 +105,14 @@ public final class GhidraLanguagePropertyKeys {
 	 * If calls are used as long-jumps this can cause problems, so it is disabled for older arm processors.
 	 */
 	public static final String ENABLE_SHARED_RETURN_ANALYSIS = "enableSharedReturnAnalysis";
+	
+	/**
+	 * Shared return analysis, option to assume contiguous functions where a function jumps to another function
+	 * across the address space of another function.
+	 * 
+	 * This could cause issues on programs with bad control flow, or bad disassembly
+	 */
+	public static final String ENABLE_ASSUME_CONTIGUOUS_FUNCTIONS_ONLY = "enableContiguousFunctionsOnly";
 
 	/**
 	 * Non returning function analysis, where a function such as exit() is known to the compiler

Ghidra/Processors/ARM/data/languages/ARMCortex.pspec

@@ -5,6 +5,7 @@
     <property key="addressesDoNotAppearDirectlyInCode" value="true"/>
     <property key="allowOffcutReferencesToFunctionStarts" value="true"/>
     <property key="useNewFunctionStackAnalysis" value="true"/>
+    <property key="enableContiguousFunctionsOnly" value="false"/>
     <property key="emulateInstructionStateModifierClass" value="ghidra.program.emulation.ARMEmulateInstructionStateModifier"/>
   </properties>
   <programcounter register="pc"/>

Ghidra/Processors/ARM/data/languages/ARM_v45.pspec

@@ -6,6 +6,7 @@
     <property key="allowOffcutReferencesToFunctionStarts" value="true"/>
     <property key="useNewFunctionStackAnalysis" value="true"/>
     <property key="enableSharedReturnAnalysis" value="false"/>
+    <property key="enableContiguousFunctionsOnly" value="false"/>
     <property key="emulateInstructionStateModifierClass" value="ghidra.program.emulation.ARMEmulateInstructionStateModifier"/>
   </properties>
   <programcounter register="pc"/>

Ghidra/Processors/ARM/data/languages/ARMt.pspec

@@ -5,6 +5,7 @@
     <property key="addressesDoNotAppearDirectlyInCode" value="true"/>
     <property key="allowOffcutReferencesToFunctionStarts" value="true"/>
     <property key="useNewFunctionStackAnalysis" value="true"/>
+    <property key="enableContiguousFunctionsOnly" value="false"/>
     <property key="emulateInstructionStateModifierClass" value="ghidra.program.emulation.ARMEmulateInstructionStateModifier"/>
     <property key="assemblyRating:ARM:BE:32:v7" value="PLATINUM"/>
     <property key="assemblyRating:ARM:LE:32:v7" value="PLATINUM"/>

Ghidra/Processors/ARM/data/languages/ARMtTHUMB.pspec

@@ -6,6 +6,7 @@
     <property key="addressesDoNotAppearDirectlyInCode" value="true"/>
     <property key="allowOffcutReferencesToFunctionStarts" value="true"/>
     <property key="useNewFunctionStackAnalysis" value="true"/>
+    <property key="enableContiguousFunctionsOnly" value="false"/>
     <property key="emulateInstructionStateModifierClass" value="ghidra.program.emulation.ARMEmulateInstructionStateModifier"/>
     <property key="assemblyRating:ARM:BE:32:v7" value="PLATINUM"/>
     <property key="assemblyRating:ARM:LE:32:v7" value="PLATINUM"/>

Ghidra/Processors/ARM/data/languages/ARMt_v45.pspec

@@ -6,6 +6,7 @@
     <property key="allowOffcutReferencesToFunctionStarts" value="true"/>
     <property key="useNewFunctionStackAnalysis" value="true"/>
     <property key="enableSharedReturnAnalysis" value="false"/>
+    <property key="enableContiguousFunctionsOnly" value="false"/>
     <property key="emulateInstructionStateModifierClass" value="ghidra.program.emulation.ARMEmulateInstructionStateModifier"/>
   </properties>
   <programcounter register="pc"/>

Ghidra/Processors/ARM/data/languages/ARMt_v6.pspec

@@ -5,6 +5,7 @@
     <property key="addressesDoNotAppearDirectlyInCode" value="true"/>
     <property key="allowOffcutReferencesToFunctionStarts" value="true"/>
     <property key="useNewFunctionStackAnalysis" value="true"/>
+    <property key="enableContiguousFunctionsOnly" value="false"/>
     <property key="emulateInstructionStateModifierClass" value="ghidra.program.emulation.ARMEmulateInstructionStateModifier"/>
     <property key="assemblyRating:ARM:BE:32:v7" value="PLATINUM"/>
     <property key="assemblyRating:ARM:LE:32:v7" value="PLATINUM"/>

Ghidra/Processors/MIPS/src/main/java/ghidra/app/plugin/core/analysis/MipsAddressAnalyzer.java

@@ -704,33 +704,40 @@ public class MipsAddressAnalyzer extends ConstantPropagationAnalyzer {
 
 		return false;
 	}
+	
+	
+
+	@Override
+	public void registerOptions(Options options, Program program) {
+		super.registerOptions(options, program);
+		
+		options.registerOption(OPTION_NAME_SWITCH_TABLE, trySwitchTables, null,
+			OPTION_DESCRIPTION_SWITCH_TABLE);
+
+		options.registerOption(OPTION_NAME_MARK_DUAL_INSTRUCTION,
+			markupDualInstructionOption, null, OPTION_DESCRIPTION_MARK_DUAL_INSTRUCTION);
+
+		options.registerOption(OPTION_NAME_ASSUME_T9_ENTRY, assumeT9EntryAddress, null,
+			OPTION_DESCRIPTION_ASSUME_T9_ENTRY);
+
+		options.registerOption(OPTION_NAME_RECOVER_GP, discoverGlobalGPSetting, null,
+			OPTION_DESCRIPTION_RECOVER_GP);
+	}
 
 	@Override
 	public void optionsChanged(Options options, Program program) {
 		super.optionsChanged(options, program);
 
-		options.registerOption(OPTION_NAME_SWITCH_TABLE, OPTION_DEFAULT_SWITCH_TABLE, null,
+		trySwitchTables = options.getBoolean(OPTION_NAME_SWITCH_TABLE, trySwitchTables);
-			OPTION_DESCRIPTION_SWITCH_TABLE);
-
-		options.registerOption(OPTION_NAME_MARK_DUAL_INSTRUCTION,
-			OPTION_DEFAULT_MARK_DUAL_INSTRUCTION, null, OPTION_DESCRIPTION_MARK_DUAL_INSTRUCTION);
-
-		options.registerOption(OPTION_NAME_ASSUME_T9_ENTRY, OPTION_DEFAULT_ASSUME_T9_ENTRY, null,
-			OPTION_DESCRIPTION_ASSUME_T9_ENTRY);
-
-		options.registerOption(OPTION_NAME_RECOVER_GP, OPTION_DEFAULT_RECOVER_GP, null,
-			OPTION_DESCRIPTION_RECOVER_GP);
-
-		trySwitchTables = options.getBoolean(OPTION_NAME_SWITCH_TABLE, OPTION_DEFAULT_SWITCH_TABLE);
 
 		markupDualInstructionOption = options.getBoolean(OPTION_NAME_MARK_DUAL_INSTRUCTION,
-			OPTION_DEFAULT_MARK_DUAL_INSTRUCTION);
+			markupDualInstructionOption);
 
 		assumeT9EntryAddress =
-			options.getBoolean(OPTION_NAME_ASSUME_T9_ENTRY, OPTION_DEFAULT_ASSUME_T9_ENTRY);
+			options.getBoolean(OPTION_NAME_ASSUME_T9_ENTRY, assumeT9EntryAddress);
 
 		discoverGlobalGPSetting =
-			options.getBoolean(OPTION_NAME_RECOVER_GP, OPTION_DEFAULT_RECOVER_GP);
+			options.getBoolean(OPTION_NAME_RECOVER_GP, discoverGlobalGPSetting);
 	}
 
 }