From 3845aaa3c31bc4c591dd83b497aeaf3a0af60f47 Mon Sep 17 00:00:00 2001
From: Clark Coleman <clc@zephyr-software.com>
Date: Tue, 14 Jan 2020 16:29:20 -0500
Subject: [PATCH] Add dot CFG output; add debugging output; fix for stack
 pointer analysis on recursive functions.

---
 include/base/SMPFunction.h |  1 +
 src/base/SMPBasicBlock.cpp | 14 +++---
 src/base/SMPFunction.cpp   | 90 +++++++++++++++++++++++++++++++-------
 src/base/SMPInstr.cpp      | 10 ++---
 src/base/SMPProgram.cpp    |  7 ++-
 5 files changed, 91 insertions(+), 31 deletions(-)

diff --git a/include/base/SMPFunction.h b/include/base/SMPFunction.h
index 76518fa1..32034e44 100644
--- a/include/base/SMPFunction.h
+++ b/include/base/SMPFunction.h
@@ -539,6 +539,7 @@ public:
 
 	// Printing methods
 	void Dump(void); // debug dump
+	void DumpDotCFG(void); // Dump CFG representation for processing by "dot" program.
 
 	// Analysis methods
 	void ResetProcessedBlocks(void); // Set Processed flag to false in all blocks
