Index: include/llvm/ADT/ScopeGuard.h =================================================================== --- /dev/null +++ include/llvm/ADT/ScopeGuard.h @@ -0,0 +1,44 @@ +//===-- 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 + +namespace llvm { + +template 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 ScopeGuardImpl ScopeGuard(F Func) { + return ScopeGuardImpl(std::move(Func)); +} +} +#endif Index: include/llvm/Bitcode/ReaderWriter.h =================================================================== --- include/llvm/Bitcode/ReaderWriter.h +++ include/llvm/Bitcode/ReaderWriter.h @@ -15,6 +15,7 @@ #define LLVM_BITCODE_READERWRITER_H #include "llvm/Support/ErrorOr.h" +#include #include namespace llvm { @@ -29,7 +30,7 @@ /// 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 getLazyBitcodeModule(MemoryBuffer *Buffer, + ErrorOr getLazyBitcodeModule(std::unique_ptr &&Buffer, LLVMContext &Context); /// getStreamedBitcodeModule - Read the header of the specified stream Index: include/llvm/IR/GVMaterializer.h =================================================================== --- include/llvm/IR/GVMaterializer.h +++ include/llvm/IR/GVMaterializer.h @@ -18,11 +18,13 @@ #ifndef LLVM_IR_GVMATERIALIZER_H #define LLVM_IR_GVMATERIALIZER_H +#include #include namespace llvm { class Function; class GlobalValue; +class MemoryBuffer; class Module; class GVMaterializer { @@ -55,7 +57,7 @@ /// virtual std::error_code MaterializeModule(Module *M) = 0; - virtual void releaseBuffer() = 0; + virtual std::unique_ptr takeBuffer() = 0; }; } // End llvm namespace Index: include/llvm/IR/Module.h =================================================================== --- include/llvm/IR/Module.h +++ include/llvm/IR/Module.h @@ -30,6 +30,7 @@ class FunctionType; class GVMaterializer; class LLVMContext; +class MemoryBuffer; class RandomNumberGenerator; class StructType; template struct DenseMapInfo; @@ -483,7 +484,8 @@ /// 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 *Buf = nullptr); /// @} /// @name Direct access to the globals list, functions list, and symbol table Index: lib/Bitcode/Reader/BitReader.cpp =================================================================== --- lib/Bitcode/Reader/BitReader.cpp +++ lib/Bitcode/Reader/BitReader.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #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" @@ -50,9 +51,15 @@ 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 Owner(unwrap(MemBuf)); + auto Release = ScopeGuard([&]() { Owner.release(); }); + std::string Message; ErrorOr ModuleOrErr = - getLazyBitcodeModule(unwrap(MemBuf), *unwrap(ContextRef)); + getLazyBitcodeModule(std::move(Owner), *unwrap(ContextRef)); if (std::error_code EC = ModuleOrErr.getError()) { *OutM = wrap((Module *)nullptr); Index: lib/Bitcode/Reader/BitcodeReader.h =================================================================== --- lib/Bitcode/Reader/BitcodeReader.h +++ lib/Bitcode/Reader/BitcodeReader.h @@ -205,11 +205,11 @@ 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 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), @@ -221,7 +221,7 @@ void FreeState(); - void releaseBuffer() override; + std::unique_ptr takeBuffer() override; bool isMaterializable(const GlobalValue *GV) const override; bool isDematerializable(const GlobalValue *GV) const override; Index: lib/Bitcode/Reader/BitcodeReader.cpp =================================================================== --- lib/Bitcode/Reader/BitcodeReader.cpp +++ lib/Bitcode/Reader/BitcodeReader.cpp @@ -9,6 +9,7 @@ #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" @@ -3278,7 +3279,9 @@ // GVMaterializer implementation //===----------------------------------------------------------------------===// -void BitcodeReader::releaseBuffer() { Buffer.release(); } +std::unique_ptr BitcodeReader::takeBuffer() { + return std::move(Buffer); +} bool BitcodeReader::isMaterializable(const GlobalValue *GV) const { if (const Function *F = dyn_cast(GV)) { @@ -3521,15 +3524,15 @@ /// /// \param[in] WillMaterializeAll Set to \c true if the caller promises to /// materialize everything -- in particular, if this isn't truly lazy. -static ErrorOr getLazyBitcodeModuleImpl(MemoryBuffer *Buffer, +static ErrorOr getLazyBitcodeModuleImpl(std::unique_ptr &&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; }; @@ -3545,9 +3548,10 @@ return M; } -ErrorOr llvm::getLazyBitcodeModule(MemoryBuffer *Buffer, - LLVMContext &Context) { - return getLazyBitcodeModuleImpl(Buffer, Context, false); +ErrorOr +llvm::getLazyBitcodeModule(std::unique_ptr &&Buffer, + LLVMContext &Context) { + return getLazyBitcodeModuleImpl(std::move(Buffer), Context, false); } Module *llvm::getStreamedBitcodeModule(const std::string &name, @@ -3568,13 +3572,19 @@ ErrorOr 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 PseudoOwner(Buffer); + auto Releaser = ScopeGuard([&]() { PseudoOwner.release(); }); + ErrorOr 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; } @@ -3587,9 +3597,15 @@ 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 PseudoOwner(Buffer); + auto Releaser = ScopeGuard([&]() { PseudoOwner.release(); }); + + BitcodeReader *R = new BitcodeReader(std::move(PseudoOwner), Context); ErrorOr Triple = R->parseTriple(); - R->releaseBuffer(); + PseudoOwner = R->takeBuffer(); delete R; if (Triple.getError()) return ""; Index: lib/IR/Module.cpp =================================================================== --- lib/IR/Module.cpp +++ lib/IR/Module.cpp @@ -24,6 +24,7 @@ #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 @@ -413,15 +414,12 @@ 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 *Buf) { + std::error_code EC = materializeAll(); + if (Buf) + *Buf = Materializer->takeBuffer(); Materializer.reset(); - return std::error_code(); + return EC; } //===----------------------------------------------------------------------===// Index: lib/IRReader/IRReader.cpp =================================================================== --- lib/IRReader/IRReader.cpp +++ lib/IRReader/IRReader.cpp @@ -29,24 +29,21 @@ 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 Buffer, SMDiagnostic &Err, LLVMContext &Context) { if (isBitcode((const unsigned char *)Buffer->getBufferStart(), (const unsigned char *)Buffer->getBufferEnd())) { std::string ErrMsg; - ErrorOr ModuleOrErr = getLazyBitcodeModule(Buffer, Context); + ErrorOr 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, @@ -59,7 +56,7 @@ return nullptr; } - return getLazyIRModule(FileOrErr.get().release(), Err, Context); + return getLazyIRModule(std::move(*FileOrErr), Err, Context); } Module *llvm::ParseIR(MemoryBuffer *Buffer, SMDiagnostic &Err, Index: lib/LTO/LTOModule.cpp =================================================================== --- lib/LTO/LTOModule.cpp +++ lib/LTO/LTOModule.cpp @@ -112,7 +112,7 @@ TargetOptions options, std::string &errMsg) { ErrorOr MOrErr = - getLazyBitcodeModule(Buffer.get(), getGlobalContext()); + getLazyBitcodeModule(std::move(Buffer), getGlobalContext()); if (std::error_code EC = MOrErr.getError()) { errMsg = EC.message(); return nullptr; @@ -146,7 +146,7 @@ TargetMachine *target = march->createTargetMachine(TripleStr, CPU, FeatureStr, options); - M->materializeAllPermanently(true); + M->materializeAllPermanently(&Buffer); M->setDataLayout(target->getSubtargetImpl()->getDataLayout()); std::unique_ptr IRObj( Index: lib/Object/IRObjectFile.cpp =================================================================== --- lib/Object/IRObjectFile.cpp +++ lib/Object/IRObjectFile.cpp @@ -114,10 +114,9 @@ } 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) @@ -270,10 +269,18 @@ ErrorOr llvm::object::IRObjectFile::createIRObjectFile( std::unique_ptr Object, LLVMContext &Context) { - ErrorOr MOrErr = getLazyBitcodeModule(Object.get(), Context); + MemoryBuffer *RawObject = Object.get(); + + ErrorOr MOrErr = getLazyBitcodeModule(std::move(FalseOwner), Context); if (std::error_code EC = MOrErr.getError()) return EC; std::unique_ptr 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)); } Index: unittests/Bitcode/BitReaderTest.cpp =================================================================== --- unittests/Bitcode/BitReaderTest.cpp +++ unittests/Bitcode/BitReaderTest.cpp @@ -53,8 +53,9 @@ SmallString<1024> &Mem, const char *Assembly) { writeModuleToBuffer(parseAssembly(Assembly), Mem); - MemoryBuffer *Buffer = MemoryBuffer::getMemBuffer(Mem.str(), "test", false); - ErrorOr ModuleOrErr = getLazyBitcodeModule(Buffer, Context); + std::unique_ptr Buffer( + MemoryBuffer::getMemBuffer(Mem.str(), "test", false)); + ErrorOr ModuleOrErr = getLazyBitcodeModule(std::move(Buffer), Context); return std::unique_ptr(ModuleOrErr.get()); } Index: unittests/ExecutionEngine/JIT/JITTest.cpp =================================================================== --- unittests/ExecutionEngine/JIT/JITTest.cpp +++ unittests/ExecutionEngine/JIT/JITTest.cpp @@ -630,12 +630,12 @@ 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 ModuleOrErr = getLazyBitcodeModule(BitcodeBuffer, Context); + std::unique_ptr BitcodeBuffer( + MemoryBuffer::getMemBuffer(Bitcode, "Bitcode for test")); + ErrorOr ModuleOrErr = + getLazyBitcodeModule(std::move(BitcodeBuffer), Context); if (std::error_code EC = ModuleOrErr.getError()) { ADD_FAILURE() << EC.message(); - delete BitcodeBuffer; return nullptr; } M = ModuleOrErr.get();