diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java index 12ee02e1a9..9c89f48518 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java +++ b/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()); } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemoryRegionBlock.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemoryRegionBlock.java index 76c7202e28..4fcc08b297 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemoryRegionBlock.java +++ b/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(); } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisterMemoryBlock.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisterMemoryBlock.java index 140ec1ef9e..d0e67cdff7 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisterMemoryBlock.java +++ b/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(); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CreateFunctionCmd.java b/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CreateFunctionCmd.java index c395312bd5..10b0aead10 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CreateFunctionCmd.java +++ b/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); - while (followInstr != null && !subSet.contains(followInstr.getMinAddress())) { + Instruction followInstr = program.getListing().getInstructionContaining(bodyAddr); + 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(); - break; + return func.getEntryPoint(); } 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; - break; + return followInstr.getMinAddress(); // may not be in a function body } 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 = new FollowFlow(program, start, dontFollow, includeOtherFunctions, false); + FollowFlow flow = + new FollowFlow(program, entry, dontFollow, includeOtherFunctions, false, true); return flow.getFlowAddressSet(monitor); } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/function/FunctionDBTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/function/FunctionDBTest.java index 5c9af3ebeb..a8eeed11e0 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/function/FunctionDBTest.java +++ b/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.getFunctionAt(entryPt); + Function f = functionManager.createFunction(name, entryPt, body, SourceType.USER_DEFINED); + 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( - "Should have gotten illegal argument exception: original entry point not in new body"); + fail("Expected exception: body must contain entry point"); } 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")); } } diff --git a/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/checksums/MyTestMemoryBlock.java b/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/checksums/MyTestMemoryBlock.java index 74e72618fe..787e8be331 100644 --- a/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/checksums/MyTestMemoryBlock.java +++ b/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; diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionManagerDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionManagerDB.java index 841ddd8269..d3eeeaa130 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionManagerDB.java +++ b/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 { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryBlockDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryBlockDB.java index 7111fe1248..c0d1741e42 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/mem/MemoryBlockDB.java +++ b/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); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/block/FollowFlow.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/block/FollowFlow.java index a9579ca53b..186f5c5250 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/block/FollowFlow.java +++ b/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 { *
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: + *
FlowType.COMPUTED_CALL + *
FlowType.CONDITIONAL_CALL + *
FlowType.UNCONDITIONAL_CALL + *
FlowType.COMPUTED_JUMP + *
FlowType.CONDITIONAL_JUMP + *
FlowType.UNCONDITIONAL_JUMP + *
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 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. */ diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlock.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlock.java index bbfcb0e7ef..130d48779b 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlock.java +++ b/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.AddressSpace; +import ghidra.program.model.address.*; import ghidra.program.model.symbol.OffsetReference; import ghidra.util.NamingUtilities; @@ -86,6 +85,12 @@ public interface MemoryBlock extends Serializable, Comparable { */ 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. * diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlockStub.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlockStub.java index b876975d22..6be0695ec5 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/mem/MemoryBlockStub.java +++ b/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();