From 9c73c86fecbd852834e7f60d2fa43c5dd9aae095 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Tue, 11 Oct 2022 17:57:55 -0400 Subject: [PATCH] GP-2534 turn on shared function contiguous function options, except for on ARM --- .../cmd/analysis/SharedReturnAnalysisCmd.java | 45 +++++++++++++++++-- .../core/function/SharedReturnAnalyzer.java | 29 ++++++------ .../lang/GhidraLanguagePropertyKeys.java | 8 ++++ .../ARM/data/languages/ARMCortex.pspec | 1 + .../ARM/data/languages/ARM_v45.pspec | 1 + .../Processors/ARM/data/languages/ARMt.pspec | 1 + .../ARM/data/languages/ARMtTHUMB.pspec | 1 + .../ARM/data/languages/ARMt_v45.pspec | 1 + .../ARM/data/languages/ARMt_v6.pspec | 1 + .../core/analysis/MipsAddressAnalyzer.java | 39 +++++++++------- 10 files changed, 94 insertions(+), 33 deletions(-) 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 882d21d5c6..edbf723db5 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 @@ -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); } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/SharedReturnAnalyzer.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/SharedReturnAnalyzer.java index fc2f8a4b20..e261d446e4 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/SharedReturnAnalyzer.java +++ b/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.Language; -import ghidra.program.model.listing.Program; +import ghidra.program.model.lang.*; +import ghidra.program.model.listing.*; 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 = - OPTION_DEFAULT_CONSIDER_CONDITIONAL_BRANCHES_ENABLED; + private boolean considerConditionalBranches = 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 = - options.getBoolean(OPTION_NAME_CONSIDER_CONDITIONAL_BRANCHES_FUNCTIONS, - OPTION_DEFAULT_CONSIDER_CONDITIONAL_BRANCHES_ENABLED); + considerConditionalBranches = options.getBoolean(OPTION_NAME_CONSIDER_CONDITIONAL_BRANCHES_FUNCTIONS, + considerConditionalBranches); } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/GhidraLanguagePropertyKeys.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/GhidraLanguagePropertyKeys.java index df76867542..9d9fd97a0c 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/lang/GhidraLanguagePropertyKeys.java +++ b/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 = "enableSharedReturnContiguousFunctionsOnly"; /** * Non returning function analysis, where a function such as exit() is known to the compiler diff --git a/Ghidra/Processors/ARM/data/languages/ARMCortex.pspec b/Ghidra/Processors/ARM/data/languages/ARMCortex.pspec index 64222b6cc8..5873ef0925 100644 --- a/Ghidra/Processors/ARM/data/languages/ARMCortex.pspec +++ b/Ghidra/Processors/ARM/data/languages/ARMCortex.pspec @@ -5,6 +5,7 @@ + diff --git a/Ghidra/Processors/ARM/data/languages/ARM_v45.pspec b/Ghidra/Processors/ARM/data/languages/ARM_v45.pspec index 865bb58652..2cbde5e7f4 100644 --- a/Ghidra/Processors/ARM/data/languages/ARM_v45.pspec +++ b/Ghidra/Processors/ARM/data/languages/ARM_v45.pspec @@ -6,6 +6,7 @@ + diff --git a/Ghidra/Processors/ARM/data/languages/ARMt.pspec b/Ghidra/Processors/ARM/data/languages/ARMt.pspec index 17d32acd3f..70e5e6c3d9 100644 --- a/Ghidra/Processors/ARM/data/languages/ARMt.pspec +++ b/Ghidra/Processors/ARM/data/languages/ARMt.pspec @@ -5,6 +5,7 @@ + diff --git a/Ghidra/Processors/ARM/data/languages/ARMtTHUMB.pspec b/Ghidra/Processors/ARM/data/languages/ARMtTHUMB.pspec index cc30cec13c..771ee02681 100644 --- a/Ghidra/Processors/ARM/data/languages/ARMtTHUMB.pspec +++ b/Ghidra/Processors/ARM/data/languages/ARMtTHUMB.pspec @@ -6,6 +6,7 @@ + diff --git a/Ghidra/Processors/ARM/data/languages/ARMt_v45.pspec b/Ghidra/Processors/ARM/data/languages/ARMt_v45.pspec index 1d04e809c7..0059c3cfaf 100644 --- a/Ghidra/Processors/ARM/data/languages/ARMt_v45.pspec +++ b/Ghidra/Processors/ARM/data/languages/ARMt_v45.pspec @@ -6,6 +6,7 @@ + diff --git a/Ghidra/Processors/ARM/data/languages/ARMt_v6.pspec b/Ghidra/Processors/ARM/data/languages/ARMt_v6.pspec index 68d853b139..bd073c9734 100644 --- a/Ghidra/Processors/ARM/data/languages/ARMt_v6.pspec +++ b/Ghidra/Processors/ARM/data/languages/ARMt_v6.pspec @@ -5,6 +5,7 @@ + diff --git a/Ghidra/Processors/MIPS/src/main/java/ghidra/app/plugin/core/analysis/MipsAddressAnalyzer.java b/Ghidra/Processors/MIPS/src/main/java/ghidra/app/plugin/core/analysis/MipsAddressAnalyzer.java index 49063c5678..54240fe063 100644 --- a/Ghidra/Processors/MIPS/src/main/java/ghidra/app/plugin/core/analysis/MipsAddressAnalyzer.java +++ b/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, - 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); + trySwitchTables = options.getBoolean(OPTION_NAME_SWITCH_TABLE, trySwitchTables); 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); } }