From fa2fcf173b5749e402599090c363e091c3c572a2 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Fri, 20 Feb 2015 19:51:56 +0000 Subject: [PATCH] [GC, RewriteStatepointsForGC] Style cleanup and bug fix When doing style cleanup, I noticed a minor bug in this code. If we have a pointer that we think is unused after a statepoint and thus doesn't need relocation, we store a null pointer into the alloca we're about to promote. This helps turn a mistake in liveness analysis into an easily debuggable crash. It turned out this code had never been updated to handle invoke statepoints. There's no test for this. Without a bug in liveness, it appears impossible to make this trigger in a way which is visible in the resulting IR. We might store the null, but when promoting the alloca, there will be no uses and thus nothing to test against. Suggestions on how to test are very welcome. llvm-svn: 230047 --- .../Transforms/Scalar/RewriteStatepointsForGC.cpp | 38 +++++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index 5ce4267..4f1e8a9 100644 --- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -1538,21 +1538,41 @@ static void relocationViaAlloca( #ifndef NDEBUG // As a debuging aid, pretend that an unrelocated pointer becomes null at // the gc.statepoint. This will turn some subtle GC problems into slightly - // easy to debug SEGVs + // easier to debug SEGVs + SmallVector ToClobber; for (auto Pair : allocaMap) { - Value *def = Pair.first; - Value *alloca = Pair.second; + Value *Def = Pair.first; + AllocaInst *Alloca = cast(Pair.second); // This value was relocated - if (visitedLiveValues.count(def)) { + if (visitedLiveValues.count(Def)) { continue; } - - auto PT = cast(def->getType()); - Constant *CPN = ConstantPointerNull::get(PT); - StoreInst *store = new StoreInst(CPN, alloca); - store->insertBefore(info.SafepointBounds.second); + ToClobber.push_back(Alloca); } + + Instruction *Statepoint = info.SafepointBounds.first; + auto InsertClobbersAt = [&](Instruction *IP) { + for (auto *AI : ToClobber) { + auto AIType = cast(AI->getType()); + auto PT = cast(AIType->getElementType()); + Constant *CPN = ConstantPointerNull::get(PT); + StoreInst *store = new StoreInst(CPN, AI); + store->insertBefore(IP); + } + }; + + // Insert the clobbering stores. These may get intermixed with the + // gc.results and gc.relocates, but that's fine. + if (auto II = dyn_cast(Statepoint)) { + InsertClobbersAt(II->getNormalDest()->getFirstInsertionPt()); + InsertClobbersAt(II->getUnwindDest()->getFirstInsertionPt()); + } else if (auto CI = dyn_cast(Statepoint)) { + BasicBlock::iterator Next(CI); + Next++; + InsertClobbersAt(Next); + } else + llvm_unreachable("illegal statepoint instruction type?"); #endif } // update use with load allocas and add store for gc_relocated -- 2.7.4