From 269b6f6700ed0eddff071d359aa8b150b3e1a5a7 Mon Sep 17 00:00:00 2001
From: clc5q <clc5q@git.zephyr-software.com>
Date: Thu, 13 Dec 2007 19:39:09 +0000
Subject: [PATCH] Add method SMPFunction::MDFixUseFP to correct IDA analysis of
 frame pointer use.

---
 SMPDataFlowAnalysis.cpp | 54 ++++++++++++++++++++++++++++++++++++-----
 SMPDataFlowAnalysis.h   |  3 ++-
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/SMPDataFlowAnalysis.cpp b/SMPDataFlowAnalysis.cpp
index 17a8ad89..0ae9fb54 100644
--- a/SMPDataFlowAnalysis.cpp
+++ b/SMPDataFlowAnalysis.cpp
@@ -565,7 +565,7 @@ void SMPInstr::AnnotateStackConstants(bool UseFP, FILE *AnnotFile) {
 							"%x %d PTRIMMEDESP STACK %d %s\n",
 							SMPcmd.ea, SMPcmd.size, offset, disasm);
 				}
-				else if (IndexReg == R_bp) { // ESP cannot be IndexReg
+				else if (UseFP && (IndexReg == R_bp)) { // ESP cannot be IndexReg
 					// EBP-relative constant offset
 					qfprintf(AnnotFile,
 							"%x %d PTRIMMEDEBP STACK %d %s\n",
@@ -580,7 +580,7 @@ void SMPInstr::AnnotateStackConstants(bool UseFP, FILE *AnnotFile) {
 							"%x %d PTRIMMEDESP STACK %d %s\n",
 							SMPcmd.ea, SMPcmd.size, offset, disasm);
 				}
-				else if (BaseReg == R_bp) {
+				else if (UseFP && (BaseReg == R_bp)) {
 					// EBP-relative constant offset
 					qfprintf(AnnotFile,
 							"%x %d PTRIMMEDEBP STACK %d %s\n",
@@ -599,7 +599,7 @@ void SMPInstr::AnnotateStackConstants(bool UseFP, FILE *AnnotFile) {
 							"%x %d PTRIMMEDESP STACK %d %s\n",
 							SMPcmd.ea, SMPcmd.size, offset, disasm);
 				}
-				else if (IndexReg == R_bp) { // ESP cannot be IndexReg
+				else if (UseFP && (IndexReg == R_bp)) { // ESP cannot be IndexReg
 					// EBP-relative constant offset
 					qfprintf(AnnotFile,
 							"%x %d PTRIMMEDEBP STACK %d %s\n",
@@ -614,7 +614,7 @@ void SMPInstr::AnnotateStackConstants(bool UseFP, FILE *AnnotFile) {
 							"%x %d PTRIMMEDESP STACK %d %s\n",
 							SMPcmd.ea, SMPcmd.size, offset, disasm);
 				}
-				else if (BaseReg == R_bp) {
+				else if (UseFP && (BaseReg == R_bp)) {
 					// EBP-relative constant offset
 					qfprintf(AnnotFile,
 							"%x %d PTRIMMEDEBP STACK %d %s\n",
@@ -880,6 +880,16 @@ void SMPFunction::SetStackFrameInfo(void) {
 				&& CurrInstr->MDIsFrameAllocInstr()) {
 				this->LocalVarsAllocInstr = addr;
 				FoundAllocInstr = true;
+				// As soon as we have found the local vars allocation,
+				//  we can try to fix incorrect sets of UseFP by IDA.
+				// NOTE: We might want to extend this in the future to
+				//  handle functions that have no locals.  **!!**
+				bool FixedUseFP = MDFixUseFP();
+#if SMP_DEBUG_FRAMEFIXUP
+				if (FixedUseFP) {
+					msg("Fixed UseFP in %s\n", this->FuncName);
+				}
+#endif
 			}
 			else if (FoundAllocInstr) {
 				// We can now start searching for the DeallocInstr.
@@ -1149,6 +1159,38 @@ bool SMPFunction::MDFixFrameInfo(void) {
 	return Changed;
 } // end of SMPFunction::MDFixFrameInfo()
 
+// IDA Pro is often confused by a function that uses the frame pointer
+//  register for other purposes. For the x86, a function that uses EBP
+//  as a frame pointer would begin with: push ebp; mov ebp,esp to save
+//  the old value of EBP and give it a new value as a frame pointer. The
+//  allocation of local variable space would have to come AFTER the move
+//  instruction. A function that begins: push ebp; push esi; sub esp,24
+//  is obviously not using EBP as a frame pointer. IDA is apparently
+//  confused by the push ebp instruction being the first instruction
+//  in the function. We will reset UseFP to false in this case.
+// NOTE: This logic should work for both Linux and Windows x86 prologues.
+bool SMPFunction::MDFixUseFP(void) {
+	list<SMPInstr>::iterator CurrInstr = this->Instrs.begin();
+	ea_t addr = CurrInstr->GetAddr();
+	if (!UseFP)
+		return false;  // Only looking to reset true to false.
+	while (addr < this->LocalVarsAllocInstr) {
+		size_t DefIndex = 0;
+		while (DefIndex < CurrInstr->NumDefs()) {
+			if (CurrInstr->GetDef(DefIndex).is_reg(R_bp))
+				return false; // EBP got set before locals were allocated
+			++DefIndex;
+		}
+		++CurrInstr;
+		addr = CurrInstr->GetAddr();
+	}
+	// If we found no defs of the frame pointer before the local vars
+	//  allocation, then the frame pointer register is not being used
+	//  as a frame pointer, just as a general callee-saved register.
+	this->UseFP = false;
+	return true;
+} // end of SMPFunction::MDFixUseFP()
+
 // Emit the annotations describing the regions of the stack frame.
 void SMPFunction::EmitStackFrameAnnotations(FILE *AnnotFile, list<SMPInstr>::iterator Instr) {
 	ea_t addr = Instr->GetAddr();
@@ -1211,9 +1253,9 @@ void SMPFunction::Analyze(void) {
 			if (isHead(InstrFlags) && isCode(InstrFlags)) {
 				SMPInstr CurrInst = SMPInstr(addr);
 				// Fill in the instruction data members.
-	#if SMP_DEBUG_CONTROLFLOW
+#if SMP_DEBUG_CONTROLFLOW
 				msg("SMPFunction::Analyze: calling CurrInst::Analyze.\n");
-	#endif
+#endif
 				CurrInst.Analyze();
 				if (SMPBinaryDebug) {
 					msg("Disasm:  %s \n", CurrInst.GetDisasm());
diff --git a/SMPDataFlowAnalysis.h b/SMPDataFlowAnalysis.h
index 14aba8aa..f78e7b66 100644
--- a/SMPDataFlowAnalysis.h
+++ b/SMPDataFlowAnalysis.h
@@ -174,7 +174,8 @@ private:
 	ea_t LocalVarsAllocInstr; // address of instr that allocates stack frame
 	ea_t LocalVarsDeallocInstr; // address of epilogue instr that deallocs frame
 	void SetStackFrameInfo(void);
-	bool MDFixFrameInfo(void);
+	bool MDFixFrameInfo(void); // Redefine stack regions for our needs
+	bool MDFixUseFP(void);  // Fix IDA errors affecting UseFP
 	void EmitStackFrameAnnotations(FILE *AnnotFile, list<SMPInstr>::iterator Instr);
 }; // end class SMPFunction
 
-- 
GitLab