GP-567 Ensure that function bodies are within a single address space only.

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java

@@ -133,7 +133,8 @@ public abstract class AbstractDBTraceProgramViewMemoryBlock implements MemoryBlo
 		return program.trace.getMemoryManager().getMemorySpace(getAddressSpace(), false);
 	}
 
-	protected AddressRange getAddressRange() {
+	@Override
+	public AddressRange getAddressRange() {
 		return new AddressRangeImpl(getStart(), getEnd());
 	}
 

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemoryRegionBlock.java

@@ -45,7 +45,7 @@ public class DBTraceProgramViewMemoryRegionBlock extends AbstractDBTraceProgramV
 	}
 
 	@Override
-	protected AddressRange getAddressRange() {
+	public AddressRange getAddressRange() {
 		return region.getRange();
 	}
 

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisterMemoryBlock.java

@@ -130,6 +130,11 @@ public class DBTraceProgramViewRegisterMemoryBlock implements MemoryBlock {
 		return range.contains(addr);
 	}
 
+	@Override
+	public AddressRange getAddressRange() {
+		return range;
+	}
+
 	@Override
 	public Address getStart() {
 		return range.getMinAddress();

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

@@ -279,6 +279,7 @@ public class CreateFunctionCmd extends BackgroundCommand {
 				// if we are not recreating the function,
 				//  then don't continue because there is already a function here.
 				if (!recreateFunction) {
+					entry = functionContaining.getEntryPoint(); // preserve entry
 					long bodySize = functionContaining.getBody().getNumAddresses();
 					if (bodySize != 1) {
 						return false;
@@ -545,26 +546,29 @@ public class CreateFunctionCmd extends BackgroundCommand {
 	}
 
 	/**
-	 * Follow flow back from the address trying to find an existing function this fragment belongs to
+	 * Follow flow back from the address trying to find an existing function or reasonable entry point 
+	 * that flows to the specified bodyAddr.  Search is very limited and gives preference to a contiguous 
+	 * fall-through flow.
 	 *
 	 * @param bodyAddr address that should be in the body of a function
-	 * @return
+	 * @return a possible entry point that flows to bodyAddr or null if a reasonable entry not found.
 	 */
 	private Address findFunctionEntry(Address bodyAddr) {
-		Address entry = bodyAddr;
+
+		AddressSpace space = bodyAddr.getAddressSpace();
 
 		// if there is no function, then just follow some flow backwards
 		AddressSet subSet = new AddressSet();
-		Instruction followInstr = program.getListing().getInstructionContaining(entry);
+		Instruction followInstr = program.getListing().getInstructionContaining(bodyAddr);
-		while (followInstr != null && !subSet.contains(followInstr.getMinAddress())) {
+		while (followInstr != null && !subSet.contains(followInstr.getMinAddress()) &&
+			followInstr.getMinAddress().getAddressSpace() == space) {
 			subSet.addRange(followInstr.getMinAddress(), followInstr.getMaxAddress());
 
 			// see if we have wandered backward into a function
 			Function func =
 				program.getFunctionManager().getFunctionContaining(followInstr.getMinAddress());
 			if (func != null) {
-				entry = func.getEntryPoint();
+				return func.getEntryPoint();
-				break;
 			}
 			Address fallFrom = followInstr.getFallFrom();
 			if (fallFrom == null) {
@@ -572,17 +576,17 @@ public class CreateFunctionCmd extends BackgroundCommand {
 				if (!iter.hasNext()) {
 					break;
 				}
+				// TODO: only considering one reference which may not be a flow
 				Reference ref = iter.next();
 				if (ref.getReferenceType().isCall()) {
-					entry = fallFrom;
+					return followInstr.getMinAddress(); // may not be in a function body
-					break;
 				}
 				fallFrom = ref.getFromAddress();
 			}
 			followInstr = program.getListing().getInstructionContaining(fallFrom);
 		}
 
-		return entry;
+		return null;
 	}
 
 	/**
@@ -636,8 +640,8 @@ 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 =
-		FollowFlow flow = new FollowFlow(program, start, dontFollow, includeOtherFunctions, false);
+			new FollowFlow(program, entry, dontFollow, includeOtherFunctions, false, true);
 		return flow.getFlowAddressSet(monitor);
 	}
 

Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/function/FunctionDBTest.java

@@ -35,6 +35,7 @@ import ghidra.program.model.lang.CompilerSpec;
 import ghidra.program.model.lang.Register;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.listing.Function.FunctionUpdateType;
+import ghidra.program.model.mem.MemoryBlock;
 import ghidra.program.model.symbol.*;
 import ghidra.program.util.ChangeManager;
 import ghidra.program.util.ProgramChangeRecord;
@@ -131,13 +132,38 @@ public class FunctionDBTest extends AbstractGhidraHeadedIntegrationTest
 	private Function createFunction(String name, Address entryPt, AddressSetView body)
 			throws DuplicateNameException, InvalidInputException, OverlappingFunctionException {
 
-		functionManager.createFunction(name, entryPt, body, SourceType.USER_DEFINED);
+		Function f = functionManager.createFunction(name, entryPt, body, SourceType.USER_DEFINED);
-		Function f = functionManager.getFunctionAt(entryPt);
+		assertNotNull(f);
+		assertEquals(f, functionManager.getFunctionAt(entryPt));
 		assertEquals(entryPt, f.getEntryPoint());
 		assertEquals(body, f.getBody());
 		return f;
 	}
 
+	@Test
+	public void testCreateFunctionBodyRestrictions() throws Exception
+	{
+		MemoryBlock ovBlock =
+			program.getMemory().createUninitializedBlock("OV", addr(200), 100, true);
+		try {
+			AddressSet set = new AddressSet(addr(100), addr(200));
+			set.add(ovBlock.getAddressRange());
+			createFunction("foo", addr(100), set);
+			fail("Expected body must contain single address space only");
+		}
+		catch (IllegalArgumentException e) {
+			assertTrue(e.getMessage().contains("body must contain single address space only"));
+		}
+
+		try {
+			createFunction("foo", addr(100), new AddressSet(addr(150), addr(200)));
+			fail("Expected body must contain entry point exception");
+		}
+		catch (IllegalArgumentException e) {
+			assertTrue(e.getMessage().contains("body must contain the entrypoint"));
+		}
+	}
+
 	@Test
 	public void testGetName() throws Exception {
 		Function f = createFunction("foo", addr(100), new AddressSet(addr(100), addr(200)));
@@ -397,7 +423,7 @@ public class FunctionDBTest extends AbstractGhidraHeadedIntegrationTest
 	}
 
 	@Test
-	public void testSetBodyInvalidEntryPoint() throws Exception {
+	public void testSetInvalidBody() throws Exception {
 
 		AddressSet asv = new AddressSet();
 		asv.addRange(addr(100), addr(350));
@@ -407,17 +433,27 @@ public class FunctionDBTest extends AbstractGhidraHeadedIntegrationTest
 		Function f = createFunction("foo", addr(100), asv);
 		functionManager.invalidateCache(false);
 		f = functionManager.getFunctionAt(addr(100));
-		asv = new AddressSet();
-		asv.addRange(addr(110), addr(120));
-		asv.addRange(addr(300), addr(400));
-		asv.addRange(addr(10), addr(20));
 
 		try {
+			asv = new AddressSet();
+			asv.addRange(addr(110), addr(120));
 			f.setBody(asv);
-			Assert.fail(
+			fail("Expected exception: body must contain entry point");
-				"Should have gotten illegal argument exception: original entry point not in new body");
 		}
 		catch (IllegalArgumentException e) {
+			assertTrue(e.getMessage().contains("body must contain the entry point"));
+		}
+
+		MemoryBlock ovBlock =
+			program.getMemory().createUninitializedBlock("OV", addr(200), 100, true);
+		try {
+			asv = new AddressSet(addr(100), addr(200));
+			asv.add(ovBlock.getAddressRange());
+			f.setBody(asv);
+			fail("Expected exception: body must contain single address space only");
+		}
+		catch (IllegalArgumentException e) {
+			assertTrue(e.getMessage().contains("body must contain single address space only"));
 		}
 
 	}

Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/checksums/MyTestMemoryBlock.java

@@ -19,7 +19,7 @@ import java.io.InputStream;
 import java.math.BigInteger;
 import java.util.List;
 
-import ghidra.program.model.address.Address;
+import ghidra.program.model.address.*;
 import ghidra.program.model.mem.*;
 
 class MyTestMemoryBlock implements MemoryBlock {
@@ -46,6 +46,11 @@ class MyTestMemoryBlock implements MemoryBlock {
 		throw new UnsupportedOperationException();
 	}
 
+	@Override
+	public AddressRange getAddressRange() {
+		return new AddressRangeImpl(start, end);
+	}
+
 	@Override
 	public Address getStart() {
 		return start;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionManagerDB.java

@@ -219,6 +219,13 @@ public class FunctionManagerDB implements FunctionManager {
 		}
 	}
 
+	static void checkSingleAddressSpaceOnly(AddressSetView set) {
+		if (set.getMinAddress().getAddressSpace() != set.getMaxAddress().getAddressSpace()) {
+			throw new IllegalArgumentException(
+					"Function body must contain single address space only");
+		}
+	}
+
 	private Function createFunction(String name, Namespace nameSpace, Address entryPoint,
 			AddressSetView body, Function thunkedFunction, SourceType source)
 			throws InvalidInputException, OverlappingFunctionException {
@@ -231,11 +238,15 @@ public class FunctionManagerDB implements FunctionManager {
 			if (body == null || !body.contains(entryPoint)) {
 				throw new IllegalArgumentException("Function body must contain the entrypoint");
 			}
+			if (body.getNumAddresses() > Integer.MAX_VALUE) {
+				throw new IllegalArgumentException(
+					"Function body size must be <= 0x7fffffff byte addresses");
+			}
 			if (codeMgr.getDefinedDataAt(entryPoint) != null) {
 				throw new IllegalArgumentException(
 					"Function entryPoint may not be created on defined data");
 			}
-
+			checkSingleAddressSpaceOnly(body);
 			if (namespaceMgr.overlapsNamespace(body) != null) {
 				throw new OverlappingFunctionException(entryPoint);
 			}
@@ -990,6 +1001,7 @@ public class FunctionManagerDB implements FunctionManager {
 			throw new IllegalArgumentException(
 				"Function body size must be <= 0x7fffffff byte addresses");
 		}
+		checkSingleAddressSpaceOnly(newBody);
 		AddressSetView oldBody = function.getBody();
 
 		try {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryBlockDB.java

@@ -155,6 +155,16 @@ public class MemoryBlockDB implements MemoryBlock {
 		return NumericUtilities.unsignedLongToBigInteger(length);
 	}
 
+	@Override
+	public AddressRange getAddressRange() {
+		try {
+			return new AddressRangeImpl(startAddress, length);
+		}
+		catch (AddressOverflowException e) {
+			throw new RuntimeException(e); // unexpected
+		}
+	}
+
 	@Override
 	public String getName() {
 		String name = record.getString(MemoryMapDBAdapter.NAME_COL);

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

@@ -22,7 +22,6 @@ import ghidra.program.model.listing.*;
 import ghidra.program.model.mem.Memory;
 import ghidra.program.model.symbol.*;
 import ghidra.util.task.TaskMonitor;
-import ghidra.util.task.TaskMonitorAdapter;
 
 /**
  * FollowFlow follows the program's code flow either forward or backward from an initial
@@ -34,7 +33,7 @@ import ghidra.util.task.TaskMonitorAdapter;
  */
 public class FollowFlow {
 	private Program program;
-	private AddressSet initialAddresses;
+	private AddressSetView initialAddresses;
 
 	private boolean followAllFlow = true;
 	private boolean followComputedCall = true;
@@ -47,6 +46,7 @@ public class FollowFlow {
 
 	private boolean followIntoFunction = true;
 	private boolean includeData = true;
+	private AddressSpace restrictedAddressSpace = null; // if set restrict flow to this space only
 	private Address nextSymbolAddr;
 
 	/**
@@ -69,7 +69,7 @@ public class FollowFlow {
 	 * <BR>FlowType.INDIRECTION
 	 *
 	 */
-	public FollowFlow(Program program, AddressSet addressSet, FlowType[] doNotFollow) {
+	public FollowFlow(Program program, AddressSetView addressSet, FlowType[] doNotFollow) {
 		this.program = program;
 		this.initialAddresses = addressSet;
 		updateFollowFlags(doNotFollow);
@@ -95,7 +95,7 @@ public class FollowFlow {
 	 * @param followIntoFunctions true if flows into (or back from) defined functions
 	 * should be followed.
 	 */
-	public FollowFlow(Program program, AddressSet addressSet, FlowType[] doNotFollow,
+	public FollowFlow(Program program, AddressSetView addressSet, FlowType[] doNotFollow,
 			boolean followIntoFunctions) {
 		this(program, addressSet, doNotFollow);
 		this.followIntoFunction = followIntoFunctions;
@@ -126,6 +126,36 @@ public class FollowFlow {
 		this.includeData = includeData;
 	}
 
+	/**
+	 * Constructor
+	 * 
+	 * @param program the program whose flow we are following.
+	 * @param address the initial address that should be flowed from or flowed to.
+	 * @param doNotFollow array of flow types that are not to be followed.
+	 * @param restrictSingleAddressSpace if true collected flows should be restricted to
+	 * a single address space identified by {@code address}.
+	 * 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, Address address, FlowType[] doNotFollow,
+			boolean followIntoFunctions, boolean includeData, boolean restrictSingleAddressSpace) {
+		this(program, new AddressSet(address, address), doNotFollow, followIntoFunctions);
+		if (restrictSingleAddressSpace) {
+			restrictedAddressSpace = address.getAddressSpace();
+		}
+		this.includeData = includeData;
+	}
+
 	/**
 	 * updateFollowFlags
 	 *
@@ -171,7 +201,7 @@ public class FollowFlow {
 	 * @param forward true to determine the flows "from" the startAddresses. false (backward) to 
 	 * determine flows "to" the startAddresses.
 	 */
-	private AddressSet getAddressFlow(TaskMonitor monitor, AddressSet startAddresses,
+	private AddressSet getAddressFlow(TaskMonitor monitor, AddressSetView startAddresses,
 			boolean forward) {
 
 		if (monitor == null) {
@@ -233,7 +263,7 @@ public class FollowFlow {
 	 * @param forward true to determine the flows from the code unit. false to determine flows
 	 * to the code unit.
 	 */
-	private void getCodeUnitFlow(TaskMonitor monitor, AddressSet startAddresses,
+	private void getCodeUnitFlow(TaskMonitor monitor, AddressSetView startAddresses,
 			AddressSet flowAddressSet, CodeUnit codeUnit, boolean forward) {
 		if (codeUnit instanceof Data) {
 			getIndirectCodeFlow(monitor, startAddresses, flowAddressSet, (Data) codeUnit, forward);
@@ -256,7 +286,7 @@ public class FollowFlow {
 		}
 	}
 
-	private void getIndirectCodeFlow(TaskMonitor monitor, AddressSet startAddresses,
+	private void getIndirectCodeFlow(TaskMonitor monitor, AddressSetView startAddresses,
 			AddressSet flowAddressSet, Data data, boolean forward) {
 		// Follow data - isolate each primitive within startAddresses
 		if (!data.isDefined()) {
@@ -289,7 +319,7 @@ public class FollowFlow {
 	 * instruction code units.
 	 * @param monitor a cancellable task monitor
 	 * @param flowAddressSet the address set to be added to
-	 * @param currentCodeUnit the code unit to flow from.
+	 * @param codeUnit the code unit to flow from.
 	 *     Appropriate flows out of this code unit will be traversed.
 	 * @param dataAddr null or the address to flow from within the currentCodeUnit for Data.
 	 */
@@ -492,7 +522,7 @@ public class FollowFlow {
 	 * instruction code units.
 	 *
 	 * @param flowAddressSet the address set to add our addresses to.
-	 * @param currentCodeUnit the Instruction object to flow from.
+	 * @param currentInstr the Instruction object to flow from.
 	 *     Appropriate flows out of this code unit will be traversed.
 	 */
 	private void followInstruction(Stack<CodeUnit> instructionStack, AddressSet flowAddressSet,
@@ -506,7 +536,8 @@ public class FollowFlow {
 		Address[] flowAddresses = getFlowsFromInstruction(currentInstr);
 		for (int index = 0; (flowAddresses != null) && (index < flowAddresses.length); index++) {
 			nextAddress = flowAddresses[index];
-			if (nextAddress != null) {
+			if (nextAddress != null && (restrictedAddressSpace == null ||
+				restrictedAddressSpace == nextAddress.getAddressSpace())) {
 				CodeUnit nextCodeUnit = program.getListing().getCodeUnitContaining(nextAddress);
 				if (nextCodeUnit != null) {
 					if (nextCodeUnit instanceof Data && includeData) {
@@ -538,7 +569,8 @@ public class FollowFlow {
 			}
 		}
 
-		if (nextAddress != null) {
+		if (nextAddress != null && (restrictedAddressSpace == null ||
+			restrictedAddressSpace == nextAddress.getAddressSpace())) {
 			Instruction nextInstruction = program.getListing().getInstructionAt(nextAddress);
 			if (nextInstruction != null) {
 				instructionStack.push(nextInstruction);
@@ -705,7 +737,7 @@ public class FollowFlow {
 	 * matching the ones that should be followed will have the address it flows
 	 * to returned.
 	 * 
-	 * @param the instruction being flowed from.
+	 * @param instr the instruction being flowed from.
 	 * @return array of the addresses being flowed to in the manner we are
 	 * interested in.
 	 */
@@ -737,7 +769,7 @@ public class FollowFlow {
 	 * matching the ones that should be followed will have the address it flows
 	 * from returned.
 	 * 
-	 * @param the instruction being flowed to.
+	 * @param instr the instruction being flowed to.
 	 * @return array of the addresses that flow to the instruction in the manner we are
 	 * interested in.
 	 */
@@ -786,8 +818,9 @@ public class FollowFlow {
 	 * reference to an instruction. If the flow at the address isn't from a pointer to 
 	 * an instruction then just the address passed to this method is added to the flow set.
 	 *
+	 * @param instructionStack code unit stack for subsequent flow analysis
 	 * @param flowAddressSet the address set to add our addresses to.
-	 * @param currentCodeUnit the Data object to flow from.
+	 * @param data the Data object to flow from.
 	 *     Appropriate flows out of this code unit will be traversed.
 	 * @param addr the flow reference address which is contained within data.
 	 */

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlock.java

@@ -21,8 +21,7 @@ import java.math.BigInteger;
 import java.util.List;
 
 import ghidra.framework.store.LockException;
-import ghidra.program.model.address.Address;
+import ghidra.program.model.address.*;
-import ghidra.program.model.address.AddressSpace;
 import ghidra.program.model.symbol.OffsetReference;
 import ghidra.util.NamingUtilities;
 
@@ -86,6 +85,12 @@ public interface MemoryBlock extends Serializable, Comparable<MemoryBlock> {
 	 */
 	public Address getEnd();
 
+	/**
+	 * Get the address range that corresponds to this block.
+	 * @return block address range
+	 */
+	public AddressRange getAddressRange();
+
 	/**
 	 * Get the number of bytes in this block.
 	 * 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlockStub.java

@@ -20,7 +20,7 @@ import java.math.BigInteger;
 import java.util.List;
 
 import ghidra.framework.store.LockException;
-import ghidra.program.model.address.Address;
+import ghidra.program.model.address.*;
 
 /**
  * MemoryBlockStub can be extended for use by tests. It throws an UnsupportedOperationException for
@@ -70,6 +70,11 @@ public class MemoryBlockStub implements MemoryBlock {
 		return end;
 	}
 
+	@Override
+	public AddressRange getAddressRange() {
+		return new AddressRangeImpl(start, end);
+	}
+
 	@Override
 	public long getSize() {
 		throw new UnsupportedOperationException();