unique_ptrify llvm::getLazyBitcodeModule's MemoryBuffer parameter

[dblaikie]

Summary

This turned out to be a bit tricky as there are some callers doing
strange things with ownership of the this MemoryBuffer.

A couple of callers (llvm::getBitcodeTargetTriple,
llvm::parseBitcodeFile) don't take ownership of the MemoryBuffer, but
still want to construct a Module (but they either ensure it's fully
materialized, or otherwise destroy it before the function is over, being
sure to take the MemoryBuffer back from the Module so it is not
destroyed). Some of these callers were adjusted to use ScopeGuards to
ensure the release of a fake local unique_ptr, others were able to be
adjusted to pass ownership transiently, then take it back again later.

Other ideas are welcome - one possible option would be to create new
shallow MemoryBuffers in these cases. Another would be to make these
data structures consistently non-owning, though that sounds difficult
for other users. I'd like to throw a few ideas around here because I
think this pattern shows up a few times in LLVM - where an API has
multiple users, some of whom want to give ownership, some of whom don't.
I'd like to figure out a good/consistent/simple answer for when we come
across this again.

[silvas]

Maybe this is worth another thread on LLVMDev, but I really dislike using unique_ptr when it complicates things. I see unique_ptr as a lightweight way ensure things are correct by default with no need to fiddle around; it should bring an immediate sense of security. Needing to have comments about ownership in code using unique_ptr defeats the purpose IMO.

I feel like this patch is really complicating things, trying to move towards unique_ptr just for the sake of using unique_ptr. There's no difference between a unique_ptr that needs to have careful comments about ownership around it vs. a raw pointer with careful comments about ownership, besides the possibility that the unique_ptr will possibly lure the reader into a false sense of security.

An unused-result call to .release() is as bad as raw new/delete: it is a manual, error-prone intervention for controlling an object's lifetime.

lib/Bitcode/Reader/BitcodeReader.cpp

3552–3554

What's the point of this? All std::move does is cast to T&&. You already have a T&&.

[• rafael]

I am somewhat OK with this approach. I think the one item I feel
should be done differently is llvm::parseBitcodeFile. It can take just
a stringref and build a fake membuf to pass down. Maybe we should do
that first (attached patch)?

• t.patch3 KBDownload

[dblaikie]

Looks like Rafael is already working on this in some more incremental
and probably better ways, so I'll defer to his work.

See Rafael's thread "[patch] Pass a MemoryBufferRef when we can avoid
taking ownership" for more details.

Here's a half-written email I had on this thread for a while. Just
sending for posterity.

include/llvm/ADT/ScopeGuard.h

+//===-- ScopeGuard.h - Utility  -----------------------------------*- C++ -*-=//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file provides utility classes that use RAII to execute cleanup/rollback
+/// operations within a scope.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ADT_SCOPEGUARD_H
+#define LLVM_ADT_SCOPEGUARD_H
+
+#include <utility>
+
+namespace llvm {
+
+template <typename F> class ScopeGuardImpl {
+  F Func;
+  bool Active;
+
+public:
+  ScopeGuardImpl(F Func) : Func(std::move(Func)), Active(true) {}
+  ScopeGuardImpl(ScopeGuardImpl &&RHS)
+      : Func(std::move(RHS.Func)), Active(RHS.Active) {
+    RHS.Dismiss();
+  }
+  void Dismiss() { Active = false; }
+  ~ScopeGuardImpl() {
+    if (Active)
+      Func();
+  }
+};
+
+template <typename F> ScopeGuardImpl<F> ScopeGuard(F Func) {
+  return ScopeGuardImpl<F>(std::move(Func));
+}
+}
+#endif

include/llvm/Bitcode/ReaderWriter.h

 // This header defines interfaces to read and write LLVM bitcode files/streams.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_BITCODE_READERWRITER_H
 #define LLVM_BITCODE_READERWRITER_H
 
 #include "llvm/Support/ErrorOr.h"
