<div dir="ltr">George is looking into what to do here.<div>They get handed to users and used by them, so it's practically owned by the memory ssa pass, but handed to callers.</div><div><br></div><div>It would be really nice to have it so if they use it after memoryssa dies, they get an obvious error instead of use after free.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 2, 2016 at 1:49 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Wed, Mar 2, 2016 at 1:16 PM, Daniel Berlin via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dannyb<br>
Date: Wed Mar  2 15:16:28 2016<br>
New Revision: 262519<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=262519&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=262519&view=rev</a><br>
Log:<br>
Really fix ASAN leak/etc issues with MemorySSA unittests<br>
<br>
Modified:<br>
    llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp<br>
    llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=262519&r1=262518&r2=262519&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp?rev=262519&r1=262518&r2=262519&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/MemorySSA.cpp Wed Mar  2 15:16:28 2016<br>
@@ -231,7 +231,7 @@ MemorySSAWalker *MemorySSA::buildMemoryS<br>
   assert(!this->AA && !this->DT &&<br>
          "MemorySSA without a walker already has AA or DT?");<br>
<br>
-  auto *Result = new CachingMemorySSAWalker(this, AA, DT);<br>
+  Walker = new CachingMemorySSAWalker(this, AA, DT);<br></blockquote><div><br></div></span><div>Any chance of using unique_ptr to manage the lifetime of the Walker(s)?</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
   this->AA = AA;<br>
   this->DT = DT;<br>
<br>
@@ -343,7 +343,7 @@ MemorySSAWalker *MemorySSA::buildMemoryS<br>
     for (auto &MA : *Accesses) {<br>
       if (auto *MU = dyn_cast<MemoryUse>(&MA)) {<br>
         Instruction *Inst = MU->getMemoryInst();<br>
-        MU->setDefiningAccess(Result->getClobberingMemoryAccess(Inst));<br>
+        MU->setDefiningAccess(Walker->getClobberingMemoryAccess(Inst));<br>
       }<br>
     }<br>
   }<br>
@@ -354,7 +354,6 @@ MemorySSAWalker *MemorySSA::buildMemoryS<br>
     if (!Visited.count(&BB))<br>
       markUnreachableAsLiveOnEntry(&BB);<br>
<br>
-  Walker = Result;<br>
   return Walker;<br>
 }<br>
<br>
<br>
Modified: llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp?rev=262519&r1=262518&r2=262519&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp?rev=262519&r1=262518&r2=262519&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp (original)<br>
+++ llvm/trunk/unittests/Transforms/Utils/MemorySSA.cpp Wed Mar  2 15:16:28 2016<br>
@@ -21,6 +21,7 @@ using namespace llvm;<br>
<br>
 TEST(MemorySSA, RemoveMemoryAccess) {<br>
   LLVMContext &C(getGlobalContext());<br>
+  std::unique_ptr<Module> M(new Module("Remove memory access", C));<br>
   IRBuilder<> B(C);<br>
   DataLayout DL("e-i64:64-f80:128-n8:16:32:64-S128");<br>
   TargetLibraryInfoImpl TLII;<br>
@@ -29,13 +30,13 @@ TEST(MemorySSA, RemoveMemoryAccess) {<br>
   // We create a diamond where there is a store on one side, and then a load<br>
   // after the merge point.  This enables us to test a bunch of different<br>
   // removal cases.<br>
-  std::unique_ptr<Function> F(Function::Create(<br>
+  Function *F = Function::Create(<br>
       FunctionType::get(B.getVoidTy(), {B.getInt8PtrTy()}, false),<br>
-      GlobalValue::ExternalLinkage, "F"));<br>
-  BasicBlock *Entry(BasicBlock::Create(C, "", F.get()));<br>
-  BasicBlock *Left(BasicBlock::Create(C, "", F.get()));<br>
-  BasicBlock *Right(BasicBlock::Create(C, "", F.get()));<br>
-  BasicBlock *Merge(BasicBlock::Create(C, "", F.get()));<br>
+      GlobalValue::ExternalLinkage, "F", M.get());<br>
+  BasicBlock *Entry(BasicBlock::Create(C, "", F));<br>
+  BasicBlock *Left(BasicBlock::Create(C, "", F));<br>
+  BasicBlock *Right(BasicBlock::Create(C, "", F));<br>
+  BasicBlock *Merge(BasicBlock::Create(C, "", F));<br>
   B.SetInsertPoint(Entry);<br>
   B.CreateCondBr(B.getTrue(), Left, Right);<br>
   B.SetInsertPoint(Left);<br>
@@ -49,10 +50,10 @@ TEST(MemorySSA, RemoveMemoryAccess) {<br>
   std::unique_ptr<MemorySSA> MSSA(new MemorySSA(*F));<br>
   std::unique_ptr<DominatorTree> DT(new DominatorTree(*F));<br>
   std::unique_ptr<AssumptionCache> AC(new AssumptionCache(*F));<br>
-  AAResults *AA = new AAResults(TLI);<br>
-  BasicAAResult *BAA = new BasicAAResult(DL, TLI, *AC, &*DT);<br>
-  AA->addAAResult(*BAA);<br>
-  MemorySSAWalker *Walker = MSSA->buildMemorySSA(AA, &*DT);<br>
+  AAResults AA(TLI);<br>
+  BasicAAResult BAA(DL, TLI, *AC, &*DT);<br>
+  AA.addAAResult(BAA);<br>
+  std::unique_ptr<MemorySSAWalker> Walker(MSSA->buildMemorySSA(&AA, &*DT));<br>
   // Before, the load will be a use of a phi<store, liveonentry>. It should be<br>
   // the same after.<br>
   MemoryUse *LoadAccess = cast<MemoryUse>(MSSA->getMemoryAccess(LoadInst));<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>