From 063d06527e4e74be341d45866723edcace817337 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Tue, 1 Sep 2009 06:31:31 +0000 Subject: Change CallGraphNode to maintain it's Function as an AssertingVH for sanity. This didn't turn up any bugs. Change CallGraphNode to maintain its "callsite" information in the call edges list as a WeakVH instead of as an instruction*. This fixes a broad class of dangling pointer bugs, and makes CallGraph have a number of useful invariants again. This fixes the class of problem indicated by PR4029 and PR3601. llvm-svn: 80663 --- llvm/lib/Analysis/IPA/CallGraphSCCPass.cpp | 48 +++++++++++++++++++----------- 1 file changed, 30 insertions(+), 18 deletions(-) (limited to 'llvm/lib/Analysis/IPA/CallGraphSCCPass.cpp') diff --git a/llvm/lib/Analysis/IPA/CallGraphSCCPass.cpp b/llvm/lib/Analysis/IPA/CallGraphSCCPass.cpp index 2d5600d6e79..1dec9c5b6b3 100644 --- a/llvm/lib/Analysis/IPA/CallGraphSCCPass.cpp +++ b/llvm/lib/Analysis/IPA/CallGraphSCCPass.cpp @@ -125,7 +125,7 @@ bool CGPassManager::RunPassOnSCC(Pass *P, std::vector &CurSCC, void CGPassManager::RefreshCallGraph(std::vector &CurSCC, CallGraph &CG) { - DenseMap CallSites; + DenseMap CallSites; DEBUG(errs() << "CGSCCPASSMGR: Refreshing SCC with " << CurSCC.size() << " nodes:\n"; @@ -146,12 +146,28 @@ void CGPassManager::RefreshCallGraph(std::vector &CurSCC, // Get the set of call sites currently in the function. for (CallGraphNode::iterator I = CGN->begin(), E = CGN->end(); I != E; ++I){ - assert(I->first.getInstruction() && - "Call site record in function should not be abstract"); - assert(!CallSites.count(I->first.getInstruction()) && + // If this call site is null, then the function pass deleted the call + // entirely and the WeakVH nulled it out. + if (I->first == 0 || + // If we've already seen this call site, then the FunctionPass RAUW'd + // one call with another, which resulted in two "uses" in the edge + // list of the same call. + CallSites.count(I->first) || + + // If the call edge is not from a call or invoke, then the function + // pass RAUW'd a call with another value. This can happen when + // constant folding happens of well known functions etc. + CallSite::get(I->first).getInstruction() == 0) { + // Just remove the edge from the set of callees. + CGN->removeCallEdge(I); + E = CGN->end(); + --I; + continue; + } + + assert(!CallSites.count(I->first) && "Call site occurs in node multiple times"); - CallSites.insert(std::make_pair(I->first.getInstruction(), - I->second)); + CallSites.insert(std::make_pair(I->first, I->second)); } // Loop over all of the instructions in the function, getting the callsites. @@ -162,7 +178,7 @@ void CGPassManager::RefreshCallGraph(std::vector &CurSCC, // If this call site already existed in the callgraph, just verify it // matches up to expectations and remove it from CallSites. - DenseMap::iterator ExistingIt = + DenseMap::iterator ExistingIt = CallSites.find(CS.getInstruction()); if (ExistingIt != CallSites.end()) { CallGraphNode *ExistingNode = ExistingIt->second; @@ -201,18 +217,14 @@ void CGPassManager::RefreshCallGraph(std::vector &CurSCC, } // After scanning this function, if we still have entries in callsites, then - // they are dangling pointers. Crap. Well, until we change CallGraph to - // use CallbackVH, we'll just zap them here. When we have that, this should - // turn into an assertion. - if (CallSites.empty()) continue; + // they are dangling pointers. WeakVH should save us for this, so abort if + // this happens. + assert(CallSites.empty() && "Dangling pointers found in call sites map"); - for (DenseMap::iterator I = CallSites.begin(), - E = CallSites.end(); I != E; ++I) - // FIXME: I had to add a special horrible form of removeCallEdgeFor to - // support this. Remove the Instruction* version of it when we can. - CGN->removeCallEdgeFor(I->first); - MadeChange = true; - CallSites.clear(); + // Periodically do an explicit clear to remove tombstones when processing + // large scc's. + if ((sccidx & 15) == 0) + CallSites.clear(); } DEBUG(if (MadeChange) { -- cgit v1.2.3