diff --git a/src/base/SMPBasicBlock.cpp b/src/base/SMPBasicBlock.cpp
index d64950c1..5e2d84ac 100644
--- a/src/base/SMPBasicBlock.cpp
+++ b/src/base/SMPBasicBlock.cpp
@@ -7314,7 +7314,8 @@ STARS_sval_t SMPBasicBlock::ComputeStackAdjustmentAfterCall(STARS_ea_t CallAddr)
 			}
 		}
 		else {
-			if (JUMP > DataFlowType) {
+			bool NopFlag = CurrInst->IsNop();
+			if ((JUMP > DataFlowType) && !NopFlag) {
 				JustFoundCallInst = false;
 				if (SMP_STACK_DELTA_ERROR_CODE == ((STARS_uval_t) CurrentDelta)) {
 					// An error will soon occur, no need for further computations.
@@ -7368,16 +7369,17 @@ STARS_sval_t SMPBasicBlock::ComputeStackAdjustmentAfterCall(STARS_ea_t CallAddr)
 			else {
 				// We want to break at calls after CallAddr, and other instructions that
 				//  hit this break would terminate the block anyway (e.g. jumps, branches).
-				if (JUMP != DataFlowType) {
+				if ((JUMP != DataFlowType) && !NopFlag) {
 					JustFoundCallInst = false;
+					break;
 				}
-				break;
 			}
 		}
-	}
+	} // end for each instruction in block
 
 	if (JustFoundCallInst) {
-		// We either ended the block with the call, or had the call followed by a jump.
+		// We either ended the block with the call, or had the call followed by a jump,
+		//  with no-ops being ignored.
 		//  In either case, we should have only one successor block to look at.
 		if (EndedWithTailCall) {
 			AdjustmentBytes = 0;
@@ -7404,7 +7406,7 @@ STARS_sval_t SMPBasicBlock::ComputeStackAdjustmentAfterCall(STARS_ea_t CallAddr)
 							(uint64_t)InstAddr, (uint64_t)CallAddr);
 						break; // stop at one adjustment, for simplicity in an already complicated case.
 					}
-					else if (SuccInst->MDIsMoveInstr()) {
+					else if (SuccInst->MDIsMoveInstr() || SuccInst->IsNop()) {
 						// Instruction scheduling might put moves before adjustment instrs.
 						;
 					}
diff --git a/src/base/SMPFunction.cpp b/src/base/SMPFunction.cpp
index 7cbdb5c2..0bac2e41 100644
--- a/src/base/SMPFunction.cpp
+++ b/src/base/SMPFunction.cpp
@@ -1962,6 +1962,10 @@ STARS_sval_t SMPFunction::ComputeGlobalStackAdjustment(bool AllowSingleCaller) {
 		return this->GlobalStackAdjustment;
 	}
 
+	bool DebugFlag = (0x8089910 == this->GetFirstFuncAddr());
+	if (DebugFlag)
+		SMP_msg("DEBUG: NumCallSites = %zu\n", NumCallSites);
+
 	if (AllowSingleCaller || (1 < NumCallSites)) { // if only one call site, it is dangerous to draw conclusions about seeming "adjustments."
 		set<STARS_ea_t>::iterator CallSiteIter;
 		for (CallSiteIter = this->AllCallSites.begin(); CallSiteIter != this->AllCallSites.end(); ++CallSiteIter) {
@@ -1972,6 +1976,10 @@ STARS_sval_t SMPFunction::ComputeGlobalStackAdjustment(bool AllowSingleCaller) {
 			SMPFunction *CallerFunc = this->GetProg()->FindFunction(CallerFirstAddr);
 			assert(nullptr != CallerFunc);
 			STARS_sval_t CurrentAdjustment = CallerFunc->GetStackAdjustmentForCallee(CallSiteAddr);
+			if (DebugFlag)
+				SMP_msg("DEBUG: CurrentAdjustment = %ld at CallSiteAddr %llx\n", (long) CurrentAdjustment,
+				(uint64_t) CallSiteAddr);
+
 			// See if CurrentAdjustment is a new, lowest positive value for GlobalAdjustment.
 			if ((0 < CurrentAdjustment) && (CurrentAdjustment < PositiveAdjustment)) {
 				PositiveAdjustment = CurrentAdjustment;
@@ -2054,6 +2062,9 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 #if SMP_COMPARE_IDA_STARS_STACK_POINTER_DELTAS
 	DebugFlag = (0 == strcmp("_dl_profile_fixup", this->GetFuncName()));
 	TraceFlag = (0 == strcmp("_dl_profile_fixup", this->GetFuncName()));
+	bool DebugFuncFound = (0x8089910 == this->GetFirstFuncAddr());
+	DebugFlag = DebugFlag || DebugFuncFound;
+	TraceFlag = TraceFlag || DebugFuncFound;
 #endif
 
 	if (!this->HasGoodRTLs()) {
@@ -2182,7 +2193,7 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 		STARS_sval_t IncomingDelta = WorkList.front().second;
 
 		if (TraceFlag) {
-			SMP_msg("TRACE: Starting block %d with IncomingDelta of %lld\n", CurrBlock->GetNumber(), (long long) IncomingDelta);
+			SMP_msg("TRACE: Starting block %d with IncomingDelta of %lld\n", CurrBlock->GetNumber(), (int64_t) IncomingDelta);
 		}
 #if SMP_COMPARE_IDA_STARS_STACK_POINTER_DELTAS
 		if (DebugFlag && IDAProSucceeded) {
@@ -2192,13 +2203,13 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 				if (IDAProDelta != IncomingDelta) {
 					intmax_t IDADelta = (intmax_t) IDAProDelta, OurDelta = (intmax_t) IncomingDelta;
 					if (sizeof(STARS_sval_t) == 8) {
-						SMP_msg("ERROR: At %p block entry IDA Pro has stack pointer delta of %jd and we compute %jd BlockProcessed: %d\n",
-							InstAddr, IDADelta, OurDelta, CurrBlock->IsProcessed());
+						SMP_msg("ERROR: At inst %p block %d entry IDA Pro has stack pointer delta of %jd and we compute %jd BlockProcessed: %d\n",
+							InstAddr, CurrBlock->GetNumber(), IDADelta, OurDelta, CurrBlock->IsProcessed());
 					}
 					else {
 						assert(sizeof(STARS_sval_t) == 4);
-						SMP_msg("ERROR: At %p block entry IDA Pro has stack pointer delta of %d and we compute %d BlockProcessed: %d\n",
-							InstAddr, IDAProDelta, IncomingDelta, CurrBlock->IsProcessed());
+						SMP_msg("ERROR: At inst %p block %d entry IDA Pro has stack pointer delta of %d and we compute %d BlockProcessed: %d\n",
+							InstAddr, CurrBlock->GetNumber(), IDAProDelta, IncomingDelta, CurrBlock->IsProcessed());
 					}
 				}
 			}
@@ -2207,7 +2218,7 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 
 		if (0 < IncomingDelta) {
 			SMP_msg("ERROR: Stack delta of %ld implies stack underflow in func at %llx\n",
-				(long) IncomingDelta, (unsigned long long) this->GetFirstFuncAddr());
+				(long) IncomingDelta, (uint64_t) this->GetFirstFuncAddr());
 			this->AnalyzedSP = false;
 			WorkList.clear();
 			break;
@@ -2232,6 +2243,11 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 				for (SuccIter = CurrBlock->GetFirstSucc(); SuccIter != CurrBlock->GetLastSucc(); ++SuccIter) {
 					pair<SMPBasicBlock *, STARS_sval_t> SuccPair (*SuccIter, SuccIncomingDelta);
 					WorkList.push_back(SuccPair);
+					if (TraceFlag) {
+						int SuccBlockNum = (*SuccIter)->GetNumber();
+						SMP_msg("TRACE: Pushing SuccBlock %d from CurrBlock %d with IncomingDelta %d\n",
+							SuccBlockNum, CurrBlock->GetNumber(), (int) SuccIncomingDelta);
+					}
 				}
 			}
 			InstIter = CurrBlock->GetFirstInst();
@@ -2263,8 +2279,10 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 					ReprocessingAllocaBlocks = true;
 					DeltaIncrement = IncomingDelta - PrevIncomingDelta;
 					if (0 != (DeltaIncrement % 32)) { // 32 bytes is our alloca fudge increment
-						SMP_msg("WARNING: DeltaIncrement of %d not alloca multiple for block at %llx\n",
-							(unsigned long long) CurrBlock->GetFirstAddr());
+						SMP_msg("WARNING: DeltaIncrement of %ld not alloca multiple for block %d at %llx\n",
+							(long)DeltaIncrement, CurrBlock->GetNumber(), (uint64_t) CurrBlock->GetFirstAddr());
+						if ((8 >= DeltaIncrement) && (0 <= DeltaIncrement))
+							this->Dump();
 					}
 					continue;  // Make the loop come around and process this block again, using
 							   //  the new incoming delta. Because we do this only when it decreases
@@ -2274,9 +2292,9 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 #if 1
 			else {
 				this->AnalyzedSP = false;
-				SMP_msg("ERROR: Stack delta: PrevIncoming is %ld NewIncoming is %ld at %llx in func %s\n",
+				SMP_msg("ERROR: Stack delta: PrevIncoming is %ld NewIncoming is %ld at %llx block %d in func %s\n",
 					(long) PrevIncomingDelta, (long) IncomingDelta, (unsigned long long) (*InstIter)->GetAddr(),
-					this->GetFuncName());
+					CurrBlock->GetNumber(), this->GetFuncName());
 				WorkList.clear();
 			}
 #endif
@@ -2349,7 +2367,7 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 				}
 				if (TraceFlag) {
 					SMP_msg("INFO: Stack delta trace: %lld at %llx\n",
-						(long long) IncomingDelta, (unsigned long long) InstAddr);
+						(long long) IncomingDelta, (uint64_t) InstAddr);
 				}
 #endif
 
@@ -2413,7 +2431,7 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 					this->AnalyzedSP = false;
 					WorkList.clear();
 					SMP_msg("ERROR: ErrorFlag=true from MDIsStackPtrSaveOrRestore() at %llx\n",
-						(unsigned long long) InstAddr);
+						(uint64_t) InstAddr);
 					break;
 				}
 				else if (CurrInst->MDIsLeaveInstr()) {
@@ -2449,16 +2467,16 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 						// For now, we ignore instructions that AND a constant into the stack pointer.
 						CurrentDelta = 0;
 						SMP_msg("WARNING: Stack pointer bitwise AND ignored at %llx\n",
-							(unsigned long long) CurrInst->GetAddr());
+							(uint64_t) CurrInst->GetAddr());
 					}
 					else if (SMP_STACK_DELTA_ERROR_CODE == ((STARS_uval_t) CurrentDelta)) {
 						this->AnalyzedSP = false;
-						SMP_msg("ERROR: Stack delta unanalyzeable at %llx\n", (unsigned long long) InstAddr);
+						SMP_msg("ERROR: Stack delta unanalyzeable at %llx\n", (uint64_t) InstAddr);
 						WorkList.clear();
 						break;
 					}
 					if (TraceFlag) {
-						SMP_msg("TRACE: CurrentDelta is %lld at %llx\n", (long long) CurrentDelta, (unsigned long long) InstAddr);
+						SMP_msg("TRACE: CurrentDelta is %lld at %llx\n", (long long) CurrentDelta, (uint64_t) InstAddr);
 					}
 					SMPitype FlowType = CurrInst->GetDataFlowType();
 					IncomingDelta += CurrentDelta;
@@ -2485,7 +2503,7 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 #if SMP_AUDIT_STACK_POINTER_DELTAS
 							if (CALLING_CONVENTION_DEFAULT_FUNCTION_STACK_DELTA != IncomingDelta) {
 								SMP_msg("WARNING: Stack delta not %d after return instruction at %llx\n", 
-									CALLING_CONVENTION_DEFAULT_FUNCTION_STACK_DELTA, (unsigned long long) CurrInst->GetAddr());
+									CALLING_CONVENTION_DEFAULT_FUNCTION_STACK_DELTA, (uint64_t) CurrInst->GetAddr());
 							}
 #endif
 						}
