[PATCH] D66206: [CodeGen] Don't keep stale pointers to LoopInfos

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66206/new/

https://reviews.llvm.org/D66206



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66206: [CodeGen] Don't keep stale pointers to LoopInfos

2019-08-19 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment.

Thanks!

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


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66206/new/

https://reviews.llvm.org/D66206



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66206: [CodeGen] Don't keep stale pointers to LoopInfos

2019-08-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66206/new/

https://reviews.llvm.org/D66206



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66206: [CodeGen] Don't keep stale pointers to LoopInfos

2019-08-14 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan created this revision.
ebevhan added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D66206

Files:
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  test/CodeGen/loop-info-asan.c


Index: test/CodeGen/loop-info-asan.c
===
--- /dev/null
+++ test/CodeGen/loop-info-asan.c
@@ -0,0 +1,16 @@
+// 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 (;;)
+;
+}
Index: lib/CodeGen/CGLoopInfo.h
===
--- lib/CodeGen/CGLoopInfo.h
+++ lib/CodeGen/CGLoopInfo.h
@@ -275,11 +275,11 @@
   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 () const { return Active.back(); }
+  const LoopInfo () 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 Active;
+  llvm::SmallVector, 4> Active;
 };
 
 } // end namespace CodeGen
Index: lib/CodeGen/CGLoopInfo.cpp
===
--- lib/CodeGen/CGLoopInfo.cpp
+++ lib/CodeGen/CGLoopInfo.cpp
@@ -554,8 +554,9 @@
 
 void LoopInfoStack::push(BasicBlock *Header, const llvm::DebugLoc ,
  const llvm::DebugLoc ) {
-  Active.push_back(LoopInfo(Header, StagedAttrs, StartLoc, EndLoc,
-Active.empty() ? nullptr : ()));
+  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();
 }
@@ -747,16 +748,16 @@
 
 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 AccessGroups;
-for (const LoopInfo  : Active) {
+for (const auto  : 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;


Index: test/CodeGen/loop-info-asan.c
===
--- /dev/null
+++ test/CodeGen/loop-info-asan.c
@@ -0,0 +1,16 @@
+// 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 (;;)
+;
+}
Index: lib/CodeGen/CGLoopInfo.h
===
--- lib/CodeGen/CGLoopInfo.h
+++ lib/CodeGen/CGLoopInfo.h
@@ -275,11 +275,11 @@
   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 () const { return Active.back(); }
+  const LoopInfo () 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 Active;
+  llvm::SmallVector, 4> Active;
 };
 
 } // end namespace CodeGen
Index: lib/CodeGen/CGLoopInfo.cpp
===
--- lib/CodeGen/CGLoopInfo.cpp
+++ lib/CodeGen/CGLoopInfo.cpp
@@ -554,8 +554,9 @@
 
 void LoopInfoStack::push(BasicBlock *Header, const llvm::DebugLoc ,
  const llvm::DebugLoc ) {
-  Active.push_back(LoopInfo(Header, StagedAttrs, StartLoc, EndLoc,
-Active.empty() ? nullptr : ()));
+  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();
 }
@@ -747,16