From 4800ead95f7a2f345df9b846b8f9219db7fb2ba5 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Mon, 25 Jul 2022 05:05:48 +0000 Subject: [PATCH 1/2] GP-2103_emteere Set function purge for x86 RETF instructions --- .../java/ghidra/app/cmd/function/CallDepthChangeInfo.java | 5 ++++- .../ghidra/app/cmd/function/FunctionPurgeAnalysisCmd.java | 4 +++- .../java/ghidra/app/util/opinion/LibrarySymbolTable.java | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CallDepthChangeInfo.java b/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CallDepthChangeInfo.java index 6116ed5393..87496fde42 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CallDepthChangeInfo.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CallDepthChangeInfo.java @@ -497,6 +497,7 @@ public class CallDepthChangeInfo { return; } + // if extrapop is has an unknown purge, check for a purge on return instructions int purge = (short) program.getCompilerSpec().getDefaultCallingConvention().getExtrapop(); final boolean possiblePurge = purge == -1 || purge > 3200 || purge < -3200; @@ -527,7 +528,9 @@ public class CallDepthChangeInfo { public boolean evaluateContext(VarnodeContext context, Instruction instr) { FlowType ftype = instr.getFlowType(); if (possiblePurge && ftype.isTerminal()) { - if (instr.getMnemonicString().compareToIgnoreCase("ret") == 0) { + String mnemonicStr = instr.getMnemonicString().toLowerCase(); + if ("ret".equals(mnemonicStr) || "retf".equals(mnemonicStr)) { + // x86 has a scalar operand to purge value from the stack int tempPurge = 0; Scalar scalar = instr.getScalar(0); if (scalar != null) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/FunctionPurgeAnalysisCmd.java b/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/FunctionPurgeAnalysisCmd.java index 58bb7a2ba7..a8afb92bcd 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/FunctionPurgeAnalysisCmd.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/FunctionPurgeAnalysisCmd.java @@ -279,7 +279,9 @@ public class FunctionPurgeAnalysisCmd extends BackgroundCommand { FlowType ftype = instr.getFlowType(); if (ftype.isTerminal()) { - if (instr.getMnemonicString().compareToIgnoreCase("ret") == 0) { + String mnemonicStr = instr.getMnemonicString().toLowerCase(); + if ("ret".equals(mnemonicStr) || "retf".equals(mnemonicStr)) { + // x86 has a scalar operand to purge value from the stack return instr; } else if (ftype.isCall()) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/LibrarySymbolTable.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/LibrarySymbolTable.java index 2adb3f8a52..8ea6d4b268 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/LibrarySymbolTable.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/LibrarySymbolTable.java @@ -207,7 +207,9 @@ class LibrarySymbolTable { } FlowType ftype = instr.getFlowType(); if (ftype.isTerminal()) { - if (instr.getMnemonicString().compareToIgnoreCase("ret") == 0) { + String mnemonicStr = instr.getMnemonicString().toLowerCase(); + if ("ret".equals(mnemonicStr) || "retf".equals(mnemonicStr)) { + // x86 has a scalar operand to purge value from the stack tempPurge = 0; Scalar scalar = instr.getScalar(0); if (scalar != null) { From f7495daea1293b7853b7d3bcbf81f2868cdc3ad1 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Mon, 25 Jul 2022 05:30:30 +0000 Subject: [PATCH 2/2] GP-2122_emteere protect use of tmodeReg on processors without one --- ...rmAggressiveInstructionFinderAnalyzer.java | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/prototype/analysis/ArmAggressiveInstructionFinderAnalyzer.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/prototype/analysis/ArmAggressiveInstructionFinderAnalyzer.java index fb3991375f..b7f0bc6d3c 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/prototype/analysis/ArmAggressiveInstructionFinderAnalyzer.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/prototype/analysis/ArmAggressiveInstructionFinderAnalyzer.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. @@ -215,15 +214,17 @@ public class ArmAggressiveInstructionFinderAnalyzer extends AbstractAnalyzer { new PseudoDisassemblerContext(curProgram.getProgramContext()); // get the current value from the program context - curValue = curProgram.getProgramContext().getValue(tmodeReg, entry, false); - // if it doesn't have one set, try to get it the last context from the instruction before - if (curValue == null) { - Instruction instr = listing.getInstructionBefore(entry); - if (instr != null) { - curValue = - curProgram.getProgramContext().getValue(tmodeReg, instr.getMinAddress(), false); - if (curValue != null) { - pseudoContext.setValue(tmodeReg, entry, curValue); + if (tmodeReg != null) { + curValue = curProgram.getProgramContext().getValue(tmodeReg, entry, false); + // if it doesn't have one set, try to get it the last context from the instruction before + if (curValue == null) { + Instruction instr = listing.getInstructionBefore(entry); + if (instr != null) { + curValue = + curProgram.getProgramContext().getValue(tmodeReg, instr.getMinAddress(), false); + if (curValue != null) { + pseudoContext.setValue(tmodeReg, entry, curValue); + } } } } @@ -257,7 +258,9 @@ public class ArmAggressiveInstructionFinderAnalyzer extends AbstractAnalyzer { addsInfo = true; } pseudoContext = new PseudoDisassemblerContext(curProgram.getProgramContext()); - pseudoContext.setValue(tmodeReg, entry, curValue); + if (tmodeReg != null) { + pseudoContext.setValue(tmodeReg, entry, curValue); + } AddressSet body = pseudo.followSubFlows(entry, pseudoContext, 1000, new PseudoFlowProcessor() { Object lastResults[] = null;