Fix RPO block numbering, after only 14 years of error.

include/base/SMPFunction.h

@@ -562,6 +562,7 @@ public:
 	void MarkDominatedBlocks(int CurrBlockNum); // Set Processed flag for all blocks below CurrBlockNum in DomTree.
 	void ResetSCCPVisitedBlocks(void); // Set SCCPVisited flag to false in all blocks
 	void RPONumberBlocks(void); // Number basic blocks in reverse post-order and place pointers in RPOBlocks.
+	void CreatePostorderBlockList(std::list<SMPBasicBlock *> &PostorderList); // Create postorder traversal, no back edges followed.
 	int FindLastBlockProcessedInChain(const int StartingBlockNum) const; // Find greatest RPO block in chain from StartingBlockNum, no back edges followed.
 	int GetFallThroughPredBlockNum(int CurrBlockNum); // return block # of block that falls through to CurrBlockNum; SMP_BLOCKNUM_UNINIT if none
 	void RemoveBlock(SMPBasicBlock *CurrBlock, std::list<SMPBasicBlock *>::iterator &BlockIter, bool IBTarget = false); // Remove a basic block and its instructions.
@@ -958,6 +959,7 @@ private:
 	void MDAuditJumpXrefs(void); // Fix missing IDA Pro jump code xrefs
 	void RebuildCallTargets(void); // Rebuild AllCallTargets as the union of the direct and indirect call targets.
 	void EmitStackFrameAnnotations(FILE *AnnotFile, SMPInstr *Instr);
+	void PostorderBlockListHelper(SMPBasicBlock *CurrBlock, std::list<SMPBasicBlock *> &PostorderList); // recursive DFS block visitor
 	void ComputeIDoms(void); // Compute immediate dominators of all blocks into IDom[]
 	int IntersectDoms(int, int) const; // Find Dom intersection (as IDom[] index) for 2 blocks
 	void ComputeDomFrontiers(void); // Compute dominance frontiers for all blocks

src/base/SMPFunction.cpp

