[llvm] r359643 - [JITLink] Make sure we explicitly deallocate memory on failure.

[Lang Hames via llvm-commits]

Author: lhames
Date: Tue Apr 30 19:43:52 2019
New Revision: 359643

URL: http://llvm.org/viewvc/llvm-project?rev=359643&view=rev
Log:
[JITLink] Make sure we explicitly deallocate memory on failure.

JITLinkGeneric phases 2 and 3 (focused on applying fixups and finalizing memory,
respectively) may fail for various reasons. If this happens, we need to
explicitly de-allocate the memory allocated in phase 1 (explicitly, because
deallocation may also fail and so is implemented as a method returning error).

No testcase yet: I am still trying to decide on the right way to test totally
platform agnostic code like this.

Modified:
    llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
    llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.h

Modified: llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp?rev=359643&r1=359642&r2=359643&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp Tue Apr 30 19:43:52 2019
@@ -90,7 +90,7 @@ void JITLinkerBase::linkPhase2(std::uniq
                                Expected<AsyncLookupResult> LR) {
   // If the lookup failed, bail out.
   if (!LR)
-    return Ctx->notifyFailed(LR.takeError());
+    return deallocateAndBailOut(LR.takeError());
 
   // Assign addresses to external atoms.
   applyLookupResult(*LR);
@@ -102,7 +102,7 @@ void JITLinkerBase::linkPhase2(std::uniq
 
   // Copy atom content to working memory and fix up.
   if (auto Err = copyAndFixUpAllAtoms(Layout, *Alloc))
-    return Ctx->notifyFailed(std::move(Err));
+    return deallocateAndBailOut(std::move(Err));
 
   LLVM_DEBUG({
     dbgs() << "Atom graph \"" << G->getName() << "\" after copy-and-fixup:\n";
@@ -110,7 +110,7 @@ void JITLinkerBase::linkPhase2(std::uniq
   });
 
   if (auto Err = runPasses(Passes.PostFixupPasses, *G))
-    return Ctx->notifyFailed(std::move(Err));
+    return deallocateAndBailOut(std::move(Err));
 
   // FIXME: Use move capture once we have c++14.
   auto *UnownedSelf = Self.release();
@@ -124,7 +124,7 @@ void JITLinkerBase::linkPhase2(std::uniq
 
 void JITLinkerBase::linkPhase3(std::unique_ptr<JITLinkerBase> Self, Error Err) {
   if (Err)
-    return Ctx->notifyFailed(std::move(Err));
+    return deallocateAndBailOut(std::move(Err));
   Ctx->notifyFinalized(std::move(Alloc));
 }
 
@@ -356,6 +356,12 @@ void JITLinkerBase::applyLookupResult(As
          "All atoms should have been resolved by this point");
 }
 
+void JITLinkerBase::deallocateAndBailOut(Error Err) {
+  assert(Err && "Should not be bailing out on success value");
+  assert(Alloc && "can not call deallocateAndBailOut before allocation");
+  Ctx->notifyFailed(joinErrors(std::move(Err), Alloc->deallocate()));
+}
+
 void JITLinkerBase::dumpGraph(raw_ostream &OS) {
   assert(G && "Graph is not set yet");
   G->dump(dbgs(), [this](Edge::Kind K) { return getEdgeKindName(K); });

Modified: llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.h?rev=359643&r1=359642&r2=359643&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.h (original)
+++ llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.h Tue Apr 30 19:43:52 2019
@@ -102,6 +102,7 @@ private:
   Error allocateSegments(const SegmentLayoutMap &Layout);
   DenseSet<StringRef> getExternalSymbolNames() const;
   void applyLookupResult(AsyncLookupResult LR);
+  void deallocateAndBailOut(Error Err);
 
   void dumpGraph(raw_ostream &OS);