GP-3599 Fix for function bodies including one byte of non-disassembled

data.  PowerPC disassembly from computed branch.

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CreateFunctionCmd.java

@@ -637,7 +637,7 @@ public class CreateFunctionCmd extends BackgroundCommand {
 		FlowType[] dontFollow = { RefType.COMPUTED_CALL, RefType.CONDITIONAL_CALL,
 			RefType.UNCONDITIONAL_CALL, RefType.INDIRECTION };
 		AddressSet start = new AddressSet(entry, entry);
-		FollowFlow flow = new FollowFlow(program, start, dontFollow, includeOtherFunctions);
+		FollowFlow flow = new FollowFlow(program, start, dontFollow, includeOtherFunctions, false);
 		return flow.getFlowAddressSet(monitor);
 	}
 

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/AbstractFollowFlowTest.java

@@ -74,6 +74,19 @@ public abstract class AbstractFollowFlowTest extends AbstractGhidraHeadedIntegra
 		return followFlow.getFlowAddressSet(TaskMonitor.DUMMY);
 	}
 
+	AddressSetView getFlowsFrom(int startAddressOffset, FlowType[] excludedFlows, boolean includeFunctions, boolean includeData) {
+		return getFlowsFrom(addr(startAddressOffset), excludedFlows, includeFunctions, includeData);
+	}
+
+	AddressSetView getFlowsFrom(Address startAddress, FlowType[] excludedFlows, boolean includeFunctions, boolean includeData) {
+		return getFlowsFrom(new AddressSet(startAddress), excludedFlows, includeFunctions, includeData);
+	}
+	
+	AddressSetView getFlowsFrom(AddressSet startSet, FlowType[] excludedFlows, boolean includeFunctions, boolean includeData) {
+		FollowFlow followFlow = new FollowFlow(program, startSet, excludedFlows, includeFunctions, includeData);
+		return followFlow.getFlowAddressSet(TaskMonitor.DUMMY);
+	}
+
 	AddressSetView getFlowsTo(int startAddressOffset, FlowType[] excludedFlows) {
 		return getFlowsTo(addr(startAddressOffset), excludedFlows);
 	}

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowFlowForwardTest.java

@@ -81,6 +81,33 @@ public class FollowFlowForwardTest extends AbstractFollowFlowTest {
 		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
 	}
 	