@@ -522,9 +522,8 @@ STARSExprSetIter SMPFunction::FindInArgExprBySerialNumber(const std::size_t Loop
 // Reset the Processed flags in all blocks to false.
 void SMPFunction::ResetProcessedBlocks(void) {
-	list<SMPBasicBlock *>::iterator CurrBlock;
-	for (CurrBlock = this->Blocks.begin(); CurrBlock != this->Blocks.end(); ++CurrBlock) {
-		(*CurrBlock)->SetProcessed(false);
+	for (SMPBasicBlock *CurrBlock : this->Blocks) {
+		CurrBlock->SetProcessed(false);
 	}
 	return;
 } // end of SMPFunction::ResetProcessedBlocks()
@@ -14095,6 +14094,20 @@ void SMPFunction::RPONumberBlocks(void) {
 	bool Renumbering = (!(this->RPOBlocks.empty()));
 	this->RPOBlocks.clear();  // we can re-do the RPO numbering as we remove unreachable blocks.
+#if 1
+	list<SMPBasicBlock *> PostorderBlockList;
+	this->CreatePostorderBlockList(PostorderBlockList);
+	this->ResetProcessedBlocks();
+	for (list<SMPBasicBlock *>::const_reverse_iterator BlockIter = PostorderBlockList.crbegin();
+		BlockIter != PostorderBlockList.crend();
+		++BlockIter) {
+		SMPBasicBlock *CurrBlock = (*BlockIter);
+		CurrBlock->SetNumber(CurrNum);
+		CurrBlock->SetProcessed(true);
+		++CurrNum;
+		this->RPOBlocks.push_back(CurrBlock);
+	}
+#else
 	// Number the first block with 0.
 	list<SMPBasicBlock *>::iterator BlockIter = this->Blocks.begin();
 	if ((BlockIter == this->Blocks.end()) || (0 == this->BlockCount)) {
@@ -14106,13 +14119,6 @@ void SMPFunction::RPONumberBlocks(void) {
 	}
 	SMPBasicBlock *CurrBlock = (*BlockIter);
-#if 0
-	if (this->RPOBlocks.capacity() <= (std::size_t) this->BlockCount) {
-		SMP_msg("Reserving %d RPOBlocks old value: %d\n", 2+this->BlockCount, this->RPOBlocks.capacity());
-		this->RPOBlocks.reserve(2 + this->BlockCount);
-		this->RPOBlocks.assign(2 + this->BlockCount, this->Blocks.end());
-	}
-#endif
 	CurrBlock->SetNumber(CurrNum);
 	CurrBlock->SetProcessed(true);
 	this->RPOBlocks.push_back(CurrBlock);
@@ -14205,6 +14211,8 @@ void SMPFunction::RPONumberBlocks(void) {
 		} // end if (change) ... else ...
 	} // end while work list is nonempty
+#endif
+
 	// Prior to construction of hell nodes for functions with indirect jumps, there
 	//  could still be unnumbered blocks because they appear to be unreachable
 	//  (no predecessors from SetLinks() because they are reached only via indirect
@@ -14215,8 +14223,7 @@ void SMPFunction::RPONumberBlocks(void) {
 	//  jump.
 	if (!Renumbering && (CurrNum < this->BlockCount)) { // already did this during the first numbering
 		if (this->HasIndirectJumps() || this->HasIndirectCalls()) {
-			for (BlockIter = this->Blocks.begin(); BlockIter != this->Blocks.end(); ++BlockIter) {
-				CurrBlock = (*BlockIter);
+			for (SMPBasicBlock *CurrBlock : this->Blocks) {
 				if (SMP_BLOCKNUM_UNINIT == CurrBlock->GetNumber()) {
 					SMP_msg("WARNING: Numbering indirectly reachable block at %llx\n", (unsigned long long) CurrBlock->GetFirstAddr());
 					CurrBlock->SetNumber(CurrNum);
@@ -14228,14 +14235,14 @@ void SMPFunction::RPONumberBlocks(void) {
 		}
 	}
 	// If we still have unnumbered blocks, it is not because of indirect jumps or calls.
-	//  We have some mysterious dead code.
+	//  We have some mysterious dead code. NOTE: Probably just try/catch blocks reachable from
+	//  addresses stored in the EH_FRAME section of the binary.
 	if (this->BlockCount > ((int) this->RPOBlocks.size())) {
 		SMP_msg("SERIOUS WARNING: RPONumberBlocks method: Function %s has BlockCount %d and RPOBlocks size %zu\n",
 			this->GetFuncName(), this->BlockCount, this->RPOBlocks.size());
-		for (BlockIter = this->Blocks.begin(); BlockIter != this->Blocks.end(); ++BlockIter) {
-			CurrBlock = (*BlockIter);
+		for (SMPBasicBlock *CurrBlock : this->Blocks) {
 			if (!(CurrBlock->IsProcessed())) {
-				SMP_msg("WARNING: Numbering apparently unreachable block at %llx\n", (unsigned long long) CurrBlock->GetFirstAddr());
+				SMP_msg("WARNING: Numbering apparently unreachable block at %llx\n", (uint64_t) CurrBlock->GetFirstAddr());
 				CurrBlock->SetNumber(CurrNum);
 				CurrBlock->SetProcessed(true);
 				this->RPOBlocks.push_back(CurrBlock);
@@ -14249,9 +14256,37 @@ void SMPFunction::RPONumberBlocks(void) {
 		STARS_MaxBlockCount = (unsigned long) this->BlockCount;
 		assert(this->BlockCount <= STARS_BLOCKNUM_MASK);
 	}
+
 	return;
 } // end of SMPFunction::RPONumberBlocks()
+void SMPFunction::CreatePostorderBlockList(list<SMPBasicBlock *> &PostorderList) {
+	this->ResetProcessedBlocks();
+	// Do a depth-first traversal, marking blocks visited, using those markings to avoid
+	//  taking back edges.
+	SMPBasicBlock *HeadBlock = this->Blocks.front();
+	HeadBlock->SetProcessed(true);
+	this->PostorderBlockListHelper(HeadBlock, PostorderList);
+
+	return;
+} // end of SMPFunction::CreatePostorderBlockList()
+
+void SMPFunction::PostorderBlockListHelper(SMPBasicBlock *CurrBlock, list<SMPBasicBlock *> &PostorderList) {
+	for (list<SMPBasicBlock *>::const_iterator SuccIter = CurrBlock->GetFirstConstSucc();
+		SuccIter != CurrBlock->GetLastConstSucc();
+		++SuccIter) {
+		SMPBasicBlock *SuccBlock = (*SuccIter);
+		if (!SuccBlock->IsProcessed()) {
+			SuccBlock->SetProcessed(true);
+			this->PostorderBlockListHelper(SuccBlock, PostorderList); // recurse
+		}
+	}
+	// After all successors are visited, push this block on the list.
+	PostorderList.push_back(CurrBlock);
+
+	return;
+} // end of SMPFunction::PostorderBlockListHelper()
+
 // Find greatest RPO block in chain from StartingBlockNum, no back edges followed.
 int SMPFunction::FindLastBlockProcessedInChain(const int StartingBlockNum) const {
 	assert(0 <= StartingBlockNum);