From 7a97c9780919a2cc66b198eddfcd165dc999c8d6 Mon Sep 17 00:00:00 2001
From: clc5q <clc5q@git.zephyr-software.com>
Date: Thu, 7 Aug 2008 20:31:30 +0000
Subject: [PATCH] Clear up fine grained false positives from accesses to
 multiple stack frames from one inst.

---
 ProfilerInformation.cpp | 167 ++++++++++++++++++++++++++++++++++++++++
 ProfilerInformation.h   |  17 +++-
 SMPFunction.cpp         |  39 +++++++++-
 SMPFunction.h           |   9 +++
 SMPProgram.h            |   4 +-
 5 files changed, 232 insertions(+), 4 deletions(-)

diff --git a/ProfilerInformation.cpp b/ProfilerInformation.cpp
index c1fb0e14..6ff2777a 100644
--- a/ProfilerInformation.cpp
+++ b/ProfilerInformation.cpp
@@ -16,6 +16,8 @@
 
 #define qfeof feof
 
+#define SMP_DEBUG_DATA_GRANULARITY 1
+
 /*
  * ascii-to-longlong conversion; Visual Studio 2005 lacks atoll() library func
  *
@@ -370,10 +372,82 @@ MemoryAccessInfo::~MemoryAccessInfo() {
 	return;
 } // end of MemoryAccessInfo destructor
 
+// DEBUG dump.
+void MemoryAccessInfo::Dump(void) {
+	msg("Debug dump of MemoryAccessInfo:\n");
+	
+	set<ea_t>::iterator MultiIter;
+	msg("\nMultiAccess set: ");
+	int count = 0;
+	for (MultiIter = this->MultipleAccessTypeSet.begin(); 
+		MultiIter != this->MultipleAccessTypeSet.end();
+		++MultiIter) {
+
+		msg("%x ", *MultiIter);
+		++count;
+		if (0 == (count % 10))
+			msg("\n");
+	}
+	msg("\n");
+
+	map<ea_t, size_t>::iterator InstType;
+	msg("\nInst to Type map: ");
+	count = 0;
+	for (InstType = this->InstrTypeMap.begin(); InstType != this->InstrTypeMap.end(); ++InstType) {
+		msg("%x %d ", InstType->first, InstType->second);
+		++count;
+		if (0 == (count % 8))
+			msg("\n");
+	}
+	msg("\n");
+
+	map<ea_t, size_t>::iterator ObjType;
+	msg("\nObject to Type map: ");
+	count = 0;
+	for (ObjType = this->ObjectTypeMap.begin(); ObjType != this->ObjectTypeMap.end(); ++ObjType) {
+		msg("%x %d ", ObjType->first, ObjType->second);
+		++count;
+		if (0 == (count % 8))
+			msg("\n");
+	}
+	msg("\n");
+
+#define SMP_DEBUG_TYPE_NUMBER 170
+	if ((0 <= SMP_DEBUG_TYPE_NUMBER) 
+		&& (this->TypeAccessMap.size() > SMP_DEBUG_TYPE_NUMBER)) {
+
+		size_t TypeNo = SMP_DEBUG_TYPE_NUMBER;
+		msg("\nAccess map for type %d\n", TypeNo);
+		count = 0;
+		for (size_t ByteNo = 0; ByteNo < this->TypeAccessMap.at(TypeNo)->GetLimit(); ++ByteNo) {
+			SMPAccessType CurrType = this->TypeAccessMap.at(TypeNo)->GetAccessType(ByteNo);
+			msg("%d ", ByteNo);
+			if (CurrType == SMP_NO_ACCESS)
+				msg("U ");
+			else if (CurrType == SMP_DIRECT)
+				msg("D ");
+			else if (CurrType == SMP_INDIRECT)
+				msg("I ");
+			else {
+				assert(CurrType == SMP_SCALED);
+				msg("S ");
+			}
+			++count;
+			if (0 == (count % 10))
+				msg("\n");
+		}
+		msg("\n");
+	}
+
+	return;
+} // end of MemoryAccessInfo::Dump()
+
 // Process the information in the rest of a line that is of type
 //  INSTR PROF_FIELD (i.e. a profiler-generated map of memory access
 //  types from the instruction at address "addr").
 void MemoryAccessInfo::ProcessMemoryAccessAnnotation(FILE *fin, int addr) {
+
+	int CurrRegion = SMP_NO_REGION;
 	// Extract the remaining fields of the annotation.
 	char NameAddrsSize[MAXSTR];
 	char ObjectName[MAXSTR];
@@ -406,7 +480,11 @@ void MemoryAccessInfo::ProcessMemoryAccessAnnotation(FILE *fin, int addr) {
 		//  sizes. This is similar to having the instruction address
 		//  operate on objects of different sizes. We must punt on
 		//  this instruction address to be safe.
+#if 1
+		msg("Ignoring PROF_FIELD annotation with size -1 at %x\n",
+			InstructionAddress);
 		this->MultipleAccessTypeSet.insert(InstructionAddress);
+#endif
 		return;
 	}
 
@@ -432,6 +510,13 @@ void MemoryAccessInfo::ProcessMemoryAccessAnnotation(FILE *fin, int addr) {
 			return;
 		}
 		AllocationAddress = NameMapIter->second;
+		CurrRegion = SMP_GLOBAL;
+	}
+	else if (0 == strcmp("sr", ObjectName)) {
+		CurrRegion = SMP_STACK;
+	}
+	else {
+		CurrRegion = SMP_HEAP;
 	}
 
 	// Add to ObjectInstrMap.
@@ -498,6 +583,7 @@ void MemoryAccessInfo::ProcessMemoryAccessAnnotation(FILE *fin, int addr) {
 	//  and there is no size mismatch. We can update the access map using
 	//  the meet function over the new accesses recorded in the bit vector
 	//  in the annotation.
+	InstrAccessIter->second->SetRegion(CurrRegion);
 	size_t BytesProcessed = 0;
 	unsigned int CurrHexGroup;
 	size_t BitsProcessed = 0; // progresses 0 -> 1 -> 2 -> 0 ...
@@ -660,6 +746,11 @@ void MemoryAccessInfo::InferDataGranularity(void) {
 			}
 		}
 	} // end for all instructions in the InstrObjectMap
+
+#if SMP_DEBUG_DATA_GRANULARITY
+	this->Dump();
+#endif
+
 	return;
 } // end of MemoryAccessInfo::InferDataGranularity()
 
@@ -738,6 +829,82 @@ bool MemoryAccessInfo::ComputeNonDirectAccessRegion(ea_t InstAddr, int &ChildOff
 	InstAccessIter = this->InstrAccessMap.find(InstAddr);
 	assert(InstAccessIter != this->InstrAccessMap.end());
 
+	// If the InstAccess was to a stack frame of a caller (not the
+	//  stack frame of the function including InstAddr), and the
+	//  function including InstAddr has multiple callers, then we
+	//  have the dangerous case in which the profiling run only exercised
+	//  one path to this function but later application runs could
+	//  exercise paths from the other callers to this function. This
+	//  would have made InstAddr a multiple-access instruction if the
+	//  profiling run had detected it. We want to treat InstAddr
+	//  as a multiple-access instruction by not emitting CHILDACCESS
+	//  annotations for it.
+	// EXAMPLE: Functions A and B call C, but only A calls C in the
+	//  profiling run. Each caller passes a ptr to an object within
+	//  its stack frame. InstAddr in C uses that pointer to write
+	//  a result to memory. Based on the profiling run and the structure
+	//  of the stack frame in A, we might emit a CHILDACCESS annotation
+	//  restricting accesses from InstAddr to bytes 20-39 inside the
+	//  memory referent. During the application run, the path from B to C
+	//  is taken, and mmStrata tries to limit InstAddr to accessing
+	//  bytes 20-39 within the stack frame of B. But the stack frame
+	//  of B might have a completely different structure than the stack
+	//  frame of A, so false positives will result. There is no annotation
+	//  we can emit right now that works for the two different cases
+	//  with two different callers. In fact, we would have to have either
+	//  a union of the byte ranges for all callers, or a context sensitive
+	//  annotation in which mmStrata checks the calling context, and we don't
+	//  have profiler help for either approach, as only one path was hit.
+	if (SMP_STACK & InstAccessIter->second->GetRegion()) {
+		// Inst accessed a stack frame. Now, see if it accessed more
+		//  than one object.
+		map<ea_t, set<ea_t> >::iterator InstObjIter;
+		InstObjIter = this->InstrObjectMap.find(InstAddr);
+		assert(InstObjIter != this->InstrObjectMap.end());
+		if (1 < InstObjIter->second.size()) {
+			// Multiple objects accessed. At least one is a stack
+			//  frame. If any object accessed is not a stack frame, then
+			//  we have a dangerous coincidence in which a heap or global
+			//  object happens to have the same size as a stack frame,
+			//  in which case we don't want to emit a CHILDACCESS annotation
+			//  as the fine grained structures cannot possibly match.
+			//  More likely, multiple stack frames are being accessed,
+			//  and their fine grained structures are unlikely to match.
+			msg("No CHILDACCESS annotation for multi-caller frame access at %x\n",
+				InstAddr);
+			return false;
+		}
+		else {
+			// Only accessed one object, which was a stack frame. If it is
+			//  a caller's stack frame, and there is more than one potential
+			//  caller to the present instruction's function, then we
+			//  have the incomplete profiling situation detailed above.
+			ea_t InstFirstEA, InstLastEA;
+			func_t *InstFunc = get_func(InstAccessIter->first);
+			assert(NULL != InstFunc);
+			InstFirstEA = InstFunc->startEA;
+			InstLastEA = InstFunc->endEA;
+			ea_t AllocationAddr = *(InstObjIter->second.begin());
+			bool AccessedCallerFrame = 
+				((InstFirstEA > AllocationAddr) || (InstLastEA < AllocationAddr));
+			if (AccessedCallerFrame) {
+				// Accessed stack frame of a caller. Final test is to
+				//  see if this callee has multiple possible callers
+				//  according to IDA's cross references.
+				SMPFunction *InstFunction = this->MyProg->FindFunction(InstFirstEA);
+				assert(NULL != InstFunction);
+				if (1 < InstFunction->GetNumCallers()) {
+					// Multiple callers. Only one caller called this
+					//  callee during the profiling run, but others
+					//  could call it during any application run.
+					msg("No CHILDACCESS annotation for multi-caller frame access at %x\n",
+						InstAddr);
+					return false;
+				}
+			}
+		}
+	} // end if SMP_STACK region access
+
 	// Find the first and last non-directly accessed byte offsets from
 	//  this particular InstAddr.
 	ChildOffset = 0;
diff --git a/ProfilerInformation.h b/ProfilerInformation.h
index bdf821dd..f52a3803 100644
--- a/ProfilerInformation.h
+++ b/ProfilerInformation.h
@@ -86,18 +86,26 @@ enum SMPAccessType {
 	SMP_INDIRECT = 4 // indirect, but not scaled
 };
 
+#define SMP_NO_REGION 0
+#define SMP_GLOBAL 1
+#define SMP_STACK 2
+#define SMP_HEAP 4
+
 // A class to hold profiler information about the memory accesses performed
 //  at an instruction address.
 class SMPMemoryAccesses {
 public:
 	// Constructors and destructors.
 	SMPMemoryAccesses(ea_t addr);
+
 	// Get functions.
 	inline ea_t GetAddr(void) const { return address; };
 	inline size_t GetLimit(void) const { return ByteLimit; };
 	inline SMPAccessType GetAccessType(size_t index) const {
 		return ProfTypes.at(index);
 	}
+	inline int GetRegion(void) const { return region; };
+
 	// Set functions.
 	inline void SetLimit(size_t NewLimit) { 
 		ByteLimit = NewLimit; 
@@ -108,12 +116,18 @@ public:
 		ProfTypes[index] = TypeSeen;
 		return;
 	}
+	inline void SetRegion(int NewRegion) {
+		region |= NewRegion;
+		return;
+	}
+
 	// Analysis functions.
 	void MeetAccessType(size_t index, SMPAccessType TypeSeen);
 
 private:
 	ea_t address;
 	size_t ByteLimit;  // number of bytes in data object
+	int region;
 	vector<SMPAccessType> ProfTypes;  // access types seen by profiler,
 						// indexed by byte offset from start of object
 }; // end class SMPMemoryAccesses
@@ -125,6 +139,8 @@ public:
 	// Contructors and destructors.
 	MemoryAccessInfo(SMPProgram *CurrProg);
 	virtual ~MemoryAccessInfo();
+	// Debug methods.
+	void Dump(void);
 	// Analysis methods.
 	void ProcessMemoryAccessAnnotation(FILE *fin, int addr);
 	void InferDataGranularity(void);
@@ -173,6 +189,5 @@ private:
 		map<ea_t, set<ea_t> >::iterator InstObjIter);
 }; // end of class MemoryAccessInfo
 
-
 #endif
 
diff --git a/SMPFunction.cpp b/SMPFunction.cpp
index 80cd45b7..628fece6 100644
--- a/SMPFunction.cpp
+++ b/SMPFunction.cpp
@@ -123,6 +123,7 @@ SMPFunction::SMPFunction(func_t *Info, SMPProgram* pgm) {
 	this->DirectCallTargets.clear();
 	this->IndirectCallTargets.clear();
 	this->AllCallTargets.clear();
+	this->AllCallSources.clear();
 	this->InstBlockMap.clear();
 	this->RPOBlocks.clear();
 	this->IDom.clear();
@@ -153,6 +154,20 @@ void SMPFunction::ResetProcessedBlocks(void) {
 	return;
 } // end of SMPFunction::ResetProcessedBlocks()
 
+// Add a caller to the list of all callers of this function.
+void SMPFunction::AddCallSource(ea_t addr) {
+	// Convert call instruction address to beginning address of the caller.
+	func_t *FuncInfo = get_func(addr);
+	if (NULL == FuncInfo) {
+		msg("SERIOUS WARNING: Call location %x not in a function.\n", addr);
+		return;
+	}
+	ea_t FirstAddr = FuncInfo->startEA;
+	assert(BADADDR != FirstAddr);
+	this->AllCallSources.insert(FirstAddr);
+	return;
+} // end of SMPFunction::AddCallSource()
+
 // Figure out the different regions of the stack frame, and find the
 //  instructions that allocate and deallocate the local variables space
 //  on the stack frame.
@@ -1305,6 +1320,7 @@ void SMPFunction::EmitStackFrameAnnotations(FILE *AnnotFile, list<SMPInstr>::ite
 //  fills all objects for instructions, basic blocks, and the function
 //  itself.
 void SMPFunction::Analyze(void) {
+	bool FoundAllCallers = false;
 	list<SMPInstr>::iterator FirstInBlock = this->Instrs.end();
 	   // For starting a basic block
 	list<SMPInstr>::iterator LastInBlock = this->Instrs.end();
@@ -1405,11 +1421,32 @@ void SMPFunction::Analyze(void) {
 					}
 				}
 
+				// Find all functions that call the current function.
+				xrefblk_t CurrXrefs;
+				if (!FoundAllCallers) {
+					for (bool ok = CurrXrefs.first_to(CurrInst.GetAddr(), XREF_ALL);
+						ok;
+						ok = CurrXrefs.next_to()) {
+						if ((CurrXrefs.from != 0) && (CurrXrefs.iscode)) {
+							// Make sure it is not a fall-through. Must be a
+							//  control-flow instruction of some sort, including
+							//  direct or indirect calls or tail calls.
+							SMPInstr CallInst(CurrXrefs.from);
+							CallInst.Analyze();
+							SMPitype CallType = CallInst.GetDataFlowType();
+							if ((COND_BRANCH <= CallType) && (RETURN >= CallType)) {
+								// Found a caller, with its call address in CurrXrefs.from
+								this->AddCallSource(CurrXrefs.from);
+							}
+						}
+					}
+					FoundAllCallers = true; // only do this for first inst
+				}
+
 				if (CurrInst.GetDataFlowType() == INDIR_CALL) {
 					this->IndirectCalls = true;
 					// See if IDA has determined all possible targets
 					//  of the indirect call.
-					xrefblk_t CurrXrefs;
 					bool LinkedToTarget = false;
 					for (bool ok = CurrXrefs.first_from(CurrInst.GetAddr(), XREF_ALL);
 						ok;
diff --git a/SMPFunction.h b/SMPFunction.h
index afcd935a..b02a43fb 100644
--- a/SMPFunction.h
+++ b/SMPFunction.h
@@ -60,6 +60,7 @@ class SMPFunction {
 public:
 	// Constructors
 	SMPFunction(func_t *Info, SMPProgram *pgm);  // Default constructor
+
 	// Get methods
 	inline SMPProgram *GetProg(void) const { return Program; };
 	inline const char *GetFuncName(void) const { return FuncName; };
@@ -75,6 +76,8 @@ public:
 	inline const ea_t GetStartAddr() const { return FuncInfo.startEA; }; // exposing the start address of the function. Used in RecurseAndMark
 	inline const vector<ea_t> GetCallTargets() const { return AllCallTargets; };
 	bool GetIsSpeculative() { return IsSpeculative; }
+	inline size_t GetNumCallers(void) const { return AllCallSources.size(); };
+
 	// Set methods
 	inline void IncTypedPhiDefs(void) { ++TypedPhiDefs; return; };
 	inline void IncUntypedPhiDefs(void) { ++UntypedPhiDefs; return; };
@@ -88,6 +91,8 @@ public:
 	inline void SetNeedsFrame(bool Status) { NeedsStackReferent = Status; return; };
 	inline void SetSpecNeedsFrame(bool Status) { SpecNeedsStackReferent = Status; return; };
 	void SetIsSpeculative(bool IsS) { IsSpeculative = IsS; }
+	void AddCallSource(ea_t addr);
+
 	// Query methods
 	inline bool HasIndirectCalls(void) const { return IndirectCalls; };
 	inline bool HasUnresolvedIndirectCalls(void) const { return UnresolvedIndirectCalls; };
@@ -105,8 +110,10 @@ public:
 	inline bool WritesAboveReturnAddress(void) const { return WritesAboveRA; };
 	inline bool IsGlobalName(op_t RefOp) const { return (GlobalNames.end() != GlobalNames.find(RefOp)); };
 	inline bool UsesFramePointer(void) const { return UseFP; };
+
 	// Printing methods
 	void Dump(void); // debug dump
+
 	// Analysis methods
 	void ResetProcessedBlocks(void); // Set Processed flag to false in all blocks
 	void Analyze(void);  // Analyze all instructions in function
@@ -122,6 +129,7 @@ public:
 	void AnalyzeMetadataLiveness(void); // Is metadata live or dead for each inst
 	bool PropagateGlobalMetadata(op_t UseOp, SMPMetadataType Status, int SSANum);
 	void FindRedundantMetadata(void); // Do consecutive DEFs have same type?
+
 private:
 	// Data
 	SMPProgram* Program;		// pointer to the program I'm part of
@@ -169,6 +177,7 @@ private:
 	vector<ea_t> DirectCallTargets; // addresses called directly
 	vector<ea_t> IndirectCallTargets; // addresses called by indirect calls
 	vector<ea_t> AllCallTargets; // union of direct and indirect
+	set<ea_t> AllCallSources; // functions that call this one
 	map<ea_t, list<SMPBasicBlock>::iterator> InstBlockMap;
 	vector<list<SMPBasicBlock>::iterator> RPOBlocks;
 	vector<int> IDom; // Immediate dominators, indexed and valued by block RPO numbers
diff --git a/SMPProgram.h b/SMPProgram.h
index 02ed88ca..8162c098 100644
--- a/SMPProgram.h
+++ b/SMPProgram.h
@@ -62,8 +62,8 @@ struct GlobalVar {
 	set<pair<size_t, bool>, LessOff> FieldOffsets; // bool = accessed through index register by any instruction?
 };
 
-class LtStr{
-	public:
+class LtStr {
+public:
 	bool operator() (const char* c1, const char* c2) const {
 		return strcmp(c1, c2) < 0;
 	}
-- 
GitLab