+	@Test
+	public void testFollowAllFlowsFromNoData0x10() {
+
+		AddressSetView flowAddresses = getFlowsFrom(0x10, followAllFlows(), true, false);
+
+		AddressSet expectedAddresses = new AddressSet();
+		expectedAddresses.add(addr(0x0), addr(0x24));
+		expectedAddresses.add(addr(0x26), addr(0x2f));
+		expectedAddresses.add(addr(0x30), addr(0x52));
+		expectedAddresses.add(addr(0x54), addr(0x5f));
+		expectedAddresses.add(addr(0x60), addr(0x84));
+		expectedAddresses.add(addr(0x86), addr(0x8f));
+		expectedAddresses.add(addr(0x90), addr(0xb4));
+		expectedAddresses.add(addr(0xb6), addr(0xbf));
+		expectedAddresses.add(addr(0x130), addr(0x131));
+		expectedAddresses.add(addr(0x160), addr(0x161));
+		expectedAddresses.add(addr(0x190), addr(0x191));
+		expectedAddresses.add(addr(0x230), addr(0x231));
+		expectedAddresses.add(addr(0x260), addr(0x261));
+		expectedAddresses.add(addr(0x290), addr(0x291));
+		expectedAddresses.add(addr(0x330), addr(0x331));
+		expectedAddresses.add(addr(0x360), addr(0x361));
+		expectedAddresses.add(addr(0x390), addr(0x391));
+
+		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
+	}
+
 	@Test
 	public void testFollowAllFlowsFrom0x17() {
 
@@ -104,6 +131,27 @@ public class FollowFlowForwardTest extends AbstractFollowFlowTest {
 
 		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
 	}
+	@Test
+	public void testFollowAllFlowsFromNoData0x17() {
+
+		AddressSetView flowAddresses = getFlowsFrom(0x17, followAllFlows(), true, false);
+
+		AddressSet expectedAddresses = new AddressSet();
+		expectedAddresses.add(addr(0x17), addr(0x24));
+		expectedAddresses.add(addr(0x26), addr(0x2f));
+		expectedAddresses.add(addr(0x60), addr(0x84));
+		expectedAddresses.add(addr(0x86), addr(0x8f));
+		expectedAddresses.add(addr(0x90), addr(0xb4));
+		expectedAddresses.add(addr(0xb6), addr(0xbf));
+		expectedAddresses.add(addr(0x230), addr(0x231));
+		expectedAddresses.add(addr(0x260), addr(0x261));
+		expectedAddresses.add(addr(0x290), addr(0x291));
+		expectedAddresses.add(addr(0x330), addr(0x331));
+		expectedAddresses.add(addr(0x360), addr(0x361));
+		expectedAddresses.add(addr(0x390), addr(0x391));
+
+		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
+	}
 
 	@Test
 	public void testFollowAllFlowsFrom0x2f() {
@@ -160,6 +208,20 @@ public class FollowFlowForwardTest extends AbstractFollowFlowTest {
 		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
 	}
 	
+	@Test
+	public void testFollowAllFlowsFromNoData0x77() {
+
+		AddressSetView flowAddresses = getFlowsFrom(0x77, followAllFlows(), true, false);
+
+		AddressSet expectedAddresses = new AddressSet();
+		expectedAddresses.add(addr(0x77), addr(0x84));
+		expectedAddresses.add(addr(0x86), addr(0x8f));
+		expectedAddresses.add(addr(0x260), addr(0x261));
+		expectedAddresses.add(addr(0x290), addr(0x291));
+
+		assertEquals(new MySelection(expectedAddresses), new MySelection(flowAddresses));
+	}
+
 	@Test
 	public void testFollowAllFlowsFrom0x5000() {
 

Ghidra/Features/Base/src/test/java/ghidra/app/cmd/function/CreateFunctionCmdWithFlowTest.java

@@ -0,0 +1,197 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.app.cmd.function;
+
+import static org.junit.Assert.*;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import generic.test.AbstractGenericTest;
+import ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand;
+import ghidra.app.plugin.core.analysis.AutoAnalysisManager;
+import ghidra.framework.cmd.Command;
+import ghidra.framework.options.Options;
+import ghidra.framework.plugintool.PluginTool;
+import ghidra.program.database.ProgramBuilder;
+import ghidra.program.database.function.OverlappingFunctionException;
+import ghidra.program.model.address.*;
+import ghidra.program.model.listing.*;
+import ghidra.program.model.symbol.SourceType;
+import ghidra.test.AbstractGhidraHeadedIntegrationTest;
+import ghidra.test.TestEnv;
+
+/**
+ * Test for the {@link CreateFunctionCmdWithFlowTest}.
+ */
+public class CreateFunctionCmdWithFlowTest extends AbstractGhidraHeadedIntegrationTest {
+
+	private TestEnv env;
+	private PluginTool tool;
+	
+	private Program program;
+	private ProgramBuilder builder;
+
+	@Before
+	public void setUp() throws Exception {
+		env = new TestEnv();
+		tool = env.getTool();
+
+		builder = new ProgramBuilder("notepad.exe", ProgramBuilder._PPC_32);
+		builder.createMemory("test", "0x07000000", 1024);
+
+		program = builder.getProgram();
+
+		//
+		// Create some functions (byte patterns, not Ghidra objects) with varying separation
+		//
+		// single function
+		builder.setBytes("0x07000008", "3d 60 07 00 61 6b 00 20 7d 69 03 a6 4e 80 04 20");
+		builder.disassemble("0x07000008", 16);
+		builder.createMemoryJumpReference("0x070000014", "0x07000020");
+
+		// Thunk to above single function
+		builder.setBytes("0x07000020", "7c 69 1b 78 88 04 00 00 38 84 00 01 7c 00 07 74 2f 80 00 00 98 09 00 00 39 29 00 01 40 9e ff e8 4e 80 00 20");
+	}
+	
+	private void analyze() {
+		// turn off some analyzers
+		setAnalysisOptions("Stack");
+		setAnalysisOptions("Embedded Media");
+		setAnalysisOptions("DWARF");
+		setAnalysisOptions("Create Address Tables");
+		setAnalysisOptions("MIPS Constant Reference Analyzer");
+
+		AutoAnalysisManager analysisMgr = AutoAnalysisManager.getAnalysisManager(program);
+		analysisMgr.reAnalyzeAll(null);
+
+		Command cmd = new AnalysisBackgroundCommand(analysisMgr, false);
+		tool.execute(cmd, program);
+		waitForBusyTool(tool);
+	}
+	
+	protected void setAnalysisOptions(String optionName) {
+		int txId = program.startTransaction("Analyze");
+		Options analysisOptions = program.getOptions(Program.ANALYSIS_PROPERTIES);
+		analysisOptions.setBoolean(optionName, false);
+		program.endTransaction(txId, true);
+	}
+
+	@Test
+	public void testCreateFunction() {
+
+		int transactionID = program.startTransaction("Perform the TEST");
+
+		CreateFunctionCmd createCmd = new CreateFunctionCmd(addr(0x07000008));
+		createCmd.applyTo(program);
+
+		program.endTransaction(transactionID, true);
+
+		Function func8 = func(addr(0x07000008));
+		assertNotNull("Created normal function", func8);
+
+		assertEquals("Normal function body size", 16, func8.getBody().getNumAddresses());
+	}
+	
+	@Test
+	public void testCreateFunctionOneByte() throws OverlappingFunctionException {
+
+		int transactionID = program.startTransaction("Perform the TEST");
+
+		CreateFunctionCmd createCmd = new CreateFunctionCmd(addr(0x07000008));
+		createCmd.applyTo(program);
+		
+		// doctor body
+		AddressSet body = new AddressSet(addr(0x07000008),addr(0x07000017));
+		body.add(addr(0x07000020));
+		
+		Function func8 = func(addr(0x07000008));
+		
+		func8.setBody(body);
+
+		assertEquals("Normal function body size", 17, func8.getBody().getNumAddresses());
+
+		builder.disassemble("0x07000020", 36);
+		
+		createCmd = new CreateFunctionCmd(addr(0x07000020));
+		createCmd.applyTo(program);
+
+		program.endTransaction(transactionID, true);
+
+		assertNotNull("Created normal function", func8);
+
+		assertEquals("Normal function body size", 16, func8.getBody().getNumAddresses());
+		
+		Function func20 = func(addr(0x07000020));
+		
+		assertNotNull("Created normal function", func20);
+
+		assertEquals("Normal function body size", 36, func20.getBody().getNumAddresses());
+	}
+	
+	@Test
+	public void testPPCDisassemblyRef() throws OverlappingFunctionException {
+
+		int transactionID = program.startTransaction("Perform the TEST");
+
+		CreateFunctionCmd createCmd = new CreateFunctionCmd(addr(0x07000008));
+		createCmd.applyTo(program);
+		
+		Function func8 = func(addr(0x07000008));
+		
+		program.getMemory().getBlock(addr(0x07000000)).setExecute(true);
+		
+		assertFalse("is not Thunk yet", func8.isThunk());
+		
+		Instruction instructionAt = program.getListing().getInstructionAt(addr(0x07000020));
+		
+		assertNull("Not disassembled yet", instructionAt);
+		
+		builder.analyze();
+		
+		assertNotNull("Created normal function", func8);
+
+		assertEquals("Normal function body size", 16, func8.getBody().getNumAddresses());
+		
+		instructionAt = program.getListing().getInstructionAt(addr(0x07000020));
+		
+		assertNotNull("Disassembled from computed branch", instructionAt);
+		
+		createCmd = new CreateFunctionCmd(addr(0x07000020));
+		createCmd.applyTo(program);
+		
+		Function func20 = func(addr(0x07000020));
+		
+		builder.analyze();
+
+		program.endTransaction(transactionID, true);
+		
+		assertTrue("is Thunk ", func8.isThunk());
+		
+		assertEquals("Normal function body size", 36, func20.getBody().getNumAddresses());
+	}
+
+	private Address addr(long l) {
+		AddressSpace addressSpace = program.getAddressFactory().getDefaultAddressSpace();
+		return addressSpace.getAddress(l);
+	}
+
+	private Function func(Address a) {
+		FunctionManager fm = program.getFunctionManager();
+		return fm.getFunctionAt(a);
+	}
+
+}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/block/FollowFlow.java

@@ -46,11 +46,15 @@ public class FollowFlow {
 	private boolean followPointers = true;
 
 	private boolean followIntoFunction = true;
+	private boolean includeData = true;
 	private Address nextSymbolAddr;
 
 	/**
 	 * Constructor
 	 * 
+	 * Note: flow into existing functions will be included
+	 * Note: flow into un-disassembled locations will be included
+	 *
 	 * @param program the program whose flow we are following.
 	 * @param addressSet the initial addresses that should be flowed from or flowed to.
 	 * @param doNotFollow array of flow types that are not to be followed.
@@ -63,6 +67,7 @@ public class FollowFlow {
 	 * <BR>FlowType.CONDITIONAL_JUMP
 	 * <BR>FlowType.UNCONDITIONAL_JUMP
 	 * <BR>FlowType.INDIRECTION
+	 *
 	 */
 	public FollowFlow(Program program, AddressSet addressSet, FlowType[] doNotFollow) {
 		this.program = program;
@@ -73,6 +78,8 @@ public class FollowFlow {
 	/**
 	 * Constructor
 	 * 
+	 * Note: flow into un-disassembled locations will be included
+	 * 
 	 * @param program the program whose flow we are following.
 	 * @param addressSet the initial addresses that should be flowed from or flowed to.
 	 * @param doNotFollow array of flow types that are not to be followed.
@@ -94,6 +101,31 @@ public class FollowFlow {
 		this.followIntoFunction = followIntoFunctions;
 	}
 	
+	/**
+	 * Constructor
+	 * 
+	 * @param program the program whose flow we are following.
+	 * @param addressSet the initial addresses that should be flowed from or flowed to.
+	 * @param doNotFollow array of flow types that are not to be followed.
+	 * null or empty array indicates follow all flows. The following are valid
+	 * flow types for the doNotFollow array:
+	 * <BR>FlowType.COMPUTED_CALL
+	 * <BR>FlowType.CONDITIONAL_CALL
+	 * <BR>FlowType.UNCONDITIONAL_CALL
+	 * <BR>FlowType.COMPUTED_JUMP
+	 * <BR>FlowType.CONDITIONAL_JUMP
+	 * <BR>FlowType.UNCONDITIONAL_JUMP
+	 * <BR>FlowType.INDIRECTION
+	 * @param followIntoFunctions true if flows into (or back from) defined functions
+	 * should be followed.
+	 * @param includeData true if instruction flows into un-disassembled data should be included
+	 */
+	public FollowFlow(Program program, AddressSet addressSet, FlowType[] doNotFollow,
+			boolean followIntoFunctions, boolean includeData) {
+		this(program, addressSet, doNotFollow, followIntoFunctions);
+		this.includeData = includeData;
+	}
+
 	/**
 	 * updateFollowFlags
 	 *
@@ -289,7 +321,9 @@ public class FollowFlow {
 			codeUnit = instructionStack.pop();
 			if (!(codeUnit instanceof Instruction)) {
 				// Probably undefined data which should be disassembled
-				flowAddressSet.addRange(codeUnit.getMinAddress(), codeUnit.getMaxAddress());
+				if (includeData) {
+					flowAddressSet.addRange(codeUnit.getMinAddress(), codeUnit.getMaxAddress());
+				}
 				continue;
 			}
 
@@ -475,7 +509,7 @@ public class FollowFlow {
 			if (nextAddress != null) {
 				CodeUnit nextCodeUnit = program.getListing().getCodeUnitContaining(nextAddress);
 				if (nextCodeUnit != null) {
-					if (nextCodeUnit instanceof Data) {
+					if (nextCodeUnit instanceof Data && includeData) {
 						followData(instructionStack, flowAddressSet, (Data) nextCodeUnit,
 							nextAddress);
 					}

Ghidra/Processors/PowerPC/src/main/java/ghidra/app/plugin/core/analysis/PowerPCAddressAnalyzer.java

@@ -226,7 +226,9 @@ public class PowerPCAddressAnalyzer extends ConstantPropagationAnalyzer {
 				public boolean evaluateReference(VarnodeContext context, Instruction instr,
 						int pcodeop, Address address, int size, DataType dataType, RefType refType) {
 
-					if (instr.getFlowType().isJump()) {
+					if (refType.isJump() && refType.isComputed() &&
+							program.getMemory().contains(address) && address.getOffset() != 0) {
+						super.evaluateReference(context, instr, pcodeop, address, size, dataType, refType);
 						// for branching instructions, if we have a good target, mark it
 						// if this isn't straight code (thunk computation), let someone else lay down the reference
 						return !symEval.encounteredBranch();