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.

llvm-svn: 207580
This commit is contained in:
David Blaikie 2014-04-29 21:52:46 +00:00
parent 836b1aed05
commit 7a1e775a7e
10 changed files with 78 additions and 57 deletions

View File

@ -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.");
}

View File

@ -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

View File

@ -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 std::string &Name,
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)

View File

@ -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;

View File

@ -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 ObjectImage {
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->getMemBuffer()).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

View File

@ -698,21 +698,23 @@ createRuntimeDyldMachO(RTDyldMemoryManager *MM, bool ProcessAllSections) {
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(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());

View File

@ -51,7 +51,12 @@ template <class ELFT> class DyldELFObject : public ELFObjectFile<ELFT> {
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 {
@ -109,6 +114,14 @@ DyldELFObject<ELFT>::DyldELFObject(MemoryBuffer *Wrapper, error_code &ec)
this->isDyldELFObject = true;
}
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) {
@ -165,7 +178,7 @@ void RuntimeDyldELF::deregisterEHFrames() {
}
ObjectImage *
RuntimeDyldELF::createObjectImageFromFile(object::ObjectFile *ObjFile) {
RuntimeDyldELF::createObjectImageFromFile(std::unique_ptr<object::ObjectFile> 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<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::createObjectImage(ObjectBuffer *Buffer) {
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>>(
auto Obj = make_unique<DyldELFObject<ELFType<support::little, 4, false>>>(
Buffer->getMemBuffer(), ec);
return new ELFObjectImage<ELFType<support::little, 4, false>>(Buffer, Obj);
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");
}

View File

@ -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

View File

@ -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));
}
};

View File

@ -534,7 +534,7 @@ int main(int argc, char **argv, char * const *envp) {
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) {