From 795593b0b96e07623e106b3c8a0c23fe6845da14 Mon Sep 17 00:00:00 2001
From: Clark Coleman <clc@zephyr-software.com>
Date: Fri, 3 May 2019 10:34:31 -0400
Subject: [PATCH] unique_ptr fix, log file ERROR message enhancement and
 reduction.

---
 include/base/SMPInstr.h                      |  6 ++++--
 include/interfaces/abstract/STARSInterface.h |  3 ++-
 include/interfaces/idapro/STARSInterface.h   |  5 +++--
 include/interfaces/irdb/STARSInterface.h     |  3 ++-
 src/base/SMPBasicBlock.cpp                   | 17 ++++++++++++-----
 src/base/SMPFunction.cpp                     | 10 ++++++----
 src/base/SMPInstr.cpp                        |  4 +++-
 src/interfaces/irdb/STARS_IRDB_Function.cpp  |  2 +-
 src/interfaces/irdb/STARS_IRDB_Interface.cpp | 16 ++++++++--------
 9 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/include/base/SMPInstr.h b/include/base/SMPInstr.h
index c15c7b95..39a739ef 100644
--- a/include/base/SMPInstr.h
+++ b/include/base/SMPInstr.h
@@ -34,6 +34,7 @@
 
 #include <string>
 #include <bitset>
+#include <memory>
 
 #include <cstddef>
 #include <cstdint>
@@ -789,6 +790,7 @@ public:
 	bool IsBasicBlockTerminator(void) const;  // kind of inst that ALWAYS terminates a block
 	inline bool IsLastInBlock(void) const { return (booleans1 & INSTR_SET_BLOCK_TERM); }; // does terminate its block
 	inline bool IsJumpTarget(void) { return (booleans1 & INSTR_SET_JUMP_TARGET); };
+	inline bool IsJumpFromFixedCall(void) const { return this->STARSInstPtr->IsJumpFromFixedCall(); };
 	bool IsBranchToFarChunk(void);  // instr jumps outside current chunk
 	bool IsBranchToOtherFunc(void); // instr branches or jumps to another function
 	inline bool IsTailCall(void) const { return (booleans1 & INSTR_SET_TAIL_CALL); };
@@ -1018,7 +1020,7 @@ public:
 	bool IsLoopExitStatement(bool &InvertedExit); // true => jump is used to exit a loop
 	inline bool AnalyzeSwitchInfo(struct SwitchTableInfo &TableInfo) { return STARSInstPtr->AnalyzeSwitchStatement(this, TableInfo); };
 
-	inline STARS_Instruction_t* GetSTARSInstPtr() { return STARSInstPtr; } // pointer to either STARS_IDA_Instruction_t or STARS_IRDB_Instruction_t
+	// inline std::unique_ptr<STARS_Instruction_t> GetSTARSInstPtr() { return STARSInstPtr; } // pointer to either STARS_IDA_Instruction_t or STARS_IRDB_Instruction_t
 
 private:
 	// Data 
@@ -1028,7 +1030,7 @@ private:
 	uint32 features; // Canonical features for SMPcmd
 #endif
 	STARS_InstructionID_t STARS_ID;  // instruction ID; could be IDA Pro address or IRDB inst ID
-	STARS_Instruction_t *STARSInstPtr; // pointer to either STARS_IDA_Instruction_t or STARS_IRDB_Instruction_t
+	std::unique_ptr<STARS_Instruction_t> STARSInstPtr; // pointer to either STARS_IDA_Instruction_t or STARS_IRDB_Instruction_t
 	SMPitype  type; // Data flow analysis category
 #if 0
 	// Get this dynamically to save memory
diff --git a/include/interfaces/abstract/STARSInterface.h b/include/interfaces/abstract/STARSInterface.h
index 768d4bf3..f9fc3713 100644
--- a/include/interfaces/abstract/STARSInterface.h
+++ b/include/interfaces/abstract/STARSInterface.h
@@ -2,6 +2,7 @@
 #define STARSInterface_h
 
 #include <cstdio>
+#include <memory>
 
 #if __unix__
 #include <sys/time.h>
@@ -51,7 +52,7 @@ class STARS_Interface_t
 		virtual void get_func_name(const STARS_ea_t &ea, char* name, const std::size_t &len) = 0;
 
 		// Instruction creation.
-		virtual STARS_Instruction_t *CreateInst(STARS_InstructionID_t InstID) = 0;
+		virtual std::unique_ptr<STARS_Instruction_t> CreateInst(STARS_InstructionID_t InstID) = 0;
 
 		// IDA Pro does not permit direct usage of common file and string library calls, so we
 		//  have to do them differently in the IDA Pro and IRDB interfaces.
diff --git a/include/interfaces/idapro/STARSInterface.h b/include/interfaces/idapro/STARSInterface.h
index 261d88cc..87f227ff 100644
--- a/include/interfaces/idapro/STARSInterface.h
+++ b/include/interfaces/idapro/STARSInterface.h
@@ -3,6 +3,7 @@
 
 #include <cstdio>
 
+#include <memory>
 #include <map>
 
 #include <pro.h>
