[llvm] r207580 - PR19553: Memory leak in RuntimeDyldELF::createObjectImageFromFile
Kaylor, Andrew
andrew.kaylor at intel.com
Wed May 7 16:18:07 PDT 2014
I'm curious about this change. Is the object::ObjectFile object ever needed after the file's contents have been loaded? Instead of owning this object, would it be possible to just delete it?
-Andy
-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of David Blaikie
Sent: Tuesday, April 29, 2014 2:53 PM
To: llvm-commits at cs.uiuc.edu
Subject: [llvm] r207580 - PR19553: Memory leak in RuntimeDyldELF::createObjectImageFromFile
Author: dblaikie
Date: Tue Apr 29 16:52:46 2014
New Revision: 207580
URL: http://llvm.org/viewvc/llvm-project?rev=207580&view=rev
Log:
PR19553: Memory leak in RuntimeDyldELF::createObjectImageFromFile
This starts in MCJIT::getSymbolAddress where the unique_ptr<object::Binary> 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.
Modified:
llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h
llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h
llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h
llvm/trunk/tools/lli/lli.cpp
Modified: llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h Tue Apr 29
+++ 16:52:46 2014
@@ -222,7 +222,7 @@ public:
/// needed by another object.
///
/// MCJIT will take ownership of the ObjectFile.
- virtual void addObjectFile(object::ObjectFile *O) {
+ virtual void addObjectFile(std::unique_ptr<object::ObjectFile> O) {
llvm_unreachable(
"ExecutionEngine subclass doesn't implement addObjectFile.");
}
Modified: llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h Tue Apr 29
+++ 16:52:46 2014
@@ -55,7 +55,7 @@ public:
/// Ownership of the input object is transferred to the ObjectImage
/// instance returned from this function if successful. In the case of load
/// failure, the input object will be deleted.
- ObjectImage *loadObject(object::ObjectFile *InputObject);
+ ObjectImage *loadObject(std::unique_ptr<object::ObjectFile>
+ InputObject);
/// Get the address of our local copy of the symbol. This may or may not
/// be the address used for relocation (clients can copy the data around
Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp Tue Apr 29 16:52:46
+++ 2014
@@ -113,8 +113,8 @@ bool MCJIT::removeModule(Module *M) {
-void MCJIT::addObjectFile(object::ObjectFile *Obj) {
- ObjectImage *LoadedObject = Dyld.loadObject(Obj);
+void MCJIT::addObjectFile(std::unique_ptr<object::ObjectFile> Obj) {
+ ObjectImage *LoadedObject = Dyld.loadObject(std::move(Obj));
if (!LoadedObject || Dyld.hasError())
report_fatal_error(Dyld.getErrorString());
@@ -308,10 +308,10 @@ uint64_t MCJIT::getSymbolAddress(const s
std::unique_ptr<object::Binary> ChildBin;
// FIXME: Support nested archives?
if (!ChildIt->getAsBinary(ChildBin) && ChildBin->isObject()) {
- object::ObjectFile *OF = reinterpret_cast<object::ObjectFile *>(
- ChildBin.release());
+ std::unique_ptr<object::ObjectFile> OF(
+ static_cast<object::ObjectFile *>(ChildBin.release()));
// This causes the object file to be loaded.
- addObjectFile(OF);
+ addObjectFile(std::move(OF));
// The address should be here now.
Addr = getExistingSymbolAddress(Name);
if (Addr)
Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h (original)
+++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h Tue Apr 29 16:52:46
+++ 2014
@@ -239,7 +239,7 @@ public:
/// @name ExecutionEngine interface implementation
/// @{
void addModule(Module *M) override;
- void addObjectFile(object::ObjectFile *O) override;
+ void addObjectFile(std::unique_ptr<object::ObjectFile> O) override;
void addArchive(object::Archive *O) override;
bool removeModule(Module *M) override;
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/ObjectImageCommon.h Tue
+++ Apr 29 16:52:46 2014
@@ -18,6 +18,8 @@
#include "llvm/ExecutionEngine/ObjectImage.h"
#include "llvm/Object/ObjectFile.h"
+#include <memory>
+
namespace llvm {
namespace object {
@@ -30,13 +32,13 @@ class ObjectImageCommon : public ObjectI
void anchor() override;
protected:
- object::ObjectFile *ObjFile;
+ std::unique_ptr<object::ObjectFile> 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<object::ObjectFile> 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<ObjectFile*>
+ // and should probably be checked for failure.
+
+ ObjFile.reset(object::ObjectFile::createObjectFile(Buffer->getMemBuffe
+ r()).get());
}
- ObjectImageCommon(object::ObjectFile* Input)
- : ObjectImage(nullptr), ObjFile(Input) {}
- virtual ~ObjectImageCommon() { delete ObjFile; }
+ ObjectImageCommon(std::unique_ptr<object::ObjectFile> 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
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Tue Apr
+++ 29 16:52:46 2014
@@ -698,21 +698,23 @@ createRuntimeDyldMachO(RTDyldMemoryManag
return Dyld;
}
-ObjectImage *RuntimeDyld::loadObject(ObjectFile *InputObject) {
+ObjectImage *RuntimeDyld::loadObject(std::unique_ptr<ObjectFile>
+InputObject) {
std::unique_ptr<ObjectImage> InputImage;
+ ObjectFile &Obj = *InputObject;
+
if (InputObject->isELF()) {
- InputImage.reset(RuntimeDyldELF::createObjectImageFromFile(InputObject));
+
+ InputImage.reset(RuntimeDyldELF::createObjectImageFromFile(std::move(I
+ nputObject)));
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());
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp Tue
+++ Apr 29 16:52:46 2014
@@ -51,7 +51,12 @@ template <class ELFT> class DyldELFObjec
typedef typename ELFDataTypeTypedefHelper<ELFT>::value_type addr_type;
+ std::unique_ptr<ObjectFile> UnderlyingFile;
+
public:
+ DyldELFObject(std::unique_ptr<ObjectFile> 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 ELFT> class ELFObjectImage : public ObjectImageCommon {
-protected:
- DyldELFObject<ELFT> *DyldObj;
bool Registered;
public:
- ELFObjectImage(ObjectBuffer *Input, DyldELFObject<ELFT> *Obj)
- : ObjectImageCommon(Input, Obj), DyldObj(Obj), Registered(false) {}
+ ELFObjectImage(ObjectBuffer *Input, std::unique_ptr<DyldELFObject<ELFT>> 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<DyldELFObject<ELFT>*>(getObjectFile())
+ ->updateSectionAddress(Sec, Addr);
}
void updateSymbolAddress(const SymbolRef &Sym, uint64_t Addr) override {
- DyldObj->updateSymbolAddress(Sym, Addr);
+ static_cast<DyldELFObject<ELFT>*>(getObjectFile())
+ ->updateSymbolAddress(Sym, Addr);
}
void registerWithDebugger() override { @@ -110,6 +115,14 @@ DyldELFObject<ELFT>::DyldELFObject(Memor
}
template <class ELFT>
+DyldELFObject<ELFT>::DyldELFObject(std::unique_ptr<ObjectFile> UnderlyingFile,
+ MemoryBuffer *Wrapper, error_code &ec)
+ : ELFObjectFile<ELFT>(Wrapper, ec),
+ UnderlyingFile(std::move(UnderlyingFile)) {
+ this->isDyldELFObject = true;
+}
+
+template <class ELFT>
void DyldELFObject<ELFT>::updateSectionAddress(const SectionRef &Sec,
uint64_t Addr) {
DataRefImpl ShdrRef = Sec.getRawDataRefImpl(); @@ -165,7 +178,7 @@ void RuntimeDyldELF::deregisterEHFrames(
}
ObjectImage *
-RuntimeDyldELF::createObjectImageFromFile(object::ObjectFile *ObjFile) {
+RuntimeDyldELF::createObjectImageFromFile(std::unique_ptr<object::Objec
+tFile> ObjFile) {
if (!ObjFile)
return nullptr;
@@ -174,21 +187,23 @@ RuntimeDyldELF::createObjectImageFromFil
MemoryBuffer::getMemBuffer(ObjFile->getData(), "", false);
if (ObjFile->getBytesInAddress() == 4 && ObjFile->isLittleEndian()) {
- DyldELFObject<ELFType<support::little, 2, false>> *Obj =
- new DyldELFObject<ELFType<support::little, 2, false>>(Buffer, ec);
- return new ELFObjectImage<ELFType<support::little, 2, false>>(nullptr, Obj);
+ auto Obj = make_unique<DyldELFObject<ELFType<support::little, 2, false>>>(std::move(ObjFile), Buffer, ec);
+ return new ELFObjectImage<ELFType<support::little, 2, false>>(
+ nullptr, std::move(Obj));
} else if (ObjFile->getBytesInAddress() == 4 && !ObjFile->isLittleEndian()) {
- DyldELFObject<ELFType<support::big, 2, false>> *Obj =
- new DyldELFObject<ELFType<support::big, 2, false>>(Buffer, ec);
- return new ELFObjectImage<ELFType<support::big, 2, false>>(nullptr, Obj);
+ auto Obj = make_unique<DyldELFObject<ELFType<support::big, 2, false>>>(
+ std::move(ObjFile), Buffer, ec);
+ return new ELFObjectImage<ELFType<support::big, 2, false>>(nullptr,
+ std::move(Obj));
} else if (ObjFile->getBytesInAddress() == 8 && !ObjFile->isLittleEndian()) {
- DyldELFObject<ELFType<support::big, 2, true>> *Obj =
- new DyldELFObject<ELFType<support::big, 2, true>>(Buffer, ec);
- return new ELFObjectImage<ELFType<support::big, 2, true>>(nullptr, Obj);
+ auto Obj = make_unique<DyldELFObject<ELFType<support::big, 2, true>>>(
+ std::move(ObjFile), Buffer, ec);
+ return new ELFObjectImage<ELFType<support::big, 2, true>>(nullptr,
+
+ std::move(Obj));
} else if (ObjFile->getBytesInAddress() == 8 && ObjFile->isLittleEndian()) {
- DyldELFObject<ELFType<support::little, 2, true>> *Obj =
- new DyldELFObject<ELFType<support::little, 2, true>>(Buffer, ec);
- return new ELFObjectImage<ELFType<support::little, 2, true>>(nullptr, Obj);
+ auto Obj = make_unique<DyldELFObject<ELFType<support::little, 2, true>>>(
+ std::move(ObjFile), Buffer, ec);
+ return new ELFObjectImage<ELFType<support::little, 2, true>>(
+ nullptr, std::move(Obj));
} else
llvm_unreachable("Unexpected ELF format"); } @@ -202,28 +217,29 @@ ObjectImage *RuntimeDyldELF::createObjec
error_code ec;
if (Ident.first == ELF::ELFCLASS32 && Ident.second == ELF::ELFDATA2LSB) {
- DyldELFObject<ELFType<support::little, 4, false>> *Obj =
- new DyldELFObject<ELFType<support::little, 4, false>>(
- Buffer->getMemBuffer(), ec);
- return new ELFObjectImage<ELFType<support::little, 4, false>>(Buffer, Obj);
+ auto Obj = make_unique<DyldELFObject<ELFType<support::little, 4, false>>>(
+ Buffer->getMemBuffer(), ec);
+ return new ELFObjectImage<ELFType<support::little, 4, false>>(
+ Buffer, std::move(Obj));
} else if (Ident.first == ELF::ELFCLASS32 &&
Ident.second == ELF::ELFDATA2MSB) {
- DyldELFObject<ELFType<support::big, 4, false>> *Obj =
- new DyldELFObject<ELFType<support::big, 4, false>>(
+ auto Obj =
+ make_unique<DyldELFObject<ELFType<support::big, 4, false>>>(
Buffer->getMemBuffer(), ec);
- return new ELFObjectImage<ELFType<support::big, 4, false>>(Buffer, Obj);
+ return new ELFObjectImage<ELFType<support::big, 4, false>>(Buffer,
+
+ std::move(Obj));
} else if (Ident.first == ELF::ELFCLASS64 &&
Ident.second == ELF::ELFDATA2MSB) {
- DyldELFObject<ELFType<support::big, 8, true>> *Obj =
- new DyldELFObject<ELFType<support::big, 8, true>>(
+ auto Obj =
+ make_unique<DyldELFObject<ELFType<support::big, 8, true>>>(
Buffer->getMemBuffer(), ec);
- return new ELFObjectImage<ELFType<support::big, 8, true>>(Buffer, Obj);
+ return new ELFObjectImage<ELFType<support::big, 8, true>>(Buffer,
+ std::move(Obj));
} else if (Ident.first == ELF::ELFCLASS64 &&
Ident.second == ELF::ELFDATA2LSB) {
- DyldELFObject<ELFType<support::little, 8, true>> *Obj =
- new DyldELFObject<ELFType<support::little, 8, true>>(
+ auto Obj =
+ make_unique<DyldELFObject<ELFType<support::little, 8, true>>>(
Buffer->getMemBuffer(), ec);
- return new ELFObjectImage<ELFType<support::little, 8, true>>(Buffer, Obj);
+ return new ELFObjectImage<ELFType<support::little, 8,
+ true>>(Buffer, std::move(Obj));
} else
llvm_unreachable("Unexpected ELF format"); }
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h Tue Apr
+++ 29 16:52:46 2014
@@ -119,7 +119,7 @@ public:
virtual ~RuntimeDyldELF();
static ObjectImage *createObjectImage(ObjectBuffer *InputBuffer);
- static ObjectImage *createObjectImageFromFile(object::ObjectFile *Obj);
+ static ObjectImage
+ *createObjectImageFromFile(std::unique_ptr<object::ObjectFile> Obj);
};
} // end namespace llvm
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h Tue
+++ Apr 29 16:52:46 2014
@@ -88,8 +88,8 @@ public:
}
static ObjectImage *
- createObjectImageFromFile(object::ObjectFile *InputObject) {
- return new ObjectImageCommon(InputObject);
+ createObjectImageFromFile(std::unique_ptr<object::ObjectFile> InputObject) {
+ return new ObjectImageCommon(std::move(InputObject));
}
};
Modified: llvm/trunk/tools/lli/lli.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lli/lli.cpp?rev=207580&r1=207579&r2=207580&view=diff
==============================================================================
--- llvm/trunk/tools/lli/lli.cpp (original)
+++ llvm/trunk/tools/lli/lli.cpp Tue Apr 29 16:52:46 2014
@@ -534,7 +534,7 @@ int main(int argc, char **argv, char * c
Err.print(argv[0], errs());
return 1;
}
- EE->addObjectFile(Obj.get());
+ EE->addObjectFile(std::unique_ptr<object::ObjectFile>(Obj.get()));
}
for (unsigned i = 0, e = ExtraArchives.size(); i != e; ++i) {
_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list