[CodeGen] Don't keep stale pointers to LoopInfos

[ebevhan]

Summary

CGLoopInfo was keeping pointers to parent loop LoopInfos,
but when the loop info vector grew, it reallocated the
storage and invalidated all of the parent pointers, causing
use-after-free.

Manage the lifetimes of the LoopInfos separately so that
the pointers aren't stale.

[aaron.ballman]

LGTM!

[ebevhan]

Thanks!

I don't have commit access, so it would be great if you could commit this for me.

[aaron.ballman]

I've commit for you in r369259, thank you for the patch!

lib/CodeGen/CGLoopInfo.h

void setPipelineInitiationInterval(unsigned C) {
     StagedAttrs.PipelineInitiationInterval = C;
   }
 
 private:
   /// Returns true if there is LoopInfo on the stack.
   bool hasInfo() const { return !Active.empty(); }
   /// Return the LoopInfo for the current loop. HasInfo should be called
   /// first to ensure LoopInfo is present.
-  const LoopInfo &getInfo() const { return Active.back(); }
+  const LoopInfo &getInfo() const { return *Active.back(); }
   /// The set of attributes that will be applied to the next pushed loop.
   LoopAttributes StagedAttrs;
   /// Stack of active loops.
-  llvm::SmallVector<LoopInfo, 4> Active;
+  llvm::SmallVector<std::unique_ptr<LoopInfo>, 4> Active;
 };
 
 } // end namespace CodeGen
 } // end namespace clang
 
 #endif

lib/CodeGen/CGLoopInfo.cpp

void LoopInfo::finish() {
 
   bool HasUserTransforms = false;
   LoopID = createMetadata(CurLoopAttr, {}, HasUserTransforms);
   TempLoopID->replaceAllUsesWith(LoopID);
 }
 
 void LoopInfoStack::push(BasicBlock *Header, const llvm::DebugLoc &StartLoc,
                          const llvm::DebugLoc &EndLoc) {
-  Active.push_back(LoopInfo(Header, StagedAttrs, StartLoc, EndLoc,
-                            Active.empty() ? nullptr : &Active.back()));
+  Active.emplace_back(
+      new LoopInfo(Header, StagedAttrs, StartLoc, EndLoc,
+                   Active.empty() ? nullptr : Active.back().get()));
   // Clear the attributes so nested loops do not inherit them.
   StagedAttrs.clear();
 }
 
 void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
                          ArrayRef<const clang::Attr *> Attrs,
                          const llvm::DebugLoc &StartLoc,
                          const llvm::DebugLoc &EndLoc) {
void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
   }
 
   /// Stage the attributes.
   push(Header, StartLoc, EndLoc);
 }
 
 void LoopInfoStack::pop() {
   assert(!Active.empty() && "No active loops to pop");
-  Active.back().finish();
+  Active.back()->finish();
   Active.pop_back();
 }
 
 void LoopInfoStack::InsertHelper(Instruction *I) const {
   if (I->mayReadOrWriteMemory()) {
     SmallVector<Metadata *, 4> AccessGroups;
-    for (const LoopInfo &AL : Active) {
+    for (const auto &AL : Active) {
       // Here we assume that every loop that has an access group is parallel.
-      if (MDNode *Group = AL.getAccessGroup())
+      if (MDNode *Group = AL->getAccessGroup())
         AccessGroups.push_back(Group);
     }
     MDNode *UnionMD = nullptr;
     if (AccessGroups.size() == 1)
       UnionMD = cast<MDNode>(AccessGroups[0]);
     else if (AccessGroups.size() >= 2)
       UnionMD = MDNode::get(I->getContext(), AccessGroups);
     I->setMetadata("llvm.access.group", UnionMD);

test/CodeGen/loop-info-asan.c

+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -o /dev/null
+
+// This test should not exhibit use-after-free in LoopInfo.
+
+int a() {
+  for (;;)
+    for (;;)
+      for (;;)
+        for (;;)
+          for (;;)
+            for (;;)
+              for (;;)
+                for (;;)
+                  for (;;)
+                    ;
+}