@@ -2495,7 +2513,7 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 						//  push will not be undone by add esp,32. It must be undone by something like mov esp,ebp.
 						if (ConflictingValuesSeen && !StackPointerRestoreSeen) {
 							SMP_msg("ERROR: Inconsistent stack deltas seen, no stack pointer restore before return instruction at %llx\n",
-								(unsigned long long) CurrInst->GetAddr());
+								(uint64_t) CurrInst->GetAddr());
 							this->AnalyzedSP = false;
 							WorkList.clear();
 							break;
@@ -2518,6 +2536,11 @@ bool SMPFunction::AnalyzeStackPointerDeltas(void) {
 					for (SuccIter = CurrBlock->GetFirstSucc(); SuccIter != CurrBlock->GetLastSucc(); ++SuccIter) {
 						pair<SMPBasicBlock *, STARS_sval_t> SuccPair (*SuccIter, IncomingDelta);
 						WorkList.push_back(SuccPair);
+						if (TraceFlag) {
+							int SuccBlockNum = (*SuccIter)->GetNumber();
+							SMP_msg("TRACE: Pushing SuccBlock %d from CurrBlock %d with IncomingDelta %d\n",
+								SuccBlockNum, CurrBlock->GetNumber(), (int) IncomingDelta);
+						}
 					}
 				}
 			}
