From b7ede746d03e0ce02d45bf686693ec550ecfbe63 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Fri, 30 Jun 2023 22:42:47 +0000 Subject: [PATCH 1/4] GP-3599 Fix for function bodies including one byte of non-disassembled data. PowerPC disassembly from computed branch. --- .../app/cmd/function/CreateFunctionCmd.java | 2 +- .../select/flow/AbstractFollowFlowTest.java | 13 ++ .../select/flow/FollowFlowForwardTest.java | 62 ++++++ .../CreateFunctionCmdWithFlowTest.java | 197 ++++++++++++++++++ .../program/model/block/FollowFlow.java | 38 +++- .../core/analysis/PowerPCAddressAnalyzer.java | 4 +- 6 files changed, 312 insertions(+), 4 deletions(-) create mode 100644 Ghidra/Features/Base/src/test/java/ghidra/app/cmd/function/CreateFunctionCmdWithFlowTest.java 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 4d2dd15f2d..c395312bd5 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 @@ -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); } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/AbstractFollowFlowTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/AbstractFollowFlowTest.java index acc578a3c2..2d756a98a0 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/AbstractFollowFlowTest.java +++ b/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); } diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowFlowForwardTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowFlowForwardTest.java index eba7d2b268..df014e8bec 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowFlowForwardTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/select/flow/FollowFlowForwardTest.java @@ -80,6 +80,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() { @@ -159,6 +207,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() { diff --git a/Ghidra/Features/Base/src/test/java/ghidra/app/cmd/function/CreateFunctionCmdWithFlowTest.java b/Ghidra/Features/Base/src/test/java/ghidra/app/cmd/function/CreateFunctionCmdWithFlowTest.java new file mode 100644 index 0000000000..b07a2634d4 --- /dev/null +++ b/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); + } + +} 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 6d03bc28c7..a9579ca53b 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 @@ -46,10 +46,14 @@ 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. @@ -63,6 +67,7 @@ public class FollowFlow { *
FlowType.CONDITIONAL_JUMP *
FlowType.UNCONDITIONAL_JUMP *
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. @@ -93,6 +100,31 @@ public class FollowFlow { this(program, addressSet, doNotFollow); 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: + *
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, 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); } diff --git a/Ghidra/Processors/PowerPC/src/main/java/ghidra/app/plugin/core/analysis/PowerPCAddressAnalyzer.java b/Ghidra/Processors/PowerPC/src/main/java/ghidra/app/plugin/core/analysis/PowerPCAddressAnalyzer.java index 4f579a5c81..c59193a3be 100644 --- a/Ghidra/Processors/PowerPC/src/main/java/ghidra/app/plugin/core/analysis/PowerPCAddressAnalyzer.java +++ b/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(); From 365f526877f66c0b0112d338aebec667a01d4f78 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Mon, 3 Jul 2023 16:10:27 +0000 Subject: [PATCH 2/4] GP-3605 Only log error message once and improve efficiency of allocating new spaces --- .../ghidra/program/util/VarnodeContext.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/program/util/VarnodeContext.java b/Ghidra/Features/Base/src/main/java/ghidra/program/util/VarnodeContext.java index a4a8a89bfe..29ab5c0657 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/program/util/VarnodeContext.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/program/util/VarnodeContext.java @@ -1383,15 +1383,9 @@ public class VarnodeContext implements ProcessorContext { return result; } - // This is bad since registers could have multiple associated spaces -// private int getSymbolSpaceID(Varnode val) { -// Register reg = trans.getRegister(val); -// if (reg == null) { -// return -1; -// } -// return getAddressSpace(reg.getName()); -// } - + // flag running out of address spaces, so error only printed once + private boolean hitMaxAddressSpaces = false; + public int getAddressSpace(String name) { int spaceID; AddressSpace regSpace = addrFactory.getAddressSpace(name); @@ -1399,7 +1393,10 @@ public class VarnodeContext implements ProcessorContext { regSpace = ((OffsetAddressFactory) addrFactory).createNewOffsetSpace(name); } if (regSpace == null) { - Msg.error(this, "VarnodeContext: out of address spaces for: " + name); + if (!hitMaxAddressSpaces) { + Msg.error(this, "VarnodeContext: out of address spaces at @" + currentAddress +" for: " + name); + hitMaxAddressSpaces = true; + } return BAD_SPACE_ID_VALUE; } spaceID = regSpace.getSpaceID(); @@ -1762,13 +1759,18 @@ class OffsetAddressFactory extends DefaultAddressFactory { } } + // Maximum space ID used to create spaces + private int curMaxID = 0; + private int getNextUniqueID() { - int maxID = 0; - AddressSpace[] spaces = getAllAddressSpaces(); - for (AddressSpace space : spaces) { - maxID = Math.max(maxID, space.getUnique()); + if (curMaxID == 0) { + AddressSpace[] spaces = getAllAddressSpaces(); + for (AddressSpace space : spaces) { + curMaxID = Math.max(curMaxID, space.getUnique()); + } } - return maxID + 1; + curMaxID += 1; + return curMaxID; } /** From ccad2abbd81929fbd548e93d1cbe4a12cf7f9add Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Wed, 5 Jul 2023 10:58:20 -0400 Subject: [PATCH 3/4] GP-0 revised x86-64 ldef entry for compat32 variant and change opinion entries to give preference to default variant --- .../FileFormats/data/languages/apport.opinion | 2 +- .../data/languages/minidump.opinion | 2 +- .../data/languages/pagedump.opinion | 2 +- .../Processors/x86/data/languages/x86.ldefs | 2 +- .../Processors/x86/data/languages/x86.opinion | 19 ++++++++++--------- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Ghidra/Features/FileFormats/data/languages/apport.opinion b/Ghidra/Features/FileFormats/data/languages/apport.opinion index 63f9aee02b..72e121eb83 100644 --- a/Ghidra/Features/FileFormats/data/languages/apport.opinion +++ b/Ghidra/Features/FileFormats/data/languages/apport.opinion @@ -1,7 +1,7 @@ - + diff --git a/Ghidra/Features/FileFormats/data/languages/minidump.opinion b/Ghidra/Features/FileFormats/data/languages/minidump.opinion index 4b55b98f2c..4ebf266d58 100644 --- a/Ghidra/Features/FileFormats/data/languages/minidump.opinion +++ b/Ghidra/Features/FileFormats/data/languages/minidump.opinion @@ -9,7 +9,7 @@ - + diff --git a/Ghidra/Features/FileFormats/data/languages/pagedump.opinion b/Ghidra/Features/FileFormats/data/languages/pagedump.opinion index dcf71103df..a35175710f 100644 --- a/Ghidra/Features/FileFormats/data/languages/pagedump.opinion +++ b/Ghidra/Features/FileFormats/data/languages/pagedump.opinion @@ -19,7 +19,7 @@ - + diff --git a/Ghidra/Processors/x86/data/languages/x86.ldefs b/Ghidra/Processors/x86/data/languages/x86.ldefs index 02f71502b2..1961485c4f 100644 --- a/Ghidra/Processors/x86/data/languages/x86.ldefs +++ b/Ghidra/Processors/x86/data/languages/x86.ldefs @@ -99,7 +99,7 @@ - + - + - + @@ -24,18 +25,18 @@ - + - + - + @@ -51,18 +52,18 @@ - + - + - + - + From 5b393764c692dcc8de7b82f3e475b44f28f688d8 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Wed, 5 Jul 2023 14:18:55 -0400 Subject: [PATCH 4/4] GP-3582 Fix arm Branch/Return waffle if correct function boundaries not created and LR is detected as a constant --- .../ghidra/app/plugin/core/analysis/ArmAnalyzer.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Ghidra/Processors/ARM/src/main/java/ghidra/app/plugin/core/analysis/ArmAnalyzer.java b/Ghidra/Processors/ARM/src/main/java/ghidra/app/plugin/core/analysis/ArmAnalyzer.java index af452124d5..cc1020fe7d 100644 --- a/Ghidra/Processors/ARM/src/main/java/ghidra/app/plugin/core/analysis/ArmAnalyzer.java +++ b/Ghidra/Processors/ARM/src/main/java/ghidra/app/plugin/core/analysis/ArmAnalyzer.java @@ -78,6 +78,7 @@ public class ArmAnalyzer extends ConstantPropagationAnalyzer { public AddressSet flowConstants(final Program program, Address flowStart, AddressSetView flowSet, final SymbolicPropogator symEval, final TaskMonitor monitor) throws CancelledException { + // follow all flows building up context // use context to fill out addresses on certain instructions ConstantPropagationContextEvaluator eval = @@ -229,11 +230,16 @@ public class ArmAnalyzer extends ConstantPropagationAnalyzer { @Override public boolean evaluateReturn(Varnode retVN, VarnodeContext context, Instruction instruction) { // check if a return is actually returning, or is branching with a constant PC - + + // if flow already overridden, don't override again + if (instruction.getFlowOverride() != FlowOverride.NONE) { + return false; + } + if (retVN != null && context.isConstant(retVN)) { long offset = retVN.getOffset(); if (offset > 3 && offset != -1) { - // need to override the return to a branch + // need to override the return flow to a branch instruction.setFlowOverride(FlowOverride.BRANCH); } }