diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/analysis/SharedReturnAnalysisCmd.java b/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/analysis/SharedReturnAnalysisCmd.java index cc64aafe59..c6fb52a393 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/analysis/SharedReturnAnalysisCmd.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/analysis/SharedReturnAnalysisCmd.java @@ -265,8 +265,9 @@ 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)) { + // check if there could be any fallthru flow to the potential entry point + if (checkIfCouldHaveFallThruTo(program, entry)) { + // if there could be, even later in analysis, don't create the function return; } AutoAnalysisManager analysisMgr = AutoAnalysisManager.getAnalysisManager(program); @@ -274,7 +275,7 @@ public class SharedReturnAnalysisCmd extends BackgroundCommand { } } - private boolean hasFallThruTo(Program program, Address location) { + private boolean checkIfCouldHaveFallThruTo(Program program, Address location) { Instruction instr= program.getListing().getInstructionAt(location); if (instr == null) { return true; @@ -283,12 +284,16 @@ public class SharedReturnAnalysisCmd extends BackgroundCommand { if (fallFrom != null) { Instruction fallInstr = program.getListing().getInstructionContaining(fallFrom); if (fallInstr != null && location.equals(fallInstr.getFallThrough())) { - // if there is a function above, then it falls into this routine - if (program.getFunctionManager().getFunctionContaining(fallFrom) != null) { - return true; - } + // if there is no instruction yet, function may not be created yet + return true; } } + + if (instr.getFlowType() == RefType.TERMINATOR) { + // a single instruction that is terminal consider + // as having a possible future fallthru to + return true; + } return false; } 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 10b0aead10..6fe3f31532 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 @@ -89,6 +89,7 @@ public class CreateFunctionCmd extends BackgroundCommand { * Constructs a new command for creating functions that automatically computes * the body of each function. * @param entries the entry points at which to create functions. + * @param findEntryPoint true if entry point is unknown and should be found */ public CreateFunctionCmd(AddressSetView entries, boolean findEntryPoint) { this(null, entries, null, SourceType.DEFAULT, findEntryPoint, false); @@ -107,6 +108,7 @@ public class CreateFunctionCmd extends BackgroundCommand { * Constructs a new command for creating functions that automatically computes * the body of each function. * @param entries the entry points at which to create functions. + * @param source SourceType for created function */ public CreateFunctionCmd(AddressSetView entries, SourceType source) { this(null, entries, null, source, false, false); @@ -156,7 +158,6 @@ public class CreateFunctionCmd extends BackgroundCommand { monitor.initialize(origEntries.getNumAddresses()); - CodeBlockModel functionModel = null; AddressIterator iter = origEntries.getAddresses(true); while (iter.hasNext() && !monitor.isCancelled()) { monitor.setProgress(++count); @@ -209,13 +210,19 @@ public class CreateFunctionCmd extends BackgroundCommand { } monitor.setMessage("Function " + funcName); - - if (functionModel == null) { - functionModel = new PartitionCodeSubModel(program); + + boolean didCreate = false; + try { + didCreate = createFunction(monitor, funcName, nameSpace, origEntry, + origBody, tmpSource); + } catch (OverlappingFunctionException e) { + // try to create again, sometimes thunks can get resolved that + // can't be detected, for example a thunk->function->thunk + // where the final thunk gets created + didCreate = createFunction(monitor, funcName, nameSpace, origEntry, + origBody, tmpSource); } -// TODO: What if function already exists ?? - if (createFunction(monitor, funcName, functionModel, nameSpace, origEntry, - origBody, tmpSource)) { + if (didCreate) { functionsCreated++; } else { @@ -244,6 +251,8 @@ public class CreateFunctionCmd extends BackgroundCommand { /** * Returns function if create command was successful + * + * @return last new created function, null if failed */ public Function getFunction() { return newFunc; @@ -259,13 +268,15 @@ public class CreateFunctionCmd extends BackgroundCommand { * the body of the new function. * @param nameSource * the source of this function's name - * @throws OverlappingFunctionException - * @throws InvalidInputException - * @throws DuplicateNameException + * + * @return true if a function was created, or already exists + * + * @throws OverlappingFunctionException if new function overlaps with existing and couldn't fix + * @throws InvalidInputException bad characters in function name */ private boolean createFunction(TaskMonitor monitor, String funcName, - CodeBlockModel functionModel, Namespace nameSpace, Address entry, AddressSetView body, - SourceType nameSource) throws DuplicateNameException, InvalidInputException, + Namespace nameSpace, Address entry, AddressSetView body, + SourceType nameSource) throws InvalidInputException, OverlappingFunctionException, CancelledException { FunctionManager functionMgr = program.getFunctionManager(); @@ -282,7 +293,7 @@ public class CreateFunctionCmd extends BackgroundCommand { entry = functionContaining.getEntryPoint(); // preserve entry long bodySize = functionContaining.getBody().getNumAddresses(); if (bodySize != 1) { - return false; + return true; // function already created } } @@ -323,21 +334,8 @@ public class CreateFunctionCmd extends BackgroundCommand { // get the function body JUST by following flow body = (body == null ? getFunctionBody(program, entry, false, monitor) : body); - // subtract out any existing functions that overlap the body - AddressSetView oldbody = body; - body = removeExistingFunctionsFromBody(entry, body, monitor); - if (body == null) { - return false; - } - Map bodyChangeMap = new HashMap<>(); - // If I ain't got nobody left after extracting overlapping functions - if (body.isEmpty() || !body.contains(entry)) { - // try subtracting this body from existing functions - // need to compute a bodyChangeMap if affecting other functions - // in case creation of this function fails - body = subtractBodyFromExisting(entry, oldbody, bodyChangeMap, monitor); - } + body = subtractBodyFromExisting(program, entry, body, bodyChangeMap, monitor); return createFunction(nameSpace, funcName, entry, body, nameSource, bodyChangeMap, monitor); } @@ -352,15 +350,14 @@ public class CreateFunctionCmd extends BackgroundCommand { * @param nameSource - source of the name * @param bodyChangeMap - change map to restore other affected functions bodies if this fails * @param monitor task monitor - * @return - * @throws OverlappingFunctionException - * @throws DuplicateNameException - * @throws InvalidInputException + * @return true if function created + * @throws OverlappingFunctionException if functions overlaps with existing function + * @throws InvalidInputException if function name has bad chars */ private boolean createFunction(Namespace nameSpace, String funcName, Address entry, AddressSetView body, SourceType nameSource, Map bodyChangeMap, TaskMonitor monitor) - throws OverlappingFunctionException, DuplicateNameException, InvalidInputException { + throws OverlappingFunctionException, InvalidInputException { Listing listing = program.getListing(); @@ -386,8 +383,7 @@ public class CreateFunctionCmd extends BackgroundCommand { } newFunc = listing.createFunction(funcName, nameSpace, entry, body, nameSource); } - catch (InvalidInputException e) { - restoreOriginalBodies(bodyChangeMap); + catch (InvalidInputException | OverlappingFunctionException e) { throw e; } @@ -395,77 +391,73 @@ public class CreateFunctionCmd extends BackgroundCommand { } /** - * subtract this functions entire body from existing functions + * Subtract this functions entire body from existing functions. + * NewFuncBody keeps any overlapping code that occurs between its entry point and the next entry point. + * This rule helps functions keep a contiguous function body if functions share code such as a return, or + * even an internal branch. + * + * NOTE: This will change the other functions bodies * * @param entry - entry point of new function - * @param body - new functions body + * @param newFuncBody - new functions body * @param bodyChangeMap - map of functions that have their bodies changed by creating this function * @param monitor task monitor - * @return - * @throws CancelledException - * @throws OverlappingFunctionException + * + * @return address set of non-overlapping function body, some addresses may be grabbed from existing functions + * + * @throws CancelledException user cancelled + * @throws OverlappingFunctionException existing functions overlap and couldn't reconcile */ - private AddressSetView subtractBodyFromExisting(Address entry, AddressSetView body, + private static AddressSetView subtractBodyFromExisting(Program program, Address entry, AddressSetView newFuncBody, Map bodyChangeMap, TaskMonitor monitor) throws CancelledException, OverlappingFunctionException { - Iterator iter = program.getFunctionManager().getFunctionsOverlapping(body); - while (iter.hasNext()) { - monitor.checkCancelled(); - Function elem = iter.next(); - AddressSetView funcBody = elem.getBody(); - if (funcBody.contains(entry)) { - bodyChangeMap.put(elem, funcBody); - // re-define the function that does contain me.... - funcBody = funcBody.subtract(body); - elem.setBody(funcBody); - } - else { - // else, the body flowed into an existing function - body = body.subtract(funcBody); - } - } - return body; - } + // get all functions that overlap new function body + Iterator iter = program.getFunctionManager().getFunctionsOverlapping(newFuncBody); - /** - * Remove any existing functions bodies from the new functions body at entry. - * - * @param entry - * @param body - * @param monitor task monitor - * @return the new body, or null if body could not be created and need to abort function creation. - * - * @throws CancelledException - */ - private AddressSetView removeExistingFunctionsFromBody(Address entry, AddressSetView body, - TaskMonitor monitor) throws CancelledException { - Iterator iter = program.getFunctionManager().getFunctionsOverlapping(body); while (iter.hasNext()) { monitor.checkCancelled(); - Function elem = iter.next(); - if (elem.getEntryPoint().equals(entry)) { - // if finding the entrypoint, need to redefine the functions body. - if (!findEntryPoint) { - long bodySize = elem.getBody().getNumAddresses(); - // if not finding the entry point, and bodysize > 1, bad function - if (bodySize != 1) { - return null; - } + Function overlapFunc = iter.next(); + Address overlapEntryPoint = overlapFunc.getEntryPoint(); + if (overlapEntryPoint.equals(entry)) { + // function found is possibly existing function + continue; + } + AddressSetView overlapFuncBody = overlapFunc.getBody(); + // catch functions that are place-holders for a function yet to be disassembled + if (overlapFuncBody.getNumAddresses() == 1) { + overlapFuncBody = getFunctionBody(program, overlapEntryPoint, false, monitor); + if (overlapFuncBody.contains(entry)) { + continue; } } - else { - AddressSetView bodyInConflict = elem.getBody(); - // catch functions that are place-holders for a function yet to be disassembled - if (bodyInConflict.getNumAddresses() == 1) { - bodyInConflict = getFunctionBody(program, elem.getEntryPoint(), false, monitor); - if (bodyInConflict.contains(entry)) { - continue; - } - } - body = body.subtract(bodyInConflict); + + // get new body for overlapping function, by carving out addresses that + // should belong in the new function + Address overlapEndAddr; + if (overlapEntryPoint.compareTo(entry) < 0) { + // overlap function is before, subtract from entry to end of new function + overlapEndAddr = newFuncBody.getMaxAddress(); + } else { + // overlap function is after, subtract from entry to entry of overlapping function + overlapEndAddr = overlapEntryPoint.previous(); } + AddressSet overlapAddrsShouldBeInNewFunction = new AddressSet(entry,overlapEndAddr); + AddressSetView newOverlapFuncBody = overlapFuncBody.subtract(overlapAddrsShouldBeInNewFunction); + try { + if (!overlapFuncBody.equals(newOverlapFuncBody)) { + overlapFunc.setBody(newOverlapFuncBody); // set overlap body with new function body addresses removed + bodyChangeMap.put(overlapFunc, overlapFuncBody); // save old overlap body + } + overlapFuncBody = newOverlapFuncBody; + } catch (OverlappingFunctionException oe) { + // do nothing, will just subtract entire overlapping body from new function body + } + + // remove overlapping functions body from new function body + newFuncBody = newFuncBody.subtract(overlapFuncBody); } - return body; + + return newFuncBody; } private boolean handleExistingFunction(TaskMonitor monitor, Address entry, @@ -475,7 +467,7 @@ public class CreateFunctionCmd extends BackgroundCommand { if (bodySize > 1) { if (!recreateFunction) { // Function at entry already exists and recreateFunction not enabled - return false; + return true; } // if it is a thunk, then we're done if (resolveThunk(entry, null, monitor)) { @@ -498,7 +490,7 @@ public class CreateFunctionCmd extends BackgroundCommand { * @param monitor task monitor * @return true if the entry resolved to a thunk * - * @throws OverlappingFunctionException + * @throws OverlappingFunctionException if thunk thunks itself */ private boolean resolveThunk(Address entry, AddressSetView body, TaskMonitor monitor) throws OverlappingFunctionException { @@ -528,9 +520,9 @@ public class CreateFunctionCmd extends BackgroundCommand { /** * using the body map revert any changes made to function bodies * - * @param bodyChangeMap + * @param bodyChangeMap map of functions to original bodies */ - private void restoreOriginalBodies(Map bodyChangeMap) { + private static void restoreOriginalBodies(Map bodyChangeMap) { Set> entries = bodyChangeMap.entrySet(); Iterator> iter = entries.iterator(); while (iter.hasNext()) { @@ -599,17 +591,9 @@ public class CreateFunctionCmd extends BackgroundCommand { * @return AddressSetView address set representing the body of the function */ public static AddressSetView getFunctionBody(TaskMonitor monitor, Program program, - Address entry) throws CancelledException { - CodeBlock block = null; + Address entry) { - PartitionCodeSubModel model = new PartitionCodeSubModel(program); - // MultEntSubModel model = new MultEntSubModel(program); - block = model.getCodeBlockAt(entry, monitor); - - if (block == null) { - return getFunctionBody(program, entry); - } - return block; + return getFunctionBody(program, entry, true, monitor); } /** @@ -705,30 +689,17 @@ public class CreateFunctionCmd extends BackgroundCommand { func.setBody(newBody); // trigger analysis } catch (OverlappingFunctionException e) { - // subtract out any existing functions that overlap the body - Iterator iter = program.getFunctionManager().getFunctionsOverlapping(newBody); - - while (iter.hasNext()) { - monitor.checkCancelled(); - Function elem = iter.next(); - if (elem.getEntryPoint().equals(entry)) { - // if finding the entrypoint, need to redefine the functions body. - continue; - } - AddressSetView bodyInConflict = elem.getBody(); - // catch functions that are place-holders for a function yet to be disassembled - if (bodyInConflict.getNumAddresses() == 1) { - bodyInConflict = getFunctionBody(program, elem.getEntryPoint(), false, monitor); - if (bodyInConflict.contains(entry)) { - continue; - } - } - newBody = newBody.subtract(bodyInConflict); - } + + Map bodyChangeMap = new HashMap<>(); try { + newBody = subtractBodyFromExisting(program, entry, newBody, bodyChangeMap, monitor); + func.setBody(newBody); // trigger analysis } - catch (OverlappingFunctionException exc) { + catch (CancelledException | OverlappingFunctionException e1) { + // something went wrong, put things back the way they were + + restoreOriginalBodies(bodyChangeMap); return false; } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java index 844cf125f8..9511b5544d 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java @@ -671,10 +671,9 @@ public class AddressTable { func.setBody(funcBody); } catch (OverlappingFunctionException e2) { + // do nothing } } - catch (CancelledException e3) { - } } }