+#include <memory>
 #include <string>
 
 namespace llvm {
   class BitstreamWriter;
   class MemoryBuffer;
   class DataStreamer;
   class LLVMContext;
   class Module;
   class ModulePass;
   class raw_ostream;
 
   /// Read the header of the specified bitcode buffer and prepare for lazy
   /// deserialization of function bodies.  If successful, this takes ownership
   /// of 'buffer. On error, this *does not* take ownership of Buffer.
-  ErrorOr<Module *> getLazyBitcodeModule(MemoryBuffer *Buffer,
+  ErrorOr<Module *> getLazyBitcodeModule(std::unique_ptr<MemoryBuffer> &&Buffer,
                                          LLVMContext &Context);
 
   /// getStreamedBitcodeModule - Read the header of the specified stream
   /// and prepare for lazy deserialization and streaming of function bodies.
   /// On error, this returns null, and fills in *ErrMsg with an error
   /// description if ErrMsg is non-null.
   Module *getStreamedBitcodeModule(const std::string &name,
                                    DataStreamer *streamer,

include/llvm/IR/GVMaterializer.h

 // or interprocedural optimizers that do not need the entire program in memory
 // at the same time.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_GVMATERIALIZER_H
 #define LLVM_IR_GVMATERIALIZER_H
 
+#include <memory>
 #include <system_error>
 
 namespace llvm {
 class Function;
 class GlobalValue;
+class MemoryBuffer;
 class Module;
 
 class GVMaterializer {
 protected:
   GVMaterializer() {}
 
 public:
   virtual ~GVMaterializer();
public:
   /// is a noop.
   ///
   virtual void Dematerialize(GlobalValue *) {}
 
   /// Make sure the entire Module has been completely read.
   ///
   virtual std::error_code MaterializeModule(Module *M) = 0;
 
-  virtual void releaseBuffer() = 0;
+  virtual std::unique_ptr<MemoryBuffer> takeBuffer() = 0;
 };
 
 } // End llvm namespace
 
 #endif

include/llvm/IR/Module.h

 #include "llvm/Support/CBindingWrapping.h"
 #include "llvm/Support/DataTypes.h"
 #include <system_error>
 
 namespace llvm {
 class FunctionType;
 class GVMaterializer;
 class LLVMContext;
+class MemoryBuffer;
 class RandomNumberGenerator;
 class StructType;
 template<typename T> struct DenseMapInfo;
 template<typename KeyT, typename ValueT, typename KeyInfoT> class DenseMap;
 
 template<> struct ilist_traits<Function>
   : public SymbolTableListTraits<Function, Module> {
 
/// @{
   void Dematerialize(GlobalValue *GV);
 
   /// Make sure all GlobalValues in this Module are fully read.
   std::error_code materializeAll();
 
   /// Make sure all GlobalValues in this Module are fully read and clear the
   /// Materializer. If the module is corrupt, this DOES NOT clear the old
   /// Materializer.
-  std::error_code materializeAllPermanently(bool ReleaseBuffer = false);
+  std::error_code
+  materializeAllPermanently(std::unique_ptr<MemoryBuffer> *Buf = nullptr);
 
 /// @}
 /// @name Direct access to the globals list, functions list, and symbol table
 /// @{
 
   /// Get the Module's list of global variables (constant).
   const GlobalListType   &getGlobalList() const       { return GlobalList; }
   /// Get the Module's list of global variables.

lib/Bitcode/Reader/BitReader.cpp

 //===-- BitReader.cpp -----------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-c/BitReader.h"
+#include "llvm/ADT/ScopeGuard.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <cstring>
 #include <string>
 
 using namespace llvm;

 
 /* Reads a module from the specified path, returning via the OutModule parameter
    a module provider which performs lazy deserialization. Returns 0 on success.
    Optionally returns a human-readable error message via OutMessage. */
 LLVMBool LLVMGetBitcodeModuleInContext(LLVMContextRef ContextRef,
                                        LLVMMemoryBufferRef MemBuf,
                                        LLVMModuleRef *OutM,
                                        char **OutMessage) {
+  // Assume ownership so ownership can be passed to getLazyBitcodeModule, but
+  // release the resource (if it wasn't taken by getLazyBitcodeModule - such as
+  // on error) before returning so the caller can keep ownership.
+  std::unique_ptr<MemoryBuffer> Owner(unwrap(MemBuf));
+  auto Release = ScopeGuard([&]() { Owner.release(); });
+
   std::string Message;
   ErrorOr<Module *> ModuleOrErr =
-      getLazyBitcodeModule(unwrap(MemBuf), *unwrap(ContextRef));
+      getLazyBitcodeModule(std::move(Owner), *unwrap(ContextRef));
 
   if (std::error_code EC = ModuleOrErr.getError()) {
     *OutM = wrap((Module *)nullptr);
     if (OutMessage)
       *OutMessage = strdup(EC.message().c_str());
     return 1;
   }
 

lib/Bitcode/Reader/BitcodeReader.h

class BitcodeReader : public GVMaterializer {
   bool WillMaterializeAllForwardRefs;
 
   /// Functions that have block addresses taken.  This is usually empty.
   SmallPtrSet<const Function *, 4> BlockAddressesTaken;
 
 public:
   std::error_code Error(BitcodeError E) { return make_error_code(E); }
 
-  explicit BitcodeReader(MemoryBuffer *buffer, LLVMContext &C)
-      : Context(C), TheModule(nullptr), Buffer(buffer), LazyStreamer(nullptr),
-        NextUnreadBit(0), SeenValueSymbolTable(false), ValueList(C),
-        MDValueList(C), SeenFirstFunctionBody(false), UseRelativeIDs(false),
-        WillMaterializeAllForwardRefs(false) {}
+  explicit BitcodeReader(std::unique_ptr<MemoryBuffer> Buffer, LLVMContext &C)
+      : Context(C), TheModule(nullptr), Buffer(std::move(Buffer)),
+        LazyStreamer(nullptr), NextUnreadBit(0), SeenValueSymbolTable(false),
+        ValueList(C), MDValueList(C), SeenFirstFunctionBody(false),
+        UseRelativeIDs(false), WillMaterializeAllForwardRefs(false) {}
   explicit BitcodeReader(DataStreamer *streamer, LLVMContext &C)
       : Context(C), TheModule(nullptr), Buffer(nullptr), LazyStreamer(streamer),
         NextUnreadBit(0), SeenValueSymbolTable(false), ValueList(C),
         MDValueList(C), SeenFirstFunctionBody(false), UseRelativeIDs(false),
         WillMaterializeAllForwardRefs(false) {}
   ~BitcodeReader() { FreeState(); }
 
   std::error_code materializeForwardReferencedFunctions();
 
   void FreeState();
 
-  void releaseBuffer() override;
+  std::unique_ptr<MemoryBuffer> takeBuffer() override;
 
   bool isMaterializable(const GlobalValue *GV) const override;
   bool isDematerializable(const GlobalValue *GV) const override;
   std::error_code Materialize(GlobalValue *GV) override;
   std::error_code MaterializeModule(Module *M) override;
   void Dematerialize(GlobalValue *GV) override;
 
   /// @brief Main interface to parsing a bitcode buffer.

lib/Bitcode/Reader/BitcodeReader.cpp

 //===- BitcodeReader.cpp - Internal BitcodeReader implementation ----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "BitcodeReader.h"
+#include "llvm/ADT/ScopeGuard.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Bitcode/LLVMBitCodes.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/IntrinsicInst.h"
std::error_code BitcodeReader::FindFunctionInStream(
   }
   return std::error_code();
 }
 
 //===----------------------------------------------------------------------===//
 // GVMaterializer implementation
 //===----------------------------------------------------------------------===//
 
-void BitcodeReader::releaseBuffer() { Buffer.release(); }
+std::unique_ptr<MemoryBuffer> BitcodeReader::takeBuffer() {
+  return std::move(Buffer);
+}
 
 bool BitcodeReader::isMaterializable(const GlobalValue *GV) const {
   if (const Function *F = dyn_cast<Function>(GV)) {
     return F->isDeclaration() &&
       DeferredFunctionInfo.count(const_cast<Function*>(F));
   }
   return false;
 }

 /// \brief Get a lazy one-at-time loading module from bitcode.
 ///
 /// This isn't always used in a lazy context.  In particular, it's also used by
 /// \a parseBitcodeFile().  If this is truly lazy, then we need to eagerly pull
 /// in forward-referenced functions from block address references.
 ///
 /// \param[in] WillMaterializeAll Set to \c true if the caller promises to
 /// materialize everything -- in particular, if this isn't truly lazy.
-static ErrorOr<Module *> getLazyBitcodeModuleImpl(MemoryBuffer *Buffer,
+static ErrorOr<Module *> getLazyBitcodeModuleImpl(std::unique_ptr<MemoryBuffer> &&Buffer,
                                                   LLVMContext &Context,
                                                   bool WillMaterializeAll) {
   Module *M = new Module(Buffer->getBufferIdentifier(), Context);
-  BitcodeReader *R = new BitcodeReader(Buffer, Context);
+  BitcodeReader *R = new BitcodeReader(std::move(Buffer), Context);
   M->setMaterializer(R);
 
   auto cleanupOnError = [&](std::error_code EC) {
-    R->releaseBuffer(); // Never take ownership on error.
+    Buffer = R->takeBuffer(); // Never take ownership on error.
     delete M;  // Also deletes R.
     return EC;
   };
 
   if (std::error_code EC = R->ParseBitcodeInto(M))
     return cleanupOnError(EC);
 
   if (!WillMaterializeAll)
     // Resolve forward references from blockaddresses.
     if (std::error_code EC = R->materializeForwardReferencedFunctions())
       return cleanupOnError(EC);
 
   return M;
 }
 
-ErrorOr<Module *> llvm::getLazyBitcodeModule(MemoryBuffer *Buffer,
-                                             LLVMContext &Context) {
-  return getLazyBitcodeModuleImpl(Buffer, Context, false);
+ErrorOr<Module *>
+llvm::getLazyBitcodeModule(std::unique_ptr<MemoryBuffer> &&Buffer,
+  return getLazyBitcodeModuleImpl(std::move(Buffer), Context, false);
 }
 
 Module *llvm::getStreamedBitcodeModule(const std::string &name,
                                        DataStreamer *streamer,
                                        LLVMContext &Context,
                                        std::string *ErrMsg) {
   Module *M = new Module(name, Context);
   BitcodeReader *R = new BitcodeReader(streamer, Context);
   M->setMaterializer(R);
   if (std::error_code EC = R->ParseBitcodeInto(M)) {
     if (ErrMsg)
       *ErrMsg = EC.message();
     delete M;  // Also deletes R.
     return nullptr;
   }
   return M;
 }
 
 ErrorOr<Module *> llvm::parseBitcodeFile(MemoryBuffer *Buffer,
                                          LLVMContext &Context) {
+  // This is a lie - the MemoryBuffer is not owned here at all, but the Module
+  // takes ownership regardless, so let's pretend. Alternatively we could
+  // create and pass a new non-owning MemoryBuffer here.
+  std::unique_ptr<MemoryBuffer> PseudoOwner(Buffer);
+  auto Releaser = ScopeGuard([&]() { PseudoOwner.release(); });
+
   ErrorOr<Module *> ModuleOrErr =
-      getLazyBitcodeModuleImpl(Buffer, Context, true);
+      getLazyBitcodeModuleImpl(std::move(PseudoOwner), Context, true);
   if (!ModuleOrErr)
     return ModuleOrErr;
   Module *M = ModuleOrErr.get();
   // Read in the entire module, and destroy the BitcodeReader.
-  if (std::error_code EC = M->materializeAllPermanently(true)) {
+  if (std::error_code EC = M->materializeAllPermanently(&PseudoOwner)) {
     delete M;
     return EC;
   }
 
   // TODO: Restore the use-lists to the in-memory state when the bitcode was
   // written.  We must defer until the Module has been fully materialized.
 
   return M;
 }
 
 std::string llvm::getBitcodeTargetTriple(MemoryBuffer *Buffer,
                                          LLVMContext &Context) {
-  BitcodeReader *R = new BitcodeReader(Buffer, Context);
+  // This is a lie - the MemoryBuffer is not owned here at all, but the
+  // BitcodeReader takes ownership regardless, so let's pretend. Alternatively
+  // we could create and pass a new non-owning MemoryBuffer here.
+  std::unique_ptr<MemoryBuffer> PseudoOwner(Buffer);
+  auto Releaser = ScopeGuard([&]() { PseudoOwner.release(); });
+
+  BitcodeReader *R = new BitcodeReader(std::move(PseudoOwner), Context);
   ErrorOr<std::string> Triple = R->parseTriple();
-  R->releaseBuffer();
+  PseudoOwner = R->takeBuffer();
   delete R;
   if (Triple.getError())
     return "";
   return Triple.get();
 }

lib/IR/Module.cpp

 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/GVMaterializer.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/LeakDetector.h"
 #include "llvm/Support/Dwarf.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/RandomNumberGenerator.h"
 #include <algorithm>
 #include <cstdarg>
 #include <cstdlib>
 using namespace llvm;
 
 //===----------------------------------------------------------------------===//

 }
 
 std::error_code Module::materializeAll() {
   if (!Materializer)
     return std::error_code();
   return Materializer->MaterializeModule(this);
 }
 
-std::error_code Module::materializeAllPermanently(bool ReleaseBuffer) {
-  if (std::error_code EC = materializeAll())
-    return EC;
-
-  if (ReleaseBuffer)
-    Materializer->releaseBuffer();
-
+std::error_code Module::materializeAllPermanently(std::unique_ptr<MemoryBuffer> *Buf) {
+  std::error_code EC = materializeAll();
+  if (Buf)
+    *Buf = Materializer->takeBuffer();
   Materializer.reset();
-  return std::error_code();
+  return EC;
 }
 
 //===----------------------------------------------------------------------===//
 // Other module related stuff.
 //
 
 
 // dropAllReferences() - This function causes all the subelements to "let go"

lib/IRReader/IRReader.cpp

 
 namespace llvm {
   extern bool TimePassesIsEnabled;
 }
 
 static const char *const TimeIRParsingGroupName = "LLVM IR Parsing";
 static const char *const TimeIRParsingName = "Parse IR";
 
-static Module *getLazyIRModule(MemoryBuffer *Buffer, SMDiagnostic &Err,
+static Module *getLazyIRModule(std::unique_ptr<MemoryBuffer> Buffer, SMDiagnostic &Err,
                                LLVMContext &Context) {
   if (isBitcode((const unsigned char *)Buffer->getBufferStart(),
                 (const unsigned char *)Buffer->getBufferEnd())) {
     std::string ErrMsg;
-    ErrorOr<Module *> ModuleOrErr = getLazyBitcodeModule(Buffer, Context);
+    ErrorOr<Module *> ModuleOrErr = getLazyBitcodeModule(std::move(Buffer), Context);
     if (std::error_code EC = ModuleOrErr.getError()) {
       Err = SMDiagnostic(Buffer->getBufferIdentifier(), SourceMgr::DK_Error,
                          EC.message());
-      // getLazyBitcodeModule does not take ownership of the Buffer in the
-      // case of an error.
-      delete Buffer;
       return nullptr;
     }
     return ModuleOrErr.get();
   }
 
-  return ParseAssembly(Buffer, nullptr, Err, Context);
+  return ParseAssembly(Buffer.release(), nullptr, Err, Context);
 }
 
 Module *llvm::getLazyIRFileModule(const std::string &Filename, SMDiagnostic &Err,
                                   LLVMContext &Context) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
       MemoryBuffer::getFileOrSTDIN(Filename);
   if (std::error_code EC = FileOrErr.getError()) {
     Err = SMDiagnostic(Filename, SourceMgr::DK_Error,
                        "Could not open input file: " + EC.message());
     return nullptr;
   }
 
-  return getLazyIRModule(FileOrErr.get().release(), Err, Context);
+  return getLazyIRModule(std::move(*FileOrErr), Err, Context);
 }
 
 Module *llvm::ParseIR(MemoryBuffer *Buffer, SMDiagnostic &Err,
                       LLVMContext &Context) {
   NamedRegionTimer T(TimeIRParsingName, TimeIRParsingGroupName,
                      TimePassesIsEnabled);
   if (isBitcode((const unsigned char *)Buffer->getBufferStart(),
                 (const unsigned char *)Buffer->getBufferEnd())) {

lib/LTO/LTOModule.cpp

if (!buffer)
     return nullptr;
   return makeLTOModule(std::move(buffer), options, errMsg);
 }
 
 LTOModule *LTOModule::makeLTOModule(std::unique_ptr<MemoryBuffer> Buffer,
                                     TargetOptions options,
                                     std::string &errMsg) {
   ErrorOr<Module *> MOrErr =
-      getLazyBitcodeModule(Buffer.get(), getGlobalContext());
+      getLazyBitcodeModule(std::move(Buffer), getGlobalContext());
   if (std::error_code EC = MOrErr.getError()) {
     errMsg = EC.message();
     return nullptr;
   }
   std::unique_ptr<Module> M(MOrErr.get());
 
   std::string TripleStr = M->getTargetTriple();
   if (TripleStr.empty())
if (Triple.isOSDarwin()) {
     else if (Triple.getArch() == llvm::Triple::x86)
       CPU = "yonah";
     else if (Triple.getArch() == llvm::Triple::aarch64)
       CPU = "cyclone";
   }
 
   TargetMachine *target = march->createTargetMachine(TripleStr, CPU, FeatureStr,
                                                      options);
-  M->materializeAllPermanently(true);
+  M->materializeAllPermanently(&Buffer);
   M->setDataLayout(target->getSubtargetImpl()->getDataLayout());
 
   std::unique_ptr<object::IRObjectFile> IRObj(
       new object::IRObjectFile(std::move(Buffer), std::move(M)));
 
   LTOModule *Ret = new LTOModule(std::move(IRObj), target);
 
   if (Ret->parseSymbols(errMsg)) {

lib/Object/IRObjectFile.cpp

case RecordStreamer::Used:
       break;
     }
     AsmSymbols.push_back(
         std::make_pair<std::string, uint32_t>(Key, std::move(Res)));
   }
 }
 
 IRObjectFile::~IRObjectFile() {
-  GVMaterializer *GVM =  M->getMaterializer();
-  if (GVM)
-    GVM->releaseBuffer();
- }
+  if (GVMaterializer *GVM =  M->getMaterializer())
+    GVM->takeBuffer().release();
 
 static const GlobalValue *getGV(DataRefImpl &Symb) {
   if ((Symb.p & 3) == 3)
     return nullptr;
 
   return reinterpret_cast<GlobalValue*>(Symb.p & ~uintptr_t(3));
 }
 
basic_symbol_iterator IRObjectFile::symbol_end_impl() const {
   uint64_t NumAsm = AsmSymbols.size();
   NumAsm <<= 2;
   Ret.p = 3 | NumAsm;
   return basic_symbol_iterator(BasicSymbolRef(Ret, this));
 }
 
 ErrorOr<IRObjectFile *> llvm::object::IRObjectFile::createIRObjectFile(
     std::unique_ptr<MemoryBuffer> Object, LLVMContext &Context) {
-  ErrorOr<Module *> MOrErr = getLazyBitcodeModule(Object.get(), Context);
+  MemoryBuffer *RawObject = Object.get();
+
+  ErrorOr<Module *> MOrErr = getLazyBitcodeModule(std::move(FalseOwner), Context);
   if (std::error_code EC = MOrErr.getError())
     return EC;
 
   std::unique_ptr<Module> M(MOrErr.get());
+
+  // Take ownership again. Though this seems like it would cause a
+  // double-delete (the MemoryBuffer being owned by both the Module and the
+  // IRRobjectFile), it's fine because IRObjectFile's dtor is sure to release
+  // ownership of the underlying MemoryBuffer in the Module.
+  Object.reset(RawObject);
   return new IRObjectFile(std::move(Object), std::move(M));
 }

unittests/Bitcode/BitReaderTest.cpp

static void writeModuleToBuffer(std::unique_ptr<Module> Mod,
   raw_svector_ostream OS(Buffer);
   WriteBitcodeToFile(Mod.get(), OS);
 }
 
 static std::unique_ptr<Module> getLazyModuleFromAssembly(LLVMContext &Context,
                                                          SmallString<1024> &Mem,
                                                          const char *Assembly) {
   writeModuleToBuffer(parseAssembly(Assembly), Mem);
-  MemoryBuffer *Buffer = MemoryBuffer::getMemBuffer(Mem.str(), "test", false);
-  ErrorOr<Module *> ModuleOrErr = getLazyBitcodeModule(Buffer, Context);
+  std::unique_ptr<MemoryBuffer> Buffer(
+      MemoryBuffer::getMemBuffer(Mem.str(), "test", false));
+  ErrorOr<Module *> ModuleOrErr = getLazyBitcodeModule(std::move(Buffer), Context);
   return std::unique_ptr<Module>(ModuleOrErr.get());
 }
 
 TEST(BitReaderTest, MaterializeFunctionsForBlockAddr) { // PR11677
   SmallString<1024> Mem;
 
   LLVMContext Context;
   std::unique_ptr<Module> M = getLazyModuleFromAssembly(

unittests/ExecutionEngine/JIT/JITTest.cpp

 
 // Returns a newly-created ExecutionEngine that reads the bitcode in 'Bitcode'
 // lazily.  The associated Module (owned by the ExecutionEngine) is returned in
 // M.  Both will be NULL on an error.  Bitcode must live at least as long as the
 // ExecutionEngine.
 ExecutionEngine *getJITFromBitcode(
   LLVMContext &Context, const std::string &Bitcode, Module *&M) {
   // c_str() is null-terminated like MemoryBuffer::getMemBuffer requires.
-  MemoryBuffer *BitcodeBuffer =
-    MemoryBuffer::getMemBuffer(Bitcode, "Bitcode for test");
-  ErrorOr<Module*> ModuleOrErr = getLazyBitcodeModule(BitcodeBuffer, Context);
+  std::unique_ptr<MemoryBuffer> BitcodeBuffer(
+      MemoryBuffer::getMemBuffer(Bitcode, "Bitcode for test"));
+  ErrorOr<Module *> ModuleOrErr =
+      getLazyBitcodeModule(std::move(BitcodeBuffer), Context);
   if (std::error_code EC = ModuleOrErr.getError()) {
     ADD_FAILURE() << EC.message();
-    delete BitcodeBuffer;
     return nullptr;
   }
   M = ModuleOrErr.get();
   std::string errMsg;
   ExecutionEngine *TheJIT = EngineBuilder(M)
     .setEngineKind(EngineKind::JIT)
     .setErrorStr(&errMsg)
     .create();