@@ -17931,6 +17954,39 @@ void SMPFunction::Dump(void) {
 	return;
 } // end of SMPFunction::Dump()
 
+// Dump CFG representation for processing by "dot" program.
+void SMPFunction::DumpDotCFG(void) {
+	string DotFileName("STARS.");
+	char FuncBuf[20] = { '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0' };
+	// itoa(this->GetFirstFuncAddr(), FuncBuf, 16);
+	SMP_snprintf(FuncBuf, 18, "%x", this->GetFirstFuncAddr());
+	string FuncName("func");
+	FuncName += string(FuncBuf);
+	string FileExtension(".gv");
+	DotFileName += FuncName;
+	DotFileName += FileExtension;
+	FILE *DotFile = SMP_fopen(DotFileName.c_str(), "w");
+	if (nullptr == DotFile) {
+		SMP_msg("ERROR: Cannot open dot file %s for CFG dump.\n", DotFileName.c_str());
+		return;
+	}
+
+	SMP_fprintf(DotFile, "digraph %s {\n", FuncName.c_str());
+
+	for (SMPBasicBlock *CurrBlock : this->RPOBlocks) {
+		if (CurrBlock->GetNumSuccessors() > 0) {
+			int CurrBlockNum = CurrBlock->GetNumber();
+			for (std::list<SMPBasicBlock *>::const_iterator CurrLink = CurrBlock->GetFirstConstSucc(); CurrLink != CurrBlock->GetLastConstSucc(); ++CurrLink) {
+				SMP_fprintf(DotFile, "  %d -> %d\n", CurrBlockNum, (*CurrLink)->GetNumber());
+			}
+		}
+	}
+	SMP_fprintf(DotFile, "}\n");
+	(void) SMP_fclose(DotFile);
+
+	return;
+} // end of SMPFunction::DumpDotCFG()
+
 #define STARS_DEBUG_DUPLICATE_SEARCHES 1
 
 // Is DefOp+DefSSANum at DefAddr used as address reg or as source operand in memory write?
