From 267cfc1d8f9ddddd240c2bc469cad9540531739a Mon Sep 17 00:00:00 2001
From: Clark Coleman <clc@zephyr-software.com>
Date: Fri, 20 Nov 2020 23:42:45 -0500
Subject: [PATCH] Fix RPO block numbering, after only 14 years of error.
---
include/base/SMPFunction.h | 2 ++
src/base/SMPFunction.cpp | 67 +++++++++++++++++++++++++++++---------
2 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/include/base/SMPFunction.h b/include/base/SMPFunction.h
index 112e3558..9c79a723 100644
--- a/include/base/SMPFunction.h
+++ b/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
diff --git a/src/base/SMPFunction.cpp b/src/base/SMPFunction.cpp
index 481c8f4c..730bd4ab 100644
--- a/src/base/SMPFunction.cpp
+++ b/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);
--
GitLab