@@ -78,8 +79,8 @@ public:
 	}
 
 	// Instruction creation.
-	virtual STARS_Instruction_t *CreateInst(STARS_InstructionID_t InstID) {
-		return new STARS_IDA_Instruction_t(InstID);
+	virtual std::unique_ptr<STARS_Instruction_t> CreateInst(STARS_InstructionID_t InstID) {
+		return std::unique_ptr<STARS_Instruction_t>{(STARS_Instruction_t *) new STARS_IDA_Instruction_t(InstID)};
 	}
 
 	// File methods.
diff --git a/include/interfaces/irdb/STARSInterface.h b/include/interfaces/irdb/STARSInterface.h
index 11f55e15..49c208e7 100644
--- a/include/interfaces/irdb/STARSInterface.h
+++ b/include/interfaces/irdb/STARSInterface.h
@@ -1,6 +1,7 @@
 #ifndef STARS_irdb_Interface_h
 #define STARS_irdb_Interface_h
 
+#include <memory>
 
 #include "interfaces/abstract/STARSInterface.h"
 #include "interfaces/abstract/STARSInstructionID.h"
@@ -140,7 +141,7 @@ public:
 	}
 
 // Instruction creation.
-	virtual STARS_Instruction_t *CreateInst(STARS_InstructionID_t InstID) ;
+	virtual std::unique_ptr<STARS_Instruction_t> CreateInst(STARS_InstructionID_t InstID) ;
 
 // File methods.
 	virtual FILE *STARS_fopen(const char *file, const char *mode) 
