From 00121bb932ddbf026297f357c2d3cdf1414f628a Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 29 Apr 2014 21:52:46 +0000 Subject: PR19553: Memory leak in RuntimeDyldELF::createObjectImageFromFile This starts in MCJIT::getSymbolAddress where the unique_ptr is release()d and (after a cast) passed to a single caller, MCJIT::addObjectFile. addObjectFile calls RuntimeDyld::loadObject. RuntimeDld::loadObject calls RuntimeDyldELF::createObjectFromFile And the pointer is never owned at this point. I say this point, because the alternative codepath, RuntimeDyldMachO::createObjectFile certainly does take ownership, so this seemed like a good hint that this was a/the right place to take ownership. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@207580 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../RuntimeDyld/ObjectImageCommon.h | 21 +++--- lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp | 10 +-- lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp | 80 +++++++++++++--------- lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h | 2 +- lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h | 4 +- 5 files changed, 69 insertions(+), 48 deletions(-) (limited to 'lib/ExecutionEngine/RuntimeDyld') diff --git a/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h b/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h index f5a4ea9328..4917b93a96 100644 --- a/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h +++ b/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h @@ -18,6 +18,8 @@ #include "llvm/ExecutionEngine/ObjectImage.h" #include "llvm/Object/ObjectFile.h" +#include + namespace llvm { namespace object { @@ -30,13 +32,13 @@ class ObjectImageCommon : public ObjectImage { void anchor() override; protected: - object::ObjectFile *ObjFile; + std::unique_ptr ObjFile; // This form of the constructor allows subclasses to use // format-specific subclasses of ObjectFile directly - ObjectImageCommon(ObjectBuffer *Input, object::ObjectFile *Obj) + ObjectImageCommon(ObjectBuffer *Input, std::unique_ptr Obj) : ObjectImage(Input), // saves Input as Buffer and takes ownership - ObjFile(Obj) + ObjFile(std::move(Obj)) { } @@ -44,12 +46,13 @@ public: ObjectImageCommon(ObjectBuffer* Input) : ObjectImage(Input) // saves Input as Buffer and takes ownership { - ObjFile = - object::ObjectFile::createObjectFile(Buffer->getMemBuffer()).get(); + // FIXME: error checking? createObjectFile returns an ErrorOr + // and should probably be checked for failure. + ObjFile.reset(object::ObjectFile::createObjectFile(Buffer->getMemBuffer()).get()); } - ObjectImageCommon(object::ObjectFile* Input) - : ObjectImage(nullptr), ObjFile(Input) {} - virtual ~ObjectImageCommon() { delete ObjFile; } + ObjectImageCommon(std::unique_ptr Input) + : ObjectImage(nullptr), ObjFile(std::move(Input)) {} + virtual ~ObjectImageCommon() { } object::symbol_iterator begin_symbols() const override { return ObjFile->symbol_begin(); } @@ -66,7 +69,7 @@ public: StringRef getData() const override { return ObjFile->getData(); } - object::ObjectFile* getObjectFile() const override { return ObjFile; } + object::ObjectFile* getObjectFile() const override { return ObjFile.get(); } // Subclasses can override these methods to update the image with loaded // addresses for sections and common symbols diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp index cc2f29d9df..12b3e8d75d 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp @@ -698,21 +698,23 @@ createRuntimeDyldMachO(RTDyldMemoryManager *MM, bool ProcessAllSections) { return Dyld; } -ObjectImage *RuntimeDyld::loadObject(ObjectFile *InputObject) { +ObjectImage *RuntimeDyld::loadObject(std::unique_ptr InputObject) { std::unique_ptr InputImage; + ObjectFile &Obj = *InputObject; + if (InputObject->isELF()) { - InputImage.reset(RuntimeDyldELF::createObjectImageFromFile(InputObject)); + InputImage.reset(RuntimeDyldELF::createObjectImageFromFile(std::move(InputObject))); if (!Dyld) Dyld = createRuntimeDyldELF(MM, ProcessAllSections).release(); } else if (InputObject->isMachO()) { - InputImage.reset(RuntimeDyldMachO::createObjectImageFromFile(InputObject)); + InputImage.reset(RuntimeDyldMachO::createObjectImageFromFile(std::move(InputObject))); if (!Dyld) Dyld = createRuntimeDyldMachO(MM, ProcessAllSections).release(); } else report_fatal_error("Incompatible object format!"); - if (!Dyld->isCompatibleFile(InputObject)) + if (!Dyld->isCompatibleFile(&Obj)) report_fatal_error("Incompatible object format!"); Dyld->loadObject(InputImage.get()); diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp index abe8b6c388..41ae701048 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp @@ -51,7 +51,12 @@ template class DyldELFObject : public ELFObjectFile { typedef typename ELFDataTypeTypedefHelper::value_type addr_type; + std::unique_ptr UnderlyingFile; + public: + DyldELFObject(std::unique_ptr UnderlyingFile, + MemoryBuffer *Wrapper, error_code &ec); + DyldELFObject(MemoryBuffer *Wrapper, error_code &ec); void updateSectionAddress(const SectionRef &Sec, uint64_t Addr); @@ -68,13 +73,11 @@ public: }; template class ELFObjectImage : public ObjectImageCommon { -protected: - DyldELFObject *DyldObj; bool Registered; public: - ELFObjectImage(ObjectBuffer *Input, DyldELFObject *Obj) - : ObjectImageCommon(Input, Obj), DyldObj(Obj), Registered(false) {} + ELFObjectImage(ObjectBuffer *Input, std::unique_ptr> Obj) + : ObjectImageCommon(Input, std::move(Obj)), Registered(false) {} virtual ~ELFObjectImage() { if (Registered) @@ -84,11 +87,13 @@ public: // Subclasses can override these methods to update the image with loaded // addresses for sections and common symbols void updateSectionAddress(const SectionRef &Sec, uint64_t Addr) override { - DyldObj->updateSectionAddress(Sec, Addr); + static_cast*>(getObjectFile()) + ->updateSectionAddress(Sec, Addr); } void updateSymbolAddress(const SymbolRef &Sym, uint64_t Addr) override { - DyldObj->updateSymbolAddress(Sym, Addr); + static_cast*>(getObjectFile()) + ->updateSymbolAddress(Sym, Addr); } void registerWithDebugger() override { @@ -109,6 +114,14 @@ DyldELFObject::DyldELFObject(MemoryBuffer *Wrapper, error_code &ec) this->isDyldELFObject = true; } +template +DyldELFObject::DyldELFObject(std::unique_ptr UnderlyingFile, + MemoryBuffer *Wrapper, error_code &ec) + : ELFObjectFile(Wrapper, ec), + UnderlyingFile(std::move(UnderlyingFile)) { + this->isDyldELFObject = true; +} + template void DyldELFObject::updateSectionAddress(const SectionRef &Sec, uint64_t Addr) { @@ -165,7 +178,7 @@ void RuntimeDyldELF::deregisterEHFrames() { } ObjectImage * -RuntimeDyldELF::createObjectImageFromFile(object::ObjectFile *ObjFile) { +RuntimeDyldELF::createObjectImageFromFile(std::unique_ptr ObjFile) { if (!ObjFile) return nullptr; @@ -174,21 +187,23 @@ RuntimeDyldELF::createObjectImageFromFile(object::ObjectFile *ObjFile) { MemoryBuffer::getMemBuffer(ObjFile->getData(), "", false); if (ObjFile->getBytesInAddress() == 4 && ObjFile->isLittleEndian()) { - DyldELFObject> *Obj = - new DyldELFObject>(Buffer, ec); - return new ELFObjectImage>(nullptr, Obj); + auto Obj = make_unique>>(std::move(ObjFile), Buffer, ec); + return new ELFObjectImage>( + nullptr, std::move(Obj)); } else if (ObjFile->getBytesInAddress() == 4 && !ObjFile->isLittleEndian()) { - DyldELFObject> *Obj = - new DyldELFObject>(Buffer, ec); - return new ELFObjectImage>(nullptr, Obj); + auto Obj = make_unique>>( + std::move(ObjFile), Buffer, ec); + return new ELFObjectImage>(nullptr, std::move(Obj)); } else if (ObjFile->getBytesInAddress() == 8 && !ObjFile->isLittleEndian()) { - DyldELFObject> *Obj = - new DyldELFObject>(Buffer, ec); - return new ELFObjectImage>(nullptr, Obj); + auto Obj = make_unique>>( + std::move(ObjFile), Buffer, ec); + return new ELFObjectImage>(nullptr, + std::move(Obj)); } else if (ObjFile->getBytesInAddress() == 8 && ObjFile->isLittleEndian()) { - DyldELFObject> *Obj = - new DyldELFObject>(Buffer, ec); - return new ELFObjectImage>(nullptr, Obj); + auto Obj = make_unique>>( + std::move(ObjFile), Buffer, ec); + return new ELFObjectImage>( + nullptr, std::move(Obj)); } else llvm_unreachable("Unexpected ELF format"); } @@ -202,28 +217,29 @@ ObjectImage *RuntimeDyldELF::createObjectImage(ObjectBuffer *Buffer) { error_code ec; if (Ident.first == ELF::ELFCLASS32 && Ident.second == ELF::ELFDATA2LSB) { - DyldELFObject> *Obj = - new DyldELFObject>( - Buffer->getMemBuffer(), ec); - return new ELFObjectImage>(Buffer, Obj); + auto Obj = make_unique>>( + Buffer->getMemBuffer(), ec); + return new ELFObjectImage>( + Buffer, std::move(Obj)); } else if (Ident.first == ELF::ELFCLASS32 && Ident.second == ELF::ELFDATA2MSB) { - DyldELFObject> *Obj = - new DyldELFObject>( + auto Obj = + make_unique>>( Buffer->getMemBuffer(), ec); - return new ELFObjectImage>(Buffer, Obj); + return new ELFObjectImage>(Buffer, + std::move(Obj)); } else if (Ident.first == ELF::ELFCLASS64 && Ident.second == ELF::ELFDATA2MSB) { - DyldELFObject> *Obj = - new DyldELFObject>( + auto Obj = + make_unique>>( Buffer->getMemBuffer(), ec); - return new ELFObjectImage>(Buffer, Obj); + return new ELFObjectImage>(Buffer, std::move(Obj)); } else if (Ident.first == ELF::ELFCLASS64 && Ident.second == ELF::ELFDATA2LSB) { - DyldELFObject> *Obj = - new DyldELFObject>( + auto Obj = + make_unique>>( Buffer->getMemBuffer(), ec); - return new ELFObjectImage>(Buffer, Obj); + return new ELFObjectImage>(Buffer, std::move(Obj)); } else llvm_unreachable("Unexpected ELF format"); } diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h index 27db5cdf42..e08f2ee439 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h @@ -119,7 +119,7 @@ public: virtual ~RuntimeDyldELF(); static ObjectImage *createObjectImage(ObjectBuffer *InputBuffer); - static ObjectImage *createObjectImageFromFile(object::ObjectFile *Obj); + static ObjectImage *createObjectImageFromFile(std::unique_ptr Obj); }; } // end namespace llvm diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h index 1006176753..85d6501a26 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h @@ -88,8 +88,8 @@ public: } static ObjectImage * - createObjectImageFromFile(object::ObjectFile *InputObject) { - return new ObjectImageCommon(InputObject); + createObjectImageFromFile(std::unique_ptr InputObject) { + return new ObjectImageCommon(std::move(InputObject)); } }; -- cgit v1.2.3