[llvm-commits] [PATCH] Fix a PassManager pointer use-after-free bug

[Zhou Sheng]

zhousheng00 added you to the CC list for the revision "Fix a PassManager pointer use-after-free bug".

Hi chandlerc,

The attached patch fixes a PassManager pointer use-after-free bug. Please kindly let me know if the patch is good to commit.
The bug can be triggered when we require LoopInfo analysis ahead of DominatorTree construction in a Module Pass.
The cause is that the LoopInfo analysis itself also invokes DominatorTree construction, therefore, when PassManager schedules LoopInfo, it will add DominatorTree first.
Then after that, when the PassManger turns to schedule DominatorTree invoked by the above ModulePass, it finds there is already a DominatorTree, so it delete the redundant one.
However, the freed pointer is still passed to FunctionPassManagerImpl::setLastUser() for access, and then results in crash

1657   FPP->add(RequiredPass);     //  may delete that passed in pass pointer
1658
1659   // Register P as the last user of RequiredPass.
1660   if (RequiredPass) {
1661     SmallVector<Pass *, 1> LU;
1662     LU.push_back(RequiredPass);
1663     FPP->setLastUser(LU,  P);                    // use-after-free
1664   }
1665 }

Here is the case to repeat the issue:

class MyMPass : public llvm::ModulePass
{
    public:
        static char ID;
        MyMPass() : ModulePass(ID) { }
        virtual bool runOnModule(Module& M);
        virtual void getAnalysisUsage(AnalysisUsage &AU) const
        {
            AU.addRequired<LoopInfo>();
            AU.addRequired<DominatorTree>();
        }
};

char MyMPass::ID = 0;
static RegisterPass<MyMPass> Y("mpass", "The module pass");

bool MyMPass::runOnModule(Module& M)
{
    for (Module::iterator i = M.begin(); i != M.end(); ++i)
    {
        if (i->isDeclaration()) continue;
        DominatorTree &DT = getAnalysis<DominatorTree>(*i);
    }
    return true;
}


http://llvm-reviews.chandlerc.com/D154

Files:
  lib/VMCore/PassManager.cpp
  unittests/VMCore/PassManagerTest.cpp

Index: lib/VMCore/PassManager.cpp
===================================================================
--- lib/VMCore/PassManager.cpp
+++ lib/VMCore/PassManager.cpp
@@ -1646,6 +1646,18 @@
 
     OnTheFlyManagers[P] = FPP;
   }
+
+  // If RequiredPass is an analysis pass and it is available then do not
+  // generate the analysis again. Stale analysis info should not be
+  // available at this point.
+  const PassInfo *PI =
+    PassRegistry::getPassRegistry()->getPassInfo(RequiredPass->getPassID());
+  if (PI && PI->isAnalysis() && 
+      FPP->getTopLevelManager()->findAnalysisPass(RequiredPass->getPassID())) {
+    delete RequiredPass;
+    return;
+  }
+
   FPP->add(RequiredPass);
 
   // Register P as the last user of RequiredPass.
Index: unittests/VMCore/PassManagerTest.cpp
===================================================================
--- unittests/VMCore/PassManagerTest.cpp
+++ unittests/VMCore/PassManagerTest.cpp
@@ -287,6 +287,27 @@
     };
     char OnTheFlyTest::ID=0;
 
+    struct OnTheFlySegfaultTest: public ModulePass {
+    public:
+      static char ID;
+      OnTheFlySegfaultTest() : ModulePass(ID) {}
+      virtual bool runOnModule(Module &M) {
+        for (Module::iterator I=M.begin(),E=M.end(); I != E; ++I) {
+          Function &F = *I;
+          {
+            SCOPED_TRACE("Running on the fly function pass to detect use-after-free issue");
+            getAnalysis<DominatorTree>(F);
+          }
+        }
+        return false;
+      }
+      virtual void getAnalysisUsage(AnalysisUsage &AU) const {
+        AU.addRequired<LoopInfo>();
+        AU.addRequired<DominatorTree>();
+      }
+    };
+    char OnTheFlySegfaultTest::ID=0;
+
     TEST(PassManager, RunOnce) {
       Module M("test-once", getGlobalContext());
       struct ModuleNDNM *mNDNM = new ModuleNDNM();
@@ -401,6 +422,18 @@
       delete M;
     }
 
+    TEST(PassManager, SegfaultOnTheFly) {
+      Module *M = makeLLVMModule();
+      {
+        SCOPED_TRACE("Running OnTheFlySegfaultTest");
+        struct OnTheFlySegfaultTest *O = new OnTheFlySegfaultTest();
+        PassManager Passes;
+        Passes.add(O);
+        Passes.run(*M);
+      }
+      delete M;
+    }
+
     Module* makeLLVMModule() {
       // Module Construction
       Module* mod = new Module("test-mem", getGlobalContext());
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D154.2.patch
Type: text/x-patch
Size: 2327 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121205/5c2d337c/attachment.bin>