diff --git a/src/base/SMPInstr.cpp b/src/base/SMPInstr.cpp
index eb11d418..930e7ec1 100644
--- a/src/base/SMPInstr.cpp
+++ b/src/base/SMPInstr.cpp
@@ -9197,10 +9197,7 @@ STARS_sval_t SMPInstr::AnalyzeStackPointerDelta(STARS_sval_t IncomingDelta, STAR
 		//  has no effect on the stack pointer.
 		; // leave InstDelta equal to negative or zero value from StackAlterationTable[]
 	}
-	else if (this->IsRecursiveCall() || TailCall) {
-		// We don't have the net stack delta for our own function yet, so we cannot
-		//  look it up. We must assume that each call has no net effect on the stack delta.
-		// Alternatively, we could call this->GetBlock()->GetFunc()->GetStackDeltaForCallee() as below.
+	else if (TailCall) {
 		// Also, a tail call happens when the stack delta is down to zero, and the callee does not
 		//  return to the caller, unlike the call cases below, so the callee's net stack delta is
 		//  irrelevant to the caller.
@@ -9216,6 +9213,7 @@ STARS_sval_t SMPInstr::AnalyzeStackPointerDelta(STARS_sval_t IncomingDelta, STAR
 		InstDelta = STARS_DEFAULT_ALLOCA_SIZE;
 	}
 	else if ((CALL == FlowType) || (INDIR_CALL == FlowType)) {
+		// NOTE: Recursive calls are now handled in this code. 13-JAN-2020
 		// A real call instruction, which pushes a return address on the stack,
 		//  not a call used as a branch within the function. A return instruction
 		//  will usually cancel out the stack push that is implicit in the call, which 
@@ -9254,13 +9252,13 @@ STARS_sval_t SMPInstr::AnalyzeStackPointerDelta(STARS_sval_t IncomingDelta, STAR
 					if (TailCall) {
 						InstDelta = CALLING_CONVENTION_DEFAULT_FUNCTION_STACK_DELTA;
 						SMP_msg("WARNING: Callee stack ptr analysis not yet performed at tail call inst %llx ; normal delta assumed\n",
-							(unsigned long long) this->GetAddr());
+							(uint64_t) this->GetAddr());
 					}
 					else {
 						AdjustmentDelta = this->GetBlock()->GetFunc()->GetStackDeltaForCallee(CalledFuncAddr);
 						InstDelta += AdjustmentDelta;
 						SMP_msg("WARNING: Callee stack ptr analysis not yet performed at inst %llx ; stack adjustment used\n",
-							(unsigned long long) this->GetAddr());
+							(uint64_t) this->GetAddr());
 					}
 				}
 				else if (!CalleeFunc->StackPtrAnalysisSucceeded()) {
diff --git a/src/base/SMPProgram.cpp b/src/base/SMPProgram.cpp
index 182deff1..a5d10c9c 100644
--- a/src/base/SMPProgram.cpp
+++ b/src/base/SMPProgram.cpp
@@ -854,9 +854,12 @@ void SMPProgram::Analyze(ProfilerInformation *pi, FILE *AnnotFile, FILE *InfoAnn
 #endif
 				changed |= CurrFunc->InferInterproceduralTypes();
 			}
-#if 0
-			if ((!changed || (IterationCounter > STARS_INTERPROCEDURAL_ITERATION_LIMIT)) && (0 == strcmp("__mktime_internal", CurrFunc->GetFuncName()))) {
+#if 1
+			// bool FuncFound = (0 == strcmp("__mktime_internal", CurrFunc->GetFuncName()));
+			bool FuncFound = (0x8089910 == CurrFunc->GetFirstFuncAddr());
+			if ((!changed || (IterationCounter > STARS_INTERPROCEDURAL_ITERATION_LIMIT)) && FuncFound) {
 				CurrFunc->Dump();
+				CurrFunc->DumpDotCFG();
 			}
 #endif		
 		}
-- 
GitLab