diff --git a/src/base/SMPBasicBlock.cpp b/src/base/SMPBasicBlock.cpp
index 6694e512..0638b0d0 100644
--- a/src/base/SMPBasicBlock.cpp
+++ b/src/base/SMPBasicBlock.cpp
@@ -4104,11 +4104,18 @@ void SMPBasicBlock::MarkAllPhiUsesAliased(STARSOpndTypePtr &PhiMemOp) {
 			assert(nullptr != DefBlock);
 			if (DefBlockNum == (size_t) this->GetNumber()) {
 				// Infinite recursion? How can Phi USE be DEFed in the same Phi?
-				SMP_msg("ERROR: Phi USE is DEFed in same Phi function, block %d, func %s \n",
-					DefBlockNum, this->GetFunc()->GetFuncName());
-				SMP_msg("PhiMemOp: ");
-				PrintOperand(PhiMemOp);
-				SMP_msg("  Phi USESSANum: %d\n", PhiUseSSANum);
+				//  Answer: If there are more than 2 Phi USEs, then one is incoming value at
+				//   atop of loop, one is new value created inside the loop, and one is
+				//   from a path through the loop that does not change this variable, which
+				//   is no problem for non-induction variables. See how many Phi USEs we
+				//   have in this function.
+				if (2 == PhiUseLimit) {
+					SMP_msg("ERROR: Phi USE is DEFed in same Phi function, block %d, func %s \n",
+						DefBlockNum, this->GetFunc()->GetFuncName());
+					SMP_msg("PhiMemOp: ");
+					PrintOperand(PhiMemOp);
+					SMP_msg("  Phi USESSANum: %d\n", PhiUseSSANum);
+				}
 			}
 			else {
 				DefBlock->MarkAllPhiUsesAliased(PhiMemOp);
diff --git a/src/base/SMPFunction.cpp b/src/base/SMPFunction.cpp
index fb4c1882..30c2a13a 100644
--- a/src/base/SMPFunction.cpp
+++ b/src/base/SMPFunction.cpp
@@ -8566,10 +8566,12 @@ STARSExpression *SMPFunction::CreateSecondaryBIVLimitExpr(const std::size_t &Loo
 	if (nullptr == FirstInsideDefInst) {
 		return nullptr;
 	}
-	// Limit to multiplier of 1 initially. Generalize later.
+	// Limit to multiplier of 1 initially. Generalize and search for SCCP constants values later.
 	bool ImmedMultiplier = IVFamily.BasicInductionVar.Multiplier.GetOp()->IsImmedOp();
-	if (!ImmedMultiplier || (1 != IVFamily.BasicInductionVar.Multiplier.GetOp()->GetImmedValue())) {
-		SMP_msg("ERROR: BIV does not have multiplier of value 1.\n");
+	STARS_uval_t MultiplierValue = ImmedMultiplier ? IVFamily.BasicInductionVar.Multiplier.GetOp()->GetImmedValue() : 0;
+	if (!ImmedMultiplier || (1 != MultiplierValue)) {
+		SMP_msg("ERROR: BIV has multiplier of value %llu in loop %zu for Func starting at %llx\n",
+			(uint64_t) MultiplierValue, LoopIndex, (uint64_t) this->GetFirstFuncAddr());
 		return nullptr;
 	}
 	STARSOpndTypePtr StrideOp = IVFamily.BasicInductionVar.Addend.GetOp()->clone();
@@ -8809,7 +8811,7 @@ STARSExpression* SMPFunction::CreateIterationsExpr(std::size_t LoopIndex, const
 				&& (BranchOperator != SMP_BELOW) && (BranchOperator != SMP_BELOW_EQUAL) && (BranchOperator != SMP_NOT_EQUAL));
 		}
 	}
-	else { // Decrement by one on each iteration
+	else { // Decrement on each iteration
 		if (this->LoopComparisonExprs[LoopIndex].ExitsLoop) {
 			Undefined = ((BranchOperator != SMP_LESS_THAN) && (BranchOperator != SMP_LESS_EQUAL)
 				&& (BranchOperator != SMP_BELOW) && (BranchOperator != SMP_BELOW_EQUAL) && (BranchOperator != SMP_EQUAL));
diff --git a/src/base/SMPInstr.cpp b/src/base/SMPInstr.cpp
index d8879598..475d5acc 100644
--- a/src/base/SMPInstr.cpp
+++ b/src/base/SMPInstr.cpp
@@ -4872,7 +4872,7 @@ SMPInstr::SMPInstr(STARS_ea_t addr) : STARS_ID(addr) {
 	this->address = addr;
 #endif
 	this->StackPtrOffset = 0;
-	this->STARSInstPtr = global_stars_interface->CreateInst(this->STARS_ID);
+	this->STARSInstPtr = std::move(global_stars_interface->CreateInst(this->STARS_ID));
 #if 0
 	this->ResetGoodRTL();
 	this->ResetJumpTarget();
@@ -4933,6 +4933,7 @@ SMPInstr::~SMPInstr() {
 		this->Uses.clear();
 	}
  
+#if 0
 	if (global_STARS_program->IsIDAProDriverMode() && (nullptr != this->STARSInstPtr)) {
 		delete this->STARSInstPtr;
 		this->STARSInstPtr = nullptr;
@@ -4942,6 +4943,7 @@ SMPInstr::~SMPInstr() {
 		delete this->STARSInstPtr;
 		this->STARSInstPtr = nullptr;
 	}
+#endif
 
 	return;
 }
diff --git a/src/interfaces/irdb/STARS_IRDB_Function.cpp b/src/interfaces/irdb/STARS_IRDB_Function.cpp
index 1c4aea07..14a62936 100644
--- a/src/interfaces/irdb/STARS_IRDB_Function.cpp
+++ b/src/interfaces/irdb/STARS_IRDB_Function.cpp
@@ -231,7 +231,7 @@ void STARS_IRDB_Function_t::FindFixedCalls(SMPFunction *CurrFunc) {
 
 		SMPInstr *CurrInst = (*InstIter);
 #if 1
-		if (CurrInst->GetSTARSInstPtr()->IsJumpFromFixedCall()) 
+		if (CurrInst->IsJumpFromFixedCall()) 
 		{
 			CurrInst->SetFixedCallJump();
 #if 0
diff --git a/src/interfaces/irdb/STARS_IRDB_Interface.cpp b/src/interfaces/irdb/STARS_IRDB_Interface.cpp
index faf9eadd..fdbe4b87 100644
--- a/src/interfaces/irdb/STARS_IRDB_Interface.cpp
+++ b/src/interfaces/irdb/STARS_IRDB_Interface.cpp
@@ -1,4 +1,4 @@
-
+#include <memory>
 
 #include "interfaces/abstract/STARSInterface.h"
 #include "interfaces/irdb/STARSSegment.h"
@@ -15,27 +15,27 @@
 #include <stdio.h>
 #include <pqxx/pqxx>
 
+using namespace std;
 
-
-STARS_Instruction_t * STARS_IRDB_Interface_t::CreateInst(STARS_InstructionID_t InstID) 
+unique_ptr<STARS_Instruction_t> STARS_IRDB_Interface_t::CreateInst(STARS_InstructionID_t InstID)
 {
 	// check for pseudo-instructions
 	if (STARS_IsSSAMarkerPseudoID(InstID.GetIDWithinFile()))
-		return new STARS_IRDB_Instruction_t(InstID);
+		return unique_ptr<STARS_Instruction_t>{(STARS_Instruction_t *) new STARS_IRDB_Instruction_t(InstID)};
+
 	// already created, just return what we need.
 	STARS_Instruction_t* insn = nullptr;
+
 	if (InstID.HasCorrespondingInstructionIR())
 	{
-		insn = (STARS_Instruction_t*) InstID.GetInstruction();
+		return unique_ptr<STARS_Instruction_t>{(STARS_Instruction_t*)InstID.GetInstruction()};
 	}
 	else
 	{
 		// Look it up in the private map, create STARS_IRDB_Instruction_t with it.
-		insn = (STARS_Instruction_t*) new STARS_IRDB_Instruction_t(this->instr_id_to_irdb_insn_map[(IRDB_SDK::DatabaseID_t) InstID.GetIDWithinFile()]);
+		return unique_ptr<STARS_Instruction_t>{(STARS_Instruction_t*) new STARS_IRDB_Instruction_t(this->instr_id_to_irdb_insn_map[(IRDB_SDK::DatabaseID_t) InstID.GetIDWithinFile()])};
 	}
 
-	assert(insn != nullptr);
-	return insn;
 };
 
 
-- 
GitLab